[PATCH v2 1/2] Y2038: make __mktime_internal compatible with __time64_t

[PATCH v2 1/2] Y2038: make __mktime_internal compatible with __time64_t

[Lukasz Majewski]

From: Paul Eggert <eggert@cs.ucla.edu>

This implies also making its callers 64-bit-time compatible
(these are mktime/localtime and timegm) and providing wrappers
for 32-bit-time userland to call.

Tested with 'make check' on x86_64-linux-gnu and i686-linux.gnu.

* include/time.h (__mktime64): Add prototype.
* include/time.h (__localtime64): Likewise.
* include/time.h (fits_in_time_t): New static function.
* time/mktime.c (__mktime64): New function.
* time/timegm.c (__timegm64): Likewise.
* time/mktime.c (mktime) [__TIMESIZE]: New wrapper function.
* time/timegm.c (timegm) [__TIMESIZE]: Likewise.

---
Applicable on top of the newest glibc's master branch
SH1: aa0e46636a5b71b609a41e9ab97134cd76ac1522

Tested with make check for x86_64, i585 and ARM setups.

Changes for v2:
Add missing space (__set_errno (EOVERFLOW)) in src/time/timegm.c
Add missing space (__set_errno (EOVERFLOW)) in src/time/mktime.c
---
 include/time.h | 36 +++++++++++++++++++++-----
 time/mktime.c  | 80 ++++++++++++++++++++++++++++++++++++++++++----------------
 time/timegm.c  | 27 +++++++++++++++++++-
 3 files changed, 114 insertions(+), 29 deletions(-)

diff --git a/include/time.h b/include/time.h
index 61dd9e180b..34e5e04820 100644
--- a/include/time.h
+++ b/include/time.h
@@ -3,6 +3,7 @@
 
 #ifndef _ISOMAC
 # include <bits/types/locale_t.h>
+# include <stdbool.h>
 
 extern __typeof (strftime_l) __strftime_l;
 libc_hidden_proto (__strftime_l)
@@ -16,6 +17,21 @@ libc_hidden_proto (localtime)
 libc_hidden_proto (strftime)
 libc_hidden_proto (strptime)
 
+#if __TIMESIZE == 64
+# define __timegm64 timegm
+# define __mktime64 mktime
+# define __timelocal64 timelocal
+#else
+extern __time64_t __timegm64 (struct tm *__tp) __THROW;
+extern __time64_t __mktime64 (struct tm *__tp) __THROW;
+/* Another name for `__mktime64'.  */
+extern __time64_t __timelocal64 (struct tm *__tp) __THROW;
+
+libc_hidden_proto (__mktime64)
+libc_hidden_proto (__timelocal64)
+#endif
+
+
 extern __typeof (clock_getres) __clock_getres;
 extern __typeof (clock_gettime) __clock_gettime;
 libc_hidden_proto (__clock_gettime)
@@ -49,13 +65,13 @@ extern void __tzset_parse_tz (const char *tz) attribute_hidden;
 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
-   normalize TP, given that a `struct tm *' maps to a `time_t' as performed
+/* Subroutine of mktime.  Return the __time64_t representation of TP and
+   normalize TP, given that a struct tm * maps to a __time64_t as performed
    by FUNC.  Record next guess for localtime-gmtime offset in *OFFSET.  */
-extern time_t __mktime_internal (struct tm *__tp,
-				 struct tm *(*__func) (const time_t *,
-						       struct tm *),
-				 long int *__offset) attribute_hidden;
+extern __time64_t __mktime_internal (struct tm *__tp,
+				     struct tm *(*__func) (const __time64_t *,
+							   struct tm *),
+				     long int *__offset) attribute_hidden;
 
 #if __TIMESIZE == 64
 # define __ctime64 ctime
@@ -155,5 +171,13 @@ extern double __difftime (time_t time1, time_t time0);
    actual clock ID.  */
 #define CLOCK_IDFIELD_SIZE	3
 
+/* Check whether a time64_t value fits in a time_t.  */
+static inline bool
+fits_in_time_t (__time64_t t64)
+{
+  time_t t = t64;
+  return t == t64;
+}
+
 #endif
 #endif
diff --git a/time/mktime.c b/time/mktime.c
index af43a6950d..5d3644e213 100644
--- a/time/mktime.c
+++ b/time/mktime.c
@@ -112,11 +112,11 @@ my_tzset (void)
    added to them, and then with another timestamp added, without
    worrying about overflow.
 
-   Much of the code uses long_int to represent time_t values, to
-   lessen the hassle of dealing with platforms where time_t is
+   Much of the code uses long_int to represent __time64_t values, to
+   lessen the hassle of dealing with platforms where __time64_t is
    unsigned, and because long_int should suffice to represent all
-   time_t values that mktime can generate even on platforms where
-   time_t is excessively wide.  */
+   __time64_t values that mktime can generate even on platforms where
+   __time64_t is excessively wide.  */
 
 #if INT_MAX <= LONG_MAX / 4 / 366 / 24 / 60 / 60
 typedef long int long_int;
@@ -144,16 +144,17 @@ shr (long_int a, int b)
 	  : a / (one << b) - (a % (one << b) < 0));
 }
 
-/* Bounds for the intersection of time_t and long_int.  */
+/* Bounds for the intersection of __time64_t and long_int.  */
 
 static long_int const mktime_min
-  = ((TYPE_SIGNED (time_t) && TYPE_MINIMUM (time_t) < TYPE_MINIMUM (long_int))
-     ? TYPE_MINIMUM (long_int) : TYPE_MINIMUM (time_t));
+  = ((TYPE_SIGNED (__time64_t)
+      && TYPE_MINIMUM (__time64_t) < TYPE_MINIMUM (long_int))
+     ? TYPE_MINIMUM (long_int) : TYPE_MINIMUM (__time64_t));
 static long_int const mktime_max
-  = (TYPE_MAXIMUM (long_int) < TYPE_MAXIMUM (time_t)
-     ? TYPE_MAXIMUM (long_int) : TYPE_MAXIMUM (time_t));
+  = (TYPE_MAXIMUM (long_int) < TYPE_MAXIMUM (__time64_t)
+     ? TYPE_MAXIMUM (long_int) : TYPE_MAXIMUM (__time64_t));
 
-verify (TYPE_IS_INTEGER (time_t));
+verify (TYPE_IS_INTEGER (__time64_t));
 
 #define EPOCH_YEAR 1970
 #define TM_YEAR_BASE 1900
@@ -252,23 +253,23 @@ tm_diff (long_int year, long_int yday, int hour, int min, int sec,
 }
 
 /* Use CONVERT to convert T to a struct tm value in *TM.  T must be in
-   range for time_t.  Return TM if successful, NULL (setting errno) on
+   range for __time64_t.  Return TM if successful, NULL (setting errno) on
    failure.  */
 static struct tm *
