From patchwork Wed Mar 27 20:06:49 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Paul Eggert X-Patchwork-Id: 32010 Received: (qmail 66510 invoked by alias); 27 Mar 2019 20:07:00 -0000 Mailing-List: contact libc-alpha-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: libc-alpha-owner@sourceware.org Delivered-To: mailing list libc-alpha@sourceware.org Received: (qmail 66495 invoked by uid 89); 27 Mar 2019 20:06:59 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-15.3 required=5.0 tests=AWL, BAYES_00, GIT_PATCH_0, GIT_PATCH_1, GIT_PATCH_2, GIT_PATCH_3, KAM_SHORT, SPF_PASS autolearn=ham version=3.3.1 spammy=alone, portable, timestamp, epoch X-HELO: zimbra.cs.ucla.edu Subject: Re: [PATCH v2 1/2] Y2038: make __mktime_internal compatible with __time64_t To: Lukasz Majewski Cc: libc-alpha@sourceware.org, Joseph Myers References: <20190227112042.1794-1-lukma@denx.de> <20190312075856.33ac3c5b@jawa> <20190319143956.52f83a48@jawa> <910c75f2-86ea-34cd-7279-71fcbf5edabc@cs.ucla.edu> <20190323125906.699ce15e@jawa> From: Paul Eggert Openpgp: preference=signencrypt Message-ID: Date: Wed, 27 Mar 2019 13:06:49 -0700 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 Thunderbird/60.6.0 MIME-Version: 1.0 In-Reply-To: <20190323125906.699ce15e@jawa> 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. From 8d6ea5e533fb3c071ae182dbf16a32b959f473b0 Mon Sep 17 00:00:00 2001 From: Paul Eggert 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 + + 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 * 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 +# include +# include