diff mbox

[[PATCH,RFC,2] 03/63] Y2038: make __tz_convert compatible with 64-bit-time

Message ID 20180418201819.15952-4-albert.aribaud@3adev.fr
State New, archived
Headers

Commit Message

Albert ARIBAUD April 18, 2018, 8:17 p.m. UTC 
  __tz_convert is used by six API functions. Making
it compatible with 64-bit time allows reusing it for
implementing 64-bit-time versions of theses six APIs.
---
 include/time.h   | 12 ++++++------
 time/gmtime.c    | 15 +++++++++++++--
 time/localtime.c | 15 +++++++++++++--
 time/offtime.c   |  8 ++++----
 time/tzfile.c    |  4 ++--
 time/tzset.c     | 24 +++++++++---------------
 6 files changed, 47 insertions(+), 31 deletions(-)

Comments

Paul Eggert April 19, 2018, 1:02 a.m. UTC | #1 
On 04/18/2018 01:17 PM, Albert ARIBAUD (3ADEV) wrote:
>   int
> -__offtime (const time_t *t, long int offset, struct tm *tp)
> +__offtime (const __time64_t t, long int offset, struct tm *tp)
>   {
>     time_t days, rem, y;
>     const unsigned short int *ip;
>   
> -  days = *t / SECS_PER_DAY;
> -  rem = *t % SECS_PER_DAY;
> +  days = t / SECS_PER_DAY;
> +  rem = t % SECS_PER_DAY;

This can overflow when time_t is 32 bits, because dividing a 64-bit 
integer by SECS_PER_DAY (i.e., by 86400) can yield an integer that does 
not fit into 32 bits. To fix this, you'll need to change the locals 
'days', 'y', 'yg' to be of type __time64_t rather than time_t.

Similarly, the local variable 't' in compute_change needs to be 
__time64_t, not time_t.

By the way, please add test cases to catch these bugs, and the bug I 
reported earlier today.

> -      tz_rules[0].change = tz_rules[1].change = (time_t) -1;
> +      tz_rules[0].change = tz_rules[1].change = (__time64_t) -1;
Instead, please just omit the cast; the cast is not needed and is more 
maintenance hassle than it is worth.
Paul Eggert April 19, 2018, 1:08 a.m. UTC | #2 
The patches to tzfile.c seem to be incomplete. Every use of time_t in 
that file (including tzfile_mtime) should be changed to __time64_t, no? 
And yet almost no instances where changed.
Florian Weimer April 19, 2018, 6:52 a.m. UTC | #3 
On 04/19/2018 03:08 AM, Paul Eggert wrote:
> The patches to tzfile.c seem to be incomplete. Every use of time_t in 
> that file (including tzfile_mtime) should be changed to __time64_t, no? 
> And yet almost no instances where changed.

The resulting parser simplification should be a separate, preliminary patch.

Albert, if you do not want to work on this, I think I have a patch for 
this somewhere.

Thanks,
Florian
Albert ARIBAUD May 2, 2018, 9:11 a.m. UTC | #4 
Hi Florian,

On Thu, 19 Apr 2018 08:52:15 +0200, Florian Weimer <fweimer@redhat.com>
wrote :

> On 04/19/2018 03:08 AM, Paul Eggert wrote:
> > The patches to tzfile.c seem to be incomplete. Every use of time_t in 
> > that file (including tzfile_mtime) should be changed to __time64_t, no? 
> > And yet almost no instances where changed.  
> 
> The resulting parser simplification should be a separate, preliminary patch.
> 
> Albert, if you do not want to work on this, I think I have a patch for 
> this somewhere.

Florian, if you have such a patch, please post it. Unless it's already
gotten pulled in by the time I post RFC 3, I will include it.

> Thanks,
> Florian

Cordialement,
Albert ARIBAUD
3ADEV
diff mbox

Patch

diff --git a/include/time.h b/include/time.h
index ba2c67a829..6ffa9b2efc 100644
--- a/include/time.h
+++ b/include/time.h
@@ -40,14 +40,14 @@  extern int __use_tzfile attribute_hidden;
 
 extern void __tzfile_read (const char *file, size_t extra,
 			   char **extrap) attribute_hidden;
-extern void __tzfile_compute (time_t timer, int use_localtime,
+extern void __tzfile_compute (__time64_t timer, int use_localtime,
 			      long int *leap_correct, int *leap_hit,
 			      struct tm *tp) attribute_hidden;
 extern void __tzfile_default (const char *std, const char *dst,
 			      long int stdoff, long int dstoff)
   attribute_hidden;
 extern void __tzset_parse_tz (const char *tz) attribute_hidden;
-extern void __tz_compute (time_t timer, struct tm *tm, int use_localtime)
+extern void __tz_compute (__time64_t timer, struct tm *tm, int use_localtime)
   __THROW attribute_hidden;
 
 /* Subroutine of `mktime'.  Return the `time_t' representation of TP and
@@ -64,11 +64,11 @@  extern struct tm *__gmtime_r (const time_t *__restrict __timer,
 			      struct tm *__restrict __tp);
 libc_hidden_proto (__gmtime_r)
 
-/* Compute the `struct tm' representation of *T,
+/* Compute the `struct tm' representation of T,
    offset OFFSET seconds east of UTC,
    and store year, yday, mon, mday, wday, hour, min, sec into *TP.
    Return nonzero if successful.  */
-extern int __offtime (const time_t *__timer,
+extern int __offtime (const __time64_t __timer,
 		      long int __offset,
 		      struct tm *__tp) attribute_hidden;
 
@@ -77,8 +77,8 @@  extern char *__asctime_r (const struct tm *__tp, char *__buf)
 extern void __tzset (void) attribute_hidden;
 
 /* Prototype for the internal function to get information based on TZ.  */
-extern struct tm *__tz_convert (const time_t *timer, int use_localtime,
-				struct tm *tp) attribute_hidden;
+extern struct tm *__tz_convert (const __time64_t timer, int use_localtime,
+			        struct tm *tp) attribute_hidden;
 
 extern int __nanosleep (const struct timespec *__requested_time,
 			struct timespec *__remaining);
diff --git a/time/gmtime.c b/time/gmtime.c
index dc33b3e68a..e38f0cbcc0 100644
--- a/time/gmtime.c
+++ b/time/gmtime.c
@@ -17,13 +17,19 @@ 
    <http://www.gnu.org/licenses/>.  */
 
 #include <time.h>
+#include <errno.h>
 
 /* Return the `struct tm' representation of *T in UTC,
    using *TP to store the result.  */
 struct tm *
 __gmtime_r (const time_t *t, struct tm *tp)
 {
-  return __tz_convert (t, 0, tp);
+  if (t == NULL)
+    {
+      __set_errno (EINVAL);
+      return NULL;
+    }
+  return __tz_convert (*t, 0, tp);
 }
 libc_hidden_def (__gmtime_r)
 weak_alias (__gmtime_r, gmtime_r)
@@ -33,5 +39,10 @@  weak_alias (__gmtime_r, gmtime_r)
 struct tm *
 gmtime (const time_t *t)
 {
-  return __tz_convert (t, 0, &_tmbuf);
+  if (t == NULL)
+    {
+      __set_errno (EINVAL);
+      return NULL;
+    }
+  return __tz_convert (*t, 0, &_tmbuf);
 }
diff --git a/time/localtime.c b/time/localtime.c
index 8684a8a971..362fdb3c21 100644
--- a/time/localtime.c
+++ b/time/localtime.c
@@ -17,6 +17,7 @@ 
    <http://www.gnu.org/licenses/>.  */
 
 #include <time.h>
+#include <errno.h>
 
 /* The C Standard says that localtime and gmtime return the same pointer.  */
 struct tm _tmbuf;
@@ -27,7 +28,12 @@  struct tm _tmbuf;
 struct tm *
 __localtime_r (const time_t *t, struct tm *tp)
 {
-  return __tz_convert (t, 1, tp);
+  if (t == NULL)
+    {
+      __set_errno (EINVAL);
+      return NULL;
+    }
+  return __tz_convert (*t, 1, tp);
 }
 weak_alias (__localtime_r, localtime_r)
 
@@ -36,6 +42,11 @@  weak_alias (__localtime_r, localtime_r)
 struct tm *
 localtime (const time_t *t)
 {
-  return __tz_convert (t, 1, &_tmbuf);
+  if (t == NULL)
+    {
+      __set_errno (EINVAL);
+      return NULL;
+    }
+  return __tz_convert (*t, 1, &_tmbuf);
 }
 libc_hidden_def (localtime)
diff --git a/time/offtime.c b/time/offtime.c
index 04c48389fc..59ed38f9fa 100644
--- a/time/offtime.c
+++ b/time/offtime.c
@@ -21,18 +21,18 @@ 
 #define	SECS_PER_HOUR	(60 * 60)
 #define	SECS_PER_DAY	(SECS_PER_HOUR * 24)
 
-/* Compute the `struct tm' representation of *T,
+/* Compute the `struct tm' representation of T,
    offset OFFSET seconds east of UTC,
    and store year, yday, mon, mday, wday, hour, min, sec into *TP.
    Return nonzero if successful.  */
 int
-__offtime (const time_t *t, long int offset, struct tm *tp)
+__offtime (const __time64_t t, long int offset, struct tm *tp)
 {
   time_t days, rem, y;
   const unsigned short int *ip;
 
-  days = *t / SECS_PER_DAY;
-  rem = *t % SECS_PER_DAY;
+  days = t / SECS_PER_DAY;
+  rem = t % SECS_PER_DAY;
   rem += offset;
   while (rem < 0)
     {
diff --git a/time/tzfile.c b/time/tzfile.c
index 3e39723148..9560eda8b4 100644
--- a/time/tzfile.c
+++ b/time/tzfile.c
@@ -630,7 +630,7 @@  __tzfile_default (const char *std, const char *dst,
 }
 
 void
-__tzfile_compute (time_t timer, int use_localtime,
+__tzfile_compute (__time64_t timer, int use_localtime,
 		  long int *leap_correct, int *leap_hit,
 		  struct tm *tp)
 {
@@ -685,7 +685,7 @@  __tzfile_compute (time_t timer, int use_localtime,
 
 	  /* Convert to broken down structure.  If this fails do not
 	     use the string.  */
-	  if (__glibc_unlikely (! __offtime (&timer, 0, tp)))
+	  if (__glibc_unlikely (! __offtime (timer, 0, tp)))
 	    goto use_last;
 
 	  /* Use the rules from the TZ string to compute the change.  */
diff --git a/time/tzset.c b/time/tzset.c
index b51786704a..efbb47716f 100644
--- a/time/tzset.c
+++ b/time/tzset.c
@@ -16,7 +16,6 @@ 
    <http://www.gnu.org/licenses/>.  */
 
 #include <ctype.h>
-#include <errno.h>
 #include <libc-lock.h>
 #include <stdbool.h>
 #include <stddef.h>
@@ -55,7 +54,7 @@  typedef struct
 
     /* We cache the computed time of change for a
        given year so we don't have to recompute it.  */
-    time_t change;	/* When to change to this zone.  */
+    __time64_t change;	/* When to change to this zone.  */
     int computed_for;	/* Year above is computed for.  */
   } tz_rule;
 
@@ -416,7 +415,7 @@  tzset_internal (int always)
       tz_rules[0].name = tz_rules[1].name = "UTC";
       if (J0 != 0)
 	tz_rules[0].type = tz_rules[1].type = J0;
-      tz_rules[0].change = tz_rules[1].change = (time_t) -1;
+      tz_rules[0].change = tz_rules[1].change = (__time64_t) -1;
       update_vars ();
       return;
     }
@@ -514,9 +513,10 @@  compute_change (tz_rule *rule, int year)
 
 
 /* Figure out the correct timezone for TM and set `__tzname',
-   `__timezone', and `__daylight' accordingly.  */
+   `__timezone', and `__daylight' accordingly.
+   NOTE: this takes a __time64_t value, so passing a __time_t value is OK. */
 void
-__tz_compute (time_t timer, struct tm *tm, int use_localtime)
+__tz_compute (__time64_t timer, struct tm *tm, int use_localtime)
 {
   compute_change (&tz_rules[0], 1900 + tm->tm_year);
   compute_change (&tz_rules[1], 1900 + tm->tm_year);
@@ -562,20 +562,14 @@  __tzset (void)
 }
 weak_alias (__tzset, tzset)
 
-/* Return the `struct tm' representation of *TIMER in the local timezone.
+/* Return the `struct tm' representation of TIMER in the local timezone.
    Use local time if USE_LOCALTIME is nonzero, UTC otherwise.  */
 struct tm *
-__tz_convert (const time_t *timer, int use_localtime, struct tm *tp)
+__tz_convert (const __time64_t timer, int use_localtime, struct tm *tp)
 {
   long int leap_correction;
   int leap_extra_secs;
 
-  if (timer == NULL)
-    {
-      __set_errno (EINVAL);
-      return NULL;
-    }
-
   __libc_lock_lock (tzset_lock);
 
   /* Update internal database according to current TZ setting.
@@ -584,14 +578,14 @@  __tz_convert (const time_t *timer, int use_localtime, struct tm *tp)
   tzset_internal (tp == &_tmbuf && use_localtime);
 
   if (__use_tzfile)
-    __tzfile_compute (*timer, use_localtime, &leap_correction,
+    __tzfile_compute (timer, use_localtime, &leap_correction,
 		      &leap_extra_secs, tp);
   else
     {
       if (! __offtime (timer, 0, tp))
 	tp = NULL;
       else
-	__tz_compute (*timer, tp, use_localtime);
+	__tz_compute (timer, tp, use_localtime);
       leap_correction = 0L;
       leap_extra_secs = 0;
     }