245338 - large files (>4 GB) are not displayed correctly in file browser

Status: RESOLVED WORKSFORME

(NSPR :: NSPR, defect)

Description

[Julien Pierre]

When browsing a JFS volume containing >4GB files on OS/2, using a URL such as
file:///f:/ , the file sizes over 4 GB wrap back to zero.

Note that a prerequisite to test this bug is the fix for 170911 in order for
NSPR on OS/2 to support > 2GB large files.  If the patch for NSPR isn't applied,
the file sizes over 2 GB will wrap in the browser. The patch allows NSPR to
support files of any size, but Mozilla still truncates them, probably due to the
use of a 32 bit unsigned int somewhere.

Comment 1

[Mike Kaply [:mkaply]]

Does this happen on linux?

Comment 2

[Boris Zbarsky [:bzbarsky]]

See nsIndexedToHTML::FormatSizeString -- AppendInt() takes a 32-bit int and the
quantity being passed in is an nsInt64 which has a conversion operator to a
32-bit int....

Comment 3

[Christian :Biesinger (don't email me, ping me on IRC)]

(In reply to comment #1)
> Does this happen on linux?

no, but nsLocalFileUnix is broken (bug 240192)

(In reply to comment #2)
> See nsIndexedToHTML::FormatSizeString -- AppendInt() takes a 32-bit int 

no, AppendInt is now available in a 64-bit version as well, which is used here.
(that was bug 240106)

Comment 4

[Christian :Biesinger (don't email me, ping me on IRC)]

(In reply to comment #3)
> (In reply to comment #1)
> > Does this happen on linux?
> 
> no, but nsLocalFileUnix is broken (bug 240192)

that should've said "yes", of course

Comment 5

[Mike Kaply [:mkaply]]

I believe you are incorrect.

This fix does NOT depend on your new changes at all - we query the file size
using stat, and it works fine for file sizes over 4GB - I debugged it.

The only problem is the display part which is not OS/2 specific.

Comment 6

[Mike Kaply [:mkaply]]

FormatSizeString Isn't a problem - it already has the bad data in the stream
(bad file sizes)

Comment 7

[Mike Kaply [:mkaply]]

Attachment: Fix

The actual problem here is that GETFILEINFO64 was calling GETFILEINFO so the
size of the file came in from stat 64, was converted to 32, and then put back
to 64.

Fix is to make GETFILEINFO64 do the right thing - no need to use OS/2 APIs
right now.

Comment 8

[Julien Pierre]

Mike,

If this NSPR patch really fixes the problem, then it sounds like my more
extensive NSPR patch in bug 170911 that uses the native OS/2 calls should also
have fixed it. I remember trying it, but it did not. I just checked and it looks
like that patch forgot to fix GetFileInfo64, it only patched GetOpenFileInfo64
... I don't know how that slipped through as my earlier patches had this in. I
think it's at the time I simplified it. I would suggest that this bug be marked
as a dupe of the other since the fix is in NPSR ...

Comment 9

[Mike Kaply [:mkaply]]

I disagree. Your patch in the other bug modified getfileinfo unnecessarily.

stat does exactly what we need, so we don't need to use the operating system API
in that case.

Comment 10

[Julien Pierre]

Will your patch work with both VAC++ and gcc ? If not, then I disagree with your
patch. The OS functions need to be used.

Comment 11

[Mike Kaply [:mkaply]]

The patch will compile with VACPP no problem.

4GB support for VACPP is irrelevant.

Comment 12

[Peter Weilbacher]

The patch here seems to never have gone in. I see that since this discussion _PR_MD_GETFILEINFO64 has changed a bit, so is this necessary any longer?

Comment 13

[Peter Weilbacher]

I tested with a Firefox of 20060522 from trunk, the current SeaMonkey from trunk and SeaMonkey from 1.8 branch and there large files display like

   really_large_file.args  5747209 KB  15/06/06  22:47:50

for a file of over 5GB, so I guess this was fixed by some other checkin. Marking WFM.

Comment 14

[Julien Pierre]

This was probably fixed by bug 170911, which is the bug about the support for 2GB files on OS/2 in NSPR. I resolved that bug a year ago in NSPR 4.6 .
There may have been other fixes too, but that was a required part of the fix for the file browser in mozilla, so I'm marking that bug as a dependency.