Proposed localtime.c changes for long years

[Paul Eggert]

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 at 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.