On Mon, Jun 14, 2004 at 08:11:26PM -0700, Ken Pizzini wrote:

That might work, depending on the local implementation of asctime(), but I'd feel more comfortable with a patch that takes care to avoid passing a NULL to asctime() in the first place. Perhaps this patch instead?

No, that patch wasn't right either: it did protect the asctime() calls fine, but it still left zdump.c with many potential dereferences of NULL localtime() result pointers. I'm not completely satisfied with how the code resulting from the patch below handles things, but at least it now completely avoids dereferencing these NULLs... (The attached patch is against a fresh copy of zdump.c from tzcode2004a; i.e., it is *not* incremental to either of the previous two patches circulated on the list.) --Ken Pizzini --- zdump.c-orig 2004-03-02 06:52:38.000000000 -0800 +++ zdump.c 2004-06-14 21:24:11.735549152 -0700 @@ -1,4 +1,4 @@ -static char elsieid[] = "@(#)zdump.c 7.31"; +static char elsieid[] = "@(#)zdump.c 7.32"; /* ** This code has been made independent of the rest of the time @@ -129,6 +129,7 @@ static size_t longest; static char * progname; static void show P((char * zone, time_t t, int v)); +static void complain P((time_t t)); int main(argc, argv) @@ -148,6 +149,7 @@ time_t hibit; struct tm tm; struct tm newtm; + struct tm *tmp; INITIALIZE(cuttime); #if HAVE_GETTEXT - 0 @@ -230,7 +232,12 @@ show(argv[i], t, TRUE); t += SECSPERHOUR * HOURSPERDAY; show(argv[i], t, TRUE); - tm = *localtime(&t); + tmp = localtime(&t); + if (tmp == NULL) { + complain(t); + continue; + } + tm = *tmp; (void) strncpy(buf, abbr(&tm), (sizeof buf) - 1); for ( ; ; ) { if (cutoff != NULL && t >= cuttime) @@ -240,12 +247,22 @@ break; if (newt <= t) break; - newtm = *localtime(&newt); + tmp = localtime(&newt); + if (tmp == NULL) { + complain(newt); + break; + } + newtm = *tmp; if (delta(&newtm, &tm) != (newt - t) || newtm.tm_isdst != tm.tm_isdst || strcmp(abbr(&newtm), buf) != 0) { newt = hunt(argv[i], t, newt); - newtm = *localtime(&newt); + tmp = localtime(&newt); + if (tmp == NULL) { + complain(newt); + break; + } + newtm = *tmp; (void) strncpy(buf, abbr(&newtm), (sizeof buf) - 1); } @@ -284,9 +301,15 @@ time_t t; struct tm lotm; struct tm tm; + struct tm *tmp; static char loab[MAX_STRING_LENGTH]; - lotm = *localtime(&lot); + tmp = localtime(&lot); + if (tmp == NULL) { + complain(lot); + return lot; + } + lotm = *tmp; (void) strncpy(loab, abbr(&lotm), (sizeof loab) - 1); while ((hit - lot) >= 2) { t = lot / 2 + hit / 2; @@ -294,7 +317,12 @@ ++t; else if (t >= hit) --t; - tm = *localtime(&t); + tmp = localtime(&t); + if (tmp == NULL) { + complain(t); + return t; + } + tm = *tmp; if (delta(&tm, &lotm) == (t - lot) && tm.tm_isdst == lotm.tm_isdst && strcmp(abbr(&tm), loab) == 0) { @@ -334,6 +362,14 @@ return result; } +static char * +nonnull_asctime(t) +const struct tm * t; +{ + static char null[] = "NULL"; + return (t == NULL) ? null : asctime(t); +} + static void show(zone, t, v) char * zone; @@ -344,12 +380,12 @@ (void) printf("%-*s ", (int) longest, zone); if (v) - (void) printf("%.24s UTC = ", asctime(gmtime(&t))); + (void) printf("%.24s UTC = ", nonnull_asctime(gmtime(&t))); tmp = localtime(&t); - (void) printf("%.24s", asctime(tmp)); - if (*abbr(tmp) != '\0') + (void) printf("%.24s", nonnull_asctime(tmp)); + if (tmp != NULL && *abbr(tmp) != '\0') (void) printf(" %s", abbr(tmp)); - if (v) { + if (tmp != NULL && v) { (void) printf(" isdst=%d", tmp->tm_isdst); #ifdef TM_GMTOFF (void) printf(" gmtoff=%ld", tmp->TM_GMTOFF); @@ -358,6 +394,13 @@ (void) printf("\n"); } +static void +complain(t) +time_t t; +{ + (void) printf("localtime() failed on input %lld\n", (long long)t); /*XXX*/ +} + static char * abbr(tmp) struct tm * tmp;

