From patchwork Mon Mar 18 21:23:07 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Paul Eggert X-Patchwork-Id: 31903 Received: (qmail 61001 invoked by alias); 18 Mar 2019 21:23:16 -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 60951 invoked by uid 89); 18 Mar 2019 21:23:15 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-15.2 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=nearest, timer, H*r:10026, Keep X-HELO: zimbra.cs.ucla.edu From: Paul Eggert Subject: [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> Openpgp: preference=signencrypt Message-ID: Date: Mon, 18 Mar 2019 14:23:07 -0700 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 Thunderbird/60.5.1 MIME-Version: 1.0 In-Reply-To: <20190312075856.33ac3c5b@jawa> On 3/11/19 11:58 PM, Lukasz Majewski wrote: > The question is if there is a more suitable place than include/time.h > header to have fits_in_time() reused by both Glibc and Gnulib? Hmm, well, on further thought, fits_in_time_t can live in include/time.h since only glibc will need it. Gnulib won't support two kinds of time_t types, so it should't need the function. (The attached patch renames it to "in_time_t_range" to avoid *_t pollution.) That being said, we will have to make a place for private .h code shared between glibc and Gnulib, because include/time.h can't easily be shared with Gnulib. I suggest using time/mktime-internal.h for this Some other comments on that patch that I noticed while looking into this. The patch added duplicate "#include " lines to time/timegm.c. I still don't get why we need __timelocal64. Glibc code can use __mktime64. User code shouldn't see that symbol. Come to think of it, user code shouldn't see __time64_t either. Yes, the name begins with two underscores so user code should avoid it, but putting __time64_t in /usr/include will just tempt users. It's easy to keep __time64_t out of /usr/include so let's do that. The body of __mktime64 can be simplified; there's no need for locals of type __time64_t, time_t, and struct tm. And it should use in_time_t_range instead of doing it by hand. The bodies of mktime and timegm [__TIMESIZE != 64] should not pass tp to __mktime64/__timegm64 directly, since *tp should not be updated if the result is in __time64_t range but out of time_t range. And timegm should use in_time_t_range instead of doing it by hand. The "#ifdef weak_alias" etc. lines can be removed now, since Gnulib does this stuff OK now. timegm.c should include libc-config.h, not config.h, as this is the current Gnulib style for code shared with Gnulib. Revised proposed patch attached. From 509d363dc0f96ea194589e90b27c7f1c2de51371 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 f7c9ee51ad..5d9f936c19 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,47 @@ +2019-03-18 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-03-16 Samuel Thibault * hurd/hurd/signal.h (_hurd_critical_section_lock): Document how EINTR diff --git a/include/time.h b/include/time.h index 61dd9e180b..3e55f7ba83 100644 --- a/include/time.h +++ b/include/time.h @@ -3,6 +3,8 @@ #ifndef _ISOMAC # include +# include +# include