626259 - nsILocalFile.diskSpaceAvailable incorrect on 64-bit OS X

Status: RESOLVED FIXED

(Core :: XPCOM, defect, P1)

Description

[Mime Čuvalo]

User-Agent:       Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10_6_6; en-US) AppleWebKit/534.10 (KHTML, like Gecko) Chrome/8.0.552.231 Safari/534.10
Build Identifier: 

Just what the summary says:  nsILocalFile.diskSpaceAvailable is telling me that my Mac has 9600 GB of space (I wish!).  On Windows and Linux it reports the correct amount of bytes.

Reproducible: Always

Steps to Reproduce:
var file = Components.classes['@mozilla.org/file/local;1'].createInstance(Components.interfaces.nsILocalFile);
file.initWithPath("/bin/sh");
alert(program.diskSpaceAvailable + ' bytes = ' + (program.diskSpaceAvailable / 1024 / 1024 / 1024) + ' GB');

Comment 1

[Mime Čuvalo]

Whoops - incorrect reproduction code.  This is correct:

var file = Components.classes['@mozilla.org/file/local;1'].createInstance(Components.interfaces.nsILocalFile);
file.initWithPath("/bin/sh");
alert(file.diskSpaceAvailable + ' bytes = ' + (file.diskSpaceAvailable / 1024 / 1024 / 1024) + ' GB');

Comment 2

[Mime Čuvalo]

Narrowing the problem down - tested on a 32-bit firefox and it worked fine.  Seems that it's only a 64-bit Mac issue.

Comment 3

[Boris Zbarsky [:bzbarsky]]

This looks like a regression from the nsLocalFileOSX elimination.  Over here, at least, I see f_bsize set to 1MB in struct statvfs, while f_frsize is set to 4KB.  The latter is the right size.  Note that the OSX documentation for statvfs says:

  f_frsize  The size in bytes of the minimum unit of allocation on this file
            system. (This corresponds to the f_bsize member of struct statfs.)

   f_bsize  The preferred length of I/O requests for files on this file
            system.  (Corresponds to the f_iosize member of struct statfs.)

So this code that's trying to use the same member names for both statfs and statvfs is just no good on OSX...

Comment 4

[Boris Zbarsky [:bzbarsky]]

An I have no idea why this would work in a 32-bit build...

Comment 5

[Boris Zbarsky [:bzbarsky]]

Attachment: Use the right statvfs fields for determining disk size.

Comment 6

[Boris Zbarsky [:bzbarsky]]

Josh, is there a sane way to write a test for this?

Comment 7

[Mime Čuvalo]

(In reply to comment #4)
> An I have no idea why this would work in a 32-bit build...
Sorry, I had incorrectly tested an Firefox 3.6 instead of the latest for the 32-bit.

Comment 8

[Josh Aas]

Comment on attachment 504341 [details] [diff] [review]
Use the right statvfs fields for determining disk size.

Wouldn't it be better to use statfs vs. statvfs macros to decide when to use what members instead of a list of operating systems?

Has this been checked on Mac OS X 10.5?

Comment 9

[Boris Zbarsky [:bzbarsky]]

> Wouldn't it be better to use statfs vs. statvfs macros to decide when to use
> what members instead of a list of operating systems?

See, that's the thing.  On Linux, the documentation for those two members is:

  unsigned long  f_bsize;    /* file system block size */
  unsigned long  f_frsize;   /* fragment size */

So there, f_bsize is clearly the right thing to be using.  It's not even clear to be what f_frsize means in that situation.  On my Linux box both happen to be 4096, but historically f_frsize has been 0 in some glibc versions (that was considered a bug, though), 1024 in some kernel versions (see http://www.mail-archive.com/[e-mail]/msg00122.html ) and so forth....

I could turn the condition around and just use bsize on Linux and frsize on all other Unices, I suppose.  But at this stage in the 2.0 game that sort of change would not make me all that happy.

I have not checked this on 10.5, no.  Would you like me to?  I think I have a 10.5 machine somewhere here if I dig hard enough...  At least per http://www.manpagez.com/man/3/statvfs/osx-10.5.php the manpage there looks the same as on 10.6.

Comment 10

[Boris Zbarsky [:bzbarsky]]

Pushed http://hg.mozilla.org/mozilla-central/rev/8435df7a0e7c