This is split off from bug 55814, which deals with mail lossage when the disk is full. nsLocalFile::GetDiskSpaceAvailable does not take quotas into account, so the checks before downloading mail which are done by mail don't work properly.

*** Bug 98076 has been marked as a duplicate of this bug. ***

taking some of the bugs that I have been working on from scc.

thought i had a patch for this, but I don't.

Marking as dataloss, as if I understand it correctly, the client can download mail which it has no way to store - depending on the type of server and the clientside and serverside configuration, this could lose mail.

Comment on attachment 376242 [details] [diff] [review] trunk patch Can you please review this one?

Comment on attachment 376242 [details] [diff] [review] trunk patch >diff -up src/configure.in.old src/configure.in >--- src/configure.in.old 2009-05-07 14:55:16.000000000 +0200 >+++ src/configure.in 2009-05-07 14:58:32.000000000 +0200 >@@ -3096,6 +3096,9 @@ AC_CHECK_HEADERS(io.h) > dnl These are all the places some variant of statfs can be hiding. > AC_CHECK_HEADERS(sys/statvfs.h sys/statfs.h sys/vfs.h sys/mount.h) > >+dnl Quota support >+AC_CHECK_HEADERS(sys/quota.h) What systems might *not* have these functions? Does Mozilla need to worry about binary compatibility with older systems or kernels when introducing this test? >diff -up src/xpcom/io/nsLocalFileUnix.cpp.old src/xpcom/io/nsLocalFileUnix.cpp >+#if defined(HAVE_SYS_QUOTA_H) >+ #include <sys/sysmacros.h> >+ #pragma GCC visibility push(default) >+ #include <sys/quota.h> >+ #pragma GCC visibility pop Instead of putting the visibility macros here, please add sys/quota.h (and sys/sysmacros.h?) to config/system-headers, which automatically wraps them for default visibility. Also, please don't indent the #includes >+static nsresult >+GetDeviceName(nsACString &deviceName, int deviceMajor, int deviceMinor) * Please order paramaters so that inparams are first and outparams are last: (int deviceMajor, int deviceMinor, nsACString &deviceName); * No need for nsresult failure codes here, just use PRBool true==success false==failure * This function should probably have a short doccomment explaining failure conditions >+{ >+ nsresult ret = NS_ERROR_FAILURE; >+ >+ #define MOUNTINFO_LINE_LENGHT 200 >+ #define MOUNTINFO_DEV_POSITION 6 * Plese make these static const int instead of defines (kMountInfoLineLength) * LENGHT is misspelled LENGTH? >+ while(fgets(mountinfo_line,MOUNTINFO_LINE_LENGHT,f)) { >+ char *p_dev = strstr(mountinfo_line,device_num); I could use a comment before this loop explaining the file format and what is being looked-for. > NS_IMETHODIMP > nsLocalFile::GetDiskSpaceAvailable(PRInt64 *aDiskSpaceAvailable) > { >@@ -1167,6 +1216,30 @@ nsLocalFile::GetDiskSpaceAvailable(PRInt > */ > *aDiskSpaceAvailable = (PRInt64)fs_buf.f_bsize * (fs_buf.f_bavail - 1); > >+#if defined(HAVE_SYS_STAT_H) || defined(HAVE_SYS_QUOTA_H) >+ >+ struct stat stat_buf; >+ if(stat(mPath.get(), &stat_buf) < 0) { Use FillStatCache/mCachedStat. >+ PRInt64 QuotaSpaceAvailable = (PRInt64)fs_buf.f_bsize * dq.dqb_bhardlimit; Use a constructor-style cast: PRInt64(fs_buf.f_bsize * ...) Overall, pretty good, just needs a little tightening.

Thanks for the review, there's an updated one.

Comment on attachment 376718 [details] [diff] [review] v2 I need the answer to the question about sys/quota.h and the binary availability of quotactl. A couple people on IRC say that the BSDs don't have that header. Since nsLocalFileUnix.cpp is shared code, we'd at least need the configure checks from the first patch. Is the format of /proc/self/mountinfo stable?

sys/quota.h is exported by glibc so I presume it safe to expect appropriate kernel support when the header is there. So I vote for the original AC_CHECK_HEADERS(sys/quota.h) in configure file. /proc/self/mountinfo should be stable since 2.6.26 kernels, see http://www.mjmwired.net/kernel/Documentation/filesystems/proc.txt

There's the v2 patch + quota.h check so it should be portable enough.

Comment on attachment 379547 [details] [diff] [review] v2+quota.h check This needs a tryserver run before pushed to -central.

http://hg.mozilla.org/mozilla-central/rev/85bf327a368d and followup http://hg.mozilla.org/mozilla-central/rev/6bf2a4703c9a

This broke Darwin. First it fails w/ sys/sysmacros.h not being found and I'm able to get around that by patching configure.in to look for it and #ifdef'ing it in nsLocalFileUnix.cpp, but then the build further fails with: g++-4.2 -arch i386 -o nsLocalFileUnix.o -c -fvisibility=hidden -DMOZILLA_INTERNAL_API -DOSTYPE=\"Darwin\" -DOSARCH=Darwin -D_IMPL_NS_COM -I.. -I/src/mozilla-central/xpcom/io -I. -I../../dist/include -I../../dist/include/nsprpub -I/src/mozilla-central/obj-i386-apple-darwin9.8.0-browser/dist/include/nspr -I/src/mozilla-central/obj-i386-apple-darwin9.8.0-browser/dist/include/nss -I/sw/include -fPIC -fno-rtti -fno-exceptions -Wall -Wpointer-arith -Woverloaded-virtual -Wsynth -Wno-ctor-dtor-privacy -Wno-non-virtual-dtor -Wcast-align -Wno-invalid-offsetof -Wno-long-long -isysroot /Developer/SDKs/MacOSX10.5.sdk -fno-strict-aliasing -fpascal-strings -fno-common -fshort-wchar -pthread -DNDEBUG -DTRIMMED -O3 -DMOZILLA_CLIENT -include ../../mozilla-config.h -Wp,-MD,.deps/nsLocalFileUnix.pp /src/mozilla-central/xpcom/io/nsLocalFileUnix.cpp /src/mozilla-central/xpcom/io/nsLocalFileUnix.cpp: In member function ‘virtual nsresult nsLocalFile::GetDiskSpaceAvailable(PRInt64*)’: /src/mozilla-central/xpcom/io/nsLocalFileUnix.cpp:1238: error: invalid conversion from ‘int’ to ‘const char*’ /src/mozilla-central/xpcom/io/nsLocalFileUnix.cpp:1238: error: initializing argument 1 of ‘int quotactl(const char*, int, int, char*)’ /src/mozilla-central/xpcom/io/nsLocalFileUnix.cpp:1238: error: invalid conversion from ‘const char*’ to ‘int’ /src/mozilla-central/xpcom/io/nsLocalFileUnix.cpp:1238: error: initializing argument 2 of ‘int quotactl(const char*, int, int, char*)’ make[1]: *** [nsLocalFileUnix.o] Error 1 make[1]: Leaving directory `/src/mozilla-central/obj-i386-apple-darwin9.8.0-browser/xpcom/io' I've filed Bug 520098 to deal with this, which has a possible solution, but I don't know the ramifications.

Is this bug really still 'helpwanted'?

Helpwanted is the Bug 520098 - build this patch on Darwin.