Message ID | 20180613070019.4639-3-albert.aribaud@3adev.fr |
---|---|
State | New |
Headers | show |
Series | Y2038 support batch 1 - __time64_t and __tz_convert | expand |
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
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.
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
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
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
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
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
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
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
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
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.
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).
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.
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
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); } -/* 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) -/* 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; }