-convert_time (struct tm *(*convert) (const time_t *, struct tm *),
+convert_time (struct tm *(*convert) (const __time64_t *, struct tm *),
 	      long_int t, struct tm *tm)
 {
-  time_t x = t;
+  __time64_t x = t;
   return convert (&x, tm);
 }
 
 /* Use CONVERT to convert *T to a broken down time in *TP.
    If *T is out of range for conversion, adjust it so that
    it is the nearest in-range value and then convert that.
-   A value is in range if it fits in both time_t and long_int.
+   A value is in range if it fits in both __time64_t and long_int.
    Return TP on success, NULL (setting errno) on failure.  */
 static struct tm *
-ranged_convert (struct tm *(*convert) (const time_t *, struct tm *),
+ranged_convert (struct tm *(*convert) (const __time64_t *, struct tm *),
 		long_int *t, struct tm *tp)
 {
   long_int t1 = (*t < mktime_min ? mktime_min
@@ -310,7 +311,7 @@ ranged_convert (struct tm *(*convert) (const time_t *, struct tm *),
 }
 
 
-/* Convert *TP to a time_t value, inverting
+/* Convert *TP to a __time64_t value, inverting
    the monotonic and mostly-unit-linear conversion function CONVERT.
    Use *OFFSET to keep track of a guess at the offset of the result,
    compared to what the result would be for UTC without leap seconds.
@@ -318,9 +319,9 @@ ranged_convert (struct tm *(*convert) (const time_t *, struct tm *),
    If successful, set *TP to the canonicalized struct tm;
    otherwise leave *TP alone, return ((time_t) -1) and set errno.
    This function is external because it is used also by timegm.c.  */
-time_t
+__time64_t
 __mktime_internal (struct tm *tp,
-		   struct tm *(*convert) (const time_t *, struct tm *),
+		   struct tm *(*convert) (const __time64_t *, struct tm *),
 		   mktime_offset_t *offset)
 {
   struct tm tm;
@@ -520,10 +521,13 @@ __mktime_internal (struct tm *tp,
 
 #if defined _LIBC || NEED_MKTIME_WORKING || NEED_MKTIME_WINDOWS
 
-/* Convert *TP to a time_t value.  */
-time_t
-mktime (struct tm *tp)
+/* Convert *TP to a __time64_t value.  */
+__time64_t
+__mktime64 (struct tm *tp)
 {
+  __time64_t t64;
+  time_t t;
+  struct tm tp0 = *tp;
   /* POSIX.1 8.1.1 requires that whenever mktime() is called, the
      time zone names contained in the external variable 'tzname' shall
      be set as if the tzset() function had been called.  */
@@ -531,7 +535,15 @@ mktime (struct tm *tp)
 
 # if defined _LIBC || NEED_MKTIME_WORKING
   static mktime_offset_t localtime_offset;
-  return __mktime_internal (tp, __localtime_r, &localtime_offset);
+  t64 = __mktime_internal (&tp0, __localtime64_r, &localtime_offset);
+  t = t64;
+  if (t != t64)
+    {
+      __set_errno (EOVERFLOW);
+      return -1;
+    }
+  *tp = tp0;
+  return t;
 # else
 #  undef mktime
   return mktime (tp);
@@ -540,6 +552,28 @@ mktime (struct tm *tp)
 #endif /* _LIBC || NEED_MKTIME_WORKING || NEED_MKTIME_WINDOWS */
 
 #ifdef weak_alias
+weak_alias (__mktime64, __timelocal64)
+#endif
+
+#ifdef _LIBC
+libc_hidden_def (__mktime64)
+libc_hidden_weak (__timelocal64)
+#endif
+
+#if __TIMESIZE != 64
+
+/* The 32-bit-time wrapper.  */
+time_t
+mktime (struct tm *tp)
+{
+  __time64_t t64 = __mktime64 (tp);
+  if (fits_in_time_t (t64))
+    return t64;
+  __set_errno (EOVERFLOW);
+  return -1;
+}
+
+#ifdef weak_alias
 weak_alias (mktime, timelocal)
 #endif
 
@@ -547,3 +581,5 @@ weak_alias (mktime, timelocal)
 libc_hidden_def (mktime)
 libc_hidden_weak (timelocal)
 #endif
+
+#endif
diff --git a/time/timegm.c b/time/timegm.c
index bfd36d0255..0cbe60a7fe 100644
--- a/time/timegm.c
+++ b/time/timegm.c
@@ -22,13 +22,38 @@
 #endif
 
 #include <time.h>
+#include <errno.h>
 
 #include "mktime-internal.h"
+#include <errno.h>
+
+__time64_t
+__timegm64 (struct tm *tmp)
+{
+  static long int gmtime_offset;
+  tmp->tm_isdst = 0;
+  return __mktime_internal (tmp, __gmtime64_r, &gmtime_offset);
+}
+
+#if __TIMESIZE != 64
 
 time_t
 timegm (struct tm *tmp)
 {
+  time_t t;
+  __time64_t t64;
+  struct tm tmp0 = *tmp;
   static mktime_offset_t gmtime_offset;
   tmp->tm_isdst = 0;
-  return __mktime_internal (tmp, __gmtime_r, &gmtime_offset);
+  t64 = __mktime_internal (&tmp0, __gmtime64_r, &gmtime_offset);
+  t = t64;
+  if (t != t64)
+    {
+      __set_errno (EOVERFLOW);
+      return -1;
+    }
+  *tmp = tmp0;
+  return t;
 }
+
+#endif
-- 
2.11.0

[PATCH v2 2/2] Fix time/mktime.c and time/gmtime.c for gnulib compatibility

[Lukasz Majewski]

From: Paul Eggert <eggert@cs.ucla.edu>

Tested with 'make check' on x86_64-linux-gnu and i686-linux.gnu.

	* time/mktime.c
	(__mktime64): Guard weak_alias with #ifdef
	(__mktime64): Guard libc_hidden_def with #if _LIBC
	(__mktime64): Guard libc_hidden_weak with #if _LIBC
	(mktime): Only build when _LIBC is defined
	* time/timegm.c:
	(timegm): Only build when _LIBC is defined
---
 time/mktime.c | 15 +++------------
 time/timegm.c |  2 +-
 2 files changed, 4 insertions(+), 13 deletions(-)

diff --git a/time/mktime.c b/time/mktime.c
index 5d3644e213..89293a76fb 100644
--- a/time/mktime.c
+++ b/time/mktime.c
@@ -549,18 +549,14 @@ __mktime64 (struct tm *tp)
   return mktime (tp);
 # endif
 }
-#endif /* _LIBC || NEED_MKTIME_WORKING || NEED_MKTIME_WINDOWS */
 
-#ifdef weak_alias
 weak_alias (__mktime64, __timelocal64)
-#endif
-
-#ifdef _LIBC
 libc_hidden_def (__mktime64)
 libc_hidden_weak (__timelocal64)
-#endif
 
-#if __TIMESIZE != 64
+#endif /* _LIBC || NEED_MKTIME_WORKING || NEED_MKTIME_WINDOWS */
+
+#if defined _LIBC && __TIMESIZE != 64
 
 /* The 32-bit-time wrapper.  */
 time_t
@@ -573,13 +569,8 @@ mktime (struct tm *tp)
   return -1;
 }
 
-#ifdef weak_alias
 weak_alias (mktime, timelocal)
-#endif
-
-#ifdef _LIBC
 libc_hidden_def (mktime)
 libc_hidden_weak (timelocal)
-#endif
 
 #endif
diff --git a/time/timegm.c b/time/timegm.c
index 0cbe60a7fe..f04428d032 100644
--- a/time/timegm.c
+++ b/time/timegm.c
@@ -35,7 +35,7 @@ __timegm64 (struct tm *tmp)
   return __mktime_internal (tmp, __gmtime64_r, &gmtime_offset);
 }
 
-#if __TIMESIZE != 64
+#if defined _LIBC &&  __TIMESIZE != 64
 
 time_t
 timegm (struct tm *tmp)
-- 
2.11.0

Re: [PATCH v2 1/2] Y2038: make __mktime_internal compatible with __time64_t

[Lukasz Majewski]

[-- Attachment #1: Type: text/plain, Size: 11067 bytes --]

Dear Joseph, Paul,

> From: Paul Eggert <eggert@cs.ucla.edu>
> 
> This implies also making its callers 64-bit-time compatible
> (these are mktime/localtime and timegm) and providing wrappers
> for 32-bit-time userland to call.
> 
> Tested with 'make check' on x86_64-linux-gnu and i686-linux.gnu.
> 
> * include/time.h (__mktime64): Add prototype.
> * include/time.h (__localtime64): Likewise.
> * include/time.h (fits_in_time_t): New static function.
> * time/mktime.c (__mktime64): New function.
> * time/timegm.c (__timegm64): Likewise.
> * time/mktime.c (mktime) [__TIMESIZE]: New wrapper function.
> * time/timegm.c (timegm) [__TIMESIZE]: Likewise.
> 
> ---
> Applicable on top of the newest glibc's master branch
> SH1: aa0e46636a5b71b609a41e9ab97134cd76ac1522
> 
> Tested with make check for x86_64, i585 and ARM setups.
> 
> Changes for v2:
> Add missing space (__set_errno (EOVERFLOW)) in src/time/timegm.c
> Add missing space (__set_errno (EOVERFLOW)) in src/time/mktime.c

Gentle ping on this series.

> ---
>  include/time.h | 36 +++++++++++++++++++++-----
>  time/mktime.c  | 80
> ++++++++++++++++++++++++++++++++++++++++++----------------
> time/timegm.c  | 27 +++++++++++++++++++- 3 files changed, 114
> insertions(+), 29 deletions(-)
> 
> diff --git a/include/time.h b/include/time.h
> index 61dd9e180b..34e5e04820 100644
> --- a/include/time.h
> +++ b/include/time.h
> @@ -3,6 +3,7 @@
>  
>  #ifndef _ISOMAC
>  # include <bits/types/locale_t.h>
> +# include <stdbool.h>
>  
>  extern __typeof (strftime_l) __strftime_l;
>  libc_hidden_proto (__strftime_l)
> @@ -16,6 +17,21 @@ libc_hidden_proto (localtime)
>  libc_hidden_proto (strftime)
>  libc_hidden_proto (strptime)
>  
> +#if __TIMESIZE == 64
> +# define __timegm64 timegm
> +# define __mktime64 mktime
> +# define __timelocal64 timelocal
> +#else
> +extern __time64_t __timegm64 (struct tm *__tp) __THROW;
> +extern __time64_t __mktime64 (struct tm *__tp) __THROW;
> +/* Another name for `__mktime64'.  */
> +extern __time64_t __timelocal64 (struct tm *__tp) __THROW;
> +
> +libc_hidden_proto (__mktime64)
> +libc_hidden_proto (__timelocal64)
> +#endif
> +
> +
>  extern __typeof (clock_getres) __clock_getres;
>  extern __typeof (clock_gettime) __clock_gettime;
>  libc_hidden_proto (__clock_gettime)
> @@ -49,13 +65,13 @@ extern void __tzset_parse_tz (const char *tz)
> attribute_hidden; 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
> -   normalize TP, given that a `struct tm *' maps to a `time_t' as
> performed +/* Subroutine of mktime.  Return the __time64_t
> representation of TP and
> +   normalize TP, given that a struct tm * maps to a __time64_t as
> performed by FUNC.  Record next guess for localtime-gmtime offset in
> *OFFSET.  */ -extern time_t __mktime_internal (struct tm *__tp,
> -				 struct tm *(*__func) (const time_t
> *,
> -						       struct tm *),
> -				 long int *__offset)
> attribute_hidden; +extern __time64_t __mktime_internal (struct tm
> *__tp,
> +				     struct tm *(*__func) (const
> __time64_t *,
> +							   struct tm
> *),
> +				     long int *__offset)
> attribute_hidden; 
>  #if __TIMESIZE == 64
>  # define __ctime64 ctime
> @@ -155,5 +171,13 @@ extern double __difftime (time_t time1, time_t
> time0); actual clock ID.  */
>  #define CLOCK_IDFIELD_SIZE	3
>  
> +/* Check whether a time64_t value fits in a time_t.  */
> +static inline bool
> +fits_in_time_t (__time64_t t64)
> +{
> +  time_t t = t64;
> +  return t == t64;
> +}
> +
>  #endif
>  #endif
> diff --git a/time/mktime.c b/time/mktime.c
> index af43a6950d..5d3644e213 100644
> --- a/time/mktime.c
> +++ b/time/mktime.c
> @@ -112,11 +112,11 @@ my_tzset (void)
>     added to them, and then with another timestamp added, without
>     worrying about overflow.
>  
> -   Much of the code uses long_int to represent time_t values, to
> -   lessen the hassle of dealing with platforms where time_t is
> +   Much of the code uses long_int to represent __time64_t values, to
> +   lessen the hassle of dealing with platforms where __time64_t is
>     unsigned, and because long_int should suffice to represent all
> -   time_t values that mktime can generate even on platforms where
> -   time_t is excessively wide.  */
> +   __time64_t values that mktime can generate even on platforms where
> +   __time64_t is excessively wide.  */
>  
>  #if INT_MAX <= LONG_MAX / 4 / 366 / 24 / 60 / 60
>  typedef long int long_int;
> @@ -144,16 +144,17 @@ shr (long_int a, int b)
>  	  : a / (one << b) - (a % (one << b) < 0));
>  }
>  
> -/* Bounds for the intersection of time_t and long_int.  */
> +/* Bounds for the intersection of __time64_t and long_int.  */
>  
>  static long_int const mktime_min
> -  = ((TYPE_SIGNED (time_t) && TYPE_MINIMUM (time_t) < TYPE_MINIMUM
> (long_int))
> -     ? TYPE_MINIMUM (long_int) : TYPE_MINIMUM (time_t));
> +  = ((TYPE_SIGNED (__time64_t)
> +      && TYPE_MINIMUM (__time64_t) < TYPE_MINIMUM (long_int))
> +     ? TYPE_MINIMUM (long_int) : TYPE_MINIMUM (__time64_t));
>  static long_int const mktime_max
> -  = (TYPE_MAXIMUM (long_int) < TYPE_MAXIMUM (time_t)
> -     ? TYPE_MAXIMUM (long_int) : TYPE_MAXIMUM (time_t));
> +  = (TYPE_MAXIMUM (long_int) < TYPE_MAXIMUM (__time64_t)
> +     ? TYPE_MAXIMUM (long_int) : TYPE_MAXIMUM (__time64_t));
>  
> -verify (TYPE_IS_INTEGER (time_t));
> +verify (TYPE_IS_INTEGER (__time64_t));
>  
>  #define EPOCH_YEAR 1970
>  #define TM_YEAR_BASE 1900
> @@ -252,23 +253,23 @@ tm_diff (long_int year, long_int yday, int
> hour, int min, int sec, }
>  
>  /* Use CONVERT to convert T to a struct tm value in *TM.  T must be
> in
> -   range for time_t.  Return TM if successful, NULL (setting errno)
> on
> +   range for __time64_t.  Return TM if successful, NULL (setting
> errno) on failure.  */
>  static struct tm *
> -convert_time (struct tm *(*convert) (const time_t *, struct tm *),
> +convert_time (struct tm *(*convert) (const __time64_t *, struct tm
> *), long_int t, struct tm *tm)
>  {
> -  time_t x = t;
> +  __time64_t x = t;
>    return convert (&x, tm);
>  }
>  
>  /* Use CONVERT to convert *T to a broken down time in *TP.
>     If *T is out of range for conversion, adjust it so that
>     it is the nearest in-range value and then convert that.
> -   A value is in range if it fits in both time_t and long_int.
> +   A value is in range if it fits in both __time64_t and long_int.
>     Return TP on success, NULL (setting errno) on failure.  */
>  static struct tm *
> -ranged_convert (struct tm *(*convert) (const time_t *, struct tm *),
> +ranged_convert (struct tm *(*convert) (const __time64_t *, struct tm
> *), long_int *t, struct tm *tp)
>  {
>    long_int t1 = (*t < mktime_min ? mktime_min
> @@ -310,7 +311,7 @@ ranged_convert (struct tm *(*convert) (const
> time_t *, struct tm *), }
>  
>  
> -/* Convert *TP to a time_t value, inverting
> +/* Convert *TP to a __time64_t value, inverting
>     the monotonic and mostly-unit-linear conversion function CONVERT.
>     Use *OFFSET to keep track of a guess at the offset of the result,
>     compared to what the result would be for UTC without leap seconds.
> @@ -318,9 +319,9 @@ ranged_convert (struct tm *(*convert) (const
> time_t *, struct tm *), If successful, set *TP to the canonicalized
> struct tm; otherwise leave *TP alone, return ((time_t) -1) and set
> errno. This function is external because it is used also by
> timegm.c.  */ -time_t
> +__time64_t
>  __mktime_internal (struct tm *tp,
> -		   struct tm *(*convert) (const time_t *, struct tm
> *),
> +		   struct tm *(*convert) (const __time64_t *, struct
> tm *), mktime_offset_t *offset)
>  {
>    struct tm tm;
> @@ -520,10 +521,13 @@ __mktime_internal (struct tm *tp,
>  
>  #if defined _LIBC || NEED_MKTIME_WORKING || NEED_MKTIME_WINDOWS
>  
> -/* Convert *TP to a time_t value.  */
> -time_t
> -mktime (struct tm *tp)
> +/* Convert *TP to a __time64_t value.  */
> +__time64_t
> +__mktime64 (struct tm *tp)
>  {
> +  __time64_t t64;
> +  time_t t;
> +  struct tm tp0 = *tp;
>    /* POSIX.1 8.1.1 requires that whenever mktime() is called, the
>       time zone names contained in the external variable 'tzname'
> shall be set as if the tzset() function had been called.  */
> @@ -531,7 +535,15 @@ mktime (struct tm *tp)
>  
>  # if defined _LIBC || NEED_MKTIME_WORKING
>    static mktime_offset_t localtime_offset;
> -  return __mktime_internal (tp, __localtime_r, &localtime_offset);
> +  t64 = __mktime_internal (&tp0, __localtime64_r, &localtime_offset);
> +  t = t64;
> +  if (t != t64)
> +    {
> +      __set_errno (EOVERFLOW);
> +      return -1;
> +    }
> +  *tp = tp0;
> +  return t;
>  # else
>  #  undef mktime
>    return mktime (tp);
> @@ -540,6 +552,28 @@ mktime (struct tm *tp)
>  #endif /* _LIBC || NEED_MKTIME_WORKING || NEED_MKTIME_WINDOWS */
>  
>  #ifdef weak_alias
> +weak_alias (__mktime64, __timelocal64)
> +#endif
> +
> +#ifdef _LIBC
> +libc_hidden_def (__mktime64)
> +libc_hidden_weak (__timelocal64)
> +#endif
> +
> +#if __TIMESIZE != 64
> +
> +/* The 32-bit-time wrapper.  */
> +time_t
> +mktime (struct tm *tp)
> +{
> +  __time64_t t64 = __mktime64 (tp);
> +  if (fits_in_time_t (t64))
> +    return t64;
> +  __set_errno (EOVERFLOW);
> +  return -1;
> +}
> +
> +#ifdef weak_alias
>  weak_alias (mktime, timelocal)
>  #endif
>  
> @@ -547,3 +581,5 @@ weak_alias (mktime, timelocal)
>  libc_hidden_def (mktime)
>  libc_hidden_weak (timelocal)
>  #endif
> +
> +#endif
> diff --git a/time/timegm.c b/time/timegm.c
> index bfd36d0255..0cbe60a7fe 100644
> --- a/time/timegm.c
> +++ b/time/timegm.c
> @@ -22,13 +22,38 @@
>  #endif
>  
>  #include <time.h>
> +#include <errno.h>
>  
>  #include "mktime-internal.h"
> +#include <errno.h>
> +
> +__time64_t
> +__timegm64 (struct tm *tmp)
> +{
> +  static long int gmtime_offset;
> +  tmp->tm_isdst = 0;
> +  return __mktime_internal (tmp, __gmtime64_r, &gmtime_offset);
> +}
> +
> +#if __TIMESIZE != 64
>  
>  time_t
>  timegm (struct tm *tmp)
>  {
> +  time_t t;
> +  __time64_t t64;
> +  struct tm tmp0 = *tmp;
>    static mktime_offset_t gmtime_offset;
>    tmp->tm_isdst = 0;
> -  return __mktime_internal (tmp, __gmtime_r, &gmtime_offset);
> +  t64 = __mktime_internal (&tmp0, __gmtime64_r, &gmtime_offset);
> +  t = t64;
> +  if (t != t64)
> +    {
> +      __set_errno (EOVERFLOW);
> +      return -1;
> +    }
> +  *tmp = tmp0;
> +  return t;
>  }
> +
> +#endif




Best regards,

Lukasz Majewski

--

DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-59 Fax: (+49)-8142-66989-80 Email: lukma@denx.de

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH v2 1/2] Y2038: make __mktime_internal compatible with __time64_t

[Paul Eggert]

On 2/27/19 3:20 AM, Lukasz Majewski wrote:
> +/* Another name for `__mktime64'.  */
> +extern __time64_t __timelocal64 (struct tm *__tp) __THROW;

In hindsight the name 'timelocal' was a mistake: it's not a portable
name and its use has not caught on. Although we need to keep 'timelocal'
for backwards compatibility, there's no need to define 'timelocal64', as
the very few people who need such a function can just call mktime64. So
I suggest removing all traces of timelocal64, __timelocal64, etc. from
the patch.


> +/* Check whether a time64_t value fits in a time_t.  */
> +static inline bool
> +fits_in_time_t (__time64_t t64)
> +{
> +  time_t t = t64;
> +  return t == t64;
> +}
> +

This function is used only in time/mktime.c, and so should be defined
there. This will help sharing with Gnulib, which doesn't have
include/time.h.

The function's name should not end with "_t" since that's in the
reserved namespace (important for Gnulib).


> -verify (TYPE_IS_INTEGER (time_t));
> +verify (TYPE_IS_INTEGER (__time64_t));

Please remove this line instead. It dates back to old POSIX, which
allowed time_t to be a floating-point type. POSIX no longer allows this
and we needn't worry about that possibility any more.


> +  __time64_t t64;
> +  time_t t;
> +  struct tm tp0 = *tp;
There is no need for both t and t64. Just declare "__time64_t t;" and
replace all uses of t64 with t. Also, please consistently rename tp0 to
tm0, since it's not a pointer.


> +  t = t64;
> +  if (t != t64)

Replace this with "if (! fits_in_time_t (t))" (but rename the function).


> +/* The 32-bit-time wrapper.  */
> +time_t
> +mktime (struct tm *tp)
> +{
> +  __time64_t t64 = __mktime64 (tp);
> +  if (fits_in_time_t (t64))
> +    return t64;
> +  __set_errno (EOVERFLOW);
> +  return -1;
> +}

Fix this so that it doesn not modify *TP when failing due to EOVERFLOW.
The manual says mktime doesn't change *TP on failure.


Similar comments apply to the implementation of timegm.

Re: [PATCH v2 2/2] Fix time/mktime.c and time/gmtime.c for gnulib compatibility

[Paul Eggert]

On 2/27/19 3:20 AM, Lukasz Majewski wrote:
> -#ifdef weak_alias
>  weak_alias (__mktime64, __timelocal64)
> -#endif

This part of the patch won't be needed if my suggestion about removing
timelocal64 (from the previous patch) is taken. Otherwise this patch
looks OK to me once the earlier patch is settled on.

Re: [PATCH v2 1/2] Y2038: make __mktime_internal compatible with __time64_t

[Joseph Myers]

On Mon, 11 Mar 2019, Paul Eggert wrote:

> On 2/27/19 3:20 AM, Lukasz Majewski wrote:
> > +/* Another name for `__mktime64'.  */
> > +extern __time64_t __timelocal64 (struct tm *__tp) __THROW;
> 
> In hindsight the name 'timelocal' was a mistake: it's not a portable
> name and its use has not caught on. Although we need to keep 'timelocal'
> for backwards compatibility, there's no need to define 'timelocal64', as
> the very few people who need such a function can just call mktime64. So
> I suggest removing all traces of timelocal64, __timelocal64, etc. from
> the patch.

Neither timelocal64 nor mktime64 will be a public API; the public API is 
to define _TIME_BITS=64 (and _FILE_OFFSET_BITS=64 because we don't want to 
support the combination of 64-bit times with 32-bit offsets) before 
including any system headers, then call the existing function names.

Given timelocal as part of the __USE_MISC API in time.h, it should be part 
of the __USE_MISC __TIME_BITS=64 API there as well - but it's fine for 
that case to redirect to __mktime64 (and thus not need __timelocal64 as 
another alias).

-- 
Joseph S. Myers
joseph@codesourcery.com

Re: [PATCH v2 1/2] Y2038: make __mktime_internal compatible with __time64_t

[Lukasz Majewski]

[-- Attachment #1: Type: text/plain, Size: 2836 bytes --]

Hi Paul,

> On 2/27/19 3:20 AM, Lukasz Majewski wrote:
> > +/* Another name for `__mktime64'.  */
> > +extern __time64_t __timelocal64 (struct tm *__tp) __THROW;  
> 
> In hindsight the name 'timelocal' was a mistake: it's not a portable
> name and its use has not caught on. Although we need to keep
> 'timelocal' for backwards compatibility, there's no need to define
> 'timelocal64', as the very few people who need such a function can
> just call mktime64. So I suggest removing all traces of timelocal64,
> __timelocal64, etc. from the patch.
> 
> 
> > +/* Check whether a time64_t value fits in a time_t.  */
> > +static inline bool
> > +fits_in_time_t (__time64_t t64)
> > +{
> > +  time_t t = t64;
> > +  return t == t64;
> > +}
> > +  
> 
> This function is used only in time/mktime.c, and so should be defined
> there. This will help sharing with Gnulib, which doesn't have
> include/time.h.

Some patches on top of this one (y2038 related) use fits_in_time_t() to
check if 64 bit wrapper on 32 bit version of function (like
__clock_gettime()) shall proceed or return EOVERFLOW.

The question is if there is a more suitable place than include/time.h
header to have fits_in_time() reused by both Glibc and Gnulib?

> 
> The function's name should not end with "_t" since that's in the
> reserved namespace (important for Gnulib).

Ok.

> 
> 
> > -verify (TYPE_IS_INTEGER (time_t));
> > +verify (TYPE_IS_INTEGER (__time64_t));  
> 
> Please remove this line instead. It dates back to old POSIX, which
> allowed time_t to be a floating-point type. POSIX no longer allows
> this and we needn't worry about that possibility any more.
> 
> 
> > +  __time64_t t64;
> > +  time_t t;
> > +  struct tm tp0 = *tp;  
> There is no need for both t and t64. Just declare "__time64_t t;" and
> replace all uses of t64 with t. Also, please consistently rename tp0
> to tm0, since it's not a pointer.
> 
> 
> > +  t = t64;
> > +  if (t != t64)  
> 
> Replace this with "if (! fits_in_time_t (t))" (but rename the
> function).
> 
> 
> > +/* The 32-bit-time wrapper.  */
> > +time_t
> > +mktime (struct tm *tp)
> > +{
> > +  __time64_t t64 = __mktime64 (tp);
> > +  if (fits_in_time_t (t64))
> > +    return t64;
> > +  __set_errno (EOVERFLOW);
> > +  return -1;
> > +}  
> 
> Fix this so that it doesn not modify *TP when failing due to
> EOVERFLOW. The manual says mktime doesn't change *TP on failure.
> 
> 
> Similar comments apply to the implementation of timegm.
> 

Thanks for your comments.


Best regards,

Lukasz Majewski

--

DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-59 Fax: (+49)-8142-66989-80 Email: lukma@denx.de

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH v2 1/2] Y2038: make __mktime_internal compatible with __time64_t

[Lukasz Majewski]

[-- Attachment #1: Type: text/plain, Size: 2669 bytes --]

Hi Joseph, Paul,

> On Mon, 11 Mar 2019, Paul Eggert wrote:
> 
> > On 2/27/19 3:20 AM, Lukasz Majewski wrote:  
> > > +/* Another name for `__mktime64'.  */
> > > +extern __time64_t __timelocal64 (struct tm *__tp) __THROW;  
> > 
> > In hindsight the name 'timelocal' was a mistake: it's not a portable
> > name and its use has not caught on. Although we need to keep
> > 'timelocal' for backwards compatibility, there's no need to define
> > 'timelocal64', as the very few people who need such a function can
> > just call mktime64. So I suggest removing all traces of
> > timelocal64, __timelocal64, etc. from the patch. 
> 
> Neither timelocal64 nor mktime64 will be a public API; the public API
> is to define _TIME_BITS=64 (and _FILE_OFFSET_BITS=64 because we don't
> want to support the combination of 64-bit times with 32-bit offsets)
> before including any system headers, then call the existing function
> names.

I do have a doubt if I understood the above comment(s). 

Please correct me if I'm wrong - this patch makes the
__mktime_internal compatible with __time_64 explicit 64 bit time data
structure.

It's goal is to replace all calls to mktime/timegm/localtime with
__mktime64/__timegm64/__localtime64 (alias to __mktime64) internal
counterparts (another level of abstraction with explicit 64 bit time
storage).

Then, the __mktime64() internal function is used to implement mktime
wrapper for 32-bit time (the same approach is for timegm).

This patch looks like a preparatory patch for further Y2038 conversion
(but doesn't provide one).

> 
> Given timelocal as part of the __USE_MISC API in time.h, it should be
> part of the __USE_MISC __TIME_BITS=64 API there as well - but it's

This patch doesn't introduce/use the _TIME_BITS=64 - instead it
operates on TIMESIZE=64.

> fine for that case to redirect to __mktime64 (and thus not need
> __timelocal64 as another alias).
> 


Do you require following code in include/time.h for timelocal:

#if defined __USE_MISC
# if defined(__REDIRECT)
  extern int __REDIRECT (timelocal, (struct tm *__tp),
                         __mktime64) __THROW);
# else
#  define timelocal __mktime64
# endif
#endif


The problem seems to be that Paul requested to remove {__}timelocal64
from this patch and you pointed out that timelocal is already supported
when __USE_MISC is defined?




Best regards,

Lukasz Majewski

--

DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-59 Fax: (+49)-8142-66989-80 Email: lukma@denx.de

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH v2 1/2] Y2038: make __mktime_internal compatible with __time64_t

[Joseph Myers]

On Sun, 17 Mar 2019, Lukasz Majewski wrote:

> Do you require following code in include/time.h for timelocal:
> 
> #if defined __USE_MISC
> # if defined(__REDIRECT)
>   extern int __REDIRECT (timelocal, (struct tm *__tp),
>                          __mktime64) __THROW);
> # else
> #  define timelocal __mktime64
> # endif
> #endif

include/time.h is an *internal* header, so should not need to test __USE_* 
or use __REDIRECT.

Something like that would be appropriate in the *installed* header 
(time/time.h), under whatever __USE_* condition is set by _TIME_BITS=64, 
but only at the later point where we're ready to add support for 
_TIME_BITS=64 to the public headers and corresponding new public symbol 
versions for the new ABIs.

By using something like that in the installed header you avoid any need to 
define the name __timelocal64 anywhere.

-- 
Joseph S. Myers
joseph@codesourcery.com

[PATCH v2 1/2] Y2038: make __mktime_internal compatible with __time64_t

[Paul Eggert]

[-- Attachment #1: Type: text/plain, Size: 1869 bytes --]

On 3/11/19 11:58 PM, Lukasz Majewski wrote:
> The question is if there is a more suitable place than include/time.h
> header to have fits_in_time() reused by both Glibc and Gnulib?

Hmm, well, on further thought, fits_in_time_t can live in include/time.h
since only glibc will need it. Gnulib won't support two kinds of time_t
types, so it should't need the function. (The attached patch renames it
to "in_time_t_range" to avoid *_t pollution.)

That being said, we will have to make a place for private .h code shared
between glibc and Gnulib, because include/time.h can't easily be shared
with Gnulib. I suggest using time/mktime-internal.h for this

Some other comments on that patch that I noticed while looking into this.

The patch added duplicate "#include <errno.h>" lines to time/timegm.c.

I still don't get why we need __timelocal64. Glibc code can use
__mktime64. User code shouldn't see that symbol.

Come to think of it, user code shouldn't see __time64_t either. Yes, the
name begins with two underscores so user code should avoid it, but
putting __time64_t in /usr/include will just tempt users. It's easy to
keep __time64_t out of /usr/include so let's do that.

The body of __mktime64 can be simplified; there's no need for locals of
type __time64_t, time_t, and struct tm. And it should use
in_time_t_range instead of doing it by hand.

The bodies of mktime and timegm [__TIMESIZE != 64] should not pass tp to
__mktime64/__timegm64 directly, since *tp should not be updated if the
result is in __time64_t range but out of time_t range. And timegm should
use in_time_t_range instead of doing it by hand.

The "#ifdef weak_alias" etc. lines can be removed now, since Gnulib does
this stuff OK now.

timegm.c should include libc-config.h, not config.h, as this is the
current Gnulib style for code shared with Gnulib.

Revised proposed patch attached.


[-- Attachment #2: 0001-Make-mktime-etc.-compatible-with-__time64_t.patch --]
[-- Type: text/x-patch, Size: 17764 bytes --]

From 509d363dc0f96ea194589e90b27c7f1c2de51371 Mon Sep 17 00:00:00 2001
From: Paul Eggert <eggert@cs.ucla.edu>
Date: Mon, 18 Mar 2019 14:14:15 -0700
Subject: [PATCH] Make mktime etc. compatible with __time64_t

Keep these functions compatible with Gnulib while adding
__time64_t support.  The basic idea is to move private API
declarations from include/time.h to time/mktime-internal.h, since
the former file cannot easily be shared with Gnulib whereas the
latter can.
Also, do some other minor cleanup while in the neighborhood.
* include/time.h: Include stdbool.h, time/mktime-internal.h.
(__mktime_internal): Move this prototype to time/mktime-internal.h,
since Gnulib needs it.
(__localtime64_r, __gmtime64_r) [__TIMESIZE == 64]:
Move these macros to time/mktime-internal.h, since Gnulib needs them.
(__mktime64, __timegm64) [__TIMESIZE != 64]: New prototypes.
(in_time_t_range): New static function.
* posix/bits/types.h (__time64_t): Move to time/mktime-internal.h,
so that glibc users are not tempted to use __time64_t.
* time/mktime-internal.h: Rewrite so that it does both glibc
and Gnulib work.  Include time.h if not _LIBC.
(mktime_offset_t) [!_LIBC]: Define for gnulib.
(__time64_t): New type or macro, moved here from
posix/bits/types.h.
(__gmtime64_r, __localtime64_r, __mktime64, __timegm64)
[!_LIBC || __TIMESIZE == 64): New macros, mostly moved here
from include/time.h.
(__gmtime_r, __localtime_r, __mktime_internal) [!_LIBC]:
New macros, taken from GNulib.
(__mktime_internal): New prototype, moved here from include/time.h.
* time/mktime.c (mktime_min, mktime_max, convert_time)
(ranged_convert, __mktime_internal, __mktime64):
* time/timegm.c (__timegm64):
Use __time64_t, not time_t.
* time/mktime.c: Stop worrying about whether time_t is floating-point.
(__mktime64) [! (_LIBC && __TIMESIZE != 64)]:
Rename from mktime.
(mktime) [_LIBC && __TIMESIZE != 64]: New function.
* time/timegm.c [!_LIBC]: Include libc-config.h, not config.h,
for libc_hidden_def.
Include errno.h.
(__timegm64) [! (_LIBC && __TIMESIZE != 64)]:
Rename from timegm.
(timegm) [_LIBC && __TIMESIZE != 64]: New function.
---
 ChangeLog              | 44 +++++++++++++++++++++++
 include/time.h         | 39 ++++++++++----------
 posix/bits/types.h     |  6 ----
 time/mktime-internal.h | 80 +++++++++++++++++++++++++++++++++++++++++-
 time/mktime.c          | 71 +++++++++++++++++++++++--------------
 time/timegm.c          | 32 ++++++++++++++---
 6 files changed, 215 insertions(+), 57 deletions(-)

diff --git a/ChangeLog b/ChangeLog
index f7c9ee51ad..5d9f936c19 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,47 @@
+2019-03-18  Paul Eggert  <eggert@cs.ucla.edu>
+
+	Make mktime etc. compatible with __time64_t
+	Keep these functions compatible with Gnulib while adding
+	__time64_t support.  The basic idea is to move private API
+	declarations from include/time.h to time/mktime-internal.h, since
+	the former file cannot easily be shared with Gnulib whereas the
+	latter can.
+	Also, do some other minor cleanup while in the neighborhood.
+	* include/time.h: Include stdbool.h, time/mktime-internal.h.
+	(__mktime_internal): Move this prototype to time/mktime-internal.h,
+	since Gnulib needs it.
+	(__localtime64_r, __gmtime64_r) [__TIMESIZE == 64]:
+	Move these macros to time/mktime-internal.h, since Gnulib needs them.
+	(__mktime64, __timegm64) [__TIMESIZE != 64]: New prototypes.
+	(in_time_t_range): New static function.
+	* posix/bits/types.h (__time64_t): Move to time/mktime-internal.h,
+	so that glibc users are not tempted to use __time64_t.
+	* time/mktime-internal.h: Rewrite so that it does both glibc
+	and Gnulib work.  Include time.h if not _LIBC.
+	(mktime_offset_t) [!_LIBC]: Define for gnulib.
+	(__time64_t): New type or macro, moved here from
+	posix/bits/types.h.
+	(__gmtime64_r, __localtime64_r, __mktime64, __timegm64)
+	[!_LIBC || __TIMESIZE == 64): New macros, mostly moved here
+	from include/time.h.
+	(__gmtime_r, __localtime_r, __mktime_internal) [!_LIBC]:
+	New macros, taken from GNulib.
+	(__mktime_internal): New prototype, moved here from include/time.h.
+	* time/mktime.c (mktime_min, mktime_max, convert_time)
+	(ranged_convert, __mktime_internal, __mktime64):
+	* time/timegm.c (__timegm64):
+	Use __time64_t, not time_t.
+	* time/mktime.c: Stop worrying about whether time_t is floating-point.
+	(__mktime64) [! (_LIBC && __TIMESIZE != 64)]:
+	Rename from mktime.
+	(mktime) [_LIBC && __TIMESIZE != 64]: New function.
+	* time/timegm.c [!_LIBC]: Include libc-config.h, not config.h,
+	for libc_hidden_def.
+	Include errno.h.
+	(__timegm64) [! (_LIBC && __TIMESIZE != 64)]:
+	Rename from timegm.
+	(timegm) [_LIBC && __TIMESIZE != 64]: New function.
+
 2019-03-16  Samuel Thibault  <samuel.thibault@ens-lyon.org>
 
 	* hurd/hurd/signal.h (_hurd_critical_section_lock): Document how EINTR
diff --git a/include/time.h b/include/time.h
index 61dd9e180b..3e55f7ba83 100644
--- a/include/time.h
+++ b/include/time.h
@@ -3,6 +3,8 @@
 
 #ifndef _ISOMAC
 # include <bits/types/locale_t.h>
+# include <stdbool.h>
+# include <time/mktime-internal.h>
 
 extern __typeof (strftime_l) __strftime_l;
 libc_hidden_proto (__strftime_l)
@@ -49,19 +51,11 @@ extern void __tzset_parse_tz (const char *tz) attribute_hidden;
 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
-   normalize TP, given that a `struct tm *' maps to a `time_t' as performed
-   by FUNC.  Record next guess for localtime-gmtime offset in *OFFSET.  */
-extern time_t __mktime_internal (struct tm *__tp,
-				 struct tm *(*__func) (const time_t *,
-						       struct tm *),
-				 long int *__offset) attribute_hidden;
-
 #if __TIMESIZE == 64
 # define __ctime64 ctime
 #else
 extern char *__ctime64 (const __time64_t *__timer) __THROW;
-libc_hidden_proto (__ctime64);
+libc_hidden_proto (__ctime64)
 #endif
 
 #if __TIMESIZE == 64
@@ -69,7 +63,7 @@ libc_hidden_proto (__ctime64);
 #else
 extern char *__ctime64_r (const __time64_t *__restrict __timer,
 		          char *__restrict __buf) __THROW;
-libc_hidden_proto (__ctime64_r);
+libc_hidden_proto (__ctime64_r)
 #endif
 
 #if __TIMESIZE == 64
@@ -81,13 +75,13 @@ libc_hidden_proto (__localtime64)
 
 extern struct tm *__localtime_r (const time_t *__timer,
 				 struct tm *__tp) attribute_hidden;
-
-#if __TIMESIZE == 64
-# define __localtime64_r __localtime_r
-#else
+#if __TIMESIZE != 64
 extern struct tm *__localtime64_r (const __time64_t *__timer,
 				   struct tm *__tp);
 libc_hidden_proto (__localtime64_r)
+
+extern __time64_t __mktime64 (struct tm *__tp) __THROW;
+libc_hidden_proto (__mktime64)
 #endif
 
 extern struct tm *__gmtime_r (const time_t *__restrict __timer,
@@ -99,14 +93,13 @@ libc_hidden_proto (__gmtime_r)
 #else
 extern struct tm *__gmtime64 (const __time64_t *__timer);
 libc_hidden_proto (__gmtime64)
-#endif
 
-#if __TIMESIZE == 64
-# define __gmtime64_r __gmtime_r
-#else
 extern struct tm *__gmtime64_r (const __time64_t *__restrict __timer,
 				struct tm *__restrict __tp);
-libc_hidden_proto (__gmtime64_r);
+libc_hidden_proto (__gmtime64_r)
+
+extern __time64_t __timegm64 (struct tm *__tp) __THROW;
+libc_hidden_proto (__timegm64)
 #endif
 
 /* Compute the `struct tm' representation of T,
@@ -155,5 +148,13 @@ extern double __difftime (time_t time1, time_t time0);
    actual clock ID.  */
 #define CLOCK_IDFIELD_SIZE	3
 
+/* Check whether T fits in time_t.  */
+static inline bool
+in_time_t_range (__time64_t t)
+{
+  time_t s = t;
+  return s != t;
+}
+
 #endif
 #endif
diff --git a/posix/bits/types.h b/posix/bits/types.h
index 0de6c59bb4..110081316b 100644
--- a/posix/bits/types.h
+++ b/posix/bits/types.h
@@ -213,12 +213,6 @@ __STD_TYPE __U32_TYPE __socklen_t;
    It is not currently necessary for this to be machine-specific.  */
 typedef int __sig_atomic_t;
 
-#if __TIMESIZE == 64
-# define __time64_t __time_t
-#else
-__STD_TYPE __TIME64_T_TYPE __time64_t;	/* Seconds since the Epoch.  */
-#endif
-
 #undef __STD_TYPE
 
 #endif /* bits/types.h */
diff --git a/time/mktime-internal.h b/time/mktime-internal.h
index 6111c22880..7cdc868f6d 100644
--- a/time/mktime-internal.h
+++ b/time/mktime-internal.h
@@ -1,2 +1,80 @@
-/* Gnulib mktime-internal.h, tailored for glibc.  */
+/* Internals of mktime and related functions
+   Copyright 2016-2019 Free Software Foundation, Inc.
+   This file is part of the GNU C Library.
+   Contributed by Paul Eggert <eggert@cs.ucla.edu>.
+
+   The GNU C Library is free software; you can redistribute it and/or
+   modify it under the terms of the GNU Lesser General Public
+   License as published by the Free Software Foundation; either
+   version 2.1 of the License, or (at your option) any later version.
+
+   The GNU C Library is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+   Lesser General Public License for more details.
+
+   You should have received a copy of the GNU Lesser General Public
+   License along with the GNU C Library; if not, see
+   <https://www.gnu.org/licenses/>.  */
+
+#ifndef _LIBC
+# include <time.h>
+#endif
+
+/* mktime_offset_t is a signed type wide enough to hold a UTC offset
+   in seconds, and used as part of the type of the offset-guess
+   argument to mktime_internal.  In Glibc, it is always long int.
+   When in Gnulib, use time_t on platforms where time_t
+   is signed, to be compatible with platforms like BeOS that export
+   this implementation detail of mktime.  On platforms where time_t is
+   unsigned, GNU and POSIX code can assume 'int' is at least 32 bits
+   which is wide enough for a UTC offset.  */
+#ifdef _LIBC
 typedef long int mktime_offset_t;
+#elif TIME_T_IS_SIGNED
+typedef time_t mktime_offset_t;
+#else
+typedef int mktime_offset_t;
+#endif
+
+/* The source code uses identifiers like __time64_t for glibc
+   timestamps that can contain 64-bit values even when time_t is only
+   32 bits.  These are just macros for the ordinary identifiers unless
+   compiling within glibc when time_t is 32 bits.  */
+#if defined _LIBC && __TIMESIZE != 64
+typedef __TIME64_T_TYPE __time64_t;
+#else
+# define __time64_t time_t
+# define __gmtime64_r __gmtime_r
+# define __localtime64_r __localtime_r
+# define __mktime64 mktime
+# define __timegm64 timegm
+#endif
+
+#ifndef _LIBC
+
+/* Although glibc source code uses leading underscores, Gnulib wants
+   ordinary names.
+
+   Portable standalone applications should supply a <time.h> that
+   declares a POSIX-compliant localtime_r, for the benefit of older
+   implementations that lack localtime_r or have a nonstandard one.
+   Similarly for gmtime_r.  See the gnulib time_r module for one way
+   to implement this.  */
+
+# undef __gmtime_r
+# undef __localtime_r
+# define __gmtime_r gmtime_r
+# define __localtime_r localtime_r
+
+# define __mktime_internal mktime_internal
+
+#endif
+
+/* Subroutine of mktime.  Return the time_t representation of TP and
+   normalize TP, given that a struct tm * maps to a time_t as performed
+   by FUNC.  Record next guess for localtime-gmtime offset in *OFFSET.  */
+extern __time64_t __mktime_internal (struct tm *tp,
+                                     struct tm *(*func) (__time64_t const *,
+                                                         struct tm *),
+                                     mktime_offset_t *offset) attribute_hidden;
diff --git a/time/mktime.c b/time/mktime.c
index 57efee9b25..8fe2f963ae 100644
--- a/time/mktime.c
+++ b/time/mktime.c
@@ -112,11 +112,11 @@ my_tzset (void)
    added to them, and then with another timestamp added, without
    worrying about overflow.
 
-   Much of the code uses long_int to represent time_t values, to
-   lessen the hassle of dealing with platforms where time_t is
+   Much of the code uses long_int to represent __time64_t values, to
+   lessen the hassle of dealing with platforms where __time64_t is
    unsigned, and because long_int should suffice to represent all
-   time_t values that mktime can generate even on platforms where
-   time_t is excessively wide.  */
+   __time64_t values that mktime can generate even on platforms where
+   __time64_t is wider than the int components of struct tm.  */
 
 #if INT_MAX <= LONG_MAX / 4 / 366 / 24 / 60 / 60
 typedef long int long_int;
@@ -144,16 +144,15 @@ shr (long_int a, int b)
 	  : a / (one << b) - (a % (one << b) < 0));
 }
 
-/* Bounds for the intersection of time_t and long_int.  */
+/* Bounds for the intersection of __time64_t and long_int.  */
 
 static long_int const mktime_min
-  = ((TYPE_SIGNED (time_t) && TYPE_MINIMUM (time_t) < TYPE_MINIMUM (long_int))
-     ? TYPE_MINIMUM (long_int) : TYPE_MINIMUM (time_t));
+  = ((TYPE_SIGNED (__time64_t)
+      && TYPE_MINIMUM (__time64_t) < TYPE_MINIMUM (long_int))
+     ? TYPE_MINIMUM (long_int) : TYPE_MINIMUM (__time64_t));
 static long_int const mktime_max
-  = (TYPE_MAXIMUM (long_int) < TYPE_MAXIMUM (time_t)
-     ? TYPE_MAXIMUM (long_int) : TYPE_MAXIMUM (time_t));
-
-verify (TYPE_IS_INTEGER (time_t));
+  = (TYPE_MAXIMUM (long_int) < TYPE_MAXIMUM (__time64_t)
+     ? TYPE_MAXIMUM (long_int) : TYPE_MAXIMUM (__time64_t));
 
 #define EPOCH_YEAR 1970
 #define TM_YEAR_BASE 1900
@@ -252,23 +251,23 @@ tm_diff (long_int year, long_int yday, int hour, int min, int sec,
 }
 
 /* Use CONVERT to convert T to a struct tm value in *TM.  T must be in
-   range for time_t.  Return TM if successful, NULL (setting errno) on
+   range for __time64_t.  Return TM if successful, NULL (setting errno) on
    failure.  */
 static struct tm *
-convert_time (struct tm *(*convert) (const time_t *, struct tm *),
+convert_time (struct tm *(*convert) (const __time64_t *, struct tm *),
 	      long_int t, struct tm *tm)
 {
-  time_t x = t;
+  __time64_t x = t;
   return convert (&x, tm);
 }
 
 /* Use CONVERT to convert *T to a broken down time in *TP.
    If *T is out of range for conversion, adjust it so that
    it is the nearest in-range value and then convert that.
-   A value is in range if it fits in both time_t and long_int.
+   A value is in range if it fits in both __time64_t and long_int.
    Return TP on success, NULL (setting errno) on failure.  */
 static struct tm *
-ranged_convert (struct tm *(*convert) (const time_t *, struct tm *),
+ranged_convert (struct tm *(*convert) (const __time64_t *, struct tm *),
 		long_int *t, struct tm *tp)
 {
   long_int t1 = (*t < mktime_min ? mktime_min
@@ -310,7 +309,7 @@ ranged_convert (struct tm *(*convert) (const time_t *, struct tm *),
 }
 
 
-/* Convert *TP to a time_t value, inverting
+/* Convert *TP to a __time64_t value, inverting
    the monotonic and mostly-unit-linear conversion function CONVERT.
    Use *OFFSET to keep track of a guess at the offset of the result,
    compared to what the result would be for UTC without leap seconds.
@@ -318,9 +317,9 @@ ranged_convert (struct tm *(*convert) (const time_t *, struct tm *),
    If successful, set *TP to the canonicalized struct tm;
    otherwise leave *TP alone, return ((time_t) -1) and set errno.
    This function is external because it is used also by timegm.c.  */
-time_t
+__time64_t
 __mktime_internal (struct tm *tp,
-		   struct tm *(*convert) (const time_t *, struct tm *),
+		   struct tm *(*convert) (const __time64_t *, struct tm *),
 		   mktime_offset_t *offset)
 {
   struct tm tm;
@@ -520,9 +519,9 @@ __mktime_internal (struct tm *tp,
 
 #if defined _LIBC || NEED_MKTIME_WORKING || NEED_MKTIME_WINDOWS
 
-/* Convert *TP to a time_t value.  */
-time_t
-mktime (struct tm *tp)
+/* Convert *TP to a __time64_t value.  */
+__time64_t
+__mktime64 (struct tm *tp)
 {
   /* POSIX.1 8.1.1 requires that whenever mktime() is called, the
      time zone names contained in the external variable 'tzname' shall
@@ -531,7 +530,7 @@ mktime (struct tm *tp)
 
 # if defined _LIBC || NEED_MKTIME_WORKING
   static mktime_offset_t localtime_offset;
-  return __mktime_internal (tp, __localtime_r, &localtime_offset);
+  return __mktime_internal (tp, __localtime64_r, &localtime_offset);
 # else
 #  undef mktime
   return mktime (tp);
@@ -539,11 +538,29 @@ mktime (struct tm *tp)
 }
 #endif /* _LIBC || NEED_MKTIME_WORKING || NEED_MKTIME_WINDOWS */
 
-#ifdef weak_alias
-weak_alias (mktime, timelocal)
+#if defined _LIBC && __TIMESIZE != 64
+
+libc_hidden_def (__mktime64)
+
+time_t
+mktime (struct tm *tp)
+{
+  struct tm tm = *tp;
+  __time64_t t = __mktime64 (&tm);
+  if (in_time_t_range (t))
+    {
+      *tp = tm;
+      return t;
+    }
+  else
+    {
+      __set_errno (EOVERFLOW);
+      return -1;
+    }
+}
+
 #endif
 
-#ifdef _LIBC
+weak_alias (mktime, timelocal)
 libc_hidden_def (mktime)
 libc_hidden_weak (timelocal)
-#endif
diff --git a/time/timegm.c b/time/timegm.c
index bfd36d0255..bae0ceee5e 100644
--- a/time/timegm.c
+++ b/time/timegm.c
@@ -18,17 +18,41 @@
    <http://www.gnu.org/licenses/>.  */
 
 #ifndef _LIBC
-# include <config.h>
+# include <libc-config.h>
 #endif
 
 #include <time.h>
+#include <errno.h>
 
 #include "mktime-internal.h"
 
-time_t
-timegm (struct tm *tmp)
+__time64_t
+__timegm64 (struct tm *tmp)
 {
   static mktime_offset_t gmtime_offset;
   tmp->tm_isdst = 0;
-  return __mktime_internal (tmp, __gmtime_r, &gmtime_offset);
+  return __mktime_internal (tmp, __gmtime64_r, &gmtime_offset);
 }
+
+#if defined _LIBC && __TIMESIZE != 64
+
+libc_hidden_def (__timegm64)
+
+time_t
+timegm (struct tm *tmp)
+{
+  struct tm tm = *tmp;
+  __time64_t t = __timegm64 (&tm);
+  if (in_time_t_range (t))
+    {
+      *tmp = tm;
+      return t;
+    }
+  else
+    {
+      __set_errno (EOVERFLOW);
+      return -1;
+    }
+}
+
+#endif
-- 
2.20.1

Re: [PATCH v2 1/2] Y2038: make __mktime_internal compatible with __time64_t

[Lukasz Majewski]

[-- Attachment #1: Type: text/plain, Size: 1315 bytes --]

Hi Joseph,

> On Sun, 17 Mar 2019, Lukasz Majewski wrote:
> 
> > Do you require following code in include/time.h for timelocal:
> > 
> > #if defined __USE_MISC
> > # if defined(__REDIRECT)
> >   extern int __REDIRECT (timelocal, (struct tm *__tp),
> >                          __mktime64) __THROW);
> > # else
> > #  define timelocal __mktime64
> > # endif
> > #endif  
> 
> include/time.h is an *internal* header, so should not need to test
> __USE_* or use __REDIRECT.
> 
> Something like that would be appropriate in the *installed* header 
> (time/time.h), under whatever __USE_* condition is set by
> _TIME_BITS=64, but only at the later point where we're ready to add
> support for _TIME_BITS=64 to the public headers and corresponding new
> public symbol versions for the new ABIs.
> 
> By using something like that in the installed header you avoid any
> need to define the name __timelocal64 anywhere.
> 

The distinction between internal and external set of headers was
unclear for me.

Thanks for clarification.

Best regards,

Lukasz Majewski

--

DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-59 Fax: (+49)-8142-66989-80 Email: lukma@denx.de

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH v2 1/2] Y2038: make __mktime_internal compatible with __time64_t

[Lukasz Majewski]

[-- Attachment #1: Type: text/plain, Size: 3346 bytes --]

Hi Paul,

> On 3/11/19 11:58 PM, Lukasz Majewski wrote:
> > The question is if there is a more suitable place than
> > include/time.h header to have fits_in_time() reused by both Glibc
> > and Gnulib?  
> 

Thanks for your patch.

> Hmm, well, on further thought, fits_in_time_t can live in
> include/time.h since only glibc will need it. Gnulib won't support
> two kinds of time_t types, so it should't need the function. (The
> attached patch renames it to "in_time_t_range" to avoid *_t
> pollution.)

Is the rewritten code correct?

+/* Check whether T fits in time_t.  */
+static inline bool
+in_time_t_range (__time64_t t)
+{
+  time_t s = t;
+  return s != t;
+}

Shouldn't we have: return s == t; ?

> 
> That being said, we will have to make a place for private .h code
> shared between glibc and Gnulib, because include/time.h can't easily
> be shared with Gnulib. I suggest using time/mktime-internal.h for this
> 

ACK/NAK for using time/mktime-internl.h shall be done by more
experienced glibc developer(s).

> Some other comments on that patch that I noticed while looking into
> this.
> 
> The patch added duplicate "#include <errno.h>" lines to time/timegm.c.

Ok.

> 
> I still don't get why we need __timelocal64. Glibc code can use
> __mktime64. User code shouldn't see that symbol.

If we don't need it for this conversion, then we should omit it. I will
add it if needed when preparing Y2038 patch set.

In the time/mktime.c there is:
weak_alias (mktime, timelocal), which makes the timelocal calls
aliases to mktime for time_t 32 and 64 bit (for Y2038 the proper
__REDIRECT will be added).

> 
> Come to think of it, user code shouldn't see __time64_t either. Yes,
> the name begins with two underscores so user code should avoid it,
> posix/bits/types.hbut putting __time64_t in /usr/include will just
> tempt users. It's easy to keep __time64_t out of /usr/include so
> let's do that.
> 

Is that the reason for removing __time64_t definition from
posix/bits/types.h ?

> The body of __mktime64 can be simplified; there's no need for locals
> of type __time64_t, time_t, and struct tm. And it should use
> in_time_t_range instead of doing it by hand.

Yes. Also there is no need to check for EOVERFLOW in the __mktime64()
function (as it operates inherently on 64 bit types).

> 
> The bodies of mktime and timegm [__TIMESIZE != 64] should not pass tp
> to __mktime64/__timegm64 directly, since *tp should not be updated if
> the result is in __time64_t range but out of time_t range. And timegm
> should use in_time_t_range instead of doing it by hand.
> 

Ok.

> The "#ifdef weak_alias" etc. lines can be removed now, since Gnulib
> does this stuff OK now.
> 
> timegm.c should include libc-config.h, not config.h, as this is the
> current Gnulib style for code shared with Gnulib.
> 
> Revised proposed patch attached.
> 

Just out of curiosity:

In the time/mktime-internal.h you added a comment regarding BeOS users
and posix time_t - do you know any :-) ?



Best regards,

Lukasz Majewski

--

DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-59 Fax: (+49)-8142-66989-80 Email: lukma@denx.de

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH v2 1/2] Y2038: make __mktime_internal compatible with __time64_t

[Paul Eggert]

Lukasz Majewski wrote:

> Shouldn't we have: return s == t; ?

Yes, absolutely. Thanks for catching that. I tested only the Gnulib version, and 
Gnulib doesn't use that code.

Do you have glibc tests to catch bugs like this? If no, please add writing some 
tests to your lists of things to do.

> In the time/mktime.c there is:
> weak_alias (mktime, timelocal), which makes the timelocal calls
> aliases to mktime for time_t 32 and 64 bit (for Y2038 the proper
> __REDIRECT will be added).

Sorry, I'm a bit lost here. How will that __REDIRECT work, exactly? Should it be 
part of this patch, or part of a later patch?

>> Come to think of it, user code shouldn't see __time64_t either....
> 
> Is that the reason for removing __time64_t definition from
> posix/bits/types.h ?

Yes.
> In the time/mktime-internal.h you added a comment regarding BeOS users
> and posix time_t - do you know any :-) ?

Just one. :-)  See:

https://lists.gnu.org/archive/html/bug-gnulib/2011-05/msg00470.html

Bruno's most recent BeOS-related submission to Gnulib was in October 2017:

https://lists.gnu.org/r/bug-gnulib/2017-10/msg00098.html

Re: [PATCH v2 1/2] Y2038: make __mktime_internal compatible with __time64_t

[Lukasz Majewski]

[-- Attachment #1: Type: text/plain, Size: 2324 bytes --]

Hi Paul,

> Lukasz Majewski wrote:
> 
> > Shouldn't we have: return s == t; ?  
> 
> Yes, absolutely. Thanks for catching that. I tested only the Gnulib
> version, and Gnulib doesn't use that code.

Do you plan to prepare (and send to mailing list) the next version of
this patch (including the above fix)?

> 
> Do you have glibc tests to catch bugs like this?

Actually yes:
https://github.com/lmajewski/y2038-tests/commits/master

> If no, please add
> writing some tests to your lists of things to do.
> 

The plan is to port above tests to glibc's test suite (as now they are
standalone).

There is also the Yocto/OE meta layer dedicated for testing/developing
Y2038 glibc with qemu:
https://github.com/lmajewski/meta-y2038/commits/master

And the Y2038 safe glibc:
https://github.com/lmajewski/y2038_glibc/commits/Y2038-2.29-glibc-11-03-2019


This code is going to be pushed also to sourceware.org git.

> > In the time/mktime.c there is:
> > weak_alias (mktime, timelocal), which makes the timelocal calls
> > aliases to mktime for time_t 32 and 64 bit (for Y2038 the proper
> > __REDIRECT will be added).  
> 
> Sorry, I'm a bit lost here. How will that __REDIRECT work, exactly?
> Should it be part of this patch, or part of a later patch?

The __REDIRECT would be a part of the latter patch - the one which adds
Y2038 support for 32 bit SoCs.

It would simply redirect calls to mktime/timegm to internal
__mktime64()/__timegm64().

> 
> >> Come to think of it, user code shouldn't see __time64_t
> >> either....  
> > 
> > Is that the reason for removing __time64_t definition from
> > posix/bits/types.h ?  
> 
> Yes.
> > In the time/mktime-internal.h you added a comment regarding BeOS
> > users and posix time_t - do you know any :-) ?  
> 
> Just one. :-)  See:
> 
> https://lists.gnu.org/archive/html/bug-gnulib/2011-05/msg00470.html
> 
> Bruno's most recent BeOS-related submission to Gnulib was in October
> 2017:
> 
> https://lists.gnu.org/r/bug-gnulib/2017-10/msg00098.html


Best regards,

Lukasz Majewski

--

DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-59 Fax: (+49)-8142-66989-80 Email: lukma@denx.de

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH v2 1/2] Y2038: make __mktime_internal compatible with __time64_t

[Paul Eggert]

[-- Attachment #1: Type: text/plain, Size: 812 bytes --]

On 3/20/19 12:03 AM, Lukasz Majewski wrote:
> Do you plan to prepare (and send to mailing list) the next version of
> this patch (including the above fix)?

Sure, attached.

> In the time/mktime.c there is:
> weak_alias (mktime, timelocal), which makes the timelocal calls
> aliases to mktime for time_t 32 and 64 bit (for Y2038 the proper
> __REDIRECT will be added).  
> Sorry, I'm a bit lost here. How will that __REDIRECT work, exactly?
> Should it be part of this patch, or part of a later patch?
> The __REDIRECT would be a part of the latter patch - the one which adds
> Y2038 support for 32 bit SoCs.
>
> It would simply redirect calls to mktime/timegm to internal
> __mktime64()/__timegm64().
>
OK, in that case, it can redirect timelocal calls to __mktime64, and
there is no need for a __timelocal64.


[-- Attachment #2: 0001-Make-mktime-etc.-compatible-with-__time64_t.patch --]
[-- Type: text/x-patch, Size: 17747 bytes --]

From 583877361f22d760fe15f7b6555cf9463c2f2d4a Mon Sep 17 00:00:00 2001
From: Paul Eggert <eggert@cs.ucla.edu>
Date: Mon, 18 Mar 2019 14:14:15 -0700
Subject: [PATCH] Make mktime etc. compatible with __time64_t

Keep these functions compatible with Gnulib while adding
__time64_t support.  The basic idea is to move private API
declarations from include/time.h to time/mktime-internal.h, since
the former file cannot easily be shared with Gnulib whereas the
latter can.
Also, do some other minor cleanup while in the neighborhood.
* include/time.h: Include stdbool.h, time/mktime-internal.h.
(__mktime_internal): Move this prototype to time/mktime-internal.h,
since Gnulib needs it.
(__localtime64_r, __gmtime64_r) [__TIMESIZE == 64]:
Move these macros to time/mktime-internal.h, since Gnulib needs them.
(__mktime64, __timegm64) [__TIMESIZE != 64]: New prototypes.
(in_time_t_range): New static function.
* posix/bits/types.h (__time64_t): Move to time/mktime-internal.h,
so that glibc users are not tempted to use __time64_t.
* time/mktime-internal.h: Rewrite so that it does both glibc
and Gnulib work.  Include time.h if not _LIBC.
(mktime_offset_t) [!_LIBC]: Define for gnulib.
(__time64_t): New type or macro, moved here from
posix/bits/types.h.
(__gmtime64_r, __localtime64_r, __mktime64, __timegm64)
[!_LIBC || __TIMESIZE == 64): New macros, mostly moved here
from include/time.h.
(__gmtime_r, __localtime_r, __mktime_internal) [!_LIBC]:
New macros, taken from GNulib.
(__mktime_internal): New prototype, moved here from include/time.h.
* time/mktime.c (mktime_min, mktime_max, convert_time)
(ranged_convert, __mktime_internal, __mktime64):
* time/timegm.c (__timegm64):
Use __time64_t, not time_t.
* time/mktime.c: Stop worrying about whether time_t is floating-point.
(__mktime64) [! (_LIBC && __TIMESIZE != 64)]:
Rename from mktime.
(mktime) [_LIBC && __TIMESIZE != 64]: New function.
* time/timegm.c [!_LIBC]: Include libc-config.h, not config.h,
for libc_hidden_def.
Include errno.h.
(__timegm64) [! (_LIBC && __TIMESIZE != 64)]:
Rename from timegm.
(timegm) [_LIBC && __TIMESIZE != 64]: New function.
---
 ChangeLog              | 44 +++++++++++++++++++++++
 include/time.h         | 39 ++++++++++----------
 posix/bits/types.h     |  6 ----
 time/mktime-internal.h | 80 +++++++++++++++++++++++++++++++++++++++++-
 time/mktime.c          | 71 +++++++++++++++++++++++--------------
 time/timegm.c          | 32 ++++++++++++++---
 6 files changed, 215 insertions(+), 57 deletions(-)

diff --git a/ChangeLog b/ChangeLog
index 45917ec56a..94a9a48ee9 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,47 @@
+2019-03-22  Paul Eggert  <eggert@cs.ucla.edu>
+
+	Make mktime etc. compatible with __time64_t
+	Keep these functions compatible with Gnulib while adding
+	__time64_t support.  The basic idea is to move private API
+	declarations from include/time.h to time/mktime-internal.h, since
+	the former file cannot easily be shared with Gnulib whereas the
+	latter can.
+	Also, do some other minor cleanup while in the neighborhood.
+	* include/time.h: Include stdbool.h, time/mktime-internal.h.
+	(__mktime_internal): Move this prototype to time/mktime-internal.h,
+	since Gnulib needs it.
+	(__localtime64_r, __gmtime64_r) [__TIMESIZE == 64]:
+	Move these macros to time/mktime-internal.h, since Gnulib needs them.
+	(__mktime64, __timegm64) [__TIMESIZE != 64]: New prototypes.
+	(in_time_t_range): New static function.
+	* posix/bits/types.h (__time64_t): Move to time/mktime-internal.h,
+	so that glibc users are not tempted to use __time64_t.
+	* time/mktime-internal.h: Rewrite so that it does both glibc
+	and Gnulib work.  Include time.h if not _LIBC.
+	(mktime_offset_t) [!_LIBC]: Define for gnulib.
+	(__time64_t): New type or macro, moved here from
+	posix/bits/types.h.
+	(__gmtime64_r, __localtime64_r, __mktime64, __timegm64)
+	[!_LIBC || __TIMESIZE == 64): New macros, mostly moved here
+	from include/time.h.
+	(__gmtime_r, __localtime_r, __mktime_internal) [!_LIBC]:
+	New macros, taken from GNulib.
+	(__mktime_internal): New prototype, moved here from include/time.h.
+	* time/mktime.c (mktime_min, mktime_max, convert_time)
+	(ranged_convert, __mktime_internal, __mktime64):
+	* time/timegm.c (__timegm64):
+	Use __time64_t, not time_t.
+	* time/mktime.c: Stop worrying about whether time_t is floating-point.
+	(__mktime64) [! (_LIBC && __TIMESIZE != 64)]:
+	Rename from mktime.
+	(mktime) [_LIBC && __TIMESIZE != 64]: New function.
+	* time/timegm.c [!_LIBC]: Include libc-config.h, not config.h,
+	for libc_hidden_def.
+	Include errno.h.
+	(__timegm64) [! (_LIBC && __TIMESIZE != 64)]:
+	Rename from timegm.
+	(timegm) [_LIBC && __TIMESIZE != 64]: New function.
+
 2019-03-22  Adhemerval Zanella  <adhemerval.zanella@linaro.org>
 
 	* benchtests/Makefile (USE_CLOCK_GETTIME) Remove.
diff --git a/include/time.h b/include/time.h
index 61dd9e180b..ac3163c2a5 100644
--- a/include/time.h
+++ b/include/time.h
@@ -3,6 +3,8 @@
 
 #ifndef _ISOMAC
 # include <bits/types/locale_t.h>
+# include <stdbool.h>
+# include <time/mktime-internal.h>
 
 extern __typeof (strftime_l) __strftime_l;
 libc_hidden_proto (__strftime_l)
@@ -49,19 +51,11 @@ extern void __tzset_parse_tz (const char *tz) attribute_hidden;
 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
-   normalize TP, given that a `struct tm *' maps to a `time_t' as performed
-   by FUNC.  Record next guess for localtime-gmtime offset in *OFFSET.  */
-extern time_t __mktime_internal (struct tm *__tp,
-				 struct tm *(*__func) (const time_t *,
-						       struct tm *),
-				 long int *__offset) attribute_hidden;
-
 #if __TIMESIZE == 64
 # define __ctime64 ctime
 #else
 extern char *__ctime64 (const __time64_t *__timer) __THROW;
-libc_hidden_proto (__ctime64);
+libc_hidden_proto (__ctime64)
 #endif
 
 #if __TIMESIZE == 64
@@ -69,7 +63,7 @@ libc_hidden_proto (__ctime64);
 #else
 extern char *__ctime64_r (const __time64_t *__restrict __timer,
 		          char *__restrict __buf) __THROW;
-libc_hidden_proto (__ctime64_r);
+libc_hidden_proto (__ctime64_r)
 #endif
 
 #if __TIMESIZE == 64
@@ -81,13 +75,13 @@ libc_hidden_proto (__localtime64)
 
 extern struct tm *__localtime_r (const time_t *__timer,
 				 struct tm *__tp) attribute_hidden;
-
-#if __TIMESIZE == 64
-# define __localtime64_r __localtime_r
-#else
+#if __TIMESIZE != 64
 extern struct tm *__localtime64_r (const __time64_t *__timer,
 				   struct tm *__tp);
 libc_hidden_proto (__localtime64_r)
+
+extern __time64_t __mktime64 (struct tm *__tp) __THROW;
+libc_hidden_proto (__mktime64)
 #endif
 
 extern struct tm *__gmtime_r (const time_t *__restrict __timer,
@@ -99,14 +93,13 @@ libc_hidden_proto (__gmtime_r)
 #else
 extern struct tm *__gmtime64 (const __time64_t *__timer);
 libc_hidden_proto (__gmtime64)
-#endif
 
-#if __TIMESIZE == 64
-# define __gmtime64_r __gmtime_r
-#else
 extern struct tm *__gmtime64_r (const __time64_t *__restrict __timer,
 				struct tm *__restrict __tp);
-libc_hidden_proto (__gmtime64_r);
+libc_hidden_proto (__gmtime64_r)
+
+extern __time64_t __timegm64 (struct tm *__tp) __THROW;
+libc_hidden_proto (__timegm64)
 #endif
 
 /* Compute the `struct tm' representation of T,
@@ -155,5 +148,13 @@ extern double __difftime (time_t time1, time_t time0);
    actual clock ID.  */
 #define CLOCK_IDFIELD_SIZE	3
 
+/* Check whether T fits in time_t.  */
+static inline bool
+in_time_t_range (__time64_t t)
+{
+  time_t s = t;
+  return s == t;
+}
+
 #endif
 #endif
diff --git a/posix/bits/types.h b/posix/bits/types.h
index 0de6c59bb4..110081316b 100644
--- a/posix/bits/types.h
+++ b/posix/bits/types.h
@@ -213,12 +213,6 @@ __STD_TYPE __U32_TYPE __socklen_t;
    It is not currently necessary for this to be machine-specific.  */
 typedef int __sig_atomic_t;
 
-#if __TIMESIZE == 64
-# define __time64_t __time_t
-#else
-__STD_TYPE __TIME64_T_TYPE __time64_t;	/* Seconds since the Epoch.  */
-#endif
-
 #undef __STD_TYPE
 
 #endif /* bits/types.h */
diff --git a/time/mktime-internal.h b/time/mktime-internal.h
index 6111c22880..7cdc868f6d 100644
--- a/time/mktime-internal.h
+++ b/time/mktime-internal.h
@@ -1,2 +1,80 @@
-/* Gnulib mktime-internal.h, tailored for glibc.  */
+/* Internals of mktime and related functions
+   Copyright 2016-2019 Free Software Foundation, Inc.
+   This file is part of the GNU C Library.
+   Contributed by Paul Eggert <eggert@cs.ucla.edu>.
+
+   The GNU C Library is free software; you can redistribute it and/or
+   modify it under the terms of the GNU Lesser General Public
+   License as published by the Free Software Foundation; either
+   version 2.1 of the License, or (at your option) any later version.
+
+   The GNU C Library is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+   Lesser General Public License for more details.
+
+   You should have received a copy of the GNU Lesser General Public
+   License along with the GNU C Library; if not, see
+   <https://www.gnu.org/licenses/>.  */
+
+#ifndef _LIBC
+# include <time.h>
+#endif
+
+/* mktime_offset_t is a signed type wide enough to hold a UTC offset
+   in seconds, and used as part of the type of the offset-guess
+   argument to mktime_internal.  In Glibc, it is always long int.
+   When in Gnulib, use time_t on platforms where time_t
+   is signed, to be compatible with platforms like BeOS that export
+   this implementation detail of mktime.  On platforms where time_t is
+   unsigned, GNU and POSIX code can assume 'int' is at least 32 bits
+   which is wide enough for a UTC offset.  */
+#ifdef _LIBC
 typedef long int mktime_offset_t;
+#elif TIME_T_IS_SIGNED
+typedef time_t mktime_offset_t;
+#else
+typedef int mktime_offset_t;
+#endif
+
+/* The source code uses identifiers like __time64_t for glibc
+   timestamps that can contain 64-bit values even when time_t is only
+   32 bits.  These are just macros for the ordinary identifiers unless
+   compiling within glibc when time_t is 32 bits.  */
+#if defined _LIBC && __TIMESIZE != 64
+typedef __TIME64_T_TYPE __time64_t;
+#else
+# define __time64_t time_t
+# define __gmtime64_r __gmtime_r
+# define __localtime64_r __localtime_r
+# define __mktime64 mktime
+# define __timegm64 timegm
+#endif
+
+#ifndef _LIBC
+
+/* Although glibc source code uses leading underscores, Gnulib wants
+   ordinary names.
+
+   Portable standalone applications should supply a <time.h> that
+   declares a POSIX-compliant localtime_r, for the benefit of older
+   implementations that lack localtime_r or have a nonstandard one.
+   Similarly for gmtime_r.  See the gnulib time_r module for one way
+   to implement this.  */
+
+# undef __gmtime_r
+# undef __localtime_r
+# define __gmtime_r gmtime_r
+# define __localtime_r localtime_r
+
+# define __mktime_internal mktime_internal
+
+#endif
+
+/* Subroutine of mktime.  Return the time_t representation of TP and
+   normalize TP, given that a struct tm * maps to a time_t as performed
+   by FUNC.  Record next guess for localtime-gmtime offset in *OFFSET.  */
+extern __time64_t __mktime_internal (struct tm *tp,
+                                     struct tm *(*func) (__time64_t const *,
+                                                         struct tm *),
+                                     mktime_offset_t *offset) attribute_hidden;
diff --git a/time/mktime.c b/time/mktime.c
index 57efee9b25..8fe2f963ae 100644
--- a/time/mktime.c
+++ b/time/mktime.c
@@ -112,11 +112,11 @@ my_tzset (void)
    added to them, and then with another timestamp added, without
    worrying about overflow.
 
-   Much of the code uses long_int to represent time_t values, to
-   lessen the hassle of dealing with platforms where time_t is
+   Much of the code uses long_int to represent __time64_t values, to
+   lessen the hassle of dealing with platforms where __time64_t is
    unsigned, and because long_int should suffice to represent all
-   time_t values that mktime can generate even on platforms where
-   time_t is excessively wide.  */
+   __time64_t values that mktime can generate even on platforms where
+   __time64_t is wider than the int components of struct tm.  */
 
 #if INT_MAX <= LONG_MAX / 4 / 366 / 24 / 60 / 60
 typedef long int long_int;
@@ -144,16 +144,15 @@ shr (long_int a, int b)
 	  : a / (one << b) - (a % (one << b) < 0));
 }
 
-/* Bounds for the intersection of time_t and long_int.  */
+/* Bounds for the intersection of __time64_t and long_int.  */
 
 static long_int const mktime_min
-  = ((TYPE_SIGNED (time_t) && TYPE_MINIMUM (time_t) < TYPE_MINIMUM (long_int))
-     ? TYPE_MINIMUM (long_int) : TYPE_MINIMUM (time_t));
+  = ((TYPE_SIGNED (__time64_t)
+      && TYPE_MINIMUM (__time64_t) < TYPE_MINIMUM (long_int))
+     ? TYPE_MINIMUM (long_int) : TYPE_MINIMUM (__time64_t));
 static long_int const mktime_max
-  = (TYPE_MAXIMUM (long_int) < TYPE_MAXIMUM (time_t)
-     ? TYPE_MAXIMUM (long_int) : TYPE_MAXIMUM (time_t));
-
-verify (TYPE_IS_INTEGER (time_t));
+  = (TYPE_MAXIMUM (long_int) < TYPE_MAXIMUM (__time64_t)
+     ? TYPE_MAXIMUM (long_int) : TYPE_MAXIMUM (__time64_t));
 
 #define EPOCH_YEAR 1970
 #define TM_YEAR_BASE 1900
@@ -252,23 +251,23 @@ tm_diff (long_int year, long_int yday, int hour, int min, int sec,
 }
 
 /* Use CONVERT to convert T to a struct tm value in *TM.  T must be in
-   range for time_t.  Return TM if successful, NULL (setting errno) on
+   range for __time64_t.  Return TM if successful, NULL (setting errno) on
    failure.  */
 static struct tm *
-convert_time (struct tm *(*convert) (const time_t *, struct tm *),
+convert_time (struct tm *(*convert) (const __time64_t *, struct tm *),
 	      long_int t, struct tm *tm)
 {
-  time_t x = t;
+  __time64_t x = t;
   return convert (&x, tm);
 }
 
 /* Use CONVERT to convert *T to a broken down time in *TP.
    If *T is out of range for conversion, adjust it so that
    it is the nearest in-range value and then convert that.
-   A value is in range if it fits in both time_t and long_int.
+   A value is in range if it fits in both __time64_t and long_int.
    Return TP on success, NULL (setting errno) on failure.  */
 static struct tm *
-ranged_convert (struct tm *(*convert) (const time_t *, struct tm *),
+ranged_convert (struct tm *(*convert) (const __time64_t *, struct tm *),
 		long_int *t, struct tm *tp)
 {
   long_int t1 = (*t < mktime_min ? mktime_min
@@ -310,7 +309,7 @@ ranged_convert (struct tm *(*convert) (const time_t *, struct tm *),
 }
 
 
-/* Convert *TP to a time_t value, inverting
+/* Convert *TP to a __time64_t value, inverting
    the monotonic and mostly-unit-linear conversion function CONVERT.
    Use *OFFSET to keep track of a guess at the offset of the result,
    compared to what the result would be for UTC without leap seconds.
@@ -318,9 +317,9 @@ ranged_convert (struct tm *(*convert) (const time_t *, struct tm *),
    If successful, set *TP to the canonicalized struct tm;
    otherwise leave *TP alone, return ((time_t) -1) and set errno.
    This function is external because it is used also by timegm.c.  */
-time_t
+__time64_t
 __mktime_internal (struct tm *tp,
-		   struct tm *(*convert) (const time_t *, struct tm *),
+		   struct tm *(*convert) (const __time64_t *, struct tm *),
 		   mktime_offset_t *offset)
 {
   struct tm tm;
@@ -520,9 +519,9 @@ __mktime_internal (struct tm *tp,
 
 #if defined _LIBC || NEED_MKTIME_WORKING || NEED_MKTIME_WINDOWS
 
-/* Convert *TP to a time_t value.  */
-time_t
-mktime (struct tm *tp)
+/* Convert *TP to a __time64_t value.  */
+__time64_t
+__mktime64 (struct tm *tp)
 {
   /* POSIX.1 8.1.1 requires that whenever mktime() is called, the
      time zone names contained in the external variable 'tzname' shall
@@ -531,7 +530,7 @@ mktime (struct tm *tp)
 
 # if defined _LIBC || NEED_MKTIME_WORKING
   static mktime_offset_t localtime_offset;
-  return __mktime_internal (tp, __localtime_r, &localtime_offset);
+  return __mktime_internal (tp, __localtime64_r, &localtime_offset);
 # else
 #  undef mktime
   return mktime (tp);
@@ -539,11 +538,29 @@ mktime (struct tm *tp)
 }
 #endif /* _LIBC || NEED_MKTIME_WORKING || NEED_MKTIME_WINDOWS */
 
-#ifdef weak_alias
-weak_alias (mktime, timelocal)
+#if defined _LIBC && __TIMESIZE != 64
+
+libc_hidden_def (__mktime64)
+
+time_t
+mktime (struct tm *tp)
+{
+  struct tm tm = *tp;
+  __time64_t t = __mktime64 (&tm);
+  if (in_time_t_range (t))
+    {
+      *tp = tm;
+      return t;
+    }
+  else
+    {
+      __set_errno (EOVERFLOW);
+      return -1;
+    }
+}
+
 #endif
 
-#ifdef _LIBC
+weak_alias (mktime, timelocal)
 libc_hidden_def (mktime)
 libc_hidden_weak (timelocal)
-#endif
diff --git a/time/timegm.c b/time/timegm.c
index bfd36d0255..bae0ceee5e 100644
--- a/time/timegm.c
+++ b/time/timegm.c
@@ -18,17 +18,41 @@
    <http://www.gnu.org/licenses/>.  */
 
 #ifndef _LIBC
-# include <config.h>
+# include <libc-config.h>
 #endif
 
 #include <time.h>
+#include <errno.h>
 
 #include "mktime-internal.h"
 
-time_t
-timegm (struct tm *tmp)
+__time64_t
+__timegm64 (struct tm *tmp)
 {
   static mktime_offset_t gmtime_offset;
   tmp->tm_isdst = 0;
-  return __mktime_internal (tmp, __gmtime_r, &gmtime_offset);
+  return __mktime_internal (tmp, __gmtime64_r, &gmtime_offset);
 }
+
+#if defined _LIBC && __TIMESIZE != 64
+
+libc_hidden_def (__timegm64)
+
+time_t
+timegm (struct tm *tmp)
+{
+  struct tm tm = *tmp;
+  __time64_t t = __timegm64 (&tm);
+  if (in_time_t_range (t))
+    {
+      *tmp = tm;
+      return t;
+    }
+  else
+    {
+      __set_errno (EOVERFLOW);
+      return -1;
+    }
+}
+
+#endif
-- 
2.20.1

Re: [PATCH v2 1/2] Y2038: make __mktime_internal compatible with __time64_t

[Lukasz Majewski]

[-- Attachment #1: Type: text/plain, Size: 2097 bytes --]

Hi Paul,

> Lukasz Majewski wrote:
> 
> > Shouldn't we have: return s == t; ?  
> 
> Yes, absolutely. Thanks for catching that. I tested only the Gnulib
> version, and Gnulib doesn't use that code.
> 
> Do you have glibc tests to catch bugs like this? If no, please add
> writing some tests to your lists of things to do.

Please find some patches to be applied on top of your patch to make the
Y2038 system working:

https://github.com/lmajewski/y2038_glibc/commits/mktime_v3_fixes

In short:

- The __time64_t needs to be exported to user space as it is a building
  block for Y2038 safe 32 bit systems' structures (like struct
  __timeval64). In the above use case the "normal" timeval is
  implicitly replaced with __timeval64.

- The correct condition for "in_time_t_range()"

- Some fixes necessary to make glibc building


Please consider merging those changes to your patch.

> 
> > In the time/mktime.c there is:
> > weak_alias (mktime, timelocal), which makes the timelocal calls
> > aliases to mktime for time_t 32 and 64 bit (for Y2038 the proper
> > __REDIRECT will be added).  
> 
> Sorry, I'm a bit lost here. How will that __REDIRECT work, exactly?
> Should it be part of this patch, or part of a later patch?
> 
> >> Come to think of it, user code shouldn't see __time64_t
> >> either....  
> > 
> > Is that the reason for removing __time64_t definition from
> > posix/bits/types.h ?  
> 
> Yes.
> > In the time/mktime-internal.h you added a comment regarding BeOS
> > users and posix time_t - do you know any :-) ?  
> 
> Just one. :-)  See:
> 
> https://lists.gnu.org/archive/html/bug-gnulib/2011-05/msg00470.html
> 
> Bruno's most recent BeOS-related submission to Gnulib was in October
> 2017:
> 
> https://lists.gnu.org/r/bug-gnulib/2017-10/msg00098.html


Best regards,

Lukasz Majewski

--

DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-59 Fax: (+49)-8142-66989-80 Email: lukma@denx.de

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH v2 1/2] Y2038: make __mktime_internal compatible with __time64_t

[Lukasz Majewski]

[-- Attachment #1: Type: text/plain, Size: 1634 bytes --]

Hi Paul,

> On 3/20/19 12:03 AM, Lukasz Majewski wrote:
> > Do you plan to prepare (and send to mailing list) the next version
> > of this patch (including the above fix)?
> 
> Sure, attached.
> 
> > In the time/mktime.c there is:
> > weak_alias (mktime, timelocal), which makes the timelocal calls
> > aliases to mktime for time_t 32 and 64 bit (for Y2038 the proper
> > __REDIRECT will be added).  
> > Sorry, I'm a bit lost here. How will that __REDIRECT work, exactly?
> > Should it be part of this patch, or part of a later patch?
> > The __REDIRECT would be a part of the latter patch - the one which
> > adds Y2038 support for 32 bit SoCs.
> >
> > It would simply redirect calls to mktime/timegm to internal
> > __mktime64()/__timegm64().
> >
> OK, in that case, it can redirect timelocal calls to __mktime64, and
> there is no need for a __timelocal64.
> 

Thanks for the updated patch. I don't know why I've just received your
e-mail (but it is from Friday).

Today (Sat, around 12:00 CET), I've spent some time on testing your
previous work and write some comments/fixes in the other mail.

The code can be found in:
https://github.com/lmajewski/y2038_glibc/commits/mktime_v3_fixes

The biggest problem from Y2038 support point of view is with removing
__time64_t from posix/bits/types.h
(more explanation in the other mail).


Best regards,

Lukasz Majewski

--

DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-59 Fax: (+49)-8142-66989-80 Email: lukma@denx.de

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH v2 1/2] Y2038: make __mktime_internal compatible with __time64_t

[Lukasz Majewski]

[-- Attachment #1: Type: text/plain, Size: 2129 bytes --]

Hi Paul,

> Hi Paul,
> 
> > On 3/20/19 12:03 AM, Lukasz Majewski wrote:  
> > > Do you plan to prepare (and send to mailing list) the next version
> > > of this patch (including the above fix)?  
> > 
> > Sure, attached.
> >   
> > > In the time/mktime.c there is:
> > > weak_alias (mktime, timelocal), which makes the timelocal calls
> > > aliases to mktime for time_t 32 and 64 bit (for Y2038 the proper
> > > __REDIRECT will be added).  
> > > Sorry, I'm a bit lost here. How will that __REDIRECT work,
> > > exactly? Should it be part of this patch, or part of a later
> > > patch? The __REDIRECT would be a part of the latter patch - the
> > > one which adds Y2038 support for 32 bit SoCs.
> > >
> > > It would simply redirect calls to mktime/timegm to internal
> > > __mktime64()/__timegm64().
> > >  
> > OK, in that case, it can redirect timelocal calls to __mktime64, and
> > there is no need for a __timelocal64.
> >   
> 
> Thanks for the updated patch. I don't know why I've just received your
> e-mail (but it is from Friday).
> 
> Today (Sat, around 12:00 CET), I've spent some time on testing your
> previous work and write some comments/fixes in the other mail.
> 
> The code can be found in:
> https://github.com/lmajewski/y2038_glibc/commits/mktime_v3_fixes

Please find the notification about this branch being updated (in the
case you have already fetched it).

> 
> The biggest problem from Y2038 support point of view is with removing
> __time64_t from posix/bits/types.h
> (more explanation in the other mail).
> 
> 
> Best regards,
> 
> Lukasz Majewski
> 
> --
> 
> DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
> HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
> Phone: (+49)-8142-66989-59 Fax: (+49)-8142-66989-80 Email:
> lukma@denx.de




Best regards,

Lukasz Majewski

--

DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-59 Fax: (+49)-8142-66989-80 Email: lukma@denx.de

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH v2 1/2] Y2038: make __mktime_internal compatible with __time64_t

[Paul Eggert]

[-- Attachment #1: Type: text/plain, Size: 1732 bytes --]

On 3/23/19 4:59 AM, Lukasz Majewski wrote:
> https://github.com/lmajewski/y2038_glibc/commits/mktime_v3_fixes
>
> In short:
>
> - The __time64_t needs to be exported to user space as it is a building
>   block for Y2038 safe 32 bit systems' structures (like struct
>   __timeval64). In the above use case the "normal" timeval is
>   implicitly replaced with __timeval64.

I looked into that URL, and I don't see any explanation of why
__time64_t needs to be visible to user code.

Surely struct timeval ought to be consistent with time_t. That is, it
ought to continue to be defined like this:

struct timeval
{
  __time_t tv_sec;        /* Seconds.  */
  __suseconds_t tv_usec;    /* Microseconds.  */
};

where __time_t is merely an alias for time_t and so is 64 bits if
_TIME_BITS=64 and is the current size (32 or 64) otherwise.

The only reason I can see why __time64_t needs to be visible to user
code, would be if a single user source file accesses both time_t and
__time64_t (or needs to access both struct timeval and struct timeval64,
or similarly for struct timespec etc.). However, we should not support a
complicated API like that, as it's typically not useful in practice and
its mere availability causes more confusion than it's worth - as I've
discovered with _FILE_OFFSET_BITS and __off64_t. Instead, glibc should
have a simple user API where _TIME_BITS=64 merely says "I want 64-bit
time_t everywhere" and a module either uses this option or it doesn't.

I did see a fix in that URL, to use "#elif defined TIME_T_IS_SIGNED"
instead of "#elif TIME_T_IS_SIGNED" for the benefit of picky glibc
compiles, and that fix is incorporated in the revised patch attached.


[-- Attachment #2: 0001-Make-mktime-etc.-compatible-with-__time64_t.patch --]
[-- Type: text/x-patch, Size: 17776 bytes --]

From 8d6ea5e533fb3c071ae182dbf16a32b959f473b0 Mon Sep 17 00:00:00 2001
From: Paul Eggert <eggert@cs.ucla.edu>
Date: Mon, 18 Mar 2019 14:14:15 -0700
Subject: [PATCH] Make mktime etc. compatible with __time64_t

Keep these functions compatible with Gnulib while adding
__time64_t support.  The basic idea is to move private API
declarations from include/time.h to time/mktime-internal.h, since
the former file cannot easily be shared with Gnulib whereas the
latter can.
Also, do some other minor cleanup while in the neighborhood.
* include/time.h: Include stdbool.h, time/mktime-internal.h.
(__mktime_internal): Move this prototype to time/mktime-internal.h,
since Gnulib needs it.
(__localtime64_r, __gmtime64_r) [__TIMESIZE == 64]:
Move these macros to time/mktime-internal.h, since Gnulib needs them.
(__mktime64, __timegm64) [__TIMESIZE != 64]: New prototypes.
(in_time_t_range): New static function.
* posix/bits/types.h (__time64_t): Move to time/mktime-internal.h,
so that glibc users are not tempted to use __time64_t.
* time/mktime-internal.h: Rewrite so that it does both glibc
and Gnulib work.  Include time.h if not _LIBC.
(mktime_offset_t) [!_LIBC]: Define for gnulib.
(__time64_t): New type or macro, moved here from
posix/bits/types.h.
(__gmtime64_r, __localtime64_r, __mktime64, __timegm64)
[!_LIBC || __TIMESIZE == 64): New macros, mostly moved here
from include/time.h.
(__gmtime_r, __localtime_r, __mktime_internal) [!_LIBC]:
New macros, taken from GNulib.
(__mktime_internal): New prototype, moved here from include/time.h.
* time/mktime.c (mktime_min, mktime_max, convert_time)
(ranged_convert, __mktime_internal, __mktime64):
* time/timegm.c (__timegm64):
Use __time64_t, not time_t.
* time/mktime.c: Stop worrying about whether time_t is floating-point.
(__mktime64) [! (_LIBC && __TIMESIZE != 64)]:
Rename from mktime.
(mktime) [_LIBC && __TIMESIZE != 64]: New function.
* time/timegm.c [!_LIBC]: Include libc-config.h, not config.h,
for libc_hidden_def.
Include errno.h.
(__timegm64) [! (_LIBC && __TIMESIZE != 64)]:
Rename from timegm.
(timegm) [_LIBC && __TIMESIZE != 64]: New function.
---
 ChangeLog              | 44 +++++++++++++++++++++++
 include/time.h         | 39 ++++++++++----------
 posix/bits/types.h     |  6 ----
 time/mktime-internal.h | 80 +++++++++++++++++++++++++++++++++++++++++-
 time/mktime.c          | 71 +++++++++++++++++++++++--------------
 time/timegm.c          | 32 ++++++++++++++---
 6 files changed, 215 insertions(+), 57 deletions(-)

diff --git a/ChangeLog b/ChangeLog
index bd76c1e28d..4de7b142a5 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,47 @@
+2019-03-27  Paul Eggert  <eggert@cs.ucla.edu>
+
+	Make mktime etc. compatible with __time64_t
+	Keep these functions compatible with Gnulib while adding
+	__time64_t support.  The basic idea is to move private API
+	declarations from include/time.h to time/mktime-internal.h, since
+	the former file cannot easily be shared with Gnulib whereas the
+	latter can.
+	Also, do some other minor cleanup while in the neighborhood.
+	* include/time.h: Include stdbool.h, time/mktime-internal.h.
+	(__mktime_internal): Move this prototype to time/mktime-internal.h,
+	since Gnulib needs it.
+	(__localtime64_r, __gmtime64_r) [__TIMESIZE == 64]:
+	Move these macros to time/mktime-internal.h, since Gnulib needs them.
+	(__mktime64, __timegm64) [__TIMESIZE != 64]: New prototypes.
+	(in_time_t_range): New static function.
+	* posix/bits/types.h (__time64_t): Move to time/mktime-internal.h,
+	so that glibc users are not tempted to use __time64_t.
+	* time/mktime-internal.h: Rewrite so that it does both glibc
+	and Gnulib work.  Include time.h if not _LIBC.
+	(mktime_offset_t) [!_LIBC]: Define for gnulib.
+	(__time64_t): New type or macro, moved here from
+	posix/bits/types.h.
+	(__gmtime64_r, __localtime64_r, __mktime64, __timegm64)
+	[!_LIBC || __TIMESIZE == 64): New macros, mostly moved here
+	from include/time.h.
+	(__gmtime_r, __localtime_r, __mktime_internal) [!_LIBC]:
+	New macros, taken from GNulib.
+	(__mktime_internal): New prototype, moved here from include/time.h.
+	* time/mktime.c (mktime_min, mktime_max, convert_time)
+	(ranged_convert, __mktime_internal, __mktime64):
+	* time/timegm.c (__timegm64):
+	Use __time64_t, not time_t.
+	* time/mktime.c: Stop worrying about whether time_t is floating-point.
+	(__mktime64) [! (_LIBC && __TIMESIZE != 64)]:
+	Rename from mktime.
+	(mktime) [_LIBC && __TIMESIZE != 64]: New function.
+	* time/timegm.c [!_LIBC]: Include libc-config.h, not config.h,
+	for libc_hidden_def.
+	Include errno.h.
+	(__timegm64) [! (_LIBC && __TIMESIZE != 64)]:
+	Rename from timegm.
+	(timegm) [_LIBC && __TIMESIZE != 64]: New function.
+
 2019-02-26  Adhemerval Zanella  <adhemerval.zanella@linaro.org>
 
 	* math/math.h (fpclassify, isfinite, isnormal, isnan): Use builtin for
diff --git a/include/time.h b/include/time.h
index 61dd9e180b..ac3163c2a5 100644
--- a/include/time.h
+++ b/include/time.h
@@ -3,6 +3,8 @@
 
 #ifndef _ISOMAC
 # include <bits/types/locale_t.h>
+# include <stdbool.h>
+# include <time/mktime-internal.h>
 
 extern __typeof (strftime_l) __strftime_l;
 libc_hidden_proto (__strftime_l)
@@ -49,19 +51,11 @@ extern void __tzset_parse_tz (const char *tz) attribute_hidden;
 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
-   normalize TP, given that a `struct tm *' maps to a `time_t' as performed
-   by FUNC.  Record next guess for localtime-gmtime offset in *OFFSET.  */
-extern time_t __mktime_internal (struct tm *__tp,
-				 struct tm *(*__func) (const time_t *,
-						       struct tm *),
-				 long int *__offset) attribute_hidden;
-
 #if __TIMESIZE == 64
 # define __ctime64 ctime
 #else
 extern char *__ctime64 (const __time64_t *__timer) __THROW;
-libc_hidden_proto (__ctime64);
+libc_hidden_proto (__ctime64)
 #endif
 
 #if __TIMESIZE == 64
@@ -69,7 +63,7 @@ libc_hidden_proto (__ctime64);
 #else
 extern char *__ctime64_r (const __time64_t *__restrict __timer,
 		          char *__restrict __buf) __THROW;
-libc_hidden_proto (__ctime64_r);
+libc_hidden_proto (__ctime64_r)
 #endif
 
 #if __TIMESIZE == 64
@@ -81,13 +75,13 @@ libc_hidden_proto (__localtime64)
 
 extern struct tm *__localtime_r (const time_t *__timer,
 				 struct tm *__tp) attribute_hidden;
-
-#if __TIMESIZE == 64
-# define __localtime64_r __localtime_r
-#else
+#if __TIMESIZE != 64
 extern struct tm *__localtime64_r (const __time64_t *__timer,
 				   struct tm *__tp);
 libc_hidden_proto (__localtime64_r)
+
+extern __time64_t __mktime64 (struct tm *__tp) __THROW;
+libc_hidden_proto (__mktime64)
 #endif
 
 extern struct tm *__gmtime_r (const time_t *__restrict __timer,
@@ -99,14 +93,13 @@ libc_hidden_proto (__gmtime_r)
 #else
 extern struct tm *__gmtime64 (const __time64_t *__timer);
 libc_hidden_proto (__gmtime64)
-#endif
 
-#if __TIMESIZE == 64
-# define __gmtime64_r __gmtime_r
-#else
 extern struct tm *__gmtime64_r (const __time64_t *__restrict __timer,
 				struct tm *__restrict __tp);
-libc_hidden_proto (__gmtime64_r);
+libc_hidden_proto (__gmtime64_r)
+
+extern __time64_t __timegm64 (struct tm *__tp) __THROW;
+libc_hidden_proto (__timegm64)
 #endif
 
 /* Compute the `struct tm' representation of T,
@@ -155,5 +148,13 @@ extern double __difftime (time_t time1, time_t time0);
    actual clock ID.  */
 #define CLOCK_IDFIELD_SIZE	3
 
+/* Check whether T fits in time_t.  */
+static inline bool
+in_time_t_range (__time64_t t)
+{
+  time_t s = t;
+  return s == t;
+}
+
 #endif
 #endif
diff --git a/posix/bits/types.h b/posix/bits/types.h
index 0de6c59bb4..110081316b 100644
--- a/posix/bits/types.h
+++ b/posix/bits/types.h
@@ -213,12 +213,6 @@ __STD_TYPE __U32_TYPE __socklen_t;
    It is not currently necessary for this to be machine-specific.  */
 typedef int __sig_atomic_t;
 
-#if __TIMESIZE == 64
-# define __time64_t __time_t
-#else
-__STD_TYPE __TIME64_T_TYPE __time64_t;	/* Seconds since the Epoch.  */
-#endif
-
 #undef __STD_TYPE
 
 #endif /* bits/types.h */
diff --git a/time/mktime-internal.h b/time/mktime-internal.h
index 6111c22880..15dec0317c 100644
--- a/time/mktime-internal.h
+++ b/time/mktime-internal.h
@@ -1,2 +1,80 @@
-/* Gnulib mktime-internal.h, tailored for glibc.  */
+/* Internals of mktime and related functions
+   Copyright 2016-2019 Free Software Foundation, Inc.
+   This file is part of the GNU C Library.
+   Contributed by Paul Eggert <eggert@cs.ucla.edu>.
+
+   The GNU C Library is free software; you can redistribute it and/or
+   modify it under the terms of the GNU Lesser General Public
+   License as published by the Free Software Foundation; either
+   version 2.1 of the License, or (at your option) any later version.
+
+   The GNU C Library is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+   Lesser General Public License for more details.
+
+   You should have received a copy of the GNU Lesser General Public
+   License along with the GNU C Library; if not, see
+   <https://www.gnu.org/licenses/>.  */
+
+#ifndef _LIBC
+# include <time.h>
+#endif
+
+/* mktime_offset_t is a signed type wide enough to hold a UTC offset
+   in seconds, and used as part of the type of the offset-guess
+   argument to mktime_internal.  In Glibc, it is always long int.
+   When in Gnulib, use time_t on platforms where time_t
+   is signed, to be compatible with platforms like BeOS that export
+   this implementation detail of mktime.  On platforms where time_t is
+   unsigned, GNU and POSIX code can assume 'int' is at least 32 bits
+   which is wide enough for a UTC offset.  */
+#ifdef _LIBC
 typedef long int mktime_offset_t;
+#elif defined TIME_T_IS_SIGNED
+typedef time_t mktime_offset_t;
+#else
+typedef int mktime_offset_t;
+#endif
+
+/* The source code uses identifiers like __time64_t for glibc
+   timestamps that can contain 64-bit values even when time_t is only
+   32 bits.  These are just macros for the ordinary identifiers unless
+   compiling within glibc when time_t is 32 bits.  */
+#if defined _LIBC && __TIMESIZE != 64
+typedef __TIME64_T_TYPE __time64_t;
+#else
+# define __time64_t time_t
+# define __gmtime64_r __gmtime_r
+# define __localtime64_r __localtime_r
+# define __mktime64 mktime
+# define __timegm64 timegm
+#endif
+
+#ifndef _LIBC
+
+/* Although glibc source code uses leading underscores, Gnulib wants
+   ordinary names.
+
+   Portable standalone applications should supply a <time.h> that
+   declares a POSIX-compliant localtime_r, for the benefit of older
+   implementations that lack localtime_r or have a nonstandard one.
+   Similarly for gmtime_r.  See the gnulib time_r module for one way
+   to implement this.  */
+
+# undef __gmtime_r
+# undef __localtime_r
+# define __gmtime_r gmtime_r
+# define __localtime_r localtime_r
+
+# define __mktime_internal mktime_internal
+
+#endif
+
+/* Subroutine of mktime.  Return the time_t representation of TP and
+   normalize TP, given that a struct tm * maps to a time_t as performed
+   by FUNC.  Record next guess for localtime-gmtime offset in *OFFSET.  */
+extern __time64_t __mktime_internal (struct tm *tp,
+                                     struct tm *(*func) (__time64_t const *,
+                                                         struct tm *),
+                                     mktime_offset_t *offset) attribute_hidden;
diff --git a/time/mktime.c b/time/mktime.c
index 57efee9b25..8fe2f963ae 100644
--- a/time/mktime.c
+++ b/time/mktime.c
@@ -112,11 +112,11 @@ my_tzset (void)
    added to them, and then with another timestamp added, without
    worrying about overflow.
 
-   Much of the code uses long_int to represent time_t values, to
-   lessen the hassle of dealing with platforms where time_t is
+   Much of the code uses long_int to represent __time64_t values, to
+   lessen the hassle of dealing with platforms where __time64_t is
    unsigned, and because long_int should suffice to represent all
-   time_t values that mktime can generate even on platforms where
-   time_t is excessively wide.  */
+   __time64_t values that mktime can generate even on platforms where
+   __time64_t is wider than the int components of struct tm.  */
 
 #if INT_MAX <= LONG_MAX / 4 / 366 / 24 / 60 / 60
 typedef long int long_int;
@@ -144,16 +144,15 @@ shr (long_int a, int b)
 	  : a / (one << b) - (a % (one << b) < 0));
 }
 
-/* Bounds for the intersection of time_t and long_int.  */
+/* Bounds for the intersection of __time64_t and long_int.  */
 
 static long_int const mktime_min
-  = ((TYPE_SIGNED (time_t) && TYPE_MINIMUM (time_t) < TYPE_MINIMUM (long_int))
-     ? TYPE_MINIMUM (long_int) : TYPE_MINIMUM (time_t));
+  = ((TYPE_SIGNED (__time64_t)
+      && TYPE_MINIMUM (__time64_t) < TYPE_MINIMUM (long_int))
+     ? TYPE_MINIMUM (long_int) : TYPE_MINIMUM (__time64_t));
 static long_int const mktime_max
-  = (TYPE_MAXIMUM (long_int) < TYPE_MAXIMUM (time_t)
-     ? TYPE_MAXIMUM (long_int) : TYPE_MAXIMUM (time_t));
-
-verify (TYPE_IS_INTEGER (time_t));
+  = (TYPE_MAXIMUM (long_int) < TYPE_MAXIMUM (__time64_t)
+     ? TYPE_MAXIMUM (long_int) : TYPE_MAXIMUM (__time64_t));
 
 #define EPOCH_YEAR 1970
 #define TM_YEAR_BASE 1900
@@ -252,23 +251,23 @@ tm_diff (long_int year, long_int yday, int hour, int min, int sec,
 }
 
 /* Use CONVERT to convert T to a struct tm value in *TM.  T must be in
-   range for time_t.  Return TM if successful, NULL (setting errno) on
+   range for __time64_t.  Return TM if successful, NULL (setting errno) on
    failure.  */
 static struct tm *
-convert_time (struct tm *(*convert) (const time_t *, struct tm *),
+convert_time (struct tm *(*convert) (const __time64_t *, struct tm *),
 	      long_int t, struct tm *tm)
 {
-  time_t x = t;
+  __time64_t x = t;
   return convert (&x, tm);
 }
 
 /* Use CONVERT to convert *T to a broken down time in *TP.
    If *T is out of range for conversion, adjust it so that
    it is the nearest in-range value and then convert that.
-   A value is in range if it fits in both time_t and long_int.
+   A value is in range if it fits in both __time64_t and long_int.
    Return TP on success, NULL (setting errno) on failure.  */
 static struct tm *
-ranged_convert (struct tm *(*convert) (const time_t *, struct tm *),
+ranged_convert (struct tm *(*convert) (const __time64_t *, struct tm *),
 		long_int *t, struct tm *tp)
 {
   long_int t1 = (*t < mktime_min ? mktime_min
@@ -310,7 +309,7 @@ ranged_convert (struct tm *(*convert) (const time_t *, struct tm *),
 }
 
 
-/* Convert *TP to a time_t value, inverting
+/* Convert *TP to a __time64_t value, inverting
    the monotonic and mostly-unit-linear conversion function CONVERT.
    Use *OFFSET to keep track of a guess at the offset of the result,
    compared to what the result would be for UTC without leap seconds.
@@ -318,9 +317,9 @@ ranged_convert (struct tm *(*convert) (const time_t *, struct tm *),
    If successful, set *TP to the canonicalized struct tm;
    otherwise leave *TP alone, return ((time_t) -1) and set errno.
    This function is external because it is used also by timegm.c.  */
-time_t
+__time64_t
 __mktime_internal (struct tm *tp,
-		   struct tm *(*convert) (const time_t *, struct tm *),
+		   struct tm *(*convert) (const __time64_t *, struct tm *),
 		   mktime_offset_t *offset)
 {
   struct tm tm;
@@ -520,9 +519,9 @@ __mktime_internal (struct tm *tp,
 
 #if defined _LIBC || NEED_MKTIME_WORKING || NEED_MKTIME_WINDOWS
 
-/* Convert *TP to a time_t value.  */
-time_t
-mktime (struct tm *tp)
+/* Convert *TP to a __time64_t value.  */
+__time64_t
+__mktime64 (struct tm *tp)
 {
   /* POSIX.1 8.1.1 requires that whenever mktime() is called, the
      time zone names contained in the external variable 'tzname' shall
@@ -531,7 +530,7 @@ mktime (struct tm *tp)
 
 # if defined _LIBC || NEED_MKTIME_WORKING
   static mktime_offset_t localtime_offset;
-  return __mktime_internal (tp, __localtime_r, &localtime_offset);
+  return __mktime_internal (tp, __localtime64_r, &localtime_offset);
 # else
 #  undef mktime
   return mktime (tp);
@@ -539,11 +538,29 @@ mktime (struct tm *tp)
 }
 #endif /* _LIBC || NEED_MKTIME_WORKING || NEED_MKTIME_WINDOWS */
 
-#ifdef weak_alias
-weak_alias (mktime, timelocal)
+#if defined _LIBC && __TIMESIZE != 64
+
+libc_hidden_def (__mktime64)
+
+time_t
+mktime (struct tm *tp)
+{
+  struct tm tm = *tp;
+  __time64_t t = __mktime64 (&tm);
+  if (in_time_t_range (t))
+    {
+      *tp = tm;
+      return t;
+    }
+  else
+    {
+      __set_errno (EOVERFLOW);
+      return -1;
+    }
+}
+
 #endif
 
-#ifdef _LIBC
+weak_alias (mktime, timelocal)
 libc_hidden_def (mktime)
 libc_hidden_weak (timelocal)
-#endif
diff --git a/time/timegm.c b/time/timegm.c
index bfd36d0255..bae0ceee5e 100644
--- a/time/timegm.c
+++ b/time/timegm.c
@@ -18,17 +18,41 @@
    <http://www.gnu.org/licenses/>.  */
 
 #ifndef _LIBC
-# include <config.h>
+# include <libc-config.h>
 #endif
 
 #include <time.h>
+#include <errno.h>
 
 #include "mktime-internal.h"
 
-time_t
-timegm (struct tm *tmp)
+__time64_t
+__timegm64 (struct tm *tmp)
 {
   static mktime_offset_t gmtime_offset;
   tmp->tm_isdst = 0;
-  return __mktime_internal (tmp, __gmtime_r, &gmtime_offset);
+  return __mktime_internal (tmp, __gmtime64_r, &gmtime_offset);
 }
+
+#if defined _LIBC && __TIMESIZE != 64
+
+libc_hidden_def (__timegm64)
+
+time_t
+timegm (struct tm *tmp)
+{
+  struct tm tm = *tmp;
+  __time64_t t = __timegm64 (&tm);
+  if (in_time_t_range (t))
+    {
+      *tmp = tm;
+      return t;
+    }
+  else
+    {
+      __set_errno (EOVERFLOW);
+      return -1;
+    }
+}
+
+#endif
-- 
2.20.1

Re: [PATCH v2 1/2] Y2038: make __mktime_internal compatible with __time64_t

[Lukasz Majewski]

[-- Attachment #1: Type: text/plain, Size: 4144 bytes --]

Hi Paul,

> On 3/23/19 4:59 AM, Lukasz Majewski wrote:
> > https://github.com/lmajewski/y2038_glibc/commits/mktime_v3_fixes
> >
> > In short:
> >
> > - The __time64_t needs to be exported to user space as it is a
> > building block for Y2038 safe 32 bit systems' structures (like
> > struct __timeval64). In the above use case the "normal" timeval is
> >   implicitly replaced with __timeval64.  
> 
> I looked into that URL, and I don't see any explanation of why
> __time64_t needs to be visible to user code.

On top of this patchset there is an ongoing work for Y2038 support:
https://github.com/lmajewski/y2038_glibc/commits/Y2038-2.29-glibc-__clock_settime64_v1-27-03-2019

The idea till now was to introduce new "installed" glibc type:

struct __timeval64
{
  __time64_t tv_sec;		/* Seconds */
  __int64_t tv_usec;		/* Microseconds */
};

Which uses explicit __time64_t to store tv_sec.

The above structure on 32bit Y2038 safe devices would replace in user
code struct timeval.


> 
> Surely struct timeval ought to be consistent with time_t. That is, it
> ought to continue to be defined like this:
> 
> struct timeval
> {
>   __time_t tv_sec;        /* Seconds.  */
>   __suseconds_t tv_usec;    /* Microseconds.  */
> };
> 
> where __time_t is merely an alias for time_t and so is 64 bits if
> _TIME_BITS=64 and is the current size (32 or 64) otherwise.

In other words - I shall not introduce new "installed" type for struct
timeval and just in posix/bits/types.h define:

#ifndef __USE_TIME_BITS64
  __STD_TYPE __TIME_T_TYPE __time_t; /* Seconds since the Epoch. */
#else
  __STD_TYPE·__TIME64_T_TYPE __time_t;
#endif

In that way all structures which use __time_t are Y2038 safe.

ONE NOTE:
I also guess that the above change keeps those structs posix compliant
for 32 bit machines ?

> 
> The only reason I can see why __time64_t needs to be visible to user
> code, would be if a single user source file accesses both time_t and
> __time64_t (or needs to access both struct timeval and struct
> timeval64, or similarly for struct timespec etc.).

The only supported use case would be that user defines -D_TIME_BITS=64
during compilation of the SW (in OE/yocto) and use Y2038 safe types.

> However, we should
> not support a complicated API like that, as it's typically not useful
> in practice and its mere availability causes more confusion than it's
> worth - as I've discovered with _FILE_OFFSET_BITS and __off64_t.


If I may ask - what were the biggest problems?


I would appreciate if we could make the decision/agreement on this -

if the 64bit time support shall be done by above redefinition of
__time_t and not "exporting" __time64_t (and struct __timeval64,
__timespec64, etc).

> Instead, glibc should have a simple user API where _TIME_BITS=64
> merely says "I want 64-bit time_t everywhere" and a module either
> uses this option or it doesn't.
> 

So according to above I shall only introduce glibc _internal_ struct
__timespec64/__timeval64 in include/bits/types/ :

#if __WORDSIZE > 32 || ! defined(__USE_TIME_BITS64)
# define __timespec64 timespec;
#else
struct __timespec64 {
	__time64_t tv_sec;
	__int64_t tv_nsec;
}
#endif

and rewrite the relevant functions/syscalls (like clock_settime() in
this particular case) to use it in glibc ?

PROBLEM(S) with internal struct __timespec64:

- Would be misleading for 32 bit architectures (minor issue)

- Needs to met specific Linux kernel's ABI as it is passed as an
  argument to Linux syscalls (like clock_settime64).


> I did see a fix in that URL, to use "#elif defined TIME_T_IS_SIGNED"
> instead of "#elif TIME_T_IS_SIGNED" for the benefit of picky glibc
> compiles, and that fix is incorporated in the revised patch attached.
> 

Thanks for the revised patch.


Best regards,

Lukasz Majewski

--

DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-59 Fax: (+49)-8142-66989-80 Email: lukma@denx.de

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH v2 1/2] Y2038: make __mktime_internal compatible with __time64_t

[Paul Eggert]

On 3/28/19 1:59 AM, Lukasz Majewski wrote:
> In other words - I shall not introduce new "installed" type for struct
> timeval and just in posix/bits/types.h define:
>
> #ifndef __USE_TIME_BITS64
>   __STD_TYPE __TIME_T_TYPE __time_t; /* Seconds since the Epoch. */
> #else
>   __STD_TYPE·__TIME64_T_TYPE __time_t;
> #endif
>
> In that way all structures which use __time_t are Y2038 safe.
>
> ONE NOTE:
> I also guess that the above change keeps those structs posix compliant
> for 32 bit machines ?

Yes, that's the idea.


>
>> However, we should
>> not support a complicated API like that, as it's typically not useful
>> in practice and its mere availability causes more confusion than it's
>> worth - as I've discovered with _FILE_OFFSET_BITS and __off64_t.
>
> If I may ask - what were the biggest problems?

The biggest problem is confusion and complexity. For example, see
/usr/include/zlib.h (assuming you have zlib installed). zlib is one of
the few libraries that tries to support both 32- and 64-bit file offsets
in user code. Almost nobody understands how it is supposed to work, and
I'm not sure that it even does work. And even in code that doesn't
attempt to support this sort of thing, there are still problems. See,
for example, the comedy of errors in
<https://stackoverflow.com/questions/22663897/unknown-type-name-off64-t>
where people suggest monstrosities like -Doff64_t=_off64_t to work
around compilation issues with the Apache Portable Runtime.

Rather than go down those rabbit holes, it's better to say that the
entire program must be compiled consistently, i.e., one must compile all
libraries with -D_TIME_BITS=64 if one plans to use -D_TIME_BITS=64 in
user code that passes time_t to these libraries. This includes OpenSSL,
GnuTLS, Kerberos, Glib/Gtk, libpng, ImageMagick, etc., etc., as all
these libraries expose time_t in their APIs. And it's not just
libraries: for example, one should compile Python with -D_TIME_BITS=64
and all Python modules requiring native code and using time_t in their
API should also be built with -D_TIME_BITS=64.

Although it'll be a real hassle to insist on -D_TIME_BITS=64 everywhere,
it's the only approach that will really work in practice. That's life.
Frankly if it were up to me, I'd make 64-bit time_t the default and ask
people to compile with -D_TIME_BITS=32 everywhere if they have
backward-compability concerns, as this will be much better in the long
run, and would help us avoid many of the issues we ran into during the
off_t debacle.


>> Instead, glibc should have a simple user API where _TIME_BITS=64
>> merely says "I want 64-bit time_t everywhere" and a module either
>> uses this option or it doesn't.
>>
> So according to above I shall only introduce glibc _internal_ struct
> __timespec64/__timeval64 in include/bits/types/ :
>
> #if __WORDSIZE > 32 || ! defined(__USE_TIME_BITS64)
> # define __timespec64 timespec;
> #else
> struct __timespec64 {
> 	__time64_t tv_sec;
> 	__int64_t tv_nsec;
> }
> #endif

No, these internal interfaces should not be in any file installed under
/usr/include where users can find them and get confused. They should be
only in .h files that are private to glibc and are not installed. A
logical place for them would be include/time.h.

>
> and rewrite the relevant functions/syscalls (like clock_settime() in
> this particular case) to use it in glibc ?

Yes, implementations of syscalls can use these internal interfaces.


>
> PROBLEM(S) with internal struct __timespec64:
>
> - Would be misleading for 32 bit architectures (minor issue)

I'm not sure I understand this point. How would we be misleading users
by keeping the __time64_t interfaces private?


>
> - Needs to met specific Linux kernel's ABI as it is passed as an
>   argument to Linux syscalls (like clock_settime64).

Yes, internally glibc will need to know what the kernel ABI is, and do
the right thing. This should be reasonably easy to do if the internal
glibc interfaces are 64-bit, which they should be.

Re: [PATCH v2 1/2] Y2038: make __mktime_internal compatible with __time64_t

[Joseph Myers]

[-- Attachment #1: Type: text/plain, Size: 808 bytes --]

On Thu, 28 Mar 2019, Lukasz Majewski wrote:

> > where __time_t is merely an alias for time_t and so is 64 bits if
> > _TIME_BITS=64 and is the current size (32 or 64) otherwise.
> 
> In other words - I shall not introduce new "installed" type for struct
> timeval and just in posix/bits/types.h define:
> 
> #ifndef __USE_TIME_BITS64
>   __STD_TYPE __TIME_T_TYPE __time_t; /* Seconds since the Epoch. */
> #else
>   __STD_TYPE·__TIME64_T_TYPE __time_t;
> #endif

Note that would conflict with the practice for all the other types such as 
__off_t.

> In that way all structures which use __time_t are Y2038 safe.

I don't think you can avoid explicit conditionals in struct timespec, 
because of the issue of endian-dependent padding around 32-bit 
nanoseconds.

-- 
Joseph S. Myers
joseph@codesourcery.com

Re: [PATCH v2 1/2] Y2038: make __mktime_internal compatible with __time64_t

[Lukasz Majewski]

[-- Attachment #1: Type: text/plain, Size: 5475 bytes --]

Hi Paul,

> On 3/28/19 1:59 AM, Lukasz Majewski wrote:
> > In other words - I shall not introduce new "installed" type for
> > struct timeval and just in posix/bits/types.h define:
> >
> > #ifndef __USE_TIME_BITS64
> >   __STD_TYPE __TIME_T_TYPE __time_t; /* Seconds since the Epoch. */
> > #else
> >   __STD_TYPE·__TIME64_T_TYPE __time_t;
> > #endif
> >
> > In that way all structures which use __time_t are Y2038 safe.
> >
> > ONE NOTE:
> > I also guess that the above change keeps those structs posix
> > compliant for 32 bit machines ?  
> 
> Yes, that's the idea.
> 
> 
> >  
> >> However, we should
> >> not support a complicated API like that, as it's typically not
> >> useful in practice and its mere availability causes more confusion
> >> than it's worth - as I've discovered with _FILE_OFFSET_BITS and
> >> __off64_t.  
> >
> > If I may ask - what were the biggest problems?  
> 
> The biggest problem is confusion and complexity. For example, see
> /usr/include/zlib.h (assuming you have zlib installed). zlib is one of
> the few libraries that tries to support both 32- and 64-bit file
> offsets in user code. Almost nobody understands how it is supposed to
> work, and I'm not sure that it even does work. And even in code that
> doesn't attempt to support this sort of thing, there are still
> problems. See, for example, the comedy of errors in
> <https://stackoverflow.com/questions/22663897/unknown-type-name-off64-t>
> where people suggest monstrosities like -Doff64_t=_off64_t to work
> around compilation issues with the Apache Portable Runtime.
> 
> Rather than go down those rabbit holes, it's better to say that the
> entire program must be compiled consistently, i.e., one must compile
> all libraries with -D_TIME_BITS=64 if one plans to use
> -D_TIME_BITS=64 in user code that passes time_t to these libraries.
> This includes OpenSSL, GnuTLS, Kerberos, Glib/Gtk, libpng,
> ImageMagick, etc., etc., as all these libraries expose time_t in
> their APIs. And it's not just libraries: for example, one should
> compile Python with -D_TIME_BITS=64 and all Python modules requiring
> native code and using time_t in their API should also be built with
> -D_TIME_BITS=64.
> 
> Although it'll be a real hassle to insist on -D_TIME_BITS=64
> everywhere, it's the only approach that will really work in practice.

The above approach is doable if building the embedded system rootfs in
OE/Yocto.

I do have a setup where only for glibc compilation I do disable
-D_TIME_BITS64, but for _all_ other source code it is enabled.

> That's life. Frankly if it were up to me, I'd make 64-bit time_t the
> default and ask people to compile with -D_TIME_BITS=32 everywhere if
> they have backward-compability concerns, as this will be much better
> in the long run, and would help us avoid many of the issues we ran
> into during the off_t debacle.
> 

I see your point.

> 
> >> Instead, glibc should have a simple user API where _TIME_BITS=64
> >> merely says "I want 64-bit time_t everywhere" and a module either
> >> uses this option or it doesn't.
> >>  
> > So according to above I shall only introduce glibc _internal_ struct
> > __timespec64/__timeval64 in include/bits/types/ :
> >
> > #if __WORDSIZE > 32 || ! defined(__USE_TIME_BITS64)
> > # define __timespec64 timespec;
> > #else
> > struct __timespec64 {
> > 	__time64_t tv_sec;
> > 	__int64_t tv_nsec;
> > }
> > #endif  
> 
> No, these internal interfaces should not be in any file installed
> under /usr/include where users can find them and get confused. 

Yes, correct.

I've proposed to install struct __timespec64/__timeval64
in ./include/bits/types directory of glibc (and as fair as I understand
those would be private).

But also, those can be placed in ./include/time.h

> They
> should be only in .h files that are private to glibc and are not
> installed. A logical place for them would be include/time.h.
> 

Ok.

> >
> > and rewrite the relevant functions/syscalls (like clock_settime() in
> > this particular case) to use it in glibc ?  
> 
> Yes, implementations of syscalls can use these internal interfaces.
> 

Ok. Good.

> 
> >
> > PROBLEM(S) with internal struct __timespec64:
> >
> > - Would be misleading for 32 bit architectures (minor issue)  
> 
> I'm not sure I understand this point. How would we be misleading users
> by keeping the __time64_t interfaces private?

With __timespec64 exported (installed in usr/include/*) - there would
be an explicit type to show that we are Y2038 safe (when e.g.
debugging).

> 
> 
> >
> > - Needs to met specific Linux kernel's ABI as it is passed as an
> >   argument to Linux syscalls (like clock_settime64).  
> 
> Yes, internally glibc will need to know what the kernel ABI is, and do
> the right thing. This should be reasonably easy to do if the internal
> glibc interfaces are 64-bit, which they should be.

As presented above - kernel expects in e.g. timespec 64 bit values for
both tv_sec and tv_nsec. Also internal types doesn't need to comply with
POSIX (tv_nsec shall be single long as seen from user space).

> 
> 




Best regards,

Lukasz Majewski

--

DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-59 Fax: (+49)-8142-66989-80 Email: lukma@denx.de

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH v2 1/2] Y2038: make __mktime_internal compatible with __time64_t

[Paul Eggert]

On 3/29/19 7:24 AM, Lukasz Majewski wrote:
> With __timespec64 exported (installed in usr/include/*) - there would
> be an explicit type to show that we are Y2038 safe (when e.g.
> debugging).

That's not much of an argument for exporting __timespec64 to user code.
When debugging a module, a developer can easily determine whether
whether time_t is 32- or 64-bit by printing sizeof (time_t) in GDB, so
the visibility of __timespec64 wouldn't give them any information that
they don't already have.

> I do have a setup where only for glibc compilation I do disable
> -D_TIME_BITS64, but for _all_ other source code it is enabled.

Sorry, I'm not following what you mean by "disable -DTIME_BITS64". Do
you mean that you compile applications with -D_TIME_BITS=64 and compile
glibc without it?

Does -D_TIME_BITS=64 affect building the GNU C library itself, and if
so, why? (Of course -D_TIME_BITS=64 will affect glibc's auxiliary
programs like zdump, but these are exceptional cases; I'm worried about
the library proper.) I thought _TIME_BITS was intended for use in user
programs, not for use in glibc itself.

The answers to the above questions should be documented in
manual/maint.texi; could you add that to your list of things to do?

Re: [PATCH v2 1/2] Y2038: make __mktime_internal compatible with __time64_t

[Lukasz Majewski]

[-- Attachment #1: Type: text/plain, Size: 2305 bytes --]

Hi Paul,

> On 3/29/19 7:24 AM, Lukasz Majewski wrote:
> > With __timespec64 exported (installed in usr/include/*) - there
> > would be an explicit type to show that we are Y2038 safe (when e.g.
> > debugging).  
> 
> That's not much of an argument for exporting __timespec64 to user
> code. When debugging a module, a developer can easily determine
> whether whether time_t is 32- or 64-bit by printing sizeof (time_t)
> in GDB, so the visibility of __timespec64 wouldn't give them any
> information that they don't already have.

Yes, indeed. From practical point of view it would be best to avoid
introducing any new types.

> 
> > I do have a setup where only for glibc compilation I do disable
> > -D_TIME_BITS64, but for _all_ other source code it is enabled.  
> 
> Sorry, I'm not following what you mean by "disable -DTIME_BITS64". Do
> you mean that you compile applications with -D_TIME_BITS=64 and
> compile glibc without it?

Yes, exactly.

That was the case when I last time was trying to use/compile the glibc
being Y2038 safe and as the only libc on the OE/Yocto created system.

However, it was some time ago (before I did some cleanup)
The meta-y2038 relevant branch (but it is outdated now a bit):
https://github.com/lmajewski/meta-y2038/commits/y2038-glibc2.29-deploy

> 
> Does -D_TIME_BITS=64 affect building the GNU C library itself, and if
> so, why? 

I did not checked it with the new code, but yes the glibc was not
buildable with -D_TIME_BITS=64

> (Of course -D_TIME_BITS=64 will affect glibc's auxiliary
> programs like zdump, but these are exceptional cases; I'm worried
> about the library proper.) I thought _TIME_BITS was intended for use
> in user programs, not for use in glibc itself.

Yes, exactly. The _TIME_BITS shall be only supported when building user
programs. Glibc shall not be build with it.

> 
> The answers to the above questions should be documented in
> manual/maint.texi; could you add that to your list of things to do?
> 

Yes, noted.


Best regards,

Lukasz Majewski

--

DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-59 Fax: (+49)-8142-66989-80 Email: lukma@denx.de

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH v2 1/2] Y2038: make __mktime_internal compatible with __time64_t

[Joseph Myers]

On Fri, 29 Mar 2019, Lukasz Majewski wrote:

> I've proposed to install struct __timespec64/__timeval64
> in ./include/bits/types directory of glibc (and as fair as I understand
> those would be private).

Almost all include/bits headers are single-line wrappers round the 
installed header.  Given that <bits/*.h> is a namespace specifically for 
*installed* header fragments included from other installed headers, you 
need a very good reason to put actual type definitions in include/bits/.

> But also, those can be placed in ./include/time.h

Yes, that's a more appropriate place for internal type definitions.

-- 
Joseph S. Myers
joseph@codesourcery.com

Re: [PATCH v2 1/2] Y2038: make __mktime_internal compatible with __time64_t

[Lukasz Majewski]

[-- Attachment #1: Type: text/plain, Size: 983 bytes --]

Hi Joseph,

> On Fri, 29 Mar 2019, Lukasz Majewski wrote:
> 
> > I've proposed to install struct __timespec64/__timeval64
> > in ./include/bits/types directory of glibc (and as fair as I
> > understand those would be private).  
> 
> Almost all include/bits headers are single-line wrappers round the 
> installed header.  Given that <bits/*.h> is a namespace specifically
> for *installed* header fragments included from other installed
> headers, you need a very good reason to put actual type definitions
> in include/bits/.
> 
> > But also, those can be placed in ./include/time.h  
> 
> Yes, that's a more appropriate place for internal type definitions.
> 

Ok, I see. Thanks for clarification.


Best regards,

Lukasz Majewski

--

DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-59 Fax: (+49)-8142-66989-80 Email: lukma@denx.de

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH v2 1/2] Y2038: make __mktime_internal compatible with __time64_t

[Paul Eggert]

[-- Attachment #1: Type: text/plain, Size: 920 bytes --]

Lukasz Majewski wrote:
> Paul, will you find any time soon to provide next version of mktime
> compatibility patch?

As far as mktime compatibility goes, I think I'd rather first cause mktime to be 
compatible with __time64_t, and worry about _TIME_BITS later. Proposed mktime 
patch attached; it's similar to my previous proposal except it leaves the 
__time64_t definition in posix/bits/types.h rather than moving it to 
time/mktime-internal.h.

This patch exposes __time64_t to user code if _LIBC is defined (because glibc 
needs __time64_t internally), or if __TIMESIZE != 64 (because of the compromise 
to expose __time64_t only on 32-bit time_t platforms). Once we add _TIME_BITS I 
expect that we should change the visibility rule to something that also involves 
_TIME_BITS, because __time64_t should not be exposed to user code if _TIME_BITS 
== 64. However, that can wait for the patches involving _TIME_BITS.

[-- Attachment #2: 0001-Make-mktime-etc.-compatible-with-__time64_t.patch --]
[-- Type: text/x-patch, Size: 18033 bytes --]

From 1a7cfd679d96b48335731c65f3e4e4d2e57fd086 Mon Sep 17 00:00:00 2001
From: Paul Eggert <eggert@cs.ucla.edu>
Date: Mon, 18 Mar 2019 14:14:15 -0700
Subject: [PATCH] Make mktime etc. compatible with __time64_t

Keep these functions compatible with Gnulib while adding
__time64_t support.  The basic idea is to move private API
declarations from include/time.h to time/mktime-internal.h, since
the former file cannot easily be shared with Gnulib whereas the
latter can.
Also, do some other minor cleanup while in the neighborhood.
* include/time.h: Include stdbool.h, time/mktime-internal.h.
(__mktime_internal): Move this prototype to time/mktime-internal.h,
since Gnulib needs it.
(__localtime64_r, __gmtime64_r) [__TIMESIZE == 64]:
Move these macros to time/mktime-internal.h, since Gnulib needs them.
(__mktime64, __timegm64) [__TIMESIZE != 64]: New prototypes.
(in_time_t_range): New static function.
* posix/bits/types.h (__time64_t) [__TIMESIZE == 64 && !defined __LIBC]:
Do not define as a macro in this case, so that portable code is
less tempted to use __time64_t.
* time/mktime-internal.h: Rewrite so that it does both glibc
and Gnulib work.  Include time.h if not _LIBC.
(mktime_offset_t) [!_LIBC]: Define for gnulib.
(__time64_t, __gmtime64_r, __localtime64_r, __mktime64, __timegm64)
[!_LIBC || __TIMESIZE == 64]: New macros, mostly moved here
from include/time.h.
(__gmtime_r, __localtime_r, __mktime_internal) [!_LIBC]:
New macros, taken from GNulib.
(__mktime_internal): New prototype, moved here from include/time.h.
* time/mktime.c (mktime_min, mktime_max, convert_time)
(ranged_convert, __mktime_internal, __mktime64):
* time/timegm.c (__timegm64):
Use __time64_t, not time_t.
* time/mktime.c: Stop worrying about whether time_t is floating-point.
(__mktime64) [! (_LIBC && __TIMESIZE != 64)]:
Rename from mktime.
(mktime) [_LIBC && __TIMESIZE != 64]: New function.
* time/timegm.c [!_LIBC]: Include libc-config.h, not config.h,
for libc_hidden_def.
Include errno.h.
(__timegm64) [! (_LIBC && __TIMESIZE != 64)]:
Rename from timegm.
(timegm) [_LIBC && __TIMESIZE != 64]: New function.

First cut at publicizing __time64_t
---
 ChangeLog              | 44 +++++++++++++++++++++++
 include/time.h         | 39 +++++++++++----------
 posix/bits/types.h     |  9 +++--
 time/mktime-internal.h | 79 +++++++++++++++++++++++++++++++++++++++++-
 time/mktime.c          | 71 ++++++++++++++++++++++---------------
 time/timegm.c          | 32 ++++++++++++++---
 6 files changed, 220 insertions(+), 54 deletions(-)

diff --git a/ChangeLog b/ChangeLog
index aac356bb69..e513c0ea06 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,47 @@
+2019-04-28  Paul Eggert  <eggert@cs.ucla.edu>
+
+	Make mktime etc. compatible with __time64_t
+	Keep these functions compatible with Gnulib while adding
+	__time64_t support.  The basic idea is to move private API
+	declarations from include/time.h to time/mktime-internal.h, since
+	the former file cannot easily be shared with Gnulib whereas the
+	latter can.
+	Also, do some other minor cleanup while in the neighborhood.
+	* include/time.h: Include stdbool.h, time/mktime-internal.h.
+	(__mktime_internal): Move this prototype to time/mktime-internal.h,
+	since Gnulib needs it.
+	(__localtime64_r, __gmtime64_r) [__TIMESIZE == 64]:
+	Move these macros to time/mktime-internal.h, since Gnulib needs them.
+	(__mktime64, __timegm64) [__TIMESIZE != 64]: New prototypes.
+	(in_time_t_range): New static function.
+	* posix/bits/types.h (__time64_t): Move to time/mktime-internal.h,
+	so that glibc users are not tempted to use __time64_t.
+	* time/mktime-internal.h: Rewrite so that it does both glibc
+	and Gnulib work.  Include time.h if not _LIBC.
+	(mktime_offset_t) [!_LIBC]: Define for gnulib.
+	(__time64_t): New type or macro, moved here from
+	posix/bits/types.h.
+	(__gmtime64_r, __localtime64_r, __mktime64, __timegm64)
+	[!_LIBC || __TIMESIZE == 64): New macros, mostly moved here
+	from include/time.h.
+	(__gmtime_r, __localtime_r, __mktime_internal) [!_LIBC]:
+	New macros, taken from GNulib.
+	(__mktime_internal): New prototype, moved here from include/time.h.
+	* time/mktime.c (mktime_min, mktime_max, convert_time)
+	(ranged_convert, __mktime_internal, __mktime64):
+	* time/timegm.c (__timegm64):
+	Use __time64_t, not time_t.
+	* time/mktime.c: Stop worrying about whether time_t is floating-point.
+	(__mktime64) [! (_LIBC && __TIMESIZE != 64)]:
+	Rename from mktime.
+	(mktime) [_LIBC && __TIMESIZE != 64]: New function.
+	* time/timegm.c [!_LIBC]: Include libc-config.h, not config.h,
+	for libc_hidden_def.
+	Include errno.h.
+	(__timegm64) [! (_LIBC && __TIMESIZE != 64)]:
+	Rename from timegm.
+	(timegm) [_LIBC && __TIMESIZE != 64]: New function.
+
 2019-04-26  Florian Weimer  <fweimer@redhat.com>
 
 	elf: Link sotruss-lib.so with BIND_NOW for --enable-bind-now.
diff --git a/include/time.h b/include/time.h
index 61dd9e180b..ac3163c2a5 100644
--- a/include/time.h
+++ b/include/time.h
@@ -3,6 +3,8 @@
 
 #ifndef _ISOMAC
 # include <bits/types/locale_t.h>
+# include <stdbool.h>
+# include <time/mktime-internal.h>
 
 extern __typeof (strftime_l) __strftime_l;
 libc_hidden_proto (__strftime_l)
@@ -49,19 +51,11 @@ extern void __tzset_parse_tz (const char *tz) attribute_hidden;
 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
-   normalize TP, given that a `struct tm *' maps to a `time_t' as performed
-   by FUNC.  Record next guess for localtime-gmtime offset in *OFFSET.  */
-extern time_t __mktime_internal (struct tm *__tp,
-				 struct tm *(*__func) (const time_t *,
-						       struct tm *),
-				 long int *__offset) attribute_hidden;
-
 #if __TIMESIZE == 64
 # define __ctime64 ctime
 #else
 extern char *__ctime64 (const __time64_t *__timer) __THROW;
-libc_hidden_proto (__ctime64);
+libc_hidden_proto (__ctime64)
 #endif
 
 #if __TIMESIZE == 64
@@ -69,7 +63,7 @@ libc_hidden_proto (__ctime64);
 #else
 extern char *__ctime64_r (const __time64_t *__restrict __timer,
 		          char *__restrict __buf) __THROW;
-libc_hidden_proto (__ctime64_r);
+libc_hidden_proto (__ctime64_r)
 #endif
 
 #if __TIMESIZE == 64
@@ -81,13 +75,13 @@ libc_hidden_proto (__localtime64)
 
 extern struct tm *__localtime_r (const time_t *__timer,
 				 struct tm *__tp) attribute_hidden;
-
-#if __TIMESIZE == 64
-# define __localtime64_r __localtime_r
-#else
+#if __TIMESIZE != 64
 extern struct tm *__localtime64_r (const __time64_t *__timer,
 				   struct tm *__tp);
 libc_hidden_proto (__localtime64_r)
+
+extern __time64_t __mktime64 (struct tm *__tp) __THROW;
+libc_hidden_proto (__mktime64)
 #endif
 
 extern struct tm *__gmtime_r (const time_t *__restrict __timer,
@@ -99,14 +93,13 @@ libc_hidden_proto (__gmtime_r)
 #else
 extern struct tm *__gmtime64 (const __time64_t *__timer);
 libc_hidden_proto (__gmtime64)
-#endif
 
-#if __TIMESIZE == 64
-# define __gmtime64_r __gmtime_r
-#else
 extern struct tm *__gmtime64_r (const __time64_t *__restrict __timer,
 				struct tm *__restrict __tp);
-libc_hidden_proto (__gmtime64_r);
+libc_hidden_proto (__gmtime64_r)
+
+extern __time64_t __timegm64 (struct tm *__tp) __THROW;
+libc_hidden_proto (__timegm64)
 #endif
 
 /* Compute the `struct tm' representation of T,
@@ -155,5 +148,13 @@ extern double __difftime (time_t time1, time_t time0);
    actual clock ID.  */
 #define CLOCK_IDFIELD_SIZE	3
 
+/* Check whether T fits in time_t.  */
+static inline bool
+in_time_t_range (__time64_t t)
+{
+  time_t s = t;
+  return s == t;
+}
+
 #endif
 #endif
diff --git a/posix/bits/types.h b/posix/bits/types.h
index 0de6c59bb4..cb737caff0 100644
--- a/posix/bits/types.h
+++ b/posix/bits/types.h
@@ -213,10 +213,13 @@ __STD_TYPE __U32_TYPE __socklen_t;
    It is not currently necessary for this to be machine-specific.  */
 typedef int __sig_atomic_t;
 
-#if __TIMESIZE == 64
+/* Seconds since the Epoch, visible to user code when time_t is too
+   narrow only for consistency with the old way of widening too-narrow
+   types.  User code should never use __time64_t.  */
+#if __TIMESIZE == 64 && defined __LIBC
 # define __time64_t __time_t
-#else
-__STD_TYPE __TIME64_T_TYPE __time64_t;	/* Seconds since the Epoch.  */
+#elif __TIMESIZE != 64
+__STD_TYPE __TIME64_T_TYPE __time64_t;
 #endif
 
 #undef __STD_TYPE
diff --git a/time/mktime-internal.h b/time/mktime-internal.h
index 6111c22880..4cdeb9c2f4 100644
--- a/time/mktime-internal.h
+++ b/time/mktime-internal.h
@@ -1,2 +1,79 @@
-/* Gnulib mktime-internal.h, tailored for glibc.  */
+/* Internals of mktime and related functions
+   Copyright 2016-2019 Free Software Foundation, Inc.
+   This file is part of the GNU C Library.
+   Contributed by Paul Eggert <eggert@cs.ucla.edu>.
+
+   The GNU C Library is free software; you can redistribute it and/or
+   modify it under the terms of the GNU Lesser General Public
+   License as published by the Free Software Foundation; either
+   version 2.1 of the License, or (at your option) any later version.
+
+   The GNU C Library is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+   Lesser General Public License for more details.
+
+   You should have received a copy of the GNU Lesser General Public
+   License along with the GNU C Library; if not, see
+   <https://www.gnu.org/licenses/>.  */
+
+#ifndef _LIBC
+# include <time.h>
+#endif
+
+/* mktime_offset_t is a signed type wide enough to hold a UTC offset
+   in seconds, and used as part of the type of the offset-guess
+   argument to mktime_internal.  In Glibc, it is always long int.
+   When in Gnulib, use time_t on platforms where time_t
+   is signed, to be compatible with platforms like BeOS that export
+   this implementation detail of mktime.  On platforms where time_t is
+   unsigned, GNU and POSIX code can assume 'int' is at least 32 bits
+   which is wide enough for a UTC offset.  */
+#ifdef _LIBC
 typedef long int mktime_offset_t;
+#elif defined TIME_T_IS_SIGNED
+typedef time_t mktime_offset_t;
+#else
+typedef int mktime_offset_t;
+#endif
+
+/* The source code uses identifiers like __time64_t for glibc
+   timestamps that can contain 64-bit values even when time_t is only
+   32 bits.  These are just macros for the ordinary identifiers unless
+   compiling within glibc when time_t is 32 bits.  */
+#if ! (defined _LIBC && __TIMESIZE != 64)
+# undef __time64_t
+# define __time64_t time_t
+# define __gmtime64_r __gmtime_r
+# define __localtime64_r __localtime_r
+# define __mktime64 mktime
+# define __timegm64 timegm
+#endif
+
+#ifndef _LIBC
+
+/* Although glibc source code uses leading underscores, Gnulib wants
+   ordinary names.
+
+   Portable standalone applications should supply a <time.h> that
+   declares a POSIX-compliant localtime_r, for the benefit of older
+   implementations that lack localtime_r or have a nonstandard one.
+   Similarly for gmtime_r.  See the gnulib time_r module for one way
+   to implement this.  */
+
+# undef __gmtime_r
+# undef __localtime_r
+# define __gmtime_r gmtime_r
+# define __localtime_r localtime_r
+
+# define __mktime_internal mktime_internal
+
+#endif
+
+/* Subroutine of mktime.  Return the time_t representation of TP and
+   normalize TP, given that a struct tm * maps to a time_t as performed
+   by FUNC.  Record next guess for localtime-gmtime offset in *OFFSET.  */
+extern __time64_t __mktime_internal (struct tm *tp,
+                                     struct tm *(*func) (__time64_t const *,
+                                                         struct tm *),
+                                     mktime_offset_t *offset) attribute_hidden;
diff --git a/time/mktime.c b/time/mktime.c
index 57efee9b25..8fe2f963ae 100644
--- a/time/mktime.c
+++ b/time/mktime.c
@@ -112,11 +112,11 @@ my_tzset (void)
    added to them, and then with another timestamp added, without
    worrying about overflow.
 
-   Much of the code uses long_int to represent time_t values, to
-   lessen the hassle of dealing with platforms where time_t is
+   Much of the code uses long_int to represent __time64_t values, to
+   lessen the hassle of dealing with platforms where __time64_t is
    unsigned, and because long_int should suffice to represent all
-   time_t values that mktime can generate even on platforms where
-   time_t is excessively wide.  */
+   __time64_t values that mktime can generate even on platforms where
+   __time64_t is wider than the int components of struct tm.  */
 
 #if INT_MAX <= LONG_MAX / 4 / 366 / 24 / 60 / 60
 typedef long int long_int;
@@ -144,16 +144,15 @@ shr (long_int a, int b)
 	  : a / (one << b) - (a % (one << b) < 0));
 }
 
-/* Bounds for the intersection of time_t and long_int.  */
+/* Bounds for the intersection of __time64_t and long_int.  */
 
 static long_int const mktime_min
-  = ((TYPE_SIGNED (time_t) && TYPE_MINIMUM (time_t) < TYPE_MINIMUM (long_int))
-     ? TYPE_MINIMUM (long_int) : TYPE_MINIMUM (time_t));
+  = ((TYPE_SIGNED (__time64_t)
+      && TYPE_MINIMUM (__time64_t) < TYPE_MINIMUM (long_int))
+     ? TYPE_MINIMUM (long_int) : TYPE_MINIMUM (__time64_t));
 static long_int const mktime_max
-  = (TYPE_MAXIMUM (long_int) < TYPE_MAXIMUM (time_t)
-     ? TYPE_MAXIMUM (long_int) : TYPE_MAXIMUM (time_t));
-
-verify (TYPE_IS_INTEGER (time_t));
+  = (TYPE_MAXIMUM (long_int) < TYPE_MAXIMUM (__time64_t)
+     ? TYPE_MAXIMUM (long_int) : TYPE_MAXIMUM (__time64_t));
 
 #define EPOCH_YEAR 1970
 #define TM_YEAR_BASE 1900
@@ -252,23 +251,23 @@ tm_diff (long_int year, long_int yday, int hour, int min, int sec,
 }
 
 /* Use CONVERT to convert T to a struct tm value in *TM.  T must be in
-   range for time_t.  Return TM if successful, NULL (setting errno) on
+   range for __time64_t.  Return TM if successful, NULL (setting errno) on
    failure.  */
 static struct tm *
-convert_time (struct tm *(*convert) (const time_t *, struct tm *),
+convert_time (struct tm *(*convert) (const __time64_t *, struct tm *),
 	      long_int t, struct tm *tm)
 {
-  time_t x = t;
+  __time64_t x = t;
   return convert (&x, tm);
 }
 
 /* Use CONVERT to convert *T to a broken down time in *TP.
    If *T is out of range for conversion, adjust it so that
    it is the nearest in-range value and then convert that.
-   A value is in range if it fits in both time_t and long_int.
+   A value is in range if it fits in both __time64_t and long_int.
    Return TP on success, NULL (setting errno) on failure.  */
 static struct tm *
-ranged_convert (struct tm *(*convert) (const time_t *, struct tm *),
+ranged_convert (struct tm *(*convert) (const __time64_t *, struct tm *),
 		long_int *t, struct tm *tp)
 {
   long_int t1 = (*t < mktime_min ? mktime_min
@@ -310,7 +309,7 @@ ranged_convert (struct tm *(*convert) (const time_t *, struct tm *),
 }
 
 
-/* Convert *TP to a time_t value, inverting
+/* Convert *TP to a __time64_t value, inverting
    the monotonic and mostly-unit-linear conversion function CONVERT.
    Use *OFFSET to keep track of a guess at the offset of the result,
    compared to what the result would be for UTC without leap seconds.
@@ -318,9 +317,9 @@ ranged_convert (struct tm *(*convert) (const time_t *, struct tm *),
    If successful, set *TP to the canonicalized struct tm;
    otherwise leave *TP alone, return ((time_t) -1) and set errno.
    This function is external because it is used also by timegm.c.  */
-time_t
+__time64_t
 __mktime_internal (struct tm *tp,
-		   struct tm *(*convert) (const time_t *, struct tm *),
+		   struct tm *(*convert) (const __time64_t *, struct tm *),
 		   mktime_offset_t *offset)
 {
   struct tm tm;
@@ -520,9 +519,9 @@ __mktime_internal (struct tm *tp,
 
 #if defined _LIBC || NEED_MKTIME_WORKING || NEED_MKTIME_WINDOWS
 
-/* Convert *TP to a time_t value.  */
-time_t
-mktime (struct tm *tp)
+/* Convert *TP to a __time64_t value.  */
+__time64_t
+__mktime64 (struct tm *tp)
 {
   /* POSIX.1 8.1.1 requires that whenever mktime() is called, the
      time zone names contained in the external variable 'tzname' shall
@@ -531,7 +530,7 @@ mktime (struct tm *tp)
 
 # if defined _LIBC || NEED_MKTIME_WORKING
   static mktime_offset_t localtime_offset;
-  return __mktime_internal (tp, __localtime_r, &localtime_offset);
+  return __mktime_internal (tp, __localtime64_r, &localtime_offset);
 # else
 #  undef mktime
   return mktime (tp);
@@ -539,11 +538,29 @@ mktime (struct tm *tp)
 }
 #endif /* _LIBC || NEED_MKTIME_WORKING || NEED_MKTIME_WINDOWS */
 
-#ifdef weak_alias
-weak_alias (mktime, timelocal)
+#if defined _LIBC && __TIMESIZE != 64
+
+libc_hidden_def (__mktime64)
+
+time_t
+mktime (struct tm *tp)
+{
+  struct tm tm = *tp;
+  __time64_t t = __mktime64 (&tm);
+  if (in_time_t_range (t))
+    {
+      *tp = tm;
+      return t;
+    }
+  else
+    {
+      __set_errno (EOVERFLOW);
+      return -1;
+    }
+}
+
 #endif
 
-#ifdef _LIBC
+weak_alias (mktime, timelocal)
 libc_hidden_def (mktime)
 libc_hidden_weak (timelocal)
-#endif
diff --git a/time/timegm.c b/time/timegm.c
index bfd36d0255..bae0ceee5e 100644
--- a/time/timegm.c
+++ b/time/timegm.c
@@ -18,17 +18,41 @@
    <http://www.gnu.org/licenses/>.  */
 
 #ifndef _LIBC
-# include <config.h>
+# include <libc-config.h>
 #endif
 
 #include <time.h>
+#include <errno.h>
 
 #include "mktime-internal.h"
 
-time_t
-timegm (struct tm *tmp)
+__time64_t
+__timegm64 (struct tm *tmp)
 {
   static mktime_offset_t gmtime_offset;
   tmp->tm_isdst = 0;
-  return __mktime_internal (tmp, __gmtime_r, &gmtime_offset);
+  return __mktime_internal (tmp, __gmtime64_r, &gmtime_offset);
 }
+
+#if defined _LIBC && __TIMESIZE != 64
+
+libc_hidden_def (__timegm64)
+
+time_t
+timegm (struct tm *tmp)
+{
+  struct tm tm = *tmp;
+  __time64_t t = __timegm64 (&tm);
+  if (in_time_t_range (t))
+    {
+      *tmp = tm;
+      return t;
+    }
+  else
+    {
+      __set_errno (EOVERFLOW);
+      return -1;
+    }
+}
+
+#endif
-- 
2.20.1

Re: [PATCH v2 1/2] Y2038: make __mktime_internal compatible with __time64_t

[Lukasz Majewski]

[-- Attachment #1: Type: text/plain, Size: 1366 bytes --]

Hi Paul,

> Lukasz Majewski wrote:
> > Paul, will you find any time soon to provide next version of mktime
> > compatibility patch?  
> 
> As far as mktime compatibility goes, I think I'd rather first cause
> mktime to be compatible with __time64_t, and worry about _TIME_BITS
> later. Proposed mktime patch attached; it's similar to my previous
> proposal except it leaves the __time64_t definition in
> posix/bits/types.h rather than moving it to time/mktime-internal.h.
> 
> This patch exposes __time64_t to user code if _LIBC is defined
> (because glibc needs __time64_t internally), or if __TIMESIZE != 64
> (because of the compromise to expose __time64_t only on 32-bit time_t
> platforms). Once we add _TIME_BITS I expect that we should change the
> visibility rule to something that also involves _TIME_BITS, because
> __time64_t should not be exposed to user code if _TIME_BITS == 64.
> However, that can wait for the patches involving _TIME_BITS.

Thank you very much for this patch - I've tested it in my setup and it
works.

Tested-by: Lukasz Majewski <lukma@denx.de>


Best regards,

Lukasz Majewski

--

DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-59 Fax: (+49)-8142-66989-80 Email: lukma@denx.de

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH v2 1/2] Y2038: make __mktime_internal compatible with __time64_t

[Paul Eggert]

On 4/30/19 12:59 AM, Lukasz Majewski wrote:
> Thank you very much for this patch - I've tested it in my setup and it
> works.

Thanks for checking. This seems ready, so I installed the patch into
glibc and propagated it into Gnulib.

Re: [PATCH v2 1/2] Y2038: make __mktime_internal compatible with __time64_t

[Lukasz Majewski]

[-- Attachment #1: Type: text/plain, Size: 560 bytes --]

Hi Paul,

> On 4/30/19 12:59 AM, Lukasz Majewski wrote:
> > Thank you very much for this patch - I've tested it in my setup and
> > it works.  
> 
> Thanks for checking. This seems ready, so I installed the patch into
> glibc and propagated it into Gnulib.
> 

Thanks for the inclusion.


Best regards,

Lukasz Majewski

--

DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-59 Fax: (+49)-8142-66989-80 Email: lukma@denx.de

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]