Proposed strftime.c and localtime.c changes for long years
Attached are changes (based on those circulated on the list earlier by Paul Eggert) to strftime.c and localtime.c; these changes allow for the possibility that adding an int year to TM_YEAR_BASE might overflow an int, so the relevant math is done with longs. There are also a couple of cosmetic changes. Note that no changes to the way strftime handles two-digit year output requests are being (deliberately) made at this point. I hope to update the ftp bundle to incorporate these changes in about a week; I'd welcome any feedback before then. --ado ------- localtime.c ------- *** /tmp/geta17327 Tue Sep 7 08:12:24 2004 --- /tmp/getb17327 Tue Sep 7 08:12:24 2004 *************** *** 5,11 **** #ifndef lint #ifndef NOID ! static char elsieid[] = "@(#)localtime.c 7.78"; #endif /* !defined NOID */ #endif /* !defined lint */ --- 5,11 ---- #ifndef lint #ifndef NOID ! static char elsieid[] = "@(#)localtime.c 7.79"; #endif /* !defined NOID */ #endif /* !defined lint */ *************** *** 134,139 **** --- 134,142 ---- static void localsub P((const time_t * timep, long offset, struct tm * tmp)); static int increment_overflow P((int * number, int delta)); + static int long_increment_overflow P((long * number, int delta)); + static int long_normalize_overflow P((long * tensptr, + int * unitsptr, int base)); static int normalize_overflow P((int * tensptr, int * unitsptr, int base)); static void settzname P((void)); *************** *** 1149,1155 **** register const struct lsinfo * lp; register long days; register long rem; ! register int y; register int yleap; register const int * ip; register long corr; --- 1152,1158 ---- register const struct lsinfo * lp; register long days; register long rem; ! register long y; register int yleap; register const int * ip; register long corr; *************** *** 1218,1224 **** y = EPOCH_YEAR; #define LEAPS_THRU_END_OF(y) ((y) / 4 - (y) / 100 + (y) / 400) while (days < 0 || days >= (long) year_lengths[yleap = isleap(y)]) { ! register int newy; newy = y + days / DAYSPERNYEAR; if (days < 0) --- 1221,1227 ---- y = EPOCH_YEAR; #define LEAPS_THRU_END_OF(y) ((y) / 4 - (y) / 100 + (y) / 400) while (days < 0 || days >= (long) year_lengths[yleap = isleap(y)]) { ! register long newy; newy = y + days / DAYSPERNYEAR; if (days < 0) *************** *** 1259,1264 **** --- 1262,1268 ---- char * buf; { struct tm tm; + extern char * asctime_r(); return asctime_r(localtime_r(timep, &tm), buf); } *************** *** 1294,1299 **** --- 1298,1315 ---- } static int + long_increment_overflow(number, delta) + long * number; + int delta; + { + long number0; + + number0 = *number; + *number += delta; + return (*number < number0) != (delta < 0); + } + + static int normalize_overflow(tensptr, unitsptr, base) int * const tensptr; int * const unitsptr; *************** *** 1309,1314 **** --- 1325,1345 ---- } static int + long_normalize_overflow(tensptr, unitsptr, base) + long * const tensptr; + int * const unitsptr; + const int base; + { + register int tensdelta; + + tensdelta = (*unitsptr >= 0) ? + (*unitsptr / base) : + (-1 - (-1 - *unitsptr) / base); + *unitsptr -= tensdelta * base; + return long_increment_overflow(tensptr, tensdelta); + } + + static int tmcomp(atmp, btmp) register const struct tm * const atmp; register const struct tm * const btmp; *************** *** 1335,1342 **** register const struct state * sp; register int dir; register int bits; ! register int i, j ; register int saved_seconds; time_t newt; time_t t; struct tm yourtm, mytm; --- 1366,1375 ---- register const struct state * sp; register int dir; register int bits; ! register int i, j; register int saved_seconds; + register long li; + long y; time_t newt; time_t t; struct tm yourtm, mytm; *************** *** 1352,1393 **** return WRONG; if (normalize_overflow(&yourtm.tm_mday, &yourtm.tm_hour, HOURSPERDAY)) return WRONG; ! if (normalize_overflow(&yourtm.tm_year, &yourtm.tm_mon, MONSPERYEAR)) return WRONG; /* ! ** Turn yourtm.tm_year into an actual year number for now. ** It is converted back to an offset from TM_YEAR_BASE later. */ ! if (increment_overflow(&yourtm.tm_year, TM_YEAR_BASE)) return WRONG; while (yourtm.tm_mday <= 0) { ! if (increment_overflow(&yourtm.tm_year, -1)) return WRONG; ! i = yourtm.tm_year + (1 < yourtm.tm_mon); ! yourtm.tm_mday += year_lengths[isleap(i)]; } while (yourtm.tm_mday > DAYSPERLYEAR) { ! i = yourtm.tm_year + (1 < yourtm.tm_mon); ! yourtm.tm_mday -= year_lengths[isleap(i)]; ! if (increment_overflow(&yourtm.tm_year, 1)) return WRONG; } for ( ; ; ) { ! i = mon_lengths[isleap(yourtm.tm_year)][yourtm.tm_mon]; if (yourtm.tm_mday <= i) break; yourtm.tm_mday -= i; if (++yourtm.tm_mon >= MONSPERYEAR) { yourtm.tm_mon = 0; ! if (increment_overflow(&yourtm.tm_year, 1)) return WRONG; } } ! if (increment_overflow(&yourtm.tm_year, -TM_YEAR_BASE)) return WRONG; if (yourtm.tm_sec >= 0 && yourtm.tm_sec < SECSPERMIN) saved_seconds = 0; ! else if (yourtm.tm_year + TM_YEAR_BASE < EPOCH_YEAR) { /* ** We can't set tm_sec to 0, because that might push the ** time below the minimum representable time. --- 1385,1438 ---- return WRONG; if (normalize_overflow(&yourtm.tm_mday, &yourtm.tm_hour, HOURSPERDAY)) return WRONG; ! y = yourtm.tm_year; ! /* ! ** Hands off tm_year for a while. ! */ ! #define tm_year USE_Y_NOT_YOURTM_TM_YEAR ! if (long_normalize_overflow(&y, &yourtm.tm_mon, MONSPERYEAR)) return WRONG; /* ! ** Turn y into an actual year number for now. ** It is converted back to an offset from TM_YEAR_BASE later. */ ! if (long_increment_overflow(&y, TM_YEAR_BASE)) return WRONG; while (yourtm.tm_mday <= 0) { ! if (long_increment_overflow(&y, -1)) return WRONG; ! li = y + (1 < yourtm.tm_mon); ! yourtm.tm_mday += year_lengths[isleap(li)]; } while (yourtm.tm_mday > DAYSPERLYEAR) { ! li = y + (1 < yourtm.tm_mon); ! yourtm.tm_mday -= year_lengths[isleap(li)]; ! if (long_increment_overflow(&y, 1)) return WRONG; } for ( ; ; ) { ! i = mon_lengths[isleap(y)][yourtm.tm_mon]; if (yourtm.tm_mday <= i) break; yourtm.tm_mday -= i; if (++yourtm.tm_mon >= MONSPERYEAR) { yourtm.tm_mon = 0; ! if (long_increment_overflow(&y, 1)) return WRONG; } } ! if (long_increment_overflow(&y, -TM_YEAR_BASE)) return WRONG; + /* + ** Hands back on tm_year. + */ + #undef tm_year + yourtm.tm_year = y; + if (yourtm.tm_year != y) + return WRONG; if (yourtm.tm_sec >= 0 && yourtm.tm_sec < SECSPERMIN) saved_seconds = 0; ! else if (y + TM_YEAR_BASE < EPOCH_YEAR) { /* ** We can't set tm_sec to 0, because that might push the ** time below the minimum representable time. ------- strftime.c ------- *** /tmp/geta17306 Tue Sep 7 08:12:07 2004 --- /tmp/getb17306 Tue Sep 7 08:12:07 2004 *************** *** 1,6 **** #ifndef lint #ifndef NOID ! static char elsieid[] = "@(#)strftime.c 7.64"; /* ** Based on the UCB version with the ID appearing below. ** This is ANSIish only when "multibyte character == plain character". --- 1,6 ---- #ifndef lint #ifndef NOID ! static char elsieid[] = "@(#)strftime.c 7.66"; /* ** Based on the UCB version with the ID appearing below. ** This is ANSIish only when "multibyte character == plain character". *************** *** 108,113 **** --- 108,114 ---- static char * _add P((const char *, char *, const char *)); static char * _conv P((int, const char *, char *, const char *)); + static char * _lconv P((long, const char *, char *, const char *)); static char * _fmt P((const char *, const struct tm *, char *, const char *, int *)); size_t strftime P((char *, size_t, const char *, const struct tm *)); *************** *** 210,217 **** ** something completely different. ** (ado, 1993-05-24) */ ! pt = _conv((t->tm_year + TM_YEAR_BASE) / 100, ! "%02d", pt, ptlim); continue; case 'c': { --- 211,219 ---- ** something completely different. ** (ado, 1993-05-24) */ ! pt = _lconv((t->tm_year + ! (long) TM_YEAR_BASE) / 100, ! "%02ld", pt, ptlim); continue; case 'c': { *************** *** 379,390 **** ** (ado, 1996-01-02) */ { ! int year; int yday; int wday; int w; ! year = t->tm_year + TM_YEAR_BASE; yday = t->tm_yday; wday = t->tm_wday; for ( ; ; ) { --- 381,393 ---- ** (ado, 1996-01-02) */ { ! long year; int yday; int wday; int w; ! year = t->tm_year; ! year += TM_YEAR_BASE; yday = t->tm_yday; wday = t->tm_wday; for ( ; ; ) { *************** *** 426,436 **** DAYSPERNYEAR; } #ifdef XPG4_1994_04_09 ! if ((w == 52 ! && t->tm_mon == TM_JANUARY) ! || (w == 1 ! && t->tm_mon == TM_DECEMBER)) ! w = 53; #endif /* defined XPG4_1994_04_09 */ if (*format == 'V') pt = _conv(w, "%02d", --- 429,439 ---- DAYSPERNYEAR; } #ifdef XPG4_1994_04_09 ! if ((w == 52 && ! t->tm_mon == TM_JANUARY) || ! (w == 1 && ! t->tm_mon == TM_DECEMBER)) ! w = 53; #endif /* defined XPG4_1994_04_09 */ if (*format == 'V') pt = _conv(w, "%02d", *************** *** 437,446 **** pt, ptlim); else if (*format == 'g') { *warnp = IN_ALL; ! pt = _conv(year % 100, "%02d", pt, ptlim); - } else pt = _conv(year, "%04d", - pt, ptlim); } continue; case 'v': --- 440,449 ---- pt, ptlim); else if (*format == 'g') { *warnp = IN_ALL; ! pt = _conv(int(year % 100), ! "%02d", pt, ptlim); ! } else pt = _lconv(year, "%04ld", pt, ptlim); } continue; case 'v': *************** *** 477,488 **** continue; case 'y': *warnp = IN_ALL; ! pt = _conv((t->tm_year + TM_YEAR_BASE) % 100, "%02d", pt, ptlim); continue; case 'Y': ! pt = _conv(t->tm_year + TM_YEAR_BASE, "%04d", ! pt, ptlim); continue; case 'Z': #ifdef TM_ZONE --- 480,493 ---- continue; case 'y': *warnp = IN_ALL; ! pt = _conv( ! (int) ((t->tm_year + ! (long) TM_YEAR_BASE) % 100), "%02d", pt, ptlim); continue; case 'Y': ! pt = _lconv(t->tm_year + (long) TM_YEAR_BASE, ! "%04ld", pt, ptlim); continue; case 'Z': #ifdef TM_ZONE *************** *** 582,587 **** --- 587,605 ---- char buf[INT_STRLEN_MAXIMUM(int) + 1]; (void) sprintf(buf, format, n); + return _add(buf, pt, ptlim); + } + + static char * + _lconv(n, format, pt, ptlim) + const long n; + const char * const format; + char * const pt; + const char * const ptlim; + { + char buf[INT_STRLEN_MAXIMUM(long) + 1]; + + (void) sprintf(buf, format, n); return _add(buf, pt, ptlim); }
Thanks for looking into that. I'm responding to the proposed localtime.c changes; a separate email will address the strftime.c changes. "Olson, Arthur David (NIH/NCI)" <olsona@dc37a.nci.nih.gov> writes:
extern char * asctime_r();
This could be a problem if asctime_r is a macro in time.h. Also, it should probably be moved to private.h, since it's a header fixup (like errno) that is useful in other modules. (Also, being an old-timer I have some qualms about declaring externs inside blocks, due to bugs in older compilers.)
! #define tm_year USE_Y_NOT_YOURTM_TM_YEAR ... + #undef tm_year
I'd remove this trick. Strictly speaking, the C Standard doesn't allow it: it says that identifiers in standard headers are reserved for use as macro names. More practically, I worry that some <time.h> somewhere might #define tm_year to something else, and the #define/#undef trick won't work there. The #define/#undef isn't needed for the code to run, so I'd leave it out. ! ** Turn y into an actual year number for now. ** It is converted back to an offset from TM_YEAR_BASE later. This hack has bothered me for a while, due to the possibility of overflow when adding TM_YEAR_BASE. It's a minor point, but how about if we add a macro like this and use it instead? #define isleap_tm(y) \ ((y) % 4 == 0 && ((y) % 100 != 0 || (y) % 400 == 100 || (y) % 400 == -300)) isleap_tm(tm_year) equals isleap(tm_year + TM_YEAR_BASE), except that it returns the correct answer even if the addition would overflow. Doing this will require changing one "y + TM_YEAR_BASE < EPOCH_YEAR" to "y < EPOCH_YEAR - TM_YEAR_BASE" (to avoid a possible overflow on addition) in localtime.c. Here's a patch embodying these proposed changes. =================================================================== RCS file: RCS/tzfile.h,v retrieving revision 1997.9 retrieving revision 1997.9.0.1 diff -pu -r1997.9 -r1997.9.0.1 --- tzfile.h 1997/12/29 14:31:51 1997.9 +++ tzfile.h 2004/09/07 18:32:29 1997.9.0.1 @@ -159,9 +159,13 @@ struct tzhead { /* ** Accurate only for the past couple of centuries; ** that will probably do. +** isleap_tm(tm_year) equals isleap(tm_year + TM_YEAR_BASE), +** except that it returns the right answer even if the addition would overflow. */ #define isleap(y) (((y) % 4) == 0 && (((y) % 100) != 0 || ((y) % 400) == 0)) +#define isleap_tm(y) \ + ((y) % 4 == 0 && ((y) % 100 != 0 || (y) % 400 == 100 || (y) % 400 == -300)) #ifndef USG =================================================================== RCS file: RCS/private.h,v retrieving revision 2003.5 retrieving revision 2003.5.1.1 diff -pu -r2003.5 -r2003.5.1.1 --- private.h 2003/12/15 14:36:35 2003.5 +++ private.h 2004/09/07 18:32:29 2003.5.1.1 @@ -195,6 +195,15 @@ extern int errno; #endif /* !defined errno */ /* +** Some time.h implementations don't declare asctime_r. +** Others might define it as a macro. +** Fix the former without affecting the latter. +*/ +#ifndef asctime_r +extern char * asctime_r(); +#endif + +/* ** Private function declarations. */ char * icalloc P((int nelem, int elsize)); =================================================================== RCS file: RCS/localtime.c,v retrieving revision 2003.5.1.1 retrieving revision 2003.5.0.3 diff -pu -r2003.5.1.1 -r2003.5.0.3 --- localtime.c 2004/09/07 12:21:29 2003.5.1.1 +++ localtime.c 2004/09/07 18:32:29 2003.5.0.3 @@ -1262,7 +1262,6 @@ const time_t * const timep; char * buf; { struct tm tm; - extern char * asctime_r(); return asctime_r(localtime_r(timep, &tm), buf); } @@ -1386,32 +1385,22 @@ const int do_norm_secs; if (normalize_overflow(&yourtm.tm_mday, &yourtm.tm_hour, HOURSPERDAY)) return WRONG; y = yourtm.tm_year; -/* -** Hands off tm_year for a while. -*/ -#define tm_year USE_Y_NOT_YOURTM_TM_YEAR if (long_normalize_overflow(&y, &yourtm.tm_mon, MONSPERYEAR)) return WRONG; - /* - ** Turn y into an actual year number for now. - ** It is converted back to an offset from TM_YEAR_BASE later. - */ - if (long_increment_overflow(&y, TM_YEAR_BASE)) - return WRONG; while (yourtm.tm_mday <= 0) { if (long_increment_overflow(&y, -1)) return WRONG; li = y + (1 < yourtm.tm_mon); - yourtm.tm_mday += year_lengths[isleap(li)]; + yourtm.tm_mday += year_lengths[isleap_tm(li)]; } while (yourtm.tm_mday > DAYSPERLYEAR) { li = y + (1 < yourtm.tm_mon); - yourtm.tm_mday -= year_lengths[isleap(li)]; + yourtm.tm_mday -= year_lengths[isleap_tm(li)]; if (long_increment_overflow(&y, 1)) return WRONG; } for ( ; ; ) { - i = mon_lengths[isleap(y)][yourtm.tm_mon]; + i = mon_lengths[isleap_tm(y)][yourtm.tm_mon]; if (yourtm.tm_mday <= i) break; yourtm.tm_mday -= i; @@ -1421,18 +1410,12 @@ const int do_norm_secs; return WRONG; } } - if (long_increment_overflow(&y, -TM_YEAR_BASE)) - return WRONG; -/* -** Hands back on tm_year. -*/ -#undef tm_year yourtm.tm_year = y; if (yourtm.tm_year != y) return WRONG; if (yourtm.tm_sec >= 0 && yourtm.tm_sec < SECSPERMIN) saved_seconds = 0; - else if (y + TM_YEAR_BASE < EPOCH_YEAR) { + else if (y < EPOCH_YEAR - TM_YEAR_BASE) { /* ** We can't set tm_sec to 0, because that might push the ** time below the minimum representable time.
The proposed strftime.c code for the %C conversion uses _lconv, but the converted value can't possibly fall outside the range INT_MIN..INT_MAX so it might be a bit more efficient to use _conv. The %C conversion expression "(t->tm_year + (long) TM_YEAR_BASE) / 100" returns the wrong result in some cases due to overflow, e.g., if t->tm_year == INT_MAX and INT_MAX==LONG_MAX. It looks like you've leaning towards having %y generate year%100 even when the year is negative, but I'd like to present a new arguemnt for generating year mod 100 instead, on the grounds of preventing buffer overruns in older code. Most programmers expect strftime %y to generate exactly two digits, in the range 00-99 as the the C Standard explicitly requires. Generating strings like "-1" or (especially) "-99" might cause errors (e.g., buffer overruns) in older code. The situation for %g is similar. %C is less straightforward, since here the C Standard requires the impossible (it says the output range must be 00-99). However, I'd argue that it's least surprising if %C is consistent with %y, i.e., if %C is (year - (year mod 100)) / 100. The following proposed patch addresses the above issues. =================================================================== RCS file: RCS/strftime.c,v retrieving revision 2001.4.1.1 retrieving revision 2001.4.0.2 diff -pu -r2001.4.1.1 -r2001.4.0.2 --- strftime.c 2004/09/07 12:21:29 2001.4.1.1 +++ strftime.c 2004/09/07 19:11:35 2001.4.0.2 @@ -211,9 +211,12 @@ label: ** something completely different. ** (ado, 1993-05-24) */ - pt = _lconv((t->tm_year + - (long) TM_YEAR_BASE) / 100, - "%02ld", pt, ptlim); + { + int c = (TM_YEAR_BASE / 100 + + t->tm_year / 100 + - (t->tm_year % 100 < 0)); + pt = _conv(c, "%02d", pt, ptlim); + } continue; case 'c': { @@ -439,9 +442,12 @@ label: pt = _conv(w, "%02d", pt, ptlim); else if (*format == 'g') { + int g = year % 100; + if (g < 0) + g += 100; *warnp = IN_ALL; - pt = _conv(int(year % 100), - "%02d", pt, ptlim); + pt = _conv(g, "%02d", + pt, ptlim); } else pt = _lconv(year, "%04ld", pt, ptlim); } @@ -479,11 +485,13 @@ label: } continue; case 'y': + { + int y = t->tm_year % 100; + if (y < 0) + y += 100; *warnp = IN_ALL; - pt = _conv( - (int) ((t->tm_year + - (long) TM_YEAR_BASE) % 100), - "%02d", pt, ptlim); + pt = _conv(y, "%02d", pt, ptlim); + } continue; case 'Y': pt = _lconv(t->tm_year + (long) TM_YEAR_BASE,
participants (2)
-
Olson, Arthur David (NIH/NCI) -
Paul Eggert