nsILocalFile.diskSpaceAvailable incorrect on 64-bit OS X

RESOLVED FIXED in mozilla2.0b10

Status

()

Core
XPCOM
P1
normal
RESOLVED FIXED
7 years ago
7 years ago

People

(Reporter: Mime Čuvalo, Assigned: bz)

Tracking

({regression})

unspecified
mozilla2.0b10
x86_64
Mac OS X
regression
Points:
---
Bug Flags:
in-testsuite ?

Firefox Tracking Flags

(blocking2.0 final+)

Details

(Whiteboard: [softblocker])

Attachments

(1 attachment)

(Reporter)

Description

7 years ago
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');
(Reporter)

Comment 1

7 years ago
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');
(Reporter)

Comment 2

7 years ago
Narrowing the problem down - tested on a 32-bit firefox and it worked fine.  Seems that it's only a 64-bit Mac issue.
Hardware: x86 → x86_64
(Reporter)

Updated

7 years ago
Summary: nsILocalFile.diskSpaceAvailable incorrect on OS X → nsILocalFile.diskSpaceAvailable incorrect on 64-bit OS X
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...
An I have no idea why this would work in a 32-bit build...
Blocks: 571193
blocking2.0: --- → ?
Component: Networking: File → XPCOM
Keywords: regression
QA Contact: networking.file → xpcom
Assignee: nobody → bzbarsky
Status: UNCONFIRMED → NEW
Ever confirmed: true
Priority: -- → P1
Whiteboard: [need review]
Created attachment 504341 [details] [diff] [review]
Use the right statvfs fields for determining disk size.
Attachment #504341 - Flags: review?(joshmoz)
Josh, is there a sane way to write a test for this?
(Reporter)

Comment 7

7 years ago
(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.

Updated

7 years ago
blocking2.0: ? → final+

Comment 8

7 years ago
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?
> 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/pvfs2-users@beowulf-underground.org/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.

Updated

7 years ago
Whiteboard: [need review] → [need review][softblocker]

Updated

7 years ago
Attachment #504341 - Flags: review?(joshmoz) → review+
Whiteboard: [need review][softblocker] → [need landing][softblocker]
Pushed http://hg.mozilla.org/mozilla-central/rev/8435df7a0e7c
Status: NEW → RESOLVED
Last Resolved: 7 years ago
Flags: in-testsuite?
Resolution: --- → FIXED
Whiteboard: [need landing][softblocker] → [softblocker]
Target Milestone: --- → mozilla2.0b10
You need to log in before you can comment on or make changes to this bug.