Closed
Bug 226793
Opened 21 years ago
Closed 11 years ago
Setting/getting last mod time differs on some systems
Categories
(Core :: XPCOM, defect, P3)
Tracking
()
RESOLVED
DUPLICATE
of bug 377307
People
(Reporter: BenB, Unassigned)
Details
(Whiteboard: [M1])
Attachments
(1 file)
2.72 KB,
patch
|
wtc
:
review+
dougt
:
superreview-
|
Details | Diff | Splinter Review |
Reproducability: Unknown. Bug appears on 2 W2K systems in California, but not on my W2K system in Germany. I don't have access to the California systems. Reproduction: 1. W2K, set timezone to PDT winter time (-0800) 2. int t1 = 1066244633000; // == 2003-10-15 21:03:53 nsIFile f = ...; f.lastModifiedFile = t1; int t2 = f.lastModifiedFile; print t2; Actual result: t2 == 1066248233000; // == 2003-10-15 22:03:53 == t1 + 1 hour Expected result: t2 == t1 Analysis: That 1 hour made me suspect a timezone problem, that the impl. of getter and setter differ. In fact, they do, the getter is implemented in NSPR and using a shortcut, the setter is implemented in xpcom/io/ and uses a number of scary local time conversions.
Reporter | ||
Updated•21 years ago
|
Status: NEW → ASSIGNED
Priority: -- → P1
Reporter | ||
Comment 1•21 years ago
|
||
Bug appears reproducably on FAT filesystems, but not on NTFS filesystems, to my knowledge. Compare <http://msdn.microsoft.com/library/default.asp?url=/library/en-us/sysinfo/base/setfiletime.asp>: "FAT records times on disk in local time. However, NTFS records times on disk in UTC."
Reporter | ||
Comment 2•21 years ago
|
||
No, wrong, can't reproduce on a FAT16 partition, I ran instead into another (documented) problem, the impreciseness of FAT's last mod time. The California systems are FAT32. Will continue to try to reproduce.
Reporter | ||
Comment 3•21 years ago
|
||
Bug appears, if the "Adjust for daylight saving time" setting of the OS is checked. Both on NTFS and FAT. Cases to be considered: (any combination of) - "Daylight saving time adjustment" on or off - File time in during daylight saving time or winter time - File time set during daylight saving time or winter time - File time read during daylight saving time or winter time - File time read by Mozilla (NSPR) or Explorer - File time set by Mozilla (XPCOM) or Windows (edit file with Notepad) - NTFS or FAT = 64 cases I didn't have time to cover them all. Despite some quite scary code in NSPR (hardcoded offset; direct reading of FILETIME strcture, although discouraged by MS), and a seemingly wrong call to LocalFileTimeToFileTime(), the main culprit seems to be the PR_LocalTimeParameters parameter to PR_ExplodeTime in XPCOM IO. Windows System time is in UTC, so converting our NSPR time into local time and feeding that to Windows System Time functions will give wrong results. Changing that parameter to PR_GMTParameters seems to fix the bug. But I also removed the LocalFileTimeToFileTime() call, to make the code match MS's example code and because it didn't make sense to me. Strangely, it doesn't seem to make a difference for the correctness of the result, in my testings. Attaching fix against Mozilla 1.4.1. Use |patch -p 0| to apply.
Reporter | ||
Comment 4•21 years ago
|
||
> = 64 cases
eh, 128 :).
Reporter | ||
Updated•21 years ago
|
Attachment #137040 -
Attachment description: Proposed fix, version 1. Use |patch -p 0|. → Proposed fix, version 1.
Attachment #137040 -
Flags: review?(dougt)
Comment 5•21 years ago
|
||
Comment on attachment 137040 [details] [diff] [review] Proposed fix, version 1. wtc needs to review this.
Attachment #137040 -
Flags: review?(dougt) → review?(wchang0222)
Reporter | ||
Updated•21 years ago
|
Priority: P1 → P3
Whiteboard: [M1]
Reporter | ||
Comment 6•20 years ago
|
||
no answer from wtc. dougt, what do we do?
Comment 7•20 years ago
|
||
Comment on attachment 137040 [details] [diff] [review] Proposed fix, version 1. r=wtc. The fix is correct. The comment change to nsprpub/pr/src/md/windows/w95io.c can be omitted. The new comment for xpcom/io/nsLocalFileWin.cpp can also be omitted. It is useful for reviewing this change, but it won't really help understand the new code. The new code is straightforward and easy to understand. By the way, in xpcom/io/nsLocalFileWin.cpp I am wondering if we should pass NULL as the third argument to SetFileTime because we should not modify the last access time.
Attachment #137040 -
Flags: review?(wchang0222) → review+
Reporter | ||
Updated•20 years ago
|
Attachment #137040 -
Flags: superreview?(scc)
Reporter | ||
Updated•20 years ago
|
Attachment #137040 -
Flags: superreview?(scc) → superreview?(shaver)
Reporter | ||
Comment 8•20 years ago
|
||
shaver?
Attachment #137040 -
Flags: superreview?(shaver) → superreview?(dougt)
Comment 9•20 years ago
|
||
nsLocalFileWin.cpp has changed since this patch was created. Specifically in version 1.125. Ben, can check out these changes and create a new diff.
Comment 10•20 years ago
|
||
Comment on attachment 137040 [details] [diff] [review] Proposed fix, version 1. ben, can you provide a new patch?
Attachment #137040 -
Flags: superreview?(dougt) → superreview-
Updated•18 years ago
|
Assignee: ben.bucksch → nobody
Status: ASSIGNED → NEW
QA Contact: xpcom
Updated•11 years ago
|
Status: NEW → RESOLVED
Closed: 11 years ago
Resolution: --- → DUPLICATE
Reporter | ||
Comment 12•11 years ago
|
||
Funny. The patch commited in bug 377307 in 2011 is - apart from comments and bitrot - *exactly* the same that I submitted here in 2003, 7 years earlier. The review here took 1 year (!), and by that time it had bitrotted and I had moved on to other projects.
Reporter | ||
Comment 13•11 years ago
|
||
(And I found it unbelievably sarcastic to wait with the review so long that the patch bitrots, and then deny the review *because of* this bitrot and ask more work from the contributor.)
You need to log in
before you can comment on or make changes to this bug.
Description
•