* [PATCH 0/2] Y2038 support batch 1 - __time64_t and __tz_convert @ 2018-06-13 7:00 Albert ARIBAUD (3ADEV) 2018-06-13 7:00 ` [PATCH 2/2] Y2038: make __tz_convert compatible with 64-bit-time Albert ARIBAUD (3ADEV) ` (2 more replies) 0 siblings, 3 replies; 27+ messages in thread From: Albert ARIBAUD (3ADEV) @ 2018-06-13 7:00 UTC (permalink / raw) To: libc-alpha; +Cc: Albert ARIBAUD (3ADEV) This is the first batch of Y2038 support patches. The first patch provides __time64_t, the 64-bit counterpart of time_t, to be used in 64-bit-time implementations of public APIs related to time. The second makes __tz_convert compatible with 64-bit time. This implies creating 64-bit-time versions of its callers and turning their original (32-bit-time) versions into wrappers. Albert ARIBAUD (3ADEV) (2): Y2038: add type __time64_t Y2038: make __tz_convert compatible with 64-bit-time bits/typesizes.h | 1 + include/time.h | 29 +++++++++++--- posix/bits/types.h | 3 +- sysdeps/mach/hurd/bits/typesizes.h | 1 + .../unix/sysv/linux/alpha/bits/typesizes.h | 1 + .../unix/sysv/linux/generic/bits/typesizes.h | 1 + sysdeps/unix/sysv/linux/s390/bits/typesizes.h | 1 + .../unix/sysv/linux/sparc/bits/typesizes.h | 1 + sysdeps/unix/sysv/linux/x86/bits/typesizes.h | 1 + time/Versions | 5 +++ time/ctime.c | 21 ++++++++-- time/ctime_r.c | 21 ++++++++-- time/gmtime.c | 38 ++++++++++++++++--- time/localtime.c | 36 ++++++++++++++++-- time/offtime.c | 12 +++--- time/tzfile.c | 14 ++----- time/tzset.c | 30 ++++++--------- 17 files changed, 160 insertions(+), 56 deletions(-) -- 2.17.1 ^ permalink raw reply [flat|nested] 27+ messages in thread
* [PATCH 2/2] Y2038: make __tz_convert compatible with 64-bit-time 2018-06-13 7:00 [PATCH 0/2] Y2038 support batch 1 - __time64_t and __tz_convert Albert ARIBAUD (3ADEV) @ 2018-06-13 7:00 ` Albert ARIBAUD (3ADEV) 2018-06-13 9:10 ` Florian Weimer ` (2 more replies) 2018-06-13 7:00 ` [PATCH 1/2] Y2038: add type __time64_t Albert ARIBAUD (3ADEV) 2018-06-13 7:59 ` [PATCH 0/2] Y2038 support batch 1 - __time64_t and __tz_convert Albert ARIBAUD 2 siblings, 3 replies; 27+ messages in thread From: Albert ARIBAUD (3ADEV) @ 2018-06-13 7:00 UTC (permalink / raw) To: libc-alpha; +Cc: Albert ARIBAUD (3ADEV) This implies that its callers be 64-bit-time compatible too. It is done by creating 64-bit-time versions of these and turning their original 32-bit-time versions into wrappers (at a slight execution time cost). The callers affected are: * localtime * localtime_r * ctime * ctime_r * gmtime * gmtime_r Note that in time/tzfile.c we do not need to check for time_t overflows anymore as introduced by commit fc79706a323 since we now use internal_time_t. --- include/time.h | 20 +++++++++++++++----- time/Versions | 5 +++++ time/ctime.c | 21 ++++++++++++++++++--- time/ctime_r.c | 21 ++++++++++++++++++--- time/gmtime.c | 38 +++++++++++++++++++++++++++++++++----- time/localtime.c | 36 ++++++++++++++++++++++++++++++++---- time/offtime.c | 12 ++++++------ time/tzfile.c | 14 ++++---------- time/tzset.c | 30 ++++++++++++------------------ 9 files changed, 143 insertions(+), 54 deletions(-) diff --git a/include/time.h b/include/time.h index 93638aa215..c67e163eb8 100644 --- a/include/time.h +++ b/include/time.h @@ -9,6 +9,8 @@ extern __typeof (strftime_l) __strftime_l; libc_hidden_proto (__strftime_l) extern __typeof (strptime_l) __strptime_l; +extern struct tm *__localtime64 (const __time64_t *__timer); + libc_hidden_proto (time) libc_hidden_proto (asctime) libc_hidden_proto (mktime) @@ -17,6 +19,8 @@ libc_hidden_proto (localtime) libc_hidden_proto (strftime) libc_hidden_proto (strptime) +libc_hidden_proto (__localtime64) + extern __typeof (clock_getres) __clock_getres; extern __typeof (clock_gettime) __clock_gettime; libc_hidden_proto (__clock_gettime) @@ -51,7 +55,7 @@ extern void __tzfile_default (const char *std, const char *dst, long int stdoff, long int dstoff) attribute_hidden; extern void __tzset_parse_tz (const char *tz) attribute_hidden; -extern void __tz_compute (time_t timer, struct tm *tm, int use_localtime) +extern void __tz_compute (internal_time_t timer, struct tm *tm, int use_localtime) __THROW attribute_hidden; /* Subroutine of `mktime'. Return the `time_t' representation of TP and @@ -64,15 +68,21 @@ extern time_t __mktime_internal (struct tm *__tp, extern struct tm *__localtime_r (const time_t *__timer, struct tm *__tp) attribute_hidden; +extern struct tm *__localtime64_r (const __time64_t *__timer, + struct tm *__tp) attribute_hidden; + extern struct tm *__gmtime_r (const time_t *__restrict __timer, struct tm *__restrict __tp); libc_hidden_proto (__gmtime_r) -/* Compute the `struct tm' representation of *T, +extern struct tm *__gmtime64_r (const __time64_t *__restrict __timer, + struct tm *__restrict __tp); + +/* Compute the `struct tm' representation of T, offset OFFSET seconds east of UTC, and store year, yday, mon, mday, wday, hour, min, sec into *TP. Return nonzero if successful. */ -extern int __offtime (const time_t *__timer, +extern int __offtime (const internal_time_t __timer, long int __offset, struct tm *__tp) attribute_hidden; @@ -81,8 +91,8 @@ extern char *__asctime_r (const struct tm *__tp, char *__buf) extern void __tzset (void) attribute_hidden; /* Prototype for the internal function to get information based on TZ. */ -extern struct tm *__tz_convert (const time_t *timer, int use_localtime, - struct tm *tp) attribute_hidden; +extern struct tm *__tz_convert (const internal_time_t timer, int use_localtime, + struct tm *tp) attribute_hidden; extern int __nanosleep (const struct timespec *__requested_time, struct timespec *__remaining); diff --git a/time/Versions b/time/Versions index fd838181e4..8b83f5b041 100644 --- a/time/Versions +++ b/time/Versions @@ -65,4 +65,9 @@ libc { GLIBC_2.16 { timespec_get; } + GLIBC_2.28 { + __ctime64; __ctime64_r; + __gmtime64; __gmtime64_r; + __localtime64; __localtime64_r; + } } diff --git a/time/ctime.c b/time/ctime.c index 1222614f29..de7f3c5bd3 100644 --- a/time/ctime.c +++ b/time/ctime.c @@ -16,13 +16,28 @@ <http://www.gnu.org/licenses/>. */ #include <time.h> +#include <errno.h> /* Return a string as returned by asctime which is the representation of *T in that form. */ char * +__ctime64 (const __time64_t *t) +{ + /* Apply the same rule as ctime: + make ctime64 (t) is equivalent to asctime (localtime64 (t)). */ + return asctime (__localtime64 (t)); +} + +/* The 32-bit time wrapper. */ +char * ctime (const time_t *t) { - /* The C Standard says ctime (t) is equivalent to asctime (localtime (t)). - In particular, ctime and asctime must yield the same pointer. */ - return asctime (localtime (t)); + __time64_t t64; + if (t == NULL) + { + __set_errno (EINVAL); + return NULL; + } + t64 = *t; + return __ctime64 (&t64); } diff --git a/time/ctime_r.c b/time/ctime_r.c index c111146d76..eb6e2f3ed6 100644 --- a/time/ctime_r.c +++ b/time/ctime_r.c @@ -18,12 +18,27 @@ <http://www.gnu.org/licenses/>. */ #include <time.h> +#include <errno.h> /* Return a string as returned by asctime which is the representation - of *T in that form. Reentrant version. */ + of *T in that form. Reentrant Y2038-proof version. */ char * -ctime_r (const time_t *t, char *buf) +__ctime64_r (const __time64_t *t, char *buf) { struct tm tm; - return __asctime_r (__localtime_r (t, &tm), buf); + return __asctime_r (__localtime64_r (t, &tm), buf); +} + +/* The 32-bit-time wrapper. */ +char * +ctime_r (const time_t *t, char *buf) +{ + __time64_t t64; + if (t == NULL) + { + __set_errno (EINVAL); + return NULL; + } + t64 = *t; + return __ctime64_r (&t64, buf); } diff --git a/time/gmtime.c b/time/gmtime.c index dc33b3e68a..b5ec84f36a 100644 --- a/time/gmtime.c +++ b/time/gmtime.c @@ -17,21 +17,49 @@ <http://www.gnu.org/licenses/>. */ #include <time.h> +#include <errno.h> -/* Return the `struct tm' representation of *T in UTC, - using *TP to store the result. */ +/* Return the `struct tm' representation of 64-bit-time *T + in UTC, using *TP to store the result. */ +struct tm * +__gmtime64_r (const __time64_t *t, struct tm *tp) +{ + return __tz_convert (*t, 0, tp); +} + +/* The 32-bit-time wrapper. */ struct tm * __gmtime_r (const time_t *t, struct tm *tp) { - return __tz_convert (t, 0, tp); + __time64_t t64; + if (t == NULL) + { + __set_errno (EINVAL); + return NULL; + } + t64 = *t; + return __gmtime64_r (&t64, tp); } libc_hidden_def (__gmtime_r) weak_alias (__gmtime_r, gmtime_r) +/* Return the `struct tm' representation of 64-bit-time *T in UTC. */ +struct tm * +__gmtime64 (const __time64_t *t) +{ + return __tz_convert (*t, 0, &_tmbuf); +} -/* Return the `struct tm' representation of *T in UTC. */ +/* The 32-bit-time wrapper. */ struct tm * gmtime (const time_t *t) { - return __tz_convert (t, 0, &_tmbuf); + __time64_t t64; + if (t == NULL) + { + __set_errno (EINVAL); + return NULL; + } + t64 = *t; + return __gmtime64 (&t64); } diff --git a/time/localtime.c b/time/localtime.c index 8684a8a971..68f5322b21 100644 --- a/time/localtime.c +++ b/time/localtime.c @@ -17,25 +17,53 @@ <http://www.gnu.org/licenses/>. */ #include <time.h> +#include <errno.h> /* The C Standard says that localtime and gmtime return the same pointer. */ struct tm _tmbuf; - /* Return the `struct tm' representation of *T in local time, using *TP to store the result. */ struct tm * +__localtime64_r (const __time64_t *t, struct tm *tp) +{ + return __tz_convert (*t, 1, tp); +} + +/* The 32-bit-time wrapper. */ +struct tm * __localtime_r (const time_t *t, struct tm *tp) { - return __tz_convert (t, 1, tp); + __time64_t t64; + if (t == NULL) + { + __set_errno (EINVAL); + return NULL; + } + t64 = *t; + return __localtime64_r (&t64, tp); } weak_alias (__localtime_r, localtime_r) - /* Return the `struct tm' representation of *T in local time. */ struct tm * +__localtime64 (const __time64_t *t) +{ + return __tz_convert (*t, 1, &_tmbuf); +} + +/* The 32-bit-time wrapper. */ +struct tm * localtime (const time_t *t) { - return __tz_convert (t, 1, &_tmbuf); + __time64_t t64; + if (t == NULL) + { + __set_errno (EINVAL); + return NULL; + } + t64 = *t; + return __localtime64 (&t64); } libc_hidden_def (localtime) + diff --git a/time/offtime.c b/time/offtime.c index 04c48389fc..1ff71e3c7e 100644 --- a/time/offtime.c +++ b/time/offtime.c @@ -21,18 +21,18 @@ #define SECS_PER_HOUR (60 * 60) #define SECS_PER_DAY (SECS_PER_HOUR * 24) -/* Compute the `struct tm' representation of *T, +/* Compute the `struct tm' representation of T, offset OFFSET seconds east of UTC, and store year, yday, mon, mday, wday, hour, min, sec into *TP. Return nonzero if successful. */ int -__offtime (const time_t *t, long int offset, struct tm *tp) +__offtime (const internal_time_t t, long int offset, struct tm *tp) { - time_t days, rem, y; + internal_time_t days, rem, y; const unsigned short int *ip; - days = *t / SECS_PER_DAY; - rem = *t % SECS_PER_DAY; + days = t / SECS_PER_DAY; + rem = t % SECS_PER_DAY; rem += offset; while (rem < 0) { @@ -60,7 +60,7 @@ __offtime (const time_t *t, long int offset, struct tm *tp) while (days < 0 || days >= (__isleap (y) ? 366 : 365)) { /* Guess a corrected year, assuming 365 days per year. */ - time_t yg = y + days / 365 - (days % 365 < 0); + internal_time_t yg = y + days / 365 - (days % 365 < 0); /* Adjust DAYS and Y to match the guessed year. */ days -= ((yg - y) * 365 diff --git a/time/tzfile.c b/time/tzfile.c index 2a385b92bc..94ca3323d5 100644 --- a/time/tzfile.c +++ b/time/tzfile.c @@ -635,16 +635,10 @@ __tzfile_compute (internal_time_t timer, int use_localtime, /* Convert to broken down structure. If this fails do not use the string. */ - { - time_t truncated = timer; - if (__glibc_unlikely (truncated != timer - || ! __offtime (&truncated, 0, tp))) - goto use_last; - } - - /* Use the rules from the TZ string to compute the change. - timer fits into time_t due to the truncation check - above. */ + if (__glibc_unlikely (! __offtime (timer, 0, tp))) + goto use_last; + + /* Use the rules from the TZ string to compute the change. */ __tz_compute (timer, tp, 1); /* If tzspec comes from posixrules loaded by __tzfile_default, diff --git a/time/tzset.c b/time/tzset.c index a828b9fb75..c2bb6cdffa 100644 --- a/time/tzset.c +++ b/time/tzset.c @@ -16,7 +16,6 @@ <http://www.gnu.org/licenses/>. */ #include <ctype.h> -#include <errno.h> #include <libc-lock.h> #include <stdbool.h> #include <stddef.h> @@ -27,7 +26,7 @@ #include <timezone/tzfile.h> -#define SECSPERDAY ((time_t) 86400) +#define SECSPERDAY ((internal_time_t) 86400) char *__tzname[2] = { (char *) "GMT", (char *) "GMT" }; int __daylight = 0; @@ -55,7 +54,7 @@ typedef struct /* We cache the computed time of change for a given year so we don't have to recompute it. */ - time_t change; /* When to change to this zone. */ + internal_time_t change; /* When to change to this zone. */ int computed_for; /* Year above is computed for. */ } tz_rule; @@ -416,7 +415,7 @@ tzset_internal (int always) tz_rules[0].name = tz_rules[1].name = "UTC"; if (J0 != 0) tz_rules[0].type = tz_rules[1].type = J0; - tz_rules[0].change = tz_rules[1].change = (time_t) -1; + tz_rules[0].change = tz_rules[1].change = (internal_time_t) -1; update_vars (); return; } @@ -424,13 +423,13 @@ tzset_internal (int always) __tzset_parse_tz (tz); } \f -/* Figure out the exact time (as a time_t) in YEAR +/* Figure out the exact time (as an internal_time_t) in YEAR when the change described by RULE will occur and put it in RULE->change, saving YEAR in RULE->computed_for. */ static void compute_change (tz_rule *rule, int year) { - time_t t; + internal_time_t t; if (year != -1 && rule->computed_for == year) /* Operations on times in 2 BC will be slower. Oh well. */ @@ -514,9 +513,10 @@ compute_change (tz_rule *rule, int year) /* Figure out the correct timezone for TM and set `__tzname', - `__timezone', and `__daylight' accordingly. */ + `__timezone', and `__daylight' accordingly. + NOTE: this takes an internal_time_t value, so passing a __time_t value is OK. */ void -__tz_compute (time_t timer, struct tm *tm, int use_localtime) +__tz_compute (internal_time_t timer, struct tm *tm, int use_localtime) { compute_change (&tz_rules[0], 1900 + tm->tm_year); compute_change (&tz_rules[1], 1900 + tm->tm_year); @@ -562,20 +562,14 @@ __tzset (void) } weak_alias (__tzset, tzset) \f -/* Return the `struct tm' representation of *TIMER in the local timezone. +/* Return the `struct tm' representation of TIMER in the local timezone. Use local time if USE_LOCALTIME is nonzero, UTC otherwise. */ struct tm * -__tz_convert (const time_t *timer, int use_localtime, struct tm *tp) +__tz_convert (const internal_time_t timer, int use_localtime, struct tm *tp) { long int leap_correction; int leap_extra_secs; - if (timer == NULL) - { - __set_errno (EINVAL); - return NULL; - } - __libc_lock_lock (tzset_lock); /* Update internal database according to current TZ setting. @@ -584,14 +578,14 @@ __tz_convert (const time_t *timer, int use_localtime, struct tm *tp) tzset_internal (tp == &_tmbuf && use_localtime); if (__use_tzfile) - __tzfile_compute (*timer, use_localtime, &leap_correction, + __tzfile_compute (timer, use_localtime, &leap_correction, &leap_extra_secs, tp); else { if (! __offtime (timer, 0, tp)) tp = NULL; else - __tz_compute (*timer, tp, use_localtime); + __tz_compute (timer, tp, use_localtime); leap_correction = 0L; leap_extra_secs = 0; } -- 2.17.1 ^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH 2/2] Y2038: make __tz_convert compatible with 64-bit-time 2018-06-13 7:00 ` [PATCH 2/2] Y2038: make __tz_convert compatible with 64-bit-time Albert ARIBAUD (3ADEV) @ 2018-06-13 9:10 ` Florian Weimer 2018-06-13 9:37 ` Albert ARIBAUD 2018-06-13 9:11 ` Paul Eggert 2018-06-13 14:18 ` Joseph Myers 2 siblings, 1 reply; 27+ messages in thread From: Florian Weimer @ 2018-06-13 9:10 UTC (permalink / raw) To: Albert ARIBAUD (3ADEV), libc-alpha On 06/13/2018 09:00 AM, Albert ARIBAUD (3ADEV) wrote: > + GLIBC_2.28 { > + __ctime64; __ctime64_r; > + __gmtime64; __gmtime64_r; > + __localtime64; __localtime64_r; > + } Functions in the private namespace should be exported as GLIBC_PRIVATE. Except __gmtime64_r, these functions have unwanted side effects and cannot really be called from other parts of glibc anyway. Thanks, Florian ^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH 2/2] Y2038: make __tz_convert compatible with 64-bit-time 2018-06-13 9:10 ` Florian Weimer @ 2018-06-13 9:37 ` Albert ARIBAUD 2018-06-13 9:40 ` Florian Weimer 0 siblings, 1 reply; 27+ messages in thread From: Albert ARIBAUD @ 2018-06-13 9:37 UTC (permalink / raw) To: Florian Weimer; +Cc: libc-alpha Hi Florian, On Wed, 13 Jun 2018 11:10:09 +0200, Florian Weimer <fweimer@redhat.com> wrote : > On 06/13/2018 09:00 AM, Albert ARIBAUD (3ADEV) wrote: > > + GLIBC_2.28 { > > + __ctime64; __ctime64_r; > > + __gmtime64; __gmtime64_r; > > + __localtime64; __localtime64_r; > > + } > > Functions in the private namespace should be exported as GLIBC_PRIVATE. > Except __gmtime64_r, these functions have unwanted side effects and > cannot really be called from other parts of glibc anyway. They're going to be implementations of APIs called from user source code if/when it defines _TIME_BITS equal to 64 (that'll be the last patch in the whole series), so I don't understand how they could be considered GLIBC_PRIVATE. As for the side effects, which ones are you thinking of? The ones I am aware of are those already present in the 32-bit-time versions and are "regrettable but established behavior". > Thanks, > Florian Cordialement, Albert ARIBAUD 3ADEV ^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH 2/2] Y2038: make __tz_convert compatible with 64-bit-time 2018-06-13 9:37 ` Albert ARIBAUD @ 2018-06-13 9:40 ` Florian Weimer 2018-06-13 10:21 ` Albert ARIBAUD 2018-06-13 14:24 ` Joseph Myers 0 siblings, 2 replies; 27+ messages in thread From: Florian Weimer @ 2018-06-13 9:40 UTC (permalink / raw) To: Albert ARIBAUD; +Cc: libc-alpha On 06/13/2018 11:36 AM, Albert ARIBAUD wrote: > Hi Florian, > > On Wed, 13 Jun 2018 11:10:09 +0200, Florian Weimer <fweimer@redhat.com> > wrote : > >> On 06/13/2018 09:00 AM, Albert ARIBAUD (3ADEV) wrote: >>> + GLIBC_2.28 { >>> + __ctime64; __ctime64_r; >>> + __gmtime64; __gmtime64_r; >>> + __localtime64; __localtime64_r; >>> + } >> >> Functions in the private namespace should be exported as GLIBC_PRIVATE. >> Except __gmtime64_r, these functions have unwanted side effects and >> cannot really be called from other parts of glibc anyway. > > They're going to be implementations of APIs called from user source code > if/when it defines _TIME_BITS equal to 64 (that'll be the last patch in > the whole series), so I don't understand how they could be considered > GLIBC_PRIVATE. Why do they use the __ prefix? We generally do not do that. > As for the side effects, which ones are you thinking of? The ones I am > aware of are those already present in the 32-bit-time versions and are > "regrettable but established behavior". The side effects simply mean that we cannot call this functions as an internal implementation detail of another function, so there should be no reason for an export in the private namespace (with the __prefix and GLIBC_PRIVATE). Thanks, Florian ^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH 2/2] Y2038: make __tz_convert compatible with 64-bit-time 2018-06-13 9:40 ` Florian Weimer @ 2018-06-13 10:21 ` Albert ARIBAUD 2018-06-13 10:55 ` Albert ARIBAUD 2018-06-13 13:08 ` Florian Weimer 2018-06-13 14:24 ` Joseph Myers 1 sibling, 2 replies; 27+ messages in thread From: Albert ARIBAUD @ 2018-06-13 10:21 UTC (permalink / raw) To: Florian Weimer; +Cc: libc-alpha Hi Florian, On Wed, 13 Jun 2018 11:40:04 +0200, Florian Weimer <fweimer@redhat.com> wrote : > On 06/13/2018 11:36 AM, Albert ARIBAUD wrote: > > Hi Florian, > > > > On Wed, 13 Jun 2018 11:10:09 +0200, Florian Weimer <fweimer@redhat.com> > > wrote : > > > >> On 06/13/2018 09:00 AM, Albert ARIBAUD (3ADEV) wrote: > [...] > >> > >> Functions in the private namespace should be exported as GLIBC_PRIVATE. > >> Except __gmtime64_r, these functions have unwanted side effects and > >> cannot really be called from other parts of glibc anyway. > > > > They're going to be implementations of APIs called from user source code > > if/when it defines _TIME_BITS equal to 64 (that'll be the last patch in > > the whole series), so I don't understand how they could be considered > > GLIBC_PRIVATE. > > Why do they use the __ prefix? We generally do not do that. I believe it stemmed from the fact that source code should not spell these functions by their explicit name -- that name is to be used by glibc only. User source code should keep using the historical names (here, "gmtime_r"); if it has defined _TIME_BITS equal to 64, then the glibc public headers will alias (or barring that, #define) gmtime_r to __gmtime64_r (the 64-bit-time implementation); otherwise, "gmtime_r" will be used as-is (the 32-bit-time implementation). So to make sure the symbols were considered to not be for (direct) public use, they have to start with an underscore. > > As for the side effects, which ones are you thinking of? The ones I am > > aware of are those already present in the 32-bit-time versions and are > > "regrettable but established behavior". > > The side effects simply mean that we cannot call this functions as an > internal implementation detail of another function, so there should be > no reason for an export in the private namespace (with the __prefix and > GLIBC_PRIVATE). Actually another function can call these functions -- the 32-bit-time wrappers do that exactly. I must be missing your point. > Thanks, > Florian Cordialement, Albert ARIBAUD 3ADEV ^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH 2/2] Y2038: make __tz_convert compatible with 64-bit-time 2018-06-13 10:21 ` Albert ARIBAUD @ 2018-06-13 10:55 ` Albert ARIBAUD 2018-06-13 13:08 ` Florian Weimer 1 sibling, 0 replies; 27+ messages in thread From: Albert ARIBAUD @ 2018-06-13 10:55 UTC (permalink / raw) To: Florian Weimer; +Cc: libc-alpha On Wed, 13 Jun 2018 12:20:50 +0200, Albert ARIBAUD <albert.aribaud@3adev.fr> wrote : > I believe it stemmed from the fact that source code should not spell ... that *user* source code... Cordialement, Albert ARIBAUD 3ADEV ^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH 2/2] Y2038: make __tz_convert compatible with 64-bit-time 2018-06-13 10:21 ` Albert ARIBAUD 2018-06-13 10:55 ` Albert ARIBAUD @ 2018-06-13 13:08 ` Florian Weimer 2018-06-13 14:29 ` Joseph Myers 1 sibling, 1 reply; 27+ messages in thread From: Florian Weimer @ 2018-06-13 13:08 UTC (permalink / raw) To: Albert ARIBAUD; +Cc: libc-alpha On 06/13/2018 12:20 PM, Albert ARIBAUD wrote: > Hi Florian, > > On Wed, 13 Jun 2018 11:40:04 +0200, Florian Weimer <fweimer@redhat.com> > wrote : > >> On 06/13/2018 11:36 AM, Albert ARIBAUD wrote: >>> Hi Florian, >>> >>> On Wed, 13 Jun 2018 11:10:09 +0200, Florian Weimer <fweimer@redhat.com> >>> wrote : >>> >>>> On 06/13/2018 09:00 AM, Albert ARIBAUD (3ADEV) wrote: >> [...] >>>> >>>> Functions in the private namespace should be exported as GLIBC_PRIVATE. >>>> Except __gmtime64_r, these functions have unwanted side effects and >>>> cannot really be called from other parts of glibc anyway. >>> >>> They're going to be implementations of APIs called from user source code >>> if/when it defines _TIME_BITS equal to 64 (that'll be the last patch in >>> the whole series), so I don't understand how they could be considered >>> GLIBC_PRIVATE. >> >> Why do they use the __ prefix? We generally do not do that. > > I believe it stemmed from the fact that source code should not spell > these functions by their explicit name -- that name is to be used by > glibc only. User source code should keep using the historical names > (here, "gmtime_r"); if it has defined _TIME_BITS equal to 64, then the > glibc public headers will alias (or barring that, #define) gmtime_r to > __gmtime64_r (the 64-bit-time implementation); otherwise, "gmtime_r" > will be used as-is (the 32-bit-time implementation). > > So to make sure the symbols were considered to not be for (direct) > public use, they have to start with an underscore. We use non-__ names for the LFS redirects (slightly trimmed): # ifdef __REDIRECT extern FILE *__REDIRECT (fopen, (const char *__restrict, const char *__restrict), fopen64); extern FILE *__REDIRECT (freopen, (const char *__restrict, const char *__restrict, FILE *__restrict), freopen64); # else # define fopen fopen64 # define freopen freopen64 # endif #endif I don't see a totally conforming way to implement this using redirects anyway, so whether __ is used or not is secondary because it doesn't address the main problem. >>> As for the side effects, which ones are you thinking of? The ones I am >>> aware of are those already present in the 32-bit-time versions and are >>> "regrettable but established behavior". >> >> The side effects simply mean that we cannot call this functions as an >> internal implementation detail of another function, so there should be >> no reason for an export in the private namespace (with the __prefix and >> GLIBC_PRIVATE). > > Actually another function can call these functions -- the 32-bit-time > wrappers do that exactly. I must be missing your point. Yes, but that those are libc-only and they won't need the exports. A hidden alias with a __ name would be sufficient under such circumstances. Thanks, Florian ^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH 2/2] Y2038: make __tz_convert compatible with 64-bit-time 2018-06-13 13:08 ` Florian Weimer @ 2018-06-13 14:29 ` Joseph Myers 0 siblings, 0 replies; 27+ messages in thread From: Joseph Myers @ 2018-06-13 14:29 UTC (permalink / raw) To: Florian Weimer; +Cc: Albert ARIBAUD, libc-alpha On Wed, 13 Jun 2018, Florian Weimer wrote: > I don't see a totally conforming way to implement this using redirects anyway, > so whether __ is used or not is secondary because it doesn't address the main > problem. Strictly speaking there's the C and POSIX rule that you can declare and call a function yourself without including the relevant header, which doesn't work with redirects in headers. But that only applies *if the function declaration doesn't need any type defined in a header* (so is only an issue for a few LFS functions, and not at all for the 64-bit time functions). If desired that issue could be addressed by having a header using "#pragma redefine_extname" which you could include from the command line with -include. -- Joseph S. Myers joseph@codesourcery.com ^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH 2/2] Y2038: make __tz_convert compatible with 64-bit-time 2018-06-13 9:40 ` Florian Weimer 2018-06-13 10:21 ` Albert ARIBAUD @ 2018-06-13 14:24 ` Joseph Myers 2018-06-18 13:34 ` Florian Weimer 1 sibling, 1 reply; 27+ messages in thread From: Joseph Myers @ 2018-06-13 14:24 UTC (permalink / raw) To: Florian Weimer; +Cc: Albert ARIBAUD, libc-alpha On Wed, 13 Jun 2018, Florian Weimer wrote: > > They're going to be implementations of APIs called from user source code > > if/when it defines _TIME_BITS equal to 64 (that'll be the last patch in > > the whole series), so I don't understand how they could be considered > > GLIBC_PRIVATE. > > Why do they use the __ prefix? We generally do not do that. Because we want to fix bug 14106, and not replicate it for new interfaces (and don't intend to have explicit *64 APIs at all for 64-bit time_t, just new ABIs, on platforms where time_t is currently 32-bit, which can be selected using _TIME_BITS=64). -- Joseph S. Myers joseph@codesourcery.com ^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH 2/2] Y2038: make __tz_convert compatible with 64-bit-time 2018-06-13 14:24 ` Joseph Myers @ 2018-06-18 13:34 ` Florian Weimer 0 siblings, 0 replies; 27+ messages in thread From: Florian Weimer @ 2018-06-18 13:34 UTC (permalink / raw) To: Joseph Myers; +Cc: Albert ARIBAUD, libc-alpha On 06/13/2018 04:24 PM, Joseph Myers wrote: > On Wed, 13 Jun 2018, Florian Weimer wrote: > >>> They're going to be implementations of APIs called from user source code >>> if/when it defines _TIME_BITS equal to 64 (that'll be the last patch in >>> the whole series), so I don't understand how they could be considered >>> GLIBC_PRIVATE. >> >> Why do they use the __ prefix? We generally do not do that. > > Because we want to fix bug 14106, and not replicate it for new interfaces > (and don't intend to have explicit *64 APIs at all for 64-bit time_t, just > new ABIs, on platforms where time_t is currently 32-bit, which can be > selected using _TIME_BITS=64). Ohh, right. On the other hand, it is quite awkward why these obscure interfaces receive protection from accidental interposition, when others where we know that there is ongoing interposition (fadd, fdiv, canonicalize, explicit_bzero, getrandom, even getline) do not. This relates to an older thread: <https://sourceware.org/ml/libc-alpha/2016-10/msg00294.html> It also affects _GNU_SOURCE avoidance for libstdc++, depending on how we view this. (Formal compliance vs avoiding collisions which occur in practice.) Thanks, Florian ^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH 2/2] Y2038: make __tz_convert compatible with 64-bit-time 2018-06-13 7:00 ` [PATCH 2/2] Y2038: make __tz_convert compatible with 64-bit-time Albert ARIBAUD (3ADEV) 2018-06-13 9:10 ` Florian Weimer @ 2018-06-13 9:11 ` Paul Eggert 2018-06-13 9:14 ` Florian Weimer 2018-06-13 14:22 ` Joseph Myers 2018-06-13 14:18 ` Joseph Myers 2 siblings, 2 replies; 27+ messages in thread From: Paul Eggert @ 2018-06-13 9:11 UTC (permalink / raw) To: Albert ARIBAUD (3ADEV), libc-alpha Albert ARIBAUD (3ADEV) wrote: > -extern void __tz_compute (time_t timer, struct tm *tm, int use_localtime) > +extern void __tz_compute (internal_time_t timer, struct tm *tm, int use_localtime) Why have two types "internal_time_t" and "__time64_t" that are always the same? Wouldn't it be simpler to get rid of "internal_time_t" and use "__time64_t" uniformly? If we do need two types, the need should be documented clearly. > /* Return a string as returned by asctime which > is the representation of *T in that form. */ > char * > +__ctime64 (const __time64_t *t) > +{ > + /* Apply the same rule as ctime: > + make ctime64 (t) is equivalent to asctime (localtime64 (t)). */ > + return asctime (__localtime64 (t)); > +} > + > +/* The 32-bit time wrapper. */ > +char * > ctime (const time_t *t) > { > - /* The C Standard says ctime (t) is equivalent to asctime (localtime (t)). > - In particular, ctime and asctime must yield the same pointer. */ > - return asctime (localtime (t)); > + __time64_t t64; > + if (t == NULL) > + { > + __set_errno (EINVAL); > + return NULL; > + } > + t64 = *t; > + return __ctime64 (&t64); > } I don't see why 64-bit platforms need two entry points. If time_t is 64 bits, ctime and __ctime64 can be aliases, no? Also, it's a bit cleaner to declare and initialize t64 at the same time, i.e., '__time64_t t64 = *t;'; we can assume this C99ism in glibc nowadays. Similarly for __ctime64_r, etc. The comment inside __ctime64 is confusing, since it says "localtime64" but the code says "__localtime64". Also, I would not remove the comment that is inside ctime, as removing it omits important motivation. > /* Return a string as returned by asctime which is the representation > - of *T in that form. Reentrant version. */ > + of *T in that form. Reentrant Y2038-proof version. */ > char * > -ctime_r (const time_t *t, char *buf) > +__ctime64_r (const __time64_t *t, char *buf) There is no need to add "Y2038-proof" to the comment. It's obvious that the code is using 64-bit times here, and we shouldn't be putting "Y2038"s all over the place in the commentary. > +/* Return the `struct tm' representation of 64-bit-time *T > + in UTC, using *TP to store the result. */ > +struct tm * > +__gmtime64_r (const __time64_t *t, struct tm *tp) > +{ > + return __tz_convert (*t, 0, tp); > +} > + > +/* The 32-bit-time wrapper. */ > struct tm * > __gmtime_r (const time_t *t, struct tm *tp) > { > - return __tz_convert (t, 0, tp); > + __time64_t t64; > + if (t == NULL) > + { > + __set_errno (EINVAL); > + return NULL; > + } > + t64 = *t; > + return __gmtime64_r (&t64, tp); > } __gmtime_r sets errno=EINVAL for a null first argument, whereas __gmtime64_r dumps core instead. Why is that? If it's intended, there should be a comment as to why the two functions differ in behavior here. Similarly for __localtime_r, gmtime, localtime, and ctime. However, won't this introduce compatibility issues? Wouldn't be better for the __time64_t versions to mimic the time_t behavior? > - tz_rules[0].change = tz_rules[1].change = (time_t) -1; > + tz_rules[0].change = tz_rules[1].change = (internal_time_t) -1; The cast is just getting making maintenance harder, so I suggest changing this to use plain "= -1;" at the end. ^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH 2/2] Y2038: make __tz_convert compatible with 64-bit-time 2018-06-13 9:11 ` Paul Eggert @ 2018-06-13 9:14 ` Florian Weimer 2018-06-13 9:30 ` Albert ARIBAUD 2018-06-13 14:22 ` Joseph Myers 1 sibling, 1 reply; 27+ messages in thread From: Florian Weimer @ 2018-06-13 9:14 UTC (permalink / raw) To: Paul Eggert, Albert ARIBAUD (3ADEV), libc-alpha On 06/13/2018 11:11 AM, Paul Eggert wrote: > Albert ARIBAUD (3ADEV) wrote: > >> -extern void __tz_compute (time_t timer, struct tm *tm, int >> use_localtime) >> +extern void __tz_compute (internal_time_t timer, struct tm *tm, int >> use_localtime) > > Why have two types "internal_time_t" and "__time64_t" that are always > the same? Wouldn't it be simpler to get rid of "internal_time_t" and use > "__time64_t" uniformly? If we do need two types, the need should be > documented clearly. Yes, we can get rid of internal_time_t if we have __time64_t. Thanks, Florian ^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH 2/2] Y2038: make __tz_convert compatible with 64-bit-time 2018-06-13 9:14 ` Florian Weimer @ 2018-06-13 9:30 ` Albert ARIBAUD 0 siblings, 0 replies; 27+ messages in thread From: Albert ARIBAUD @ 2018-06-13 9:30 UTC (permalink / raw) To: Florian Weimer; +Cc: Paul Eggert, libc-alpha Hi Florian, On Wed, 13 Jun 2018 11:14:22 +0200, Florian Weimer <fweimer@redhat.com> wrote : > On 06/13/2018 11:11 AM, Paul Eggert wrote: > > Albert ARIBAUD (3ADEV) wrote: > > > >> -extern void __tz_compute (time_t timer, struct tm *tm, int > >> use_localtime) > >> +extern void __tz_compute (internal_time_t timer, struct tm *tm, int > >> use_localtime) > > > > Why have two types "internal_time_t" and "__time64_t" that are always > > the same? Wouldn't it be simpler to get rid of "internal_time_t" and use > > "__time64_t" uniformly? If we do need two types, the need should be > > documented clearly. > > Yes, we can get rid of internal_time_t if we have __time64_t. Fine for me. > Thanks, > Florian Cordialement, Albert ARIBAUD 3ADEV ^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH 2/2] Y2038: make __tz_convert compatible with 64-bit-time 2018-06-13 9:11 ` Paul Eggert 2018-06-13 9:14 ` Florian Weimer @ 2018-06-13 14:22 ` Joseph Myers 1 sibling, 0 replies; 27+ messages in thread From: Joseph Myers @ 2018-06-13 14:22 UTC (permalink / raw) To: Paul Eggert; +Cc: Albert ARIBAUD (3ADEV), libc-alpha On Wed, 13 Jun 2018, Paul Eggert wrote: > I don't see why 64-bit platforms need two entry points. If time_t is 64 bits, > ctime and __ctime64 can be aliases, no? Also, it's a bit cleaner to declare > and initialize t64 at the same time, i.e., '__time64_t t64 = *t;'; we can > assume this C99ism in glibc nowadays. My suggestion is that an internal header does #if !__TIME32_SUPPORTED # define __ctime64 ctime #endif and then the definition of the 32-bit ctime wrapper is conditional on "#if __TIME32_SUPPORTED", so that the main 64-bit version just gets compiled as ctime in the case where time_t has always only been 64-bit for this ABI. That way, internal code can call the __*64 interfaces unconditionally and the calls will just end up calling the non-*64 interfaces when the *64 interfaces don't need to exist as separate functions - and no exports of *64 in the public ABI are needed in that case either. -- Joseph S. Myers joseph@codesourcery.com ^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH 2/2] Y2038: make __tz_convert compatible with 64-bit-time 2018-06-13 7:00 ` [PATCH 2/2] Y2038: make __tz_convert compatible with 64-bit-time Albert ARIBAUD (3ADEV) 2018-06-13 9:10 ` Florian Weimer 2018-06-13 9:11 ` Paul Eggert @ 2018-06-13 14:18 ` Joseph Myers 2 siblings, 0 replies; 27+ messages in thread From: Joseph Myers @ 2018-06-13 14:18 UTC (permalink / raw) To: Albert ARIBAUD (3ADEV); +Cc: libc-alpha On Wed, 13 Jun 2018, Albert ARIBAUD (3ADEV) wrote: > diff --git a/time/Versions b/time/Versions > index fd838181e4..8b83f5b041 100644 > --- a/time/Versions > +++ b/time/Versions > @@ -65,4 +65,9 @@ libc { > GLIBC_2.16 { > timespec_get; > } > + GLIBC_2.28 { > + __ctime64; __ctime64_r; > + __gmtime64; __gmtime64_r; > + __localtime64; __localtime64_r; > + } I suggest not adding any such public ABI exports until everything necessary to support _TIME_BITS=64 is in glibc. (If a function is needed from another glibc shared library, a GLIBC_PRIVATE export may be added.) > + if (t == NULL) > + { > + __set_errno (EINVAL); > + return NULL; > + } Such explicit NULL checks, for functions where NULL is not a valid input, are contrary to glibc standards; you should just dereference the input unconditionally. https://sourceware.org/glibc/wiki/Style_and_Conventions#Error_Handling -- Joseph S. Myers joseph@codesourcery.com ^ permalink raw reply [flat|nested] 27+ messages in thread
* [PATCH 1/2] Y2038: add type __time64_t 2018-06-13 7:00 [PATCH 0/2] Y2038 support batch 1 - __time64_t and __tz_convert Albert ARIBAUD (3ADEV) 2018-06-13 7:00 ` [PATCH 2/2] Y2038: make __tz_convert compatible with 64-bit-time Albert ARIBAUD (3ADEV) @ 2018-06-13 7:00 ` Albert ARIBAUD (3ADEV) 2018-06-13 8:38 ` Paul Eggert 2018-06-13 14:13 ` Joseph Myers 2018-06-13 7:59 ` [PATCH 0/2] Y2038 support batch 1 - __time64_t and __tz_convert Albert ARIBAUD 2 siblings, 2 replies; 27+ messages in thread From: Albert ARIBAUD (3ADEV) @ 2018-06-13 7:00 UTC (permalink / raw) To: libc-alpha; +Cc: Albert ARIBAUD (3ADEV) This type is public, so that time_t can be a typedef of it when we switch the public API to 64-bit time. Also, provide a function to check if a __time64_t value fits in a (32-bit) __time_t. --- bits/typesizes.h | 1 + include/time.h | 9 ++++++++- posix/bits/types.h | 3 ++- sysdeps/mach/hurd/bits/typesizes.h | 1 + sysdeps/unix/sysv/linux/alpha/bits/typesizes.h | 1 + sysdeps/unix/sysv/linux/generic/bits/typesizes.h | 1 + sysdeps/unix/sysv/linux/s390/bits/typesizes.h | 1 + sysdeps/unix/sysv/linux/sparc/bits/typesizes.h | 1 + sysdeps/unix/sysv/linux/x86/bits/typesizes.h | 1 + 9 files changed, 17 insertions(+), 2 deletions(-) diff --git a/bits/typesizes.h b/bits/typesizes.h index 85eacf2518..0b6a19c230 100644 --- a/bits/typesizes.h +++ b/bits/typesizes.h @@ -48,6 +48,7 @@ #define __ID_T_TYPE __U32_TYPE #define __CLOCK_T_TYPE __SLONGWORD_TYPE #define __TIME_T_TYPE __SLONGWORD_TYPE +#define __TIME64_T_TYPE __SQUAD_TYPE #define __USECONDS_T_TYPE __U32_TYPE #define __SUSECONDS_T_TYPE __SLONGWORD_TYPE #define __DADDR_T_TYPE __S32_TYPE diff --git a/include/time.h b/include/time.h index 23d2580528..93638aa215 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) @@ -101,10 +102,16 @@ extern char * __strptime_internal (const char *rp, const char *fmt, extern double __difftime (time_t time1, time_t time0); - /* Use in the clock_* functions. Size of the field representing the 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 t) +{ + return t == (time_t) t; +} + #endif #endif diff --git a/posix/bits/types.h b/posix/bits/types.h index 5e22ce41bf..4c6553a266 100644 --- a/posix/bits/types.h +++ b/posix/bits/types.h @@ -155,7 +155,8 @@ __STD_TYPE __CLOCK_T_TYPE __clock_t; /* Type of CPU usage counts. */ __STD_TYPE __RLIM_T_TYPE __rlim_t; /* Type for resource measurement. */ __STD_TYPE __RLIM64_T_TYPE __rlim64_t; /* Type for resource measurement (LFS). */ __STD_TYPE __ID_T_TYPE __id_t; /* General type for IDs. */ -__STD_TYPE __TIME_T_TYPE __time_t; /* Seconds since the Epoch. */ +__STD_TYPE __TIME_T_TYPE __time_t; /* Seconds since the Epoch, Y2038-unsafe. */ +__STD_TYPE __TIME64_T_TYPE __time64_t; /* Seconds since the Epoch, Y2038-safe. */ __STD_TYPE __USECONDS_T_TYPE __useconds_t; /* Count of microseconds. */ __STD_TYPE __SUSECONDS_T_TYPE __suseconds_t; /* Signed count of microseconds. */ diff --git a/sysdeps/mach/hurd/bits/typesizes.h b/sysdeps/mach/hurd/bits/typesizes.h index d3026ba66e..c9dbe2bf33 100644 --- a/sysdeps/mach/hurd/bits/typesizes.h +++ b/sysdeps/mach/hurd/bits/typesizes.h @@ -48,6 +48,7 @@ #define __ID_T_TYPE __U32_TYPE #define __CLOCK_T_TYPE __SLONGWORD_TYPE #define __TIME_T_TYPE __SLONGWORD_TYPE +#define __TIME64_T_TYPE __SQUAD_TYPE #define __USECONDS_T_TYPE __U32_TYPE #define __SUSECONDS_T_TYPE __SLONGWORD_TYPE #define __DADDR_T_TYPE __S32_TYPE diff --git a/sysdeps/unix/sysv/linux/alpha/bits/typesizes.h b/sysdeps/unix/sysv/linux/alpha/bits/typesizes.h index d9a0b0467c..47b1e36165 100644 --- a/sysdeps/unix/sysv/linux/alpha/bits/typesizes.h +++ b/sysdeps/unix/sysv/linux/alpha/bits/typesizes.h @@ -47,6 +47,7 @@ #define __ID_T_TYPE __U32_TYPE #define __CLOCK_T_TYPE __SLONGWORD_TYPE #define __TIME_T_TYPE __SLONGWORD_TYPE +#define __TIME64_T_TYPE __SQUAD_TYPE #define __USECONDS_T_TYPE __U32_TYPE #define __SUSECONDS_T_TYPE __S64_TYPE #define __DADDR_T_TYPE __S32_TYPE diff --git a/sysdeps/unix/sysv/linux/generic/bits/typesizes.h b/sysdeps/unix/sysv/linux/generic/bits/typesizes.h index a2cb3433bf..db03b1ae8b 100644 --- a/sysdeps/unix/sysv/linux/generic/bits/typesizes.h +++ b/sysdeps/unix/sysv/linux/generic/bits/typesizes.h @@ -49,6 +49,7 @@ #define __ID_T_TYPE __U32_TYPE #define __CLOCK_T_TYPE __SLONGWORD_TYPE #define __TIME_T_TYPE __SLONGWORD_TYPE +#define __TIME64_T_TYPE __SQUAD_TYPE #define __USECONDS_T_TYPE __U32_TYPE #define __SUSECONDS_T_TYPE __SLONGWORD_TYPE #define __DADDR_T_TYPE __S32_TYPE diff --git a/sysdeps/unix/sysv/linux/s390/bits/typesizes.h b/sysdeps/unix/sysv/linux/s390/bits/typesizes.h index fdaa421958..138c6d6d10 100644 --- a/sysdeps/unix/sysv/linux/s390/bits/typesizes.h +++ b/sysdeps/unix/sysv/linux/s390/bits/typesizes.h @@ -48,6 +48,7 @@ #define __ID_T_TYPE __U32_TYPE #define __CLOCK_T_TYPE __SLONGWORD_TYPE #define __TIME_T_TYPE __SLONGWORD_TYPE +#define __TIME64_T_TYPE __SQUAD_TYPE #define __USECONDS_T_TYPE __U32_TYPE #define __SUSECONDS_T_TYPE __SLONGWORD_TYPE #define __DADDR_T_TYPE __S32_TYPE diff --git a/sysdeps/unix/sysv/linux/sparc/bits/typesizes.h b/sysdeps/unix/sysv/linux/sparc/bits/typesizes.h index b59f2ced75..5d6fb00a93 100644 --- a/sysdeps/unix/sysv/linux/sparc/bits/typesizes.h +++ b/sysdeps/unix/sysv/linux/sparc/bits/typesizes.h @@ -48,6 +48,7 @@ #define __ID_T_TYPE __U32_TYPE #define __CLOCK_T_TYPE __SLONGWORD_TYPE #define __TIME_T_TYPE __SLONGWORD_TYPE +#define __TIME64_T_TYPE __SQUAD_TYPE #define __USECONDS_T_TYPE __U32_TYPE #define __SUSECONDS_T_TYPE __S32_TYPE #define __DADDR_T_TYPE __S32_TYPE diff --git a/sysdeps/unix/sysv/linux/x86/bits/typesizes.h b/sysdeps/unix/sysv/linux/x86/bits/typesizes.h index e6f7481a19..f5d792d8f3 100644 --- a/sysdeps/unix/sysv/linux/x86/bits/typesizes.h +++ b/sysdeps/unix/sysv/linux/x86/bits/typesizes.h @@ -62,6 +62,7 @@ #define __ID_T_TYPE __U32_TYPE #define __CLOCK_T_TYPE __SYSCALL_SLONG_TYPE #define __TIME_T_TYPE __SYSCALL_SLONG_TYPE +#define __TIME64_T_TYPE __SQUAD_TYPE #define __USECONDS_T_TYPE __U32_TYPE #define __SUSECONDS_T_TYPE __SYSCALL_SLONG_TYPE #define __DADDR_T_TYPE __S32_TYPE -- 2.17.1 ^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH 1/2] Y2038: add type __time64_t 2018-06-13 7:00 ` [PATCH 1/2] Y2038: add type __time64_t Albert ARIBAUD (3ADEV) @ 2018-06-13 8:38 ` Paul Eggert 2018-06-13 12:36 ` Albert ARIBAUD 2018-06-13 14:13 ` Joseph Myers 1 sibling, 1 reply; 27+ messages in thread From: Paul Eggert @ 2018-06-13 8:38 UTC (permalink / raw) To: Albert ARIBAUD (3ADEV), libc-alpha Albert ARIBAUD (3ADEV) wrote: > -__STD_TYPE __TIME_T_TYPE __time_t; /* Seconds since the Epoch. */ > +__STD_TYPE __TIME_T_TYPE __time_t; /* Seconds since the Epoch, Y2038-unsafe. */ This change to commentary is confusing, as __time_t is Y2038-safe on 64-bit hosts. I suggest omitting the change, and changing the comment on the next line to "Seconds since the Epoch (TIME_BITS==64)" (or whatever the name of that macro is). ^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH 1/2] Y2038: add type __time64_t 2018-06-13 8:38 ` Paul Eggert @ 2018-06-13 12:36 ` Albert ARIBAUD 0 siblings, 0 replies; 27+ messages in thread From: Albert ARIBAUD @ 2018-06-13 12:36 UTC (permalink / raw) To: Paul Eggert; +Cc: libc-alpha Hi Paul, On Wed, 13 Jun 2018 01:38:09 -0700, Paul Eggert <eggert@cs.ucla.edu> wrote : > Albert ARIBAUD (3ADEV) wrote: > > -__STD_TYPE __TIME_T_TYPE __time_t; /* Seconds since the Epoch. */ > > +__STD_TYPE __TIME_T_TYPE __time_t; /* Seconds since the Epoch, Y2038-unsafe. */ > > This change to commentary is confusing, as __time_t is Y2038-safe on 64-bit > hosts. I suggest omitting the change, and changing the comment on the next line > to "Seconds since the Epoch (TIME_BITS==64)" (or whatever the name of that macro > is). Fixed locally as suggested, will repost once 2/2 discussion is complete. Cordialement, Albert ARIBAUD 3ADEV ^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH 1/2] Y2038: add type __time64_t 2018-06-13 7:00 ` [PATCH 1/2] Y2038: add type __time64_t Albert ARIBAUD (3ADEV) 2018-06-13 8:38 ` Paul Eggert @ 2018-06-13 14:13 ` Joseph Myers 2018-06-13 16:19 ` Albert ARIBAUD 1 sibling, 1 reply; 27+ messages in thread From: Joseph Myers @ 2018-06-13 14:13 UTC (permalink / raw) To: Albert ARIBAUD (3ADEV); +Cc: libc-alpha There is a key definition you are missing in this patch: one that says whether the particular glibc configuration supports 32-bit time_t at all. This new macro could go in bits/wordsize.h, or in its own installed header. The value is normally the same as (__WORDSIZE == 32), *except* on x32, which already has 64-bit time_t despite 32-bit wordsize. The new macro would be used, in the installed headers, to control whether an explicit _TIME_BITS=32 is valid or not. It would also be used, in glibc internals, to control whether there are separate 32-bit interfaces that wrap 64-bit ones, or whether the 64-bit names are just #defined to the generic ones (#define __clock_gettime64 __clock_gettime, in some internal header, for example) to avoid creating any new interfaces in the case where time_t is already 64-bit, while avoiding the need for lots of conditionals where glibc-internal code written for 64-bit time_t wishes to call such interfaces. -- Joseph S. Myers joseph@codesourcery.com ^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH 1/2] Y2038: add type __time64_t 2018-06-13 14:13 ` Joseph Myers @ 2018-06-13 16:19 ` Albert ARIBAUD 2018-06-13 16:35 ` Paul Eggert 0 siblings, 1 reply; 27+ messages in thread From: Albert ARIBAUD @ 2018-06-13 16:19 UTC (permalink / raw) To: Joseph Myers; +Cc: libc-alpha Hi Joseph, On Wed, 13 Jun 2018 14:13:00 +0000, Joseph Myers <joseph@codesourcery.com> wrote : > There is a key definition you are missing in this patch: one that says > whether the particular glibc configuration supports 32-bit time_t at all. > > This new macro could go in bits/wordsize.h, or in its own installed > header. The value is normally the same as (__WORDSIZE == 32), *except* on > x32, which already has 64-bit time_t despite 32-bit wordsize. > > The new macro would be used, in the installed headers, to control whether > an explicit _TIME_BITS=32 is valid or not. It would also be used, in > glibc internals, to control whether there are separate 32-bit interfaces > that wrap 64-bit ones, or whether the 64-bit names are just #defined to > the generic ones (#define __clock_gettime64 __clock_gettime, in some > internal header, for example) to avoid creating any new interfaces in the > case where time_t is already 64-bit, while avoiding the need for lots of > conditionals where glibc-internal code written for 64-bit time_t wishes to > call such interfaces. Which would be best: - a "boolean" macro (e.g. __TIMESIZE32) defined as (__WORDSIZE == 32) except for x32 where it would be defined as 1, or - a "size" macro (e.g. __TIMESIZE) defined equal to __WORDSIZE except for x32 where it would be defined as 64? I have a slight preference for the "size" form. Cordialement, Albert ARIBAUD 3ADEV ^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH 1/2] Y2038: add type __time64_t 2018-06-13 16:19 ` Albert ARIBAUD @ 2018-06-13 16:35 ` Paul Eggert 2018-06-13 16:39 ` Joseph Myers 0 siblings, 1 reply; 27+ messages in thread From: Paul Eggert @ 2018-06-13 16:35 UTC (permalink / raw) To: Albert ARIBAUD, Joseph Myers; +Cc: libc-alpha Albert ARIBAUD wrote: > - a "size" macro (e.g. __TIMESIZE) defined equal to __WORDSIZE except > for x32 where it would be defined as 64? > > I have a slight preference for the "size" form. Works for me. Though C11 uses _WIDTH for names that count bit widths of integers, so I'd mildly prefer __TIME_WIDTH as it is a new name. ^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH 1/2] Y2038: add type __time64_t 2018-06-13 16:35 ` Paul Eggert @ 2018-06-13 16:39 ` Joseph Myers 2018-06-13 18:46 ` Paul Eggert 0 siblings, 1 reply; 27+ messages in thread From: Joseph Myers @ 2018-06-13 16:39 UTC (permalink / raw) To: Paul Eggert; +Cc: Albert ARIBAUD, libc-alpha On Wed, 13 Jun 2018, Paul Eggert wrote: > Albert ARIBAUD wrote: > > - a "size" macro (e.g. __TIMESIZE) defined equal to __WORDSIZE except > > for x32 where it would be defined as 64? > > > > I have a slight preference for the "size" form. > > Works for me. Though C11 uses _WIDTH for names that count bit widths of > integers, so I'd mildly prefer __TIME_WIDTH as it is a new name. Whatever the name, it needs to be clear that this macro relates to the time_t used by default (unsuffixed) functions in this glibc - *not* necessarily the time_t for the current compilation. (Rather, _TIME_BITS == 64 && (new macro) == 32 would be the condition for the installed headers to need to redirect calls to time-related functions.) The width macros are from TS 18661-1, not C11 (hopefully they'll end up as an unconditional feature of C2x without requiring __STDC_WANT_IEC_60559_BFP_EXT__, since there is demand for them outside the floating-point context of passing them to the fromfp functions). If there were a TIME_WIDTH addition to that set of macros, I'd expect it to relate to the current compilation. -- Joseph S. Myers joseph@codesourcery.com ^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH 1/2] Y2038: add type __time64_t 2018-06-13 16:39 ` Joseph Myers @ 2018-06-13 18:46 ` Paul Eggert 0 siblings, 0 replies; 27+ messages in thread From: Paul Eggert @ 2018-06-13 18:46 UTC (permalink / raw) To: Joseph Myers; +Cc: Albert ARIBAUD, libc-alpha On 06/13/2018 09:39 AM, Joseph Myers wrote: > Whatever the name, it needs to be clear that this macro relates to the > time_t used by default (unsuffixed) functions in this glibc -*not* > necessarily the time_t for the current compilation. OK, in that case perhaps it's better to stick to __TIMESIZE, as the _WIDTH names are intended to apply to the current compilation. (Sorry about confusing C11 with TS 18661-1.) ^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH 0/2] Y2038 support batch 1 - __time64_t and __tz_convert 2018-06-13 7:00 [PATCH 0/2] Y2038 support batch 1 - __time64_t and __tz_convert Albert ARIBAUD (3ADEV) 2018-06-13 7:00 ` [PATCH 2/2] Y2038: make __tz_convert compatible with 64-bit-time Albert ARIBAUD (3ADEV) 2018-06-13 7:00 ` [PATCH 1/2] Y2038: add type __time64_t Albert ARIBAUD (3ADEV) @ 2018-06-13 7:59 ` Albert ARIBAUD 2018-06-13 14:08 ` Joseph Myers 2 siblings, 1 reply; 27+ messages in thread From: Albert ARIBAUD @ 2018-06-13 7:59 UTC (permalink / raw) To: libc-alpha Added note (will be part of cover letter for all future Y2038 patch posts): All Y2038 patches are tested functionally using an ad hoc framework published at https://github.com/3adev/y2038 which runs tests using times below, at and beyond the Y2038 limit. The tests are run on ARM architecture. These patches have also been tested using build-many-glibcs.py, always for x86_64, aarch64, arm and powerpc compilers, and regularly for all compilers, and found to give the same results as those of the current glibc master above which the patches were created. On Wed, 13 Jun 2018 09:00:17 +0200, "Albert ARIBAUD (3ADEV)" <albert.aribaud@3adev.fr> wrote : > This is the first batch of Y2038 support patches. > > The first patch provides __time64_t, the 64-bit counterpart of time_t, > to be used in 64-bit-time implementations of public APIs related to time. > > The second makes __tz_convert compatible with 64-bit time. This implies > creating 64-bit-time versions of its callers and turning their original > (32-bit-time) versions into wrappers. > > Albert ARIBAUD (3ADEV) (2): > Y2038: add type __time64_t > Y2038: make __tz_convert compatible with 64-bit-time > > bits/typesizes.h | 1 + > include/time.h | 29 +++++++++++--- > posix/bits/types.h | 3 +- > sysdeps/mach/hurd/bits/typesizes.h | 1 + > .../unix/sysv/linux/alpha/bits/typesizes.h | 1 + > .../unix/sysv/linux/generic/bits/typesizes.h | 1 + > sysdeps/unix/sysv/linux/s390/bits/typesizes.h | 1 + > .../unix/sysv/linux/sparc/bits/typesizes.h | 1 + > sysdeps/unix/sysv/linux/x86/bits/typesizes.h | 1 + > time/Versions | 5 +++ > time/ctime.c | 21 ++++++++-- > time/ctime_r.c | 21 ++++++++-- > time/gmtime.c | 38 ++++++++++++++++--- > time/localtime.c | 36 ++++++++++++++++-- > time/offtime.c | 12 +++--- > time/tzfile.c | 14 ++----- > time/tzset.c | 30 ++++++--------- > 17 files changed, 160 insertions(+), 56 deletions(-) > Cordialement, Albert ARIBAUD 3ADEV ^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH 0/2] Y2038 support batch 1 - __time64_t and __tz_convert 2018-06-13 7:59 ` [PATCH 0/2] Y2038 support batch 1 - __time64_t and __tz_convert Albert ARIBAUD @ 2018-06-13 14:08 ` Joseph Myers 2018-06-13 15:38 ` Albert ARIBAUD 0 siblings, 1 reply; 27+ messages in thread From: Joseph Myers @ 2018-06-13 14:08 UTC (permalink / raw) To: Albert ARIBAUD; +Cc: libc-alpha On Wed, 13 Jun 2018, Albert ARIBAUD wrote: > These patches have also been tested using build-many-glibcs.py, always > for x86_64, aarch64, arm and powerpc compilers, and regularly for all > compilers, and found to give the same results as those of the current > glibc master above which the patches were created. Since build-many-glibcs.py should give clean results everywhere except i686-gnu, and since this patch adds Versions entries at a public ABI version but has no *.abilist updates, I'm skeptical of the above description of testing. You should have no ABI test failures before the patch series, anywhere, whether in your normal ARM testing or with build-many-glibcs.py, and without *.abilist updates you should be seeing such failures after the patch series. Any patch adding new ABIs at public versions always needs to include the ABI baseline updates for *all* glibc configurations. You should do full execution testing for at least one 64-bit configuration, as well. -- Joseph S. Myers joseph@codesourcery.com ^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH 0/2] Y2038 support batch 1 - __time64_t and __tz_convert 2018-06-13 14:08 ` Joseph Myers @ 2018-06-13 15:38 ` Albert ARIBAUD 0 siblings, 0 replies; 27+ messages in thread From: Albert ARIBAUD @ 2018-06-13 15:38 UTC (permalink / raw) To: Joseph Myers; +Cc: libc-alpha Hi Joseph, On Wed, 13 Jun 2018 14:08:16 +0000, Joseph Myers <joseph@codesourcery.com> wrote : > On Wed, 13 Jun 2018, Albert ARIBAUD wrote: > > > These patches have also been tested using build-many-glibcs.py, always > > for x86_64, aarch64, arm and powerpc compilers, and regularly for all > > compilers, and found to give the same results as those of the current > > glibc master above which the patches were created. > > Since build-many-glibcs.py should give clean results everywhere except > i686-gnu, and since this patch adds Versions entries at a public ABI > version but has no *.abilist updates, I'm skeptical of the above > description of testing. You should have no ABI test failures before the > patch series, anywhere, whether in your normal ARM testing or with > build-many-glibcs.py, and without *.abilist updates you should be seeing > such failures after the patch series. > > Any patch adding new ABIs at public versions always needs to include the > ABI baseline updates for *all* glibc configurations. Right now I only update abilists for ARM and PowerPC as these are the ones for which I run functional testing. I can update others but I won't be able to test them functionally. In any case, I'll group together related changes to Versions and abilists in the same patch. > You should do full execution testing for at least one 64-bit > configuration, as well. My minimal set includes x86_64-linux-gnu, aarch64-linux-gnu, arm-linux-gnueabi and powerpc-linux-gnu, so this covers two different 64-bit configurations. (my maximal set is 'all compilers and all glibcs' -- including i686-gnu for the sake of it -- but as this takes around 30 hours on a reasonably fast machine, I don't run it for every change.) Cordialement, Albert ARIBAUD 3ADEV ^ permalink raw reply [flat|nested] 27+ messages in thread
end of thread, other threads:[~2018-06-18 13:34 UTC | newest] Thread overview: 27+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2018-06-13 7:00 [PATCH 0/2] Y2038 support batch 1 - __time64_t and __tz_convert Albert ARIBAUD (3ADEV) 2018-06-13 7:00 ` [PATCH 2/2] Y2038: make __tz_convert compatible with 64-bit-time Albert ARIBAUD (3ADEV) 2018-06-13 9:10 ` Florian Weimer 2018-06-13 9:37 ` Albert ARIBAUD 2018-06-13 9:40 ` Florian Weimer 2018-06-13 10:21 ` Albert ARIBAUD 2018-06-13 10:55 ` Albert ARIBAUD 2018-06-13 13:08 ` Florian Weimer 2018-06-13 14:29 ` Joseph Myers 2018-06-13 14:24 ` Joseph Myers 2018-06-18 13:34 ` Florian Weimer 2018-06-13 9:11 ` Paul Eggert 2018-06-13 9:14 ` Florian Weimer 2018-06-13 9:30 ` Albert ARIBAUD 2018-06-13 14:22 ` Joseph Myers 2018-06-13 14:18 ` Joseph Myers 2018-06-13 7:00 ` [PATCH 1/2] Y2038: add type __time64_t Albert ARIBAUD (3ADEV) 2018-06-13 8:38 ` Paul Eggert 2018-06-13 12:36 ` Albert ARIBAUD 2018-06-13 14:13 ` Joseph Myers 2018-06-13 16:19 ` Albert ARIBAUD 2018-06-13 16:35 ` Paul Eggert 2018-06-13 16:39 ` Joseph Myers 2018-06-13 18:46 ` Paul Eggert 2018-06-13 7:59 ` [PATCH 0/2] Y2038 support batch 1 - __time64_t and __tz_convert Albert ARIBAUD 2018-06-13 14:08 ` Joseph Myers 2018-06-13 15:38 ` Albert ARIBAUD
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).