Tcl Source Code

View Ticket
Login
Ticket UUID: 1353840
Title: windows stat has daylight savings problems
Type: Bug Version: None
Submitter: matt-newman Created on: 2005-11-11 11:05:40
Subsystem: 36. Pathname Management Assigned To: kennykb
Priority: 7 High Severity:
Status: Closed Last Modified: 2005-12-03 10:21:01
Resolution: Fixed Closed By: sf-robot
    Closed on: 2005-12-03 03:21:01
Description:
Set your date to something this summer and create a file with that 
timestamp then to a tcl stat and print out the mtime field. Then go to 
control panel and UNCHECK the adjust for daylight saving and re-
stat the file - the mtime reported will be different.

The bug is very old - it is due to a misunderstand of the problems 
encountered between borland and ms C runtimes - the comments 
indicate that borland incorrectly returned UTC, which caused a 
specific work around to be added using GetFileAttributes.

The work around is fine EXCEPT the function ToCTime() is broken. 

Attached is a fix that uses direct math to convert the FIELTIME to 
time_t and is therefore immune to these issues.

I would rate this bug as critical for those of us that build applications 
that rely on timestamps....

Matt
User Comments: sf-robot added on 2005-12-03 10:21:01:
Logged In: YES 
user_id=1312539

This Tracker item was closed automatically by the system. It was
previously set to a Pending status, and the original submitter
did not respond within 14 days (the time period specified by
the administrator of this Tracker).

kennykb added on 2005-11-19 02:46:59:
Logged In: YES 
user_id=99768

Steve Offutt reports that Matt is wrong about FAT32, and in
fact we have (with the new code):

NTFS with or without intervening reboot - ok 
FAT32 with intervening reboot - ok

leaving us with "FAT32 - no intervening reboot" as the
failure case.  This is as good as we're going to get it,
particularly in light of the fact that for the "Fall
back" case the FAT32 timestamp is ambiguous.

I'm changing the bug status to Pending; it'll close
wutomatically if nobody reopens.

kennykb added on 2005-11-15 23:45:40:
Logged In: YES 
user_id=99768

I knew I had looked at this once before, and found it today:
[Bug 926106].

I had already fixed this issue in 8.5 - in much the way that
you suggest, but with some additions.  Essentially, we have
to avoid all use of the MSVC stat or _stati64 functions,
because they have the exact same bug.  Moreover, we have to
avoid use of the MSVC utime function as well - because it
has an "anti-bug" to perform the reverse adjustment to match
'stat'.

I had initially resisted backporting for two reasons:

(1) A backport risked damaging legacy code with a potential
incompatibility.

(2) In the new code, [clock format [file mtime $file]] gives
different results from the time shown in Windows Explorer,
because, guess what! Explorer has the same bug, too!

You've convinced me, though, that getting the timestamps
right is The Right Thing, no matter how many Microsoft tools
get them wrong.

I've committed the 8.5 fix into the 8.4 branch.  I'm leaving
this bug open because we most likely *do* want to put in an
"anti-bug" for FAT32 - that would constrain the cases we get
wrong to "FAT32 + DST change + no reboot" - surely the most
unusual of the cases.

kennykb added on 2005-11-15 23:45:39:

data_type - 110894

vincentdarley added on 2005-11-15 19:55:14:
Logged In: YES 
user_id=32170

FYI. If it is deemed sensible to have different behaviour on
FAT vs NTFS, you can use
'tclWinProcs->getVolumeInformationProc()' to determine the
volume type (see e.g. TclpFilesystemPathType).

But, if I understand Matt correctly, there is no valid fix
for 'FAT' that works both before and after a reboot. Hence
we should probably just apply the right code for NTFS and be
done with it.

matt-newman added on 2005-11-15 17:50:43:
Logged In: YES 
user_id=1333796

Let me try and be clear:
1. set clock to july this year
create a file on ntfs *and* fat (same system)
old & new stat returns *7072. good so far.

2. advance clock to today
old stat returns *3470 for ntfs and fat (wrong)
new stat returns *7072 for ntfs and fat (correct)

3. reboot
old stat returns *3470 for ntfs (wrong)
new stat returns *7072 for ntfs (correct)
old stat returns *7072 for fat (correct)
new stat returns *0672 for fat (wrong)

so you can see that the old code is *always* wrong for ntfs once you 
moving into a different DST slice.
Also the old code is also always wrong for fat too until a reboot takes 
place.....

The new code is only wrong for fat - post reboot - but this is because of 
a bug in MS's FIlETIME conversions in the kernel, and a side-effect of 
storing local time... Since there is no practical way to second-guess if 
the fat time is pre or post reboot there is no reasonable solution that I 
am aware of.

Also I would argue strongly, that such precision is very important in the 
corporate/business market, where NTFS is the rule and FAT the 
exception, whereas in the consumer market such things are less 
important, and are already broken for FAT anyway... (see above pre/
post reboot example)

Matt

dkf added on 2005-11-15 16:00:11:
Logged In: YES 
user_id=79902

Certainly as recently as one year ago, some system vendors
were shipping machines preconfigured with XP and FAT
filesystems. (I know because I purchased one!) OTOH,
anything running with FAT is likely to be rebooted fairly
often (i.e. be a home desktop).

kennykb added on 2005-11-15 04:38:00:
Logged In: YES 
user_id=99768

Matt,

The worst thing about this issue is that it's filesystem
dependent.  What you just described is correct for NTFS,
I *think*.  FAT, on the other hand, stores file times as
local time.  GetFileTime is documented to return a different
result from FindFirstFile, and they *both* appear to be
wrong in different ways after a Daylight Saving Time
transition.

At least, that's the way that I read the amphigory at
http://msdn.microsoft.com/library/default.asp?url=/library/en-us/sysinfo/base/file_times.asp
Do you understand what it's saying about FAT any better
than I do?  Do we care about FAT? (I think so - although
if we have to make one of them buggy, better FAT than
NTFS, I concede.)

Still thinking about applying this patch.  I doubt,
the comments notwithstanding, that it's a Borland-vs-MSVC
issue; it's more of a FAT32-vs-NTFS issue.

hobbs added on 2005-11-12 07:22:35:
Logged In: YES 
user_id=72656

This all looks good to me, but I'm passing to the Master of
Time, as he's fiddled with other bits related to Tcl's time
handling.

vincentdarley added on 2005-11-11 22:34:38:
Logged In: YES 
user_id=32170

I'm honestly not qualfied to judge whether this is correct
or not. Anyone with more experience?

matt-newman added on 2005-11-11 18:05:44:

File Added - 155858: tclWinFile.c.diff

Attachments: