Re: misc/22221: Chicago time zone file problem

[David Holland <dholland-bugs%netbsd.org@localhost>]

On Thu, Apr 03, 2008 at 05:59:36PM +0200, Alan Barrett wrote:
 > > I think another valid question to raise is whether this global
 > > variable needs to continue to be supported...
 > 
 > It's required by POSIX
 > <http://www.opengroup.org/onlinepubs/009695399/functions/timezone.html>,
 > but I don't see it documented in NetBSD's tzset(3) man page or anywhere

That's not POSIX. But I guess we still probably shouldn't get rid of
it. Pity.

 > The loop starting at line 281 of src/lib/libc/time/localtime.c looks
 > suspicious.  I don't immediately see what sp->typecnt really means, but
 > it does appear that that timezone and altzone may be set more than once.

It is apparently the number of distinct zone types/names that can
appear. I put some debug output in there and got this:

America/Denver:
   test: sp->typecnt is 4
   test: type 0: isdst: 1  tzname: MDT  gmtoff: -21600
   test: type 1: isdst: 0  tzname: MST  gmtoff: -25200
   test: type 2: isdst: 1  tzname: MWT  gmtoff: -21600
   test: type 3: isdst: 1  tzname: MPT  gmtoff: -21600

America/Chicago:
   test: sp->typecnt is 5
   test: type 0: isdst: 1  tzname: CDT  gmtoff: -18000
   test: type 1: isdst: 0  tzname: CST  gmtoff: -21600
   test: type 2: isdst: 0  tzname: EST  gmtoff: -18000
   test: type 3: isdst: 1  tzname: CWT  gmtoff: -18000
   test: type 4: isdst: 1  tzname: CPT  gmtoff: -18000

(MWT/MPT/CWT/CPT are from WWII, and EST appears in the Chicago list
because apparently Chicago put itself on Eastern time briefly in the
1930s.)

So the value of "daylight" we get out is from whatever one of these
happens to come last, and the value of "timezone" is the GMT offset
from the last one not marked DST, which in the case of Chicago is
wrong, but for Denver it's only right because there aren't any other
random entries appearing, i.e., by accident.

A bit further below there's a loop over sp->timecnt, instead of
sp->typecnt, that sets tzname[] again. This turns out to loop over
each distinct time conversion rule, in order. So the code for setting
daylight, timezone, and altzone is actually correct if moved to that
loop. Patch follows.

(It is not clear to me what purpose the sp->typecnt loop serves at all
now, except to maybe misinitialize tzname[1] in an area that has more
than one name for DST and has never used any of them. But I'm
unwilling to remove it without further investigation.)

Index: time/localtime.c
===================================================================
RCS file: /cvsroot/src/lib/libc/time/localtime.c,v
retrieving revision 1.39
diff -U 6 -r1.39 localtime.c
--- time/localtime.c    22 Mar 2006 14:01:30 -0000      1.39
+++ time/localtime.c    5 Apr 2008 16:40:12 -0000
@@ -280,33 +280,33 @@
 #endif /* defined ALL_STATE */
        for (i = 0; i < sp->typecnt; ++i) {
                register const struct ttinfo * const    ttisp = &sp->ttis[i];
 
                tzname[ttisp->tt_isdst] =
                        &sp->chars[ttisp->tt_abbrind];
-#ifdef USG_COMPAT
-               if (ttisp->tt_isdst)
-                       daylight = 1;
-               if (i == 0 || !ttisp->tt_isdst)
-                       timezone = -(ttisp->tt_gmtoff);
-#endif /* defined USG_COMPAT */
-#ifdef ALTZONE
-               if (i == 0 || ttisp->tt_isdst)
-                       altzone = -(ttisp->tt_gmtoff);
-#endif /* defined ALTZONE */
        }
        /*
        ** And to get the latest zone names into tzname. . .
        */
        for (i = 0; i < sp->timecnt; ++i) {
                register const struct ttinfo * const    ttisp =
                                                        &sp->ttis[
                                                                sp->types[i]];
 
                tzname[ttisp->tt_isdst] =
                        &sp->chars[ttisp->tt_abbrind];
+#ifdef USG_COMPAT
+               if (ttisp->tt_isdst)
+                       daylight = 1;
+               if (i == 0 || !ttisp->tt_isdst)
+                       timezone = -(ttisp->tt_gmtoff);
+#endif /* defined USG_COMPAT */
+#ifdef ALTZONE
+               if (i == 0 || ttisp->tt_isdst)
+                       altzone = -(ttisp->tt_gmtoff);
+#endif /* defined ALTZONE */
        }
 }
 
 static int
 tzload(name, sp)
 register const char *          name;


-- 
David A. Holland
dholland%netbsd.org@localhost