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.