Ken Pizzini <"tz."@explicate.org> writes:

I'd feel more comfortable with a patch that takes care to avoid passing a NULL to asctime() in the first place.

Yes, asctime has undefined behavior if you give it null pointer. But your patch doesn't suffice either, since asctime also has undefined behavior if you give it a struct tm whose tm_year member is less than -999 - 1900 or greater than 9999 - 1900. (This is because it can overrun its 26-byte buffer in that case.) Worse, HP-UX asctime rejects any tm_year that is negative; this violates the C standard, but HP won't take my bug reports since I'm not a HP-UX customer. I looked and found several other problems with 64-bit time_t in zdump.c. Here is a proposed patch. The basic ideas are as follows. * Use 'long' when computing years, not 'int', to avoid spurious overflow on 64-bit hosts. * Use our (more generous) asctime implementation than relying on the system asctime, so that we can print years before 1900 and after 9999 (even on HP-UX). * When localtime/gmtime fails, print the raw time_t value as an integer than the less-informative "NULL". * Don't attempt to print out all the 64-bit time_t space, as that will take nearly forever; instead, default to printing about 50 years into the future, and don't bother to print times before about 1833. This suffices for all the current tz data and is good enough for POSIX tz values until the year 2106. We can revisit this when we add true 64-bit support for tz data. =================================================================== RCS file: RCS/asctime.c,v retrieving revision 2004.1 retrieving revision 2004.1.0.1 diff -pu -r2004.1 -r2004.1.0.1 --- asctime.c 1998/05/28 13:56:06 2004.1 +++ asctime.c 2004/06/15 04:23:43 2004.1.0.1 @@ -41,14 +41,14 @@ char * buf; else mn = mon_name[timeptr->tm_mon]; /* ** The X3J11-suggested format is - ** "%.3s %.3s%3d %02.2d:%02.2d:%02.2d %d\n" - ** Since the .2 in 02.2d is ignored, we drop it. + ** "%.3s %.3s%3d %.2d:%.2d:%.2d %d\n" + ** Use "%02d", as it is a bit more portable than "%.2d". */ - (void) sprintf(buf, "%.3s %.3s%3d %02d:%02d:%02d %d\n", + (void) sprintf(buf, "%.3s %.3s%3d %02d:%02d:%02d %ld\n", wn, mn, timeptr->tm_mday, timeptr->tm_hour, timeptr->tm_min, timeptr->tm_sec, - TM_YEAR_BASE + timeptr->tm_year); + timeptr->tm_year + (long) TM_YEAR_BASE); return buf; } =================================================================== RCS file: RCS/zdump.c,v retrieving revision 2003.3 retrieving revision 2003.3.0.2 diff -pu -r2003.3 -r2003.3.0.2 --- zdump.c 2003/09/16 11:12:41 2003.3 +++ zdump.c 2004/06/15 05:24:21 2003.3.0.2 @@ -6,6 +6,7 @@ static char elsieid[] = "@(#)zdump.c 7.3 ** You can use this code to help in verifying other implementations. */ +#include "limits.h" /* for CHAR_BIT */ #include "stdio.h" /* for stdout, stderr, perror */ #include "string.h" /* for strcpy */ #include "sys/types.h" /* for time_t */ @@ -24,6 +25,25 @@ static char elsieid[] = "@(#)zdump.c 7.3 #define FALSE 0 #endif /* !defined FALSE */ +#ifndef TYPE_BIT +#define TYPE_BIT(type) (sizeof (type) * CHAR_BIT) +#endif /* !defined TYPE_BIT */ + +#ifndef TYPE_SIGNED +#define TYPE_SIGNED(type) (((type) -1) < 0) +#endif /* !defined TYPE_SIGNED */ + +#ifndef INT_STRLEN_MAXIMUM +/* +** 302 / 1000 is log10(2.0) rounded up. +** Subtract one for the sign bit if the type is signed; +** add one for integer division truncation; +** add one more for a minus sign if the type is signed. +*/ +#define INT_STRLEN_MAXIMUM(type) \ + ((TYPE_BIT(type) - TYPE_SIGNED(type)) * 302 / 1000 + 1 + TYPE_SIGNED(type)) +#endif /* !defined INT_STRLEN_MAXIMUM */ + #ifndef EXIT_SUCCESS #define EXIT_SUCCESS 0 #endif /* !defined EXIT_SUCCESS */ @@ -56,10 +76,18 @@ static char elsieid[] = "@(#)zdump.c 7.3 #define TM_YEAR_BASE 1900 #endif /* !defined TM_YEAR_BASE */ +#ifndef DAYSPERWEEK +#define DAYSPERWEEK 7 +#endif /* !defined DAYSPERWEEK */ + #ifndef DAYSPERNYEAR #define DAYSPERNYEAR 365 #endif /* !defined DAYSPERNYEAR */ +#ifndef MONSPERYEAR +#define MONSPERYEAR 12 +#endif /* !defined MONSPERYEAR */ + #ifndef isleap #define isleap(y) ((((y) % 4) == 0 && ((y) % 100) != 0) || ((y) % 400) == 0) #endif /* !defined isleap */ @@ -128,6 +156,9 @@ static long delta P((struct tm * newp, s static time_t hunt P((char * name, time_t lot, time_t hit)); static size_t longest; static char * progname; +static char * my_asctime_r P((const struct tm *, char *)); +static char * my_asctime P((const struct tm *)); +static void showtime P((time_t, const struct tm *)); static void show P((char * zone, time_t t, int v)); int @@ -140,16 +171,17 @@ char * argv[]; register int vflag; register char * cutoff; register int cutyear; - register long cuttime; + register time_t lasttime; char ** fakeenv; time_t now; time_t t; time_t newt; time_t hibit; + time_t time_t_min; + time_t time_t_max; struct tm tm; - struct tm newtm; + struct tm * newtm; - INITIALIZE(cuttime); #if HAVE_GETTEXT - 0 (void) setlocale(LC_MESSAGES, ""); #ifdef TZ_DOMAINDIR @@ -176,22 +208,39 @@ _("%s: usage is %s [ --version ] [ -v ] argv[0], argv[0]); (void) exit(EXIT_FAILURE); } + for (hibit = 1; (hibit << 1) != 0; hibit <<= 1) + continue; + time_t_max = ~((time_t) 0); + if (TYPE_SIGNED(time_t)) + time_t_max &= ~hibit; + time_t_min = TYPE_SIGNED(time_t) ? hibit : 0; + (void) time(&now); if (cutoff != NULL) { int y; cutyear = atoi(cutoff); - cuttime = 0; + lasttime = 0; for (y = EPOCH_YEAR; y < cutyear; ++y) - cuttime += DAYSPERNYEAR + isleap(y); - cuttime *= SECSPERHOUR * HOURSPERDAY; + lasttime += DAYSPERNYEAR + isleap(y); + lasttime *= SECSPERHOUR * HOURSPERDAY; + lasttime -= TYPE_SIGNED(time_t) || lasttime != 0; + } else { + /* + ** By default, the cutoff is the maximum time value, + ** or about 50 years from now, whichever comes first. + ** This prevents us from generating too much output if + ** time_t is 64 bits. 31556952 is 365.2425 * 24 * 60 + ** * 60, but don't trust the C compiler to calculate + ** it as there may be bugs or rounding problems. + */ + long seconds_per_mean_gregorian_year = 31556952; + time_t now50 = now + 50 * seconds_per_mean_gregorian_year - 1; + lasttime = now < now50 ? now50 : time_t_max; } - (void) time(&now); longest = 0; for (i = optind; i < argc; ++i) if (strlen(argv[i]) > longest) longest = strlen(argv[i]); - for (hibit = 1; (hibit << 1) != 0; hibit <<= 1) - continue; { register int from; register int to; @@ -224,40 +273,47 @@ _("%s: usage is %s [ --version ] [ -v ] /* ** Get lowest value of t. */ - t = hibit; - if (t > 0) /* time_t is unsigned */ - t = 0; + t = time_t_min; show(argv[i], t, TRUE); t += SECSPERHOUR * HOURSPERDAY; show(argv[i], t, TRUE); + /* + ** But don't go below - 2**32 in the following loop, + ** as that predates standard time, would cause us to + ** generate too much output, and could dump core if + ** localtime returns NULL. The earliest known use of + ** standard time is 1837 (Iceland) and - 2**32 is a + ** bit before that. + */ + if ((t + 1) / 2 < -2147483647) { + t = -1 - 2147483647; + t *= 2; + } tm = *localtime(&t); (void) strncpy(buf, abbr(&tm), (sizeof buf) - 1); for ( ; ; ) { - if (cutoff != NULL && t >= cuttime) + if (lasttime < t) break; newt = t + SECSPERHOUR * 12; - if (cutoff != NULL && newt >= cuttime) + if (lasttime < newt) break; if (newt <= t) break; - newtm = *localtime(&newt); - if (delta(&newtm, &tm) != (newt - t) || - newtm.tm_isdst != tm.tm_isdst || - strcmp(abbr(&newtm), buf) != 0) { + newtm = localtime(&newt); + if (newtm == NULL) + break; + if (delta(newtm, &tm) != (newt - t) || + newtm->tm_isdst != tm.tm_isdst || + strcmp(abbr(newtm), buf) != 0) { newt = hunt(argv[i], t, newt); - newtm = *localtime(&newt); - (void) strncpy(buf, abbr(&newtm), + newtm = localtime(&newt); + (void) strncpy(buf, abbr(newtm), (sizeof buf) - 1); } t = newt; - tm = newtm; + tm = *newtm; } - /* - ** Get highest value of t. - */ - t = ~((time_t) 0); - if (t < 0) /* time_t is signed */ - t &= ~hibit; + t = time_t_max; t -= SECSPERHOUR * HOURSPERDAY; show(argv[i], t, TRUE); t += SECSPERHOUR * HOURSPERDAY; @@ -323,7 +379,7 @@ struct tm * oldp; return -delta(oldp, newp); result = 0; for (tmy = oldp->tm_year; tmy < newp->tm_year; ++tmy) - result += DAYSPERNYEAR + isleap(tmy + TM_YEAR_BASE); + result += DAYSPERNYEAR + isleap(tmy + (long) TM_YEAR_BASE); result += newp->tm_yday - oldp->tm_yday; result *= HOURSPERDAY; result += newp->tm_hour - oldp->tm_hour; @@ -334,6 +390,80 @@ struct tm * oldp; return result; } +/* +** my_asctime_r and my_asctime are stolen from asctime.c. Unlike +** Standard C asctime, my_asctime has well-defined behavior even if +** the year is less than -999 or greater than 9999. +*/ + +static char * +my_asctime_r(timeptr, buf) +register const struct tm * timeptr; +char * buf; +{ + static const char wday_name[][3] = { + "Sun", "Mon", "Tue", "Wed", "Thu", "Fri", "Sat" + }; + static const char mon_name[][3] = { + "Jan", "Feb", "Mar", "Apr", "May", "Jun", + "Jul", "Aug", "Sep", "Oct", "Nov", "Dec" + }; + register const char * wn; + register const char * mn; + + if (timeptr->tm_wday < 0 || timeptr->tm_wday >= DAYSPERWEEK) + wn = "???"; + else wn = wday_name[timeptr->tm_wday]; + if (timeptr->tm_mon < 0 || timeptr->tm_mon >= MONSPERYEAR) + mn = "???"; + else mn = mon_name[timeptr->tm_mon]; + /* + ** The X3J11-suggested format is + ** "%.3s %.3s%3d %.2d:%.2d:%.2d %d\n" + ** Use "%02d", as it is a bit more portable than "%.2d". + */ + (void) sprintf(buf, "%.3s %.3s%3d %02d:%02d:%02d %ld\n", + wn, mn, + timeptr->tm_mday, timeptr->tm_hour, + timeptr->tm_min, timeptr->tm_sec, + timeptr->tm_year + (long) TM_YEAR_BASE); + return buf; +} + +static char * +my_asctime(timeptr) +register const struct tm * timeptr; +{ + /* + ** Big enough for something such as + ** ??? ???-2147483648 -2147483648:-2147483648:-2147483648 -2147483648\n + ** (two three-character abbreviations, five strings denoting integers, + ** three explicit spaces, two explicit colons, a newline, + ** and a trailing ASCII nul). + */ + static char result[3 * 2 + 5 * INT_STRLEN_MAXIMUM(int) + + 3 + 2 + 1 + 1]; + + return my_asctime_r(timeptr, result); +} + +static void +showtime(t, timeptr) +time_t t; +const struct tm * timeptr; +{ + if (timeptr != NULL) { + const char *p; + for (p = my_asctime (timeptr); *p != '\n'; p++) + putchar (*p); + } else { + if (TYPE_SIGNED(time_t)) + printf ("%ld\n", (long) t); + else + printf ("%lu\n", (unsigned long) t); + } +} + static void show(zone, t, v) char * zone; @@ -343,13 +473,15 @@ int v; struct tm * tmp; (void) printf("%-*s ", (int) longest, zone); - if (v) - (void) printf("%.24s UTC = ", asctime(gmtime(&t))); + if (v) { + showtime(t, gmtime(&t)); + (void) printf(" UTC = "); + } tmp = localtime(&t); - (void) printf("%.24s", asctime(tmp)); + showtime(t, tmp); if (*abbr(tmp) != '\0') (void) printf(" %s", abbr(tmp)); - if (v) { + if (v && tmp != NULL) { (void) printf(" isdst=%d", tmp->tm_isdst); #ifdef TM_GMTOFF (void) printf(" gmtoff=%ld", tmp->TM_GMTOFF); @@ -365,7 +497,7 @@ struct tm * tmp; register char * result; static char nada; - if (tmp->tm_isdst != 0 && tmp->tm_isdst != 1) + if (!tmp || (tmp->tm_isdst != 0 && tmp->tm_isdst != 1)) return &nada; result = tzname[tmp->tm_isdst]; return (result == NULL) ? &nada : result; =================================================================== RCS file: RCS/zdump.8,v retrieving revision 2004.1 retrieving revision 2004.1.0.1 diff -pu -r2004.1 -r2004.1.0.1 --- zdump.8 2003/09/16 11:12:41 2004.1 +++ zdump.8 2004/06/15 05:24:21 2004.1.0.1 @@ -40,6 +40,12 @@ otherwise. .TP .BI "\-c " cutoffyear Cut off the verbose output near the start of the given year. +By default, verbose output is cut off approximately 50 years after the +current date and time. .SH "SEE ALSO" newctime(3), tzfile(5), zic(8) +.SH "BUGS" +Time discontinuities are detected via a heuristic that suffices for +all historically valid discontinuities, but one can construct +artificial time zones for which the heuristic fails. .\" @(#)zdump.8 7.4