From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on dcvr.yhbt.net X-Spam-Level: X-Spam-ASN: AS31976 209.132.180.0/23 X-Spam-Status: No, score=-4.2 required=3.0 tests=AWL,BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_EF,HEADER_FROM_DIFFERENT_DOMAINS, MAILING_LIST_MULTI,RCVD_IN_DNSWL_MED,SPF_HELO_PASS,SPF_PASS shortcircuit=no autolearn=ham autolearn_force=no version=3.4.2 Received: from sourceware.org (server1.sourceware.org [209.132.180.131]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by dcvr.yhbt.net (Postfix) with ESMTPS id 5A0D120248 for ; Tue, 19 Mar 2019 13:40:17 +0000 (UTC) DomainKey-Signature: a=rsa-sha1; c=nofws; d=sourceware.org; h=list-id :list-unsubscribe:list-subscribe:list-archive:list-post :list-help:sender:date:from:to:cc:subject:message-id:in-reply-to :references:mime-version:content-type; q=dns; s=default; b=sVvcp L0YYa1Qp1L0FjseOqvNds838xvrZ2qm/rfWOg3VD71wUu/4S2JE09apwiCqsz9Ah 3jkGvpQPqFPyR549SYFPHLJ7YaN+gLHofiSLGpyLLkx/f1nwykFV063JmYnLxBGU jEiqdswYb/IhrYnB6thbaQ/S2baIN2QoNG/Y1o= DKIM-Signature: v=1; a=rsa-sha1; c=relaxed; d=sourceware.org; h=list-id :list-unsubscribe:list-subscribe:list-archive:list-post :list-help:sender:date:from:to:cc:subject:message-id:in-reply-to :references:mime-version:content-type; s=default; bh=k+9t2BFew6G XXdKkX40KfvpHG6E=; b=f3VfwCufOBm+H2H6YKluoDsmE395DrXp42CutyimOXa Wy/B1Hh3iLQvk38h6tGSJ1gTkHGteT5iJfkazDqUo40cvWdXNvH+wPpmbjX55hs9 hEVb+OUfmjGAADXaCfezUgUgwFP18qHtYmk0GM5LbzpCepa/EreARTvxN1YcukW8 = Received: (qmail 87527 invoked by alias); 19 Mar 2019 13:40:14 -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 Received: (qmail 87504 invoked by uid 89); 19 Mar 2019 13:40:13 -0000 Authentication-Results: sourceware.org; auth=none X-HELO: mail-out.m-online.net Date: Tue, 19 Mar 2019 14:39:56 +0100 From: Lukasz Majewski To: Paul Eggert Cc: libc-alpha@sourceware.org, Joseph Myers Subject: Re: [PATCH v2 1/2] Y2038: make __mktime_internal compatible with __time64_t Message-ID: <20190319143956.52f83a48@jawa> In-Reply-To: References: <20190227112042.1794-1-lukma@denx.de> <20190312075856.33ac3c5b@jawa> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha256; boundary="Sig_/=bAM7VWtYYB_K1q_jFTOT.D"; protocol="application/pgp-signature" --Sig_/=bAM7VWtYYB_K1q_jFTOT.D Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: quoted-printable Hi Paul, > On 3/11/19 11:58 PM, Lukasz Majewski wrote: > > The question is if there is a more suitable place than > > include/time.h header to have fits_in_time() reused by both Glibc > > and Gnulib? =20 >=20 Thanks for your patch. > Hmm, well, on further thought, fits_in_time_t can live in > include/time.h since only glibc will need it. Gnulib won't support > two kinds of time_t types, so it should't need the function. (The > attached patch renames it to "in_time_t_range" to avoid *_t > pollution.) Is the rewritten code correct? +/* Check whether T fits in time_t. */ +static inline bool +in_time_t_range (__time64_t t) +{ + time_t s =3D t; + return s !=3D t; +} Shouldn't we have: return s =3D=3D t; ? >=20 > 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 >=20 ACK/NAK for using time/mktime-internl.h shall be done by more experienced glibc developer(s). > Some other comments on that patch that I noticed while looking into > this. >=20 > The patch added duplicate "#include " lines to time/timegm.c. Ok. >=20 > I still don't get why we need __timelocal64. Glibc code can use > __mktime64. User code shouldn't see that symbol. If we don't need it for this conversion, then we should omit it. I will add it if needed when preparing Y2038 patch set. In the time/mktime.c there is: weak_alias (mktime, timelocal), which makes the timelocal calls aliases to mktime for time_t 32 and 64 bit (for Y2038 the proper __REDIRECT will be added). >=20 > Come to think of it, user code shouldn't see __time64_t either. Yes, > the name begins with two underscores so user code should avoid it, > posix/bits/types.hbut putting __time64_t in /usr/include will just > tempt users. It's easy to keep __time64_t out of /usr/include so > let's do that. >=20 Is that the reason for removing __time64_t definition from posix/bits/types.h ? > The body of __mktime64 can be simplified; there's no need for locals > of type __time64_t, time_t, and struct tm. And it should use > in_time_t_range instead of doing it by hand. Yes. Also there is no need to check for EOVERFLOW in the __mktime64() function (as it operates inherently on 64 bit types). >=20 > The bodies of mktime and timegm [__TIMESIZE !=3D 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. >=20 Ok. > The "#ifdef weak_alias" etc. lines can be removed now, since Gnulib > does this stuff OK now. >=20 > timegm.c should include libc-config.h, not config.h, as this is the > current Gnulib style for code shared with Gnulib. >=20 > Revised proposed patch attached. >=20 Just out of curiosity: In the time/mktime-internal.h you added a comment regarding BeOS users and posix time_t - do you know any :-) ? Best regards, Lukasz Majewski -- DENX Software Engineering GmbH, Managing Director: Wolfgang Denk HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: (+49)-8142-66989-59 Fax: (+49)-8142-66989-80 Email: lukma@denx.de --Sig_/=bAM7VWtYYB_K1q_jFTOT.D Content-Type: application/pgp-signature Content-Description: OpenPGP digital signature -----BEGIN PGP SIGNATURE----- iQEzBAEBCAAdFiEEgAyFJ+N6uu6+XupJAR8vZIA0zr0FAlyQ8SwACgkQAR8vZIA0 zr2Y2Af+Os4zbodqwN5cLqCI63EpVaa3N1mA76KJMZ4k4FoQPSZoztD+uZMYjrUV XtaUaSDhiMSbeh7LMFvsoKd1z4Z+vt1/mvwW20sONaKVdPpsF7y9iOV8fsipmevG vyDiDsst/dmUt4G6rMb9eeDkGvE0G2Frfzs6lK6ym+0xysyIo8z4i1RsLjbR+GQp OYsYtCWGKMhDl4x0PEELyrpZdLqH4AZtZhruGRaapIobS618HVH54eRpCS4QR6KQ DyBT1I1jHsqzMCnOIV4KudlKCPfhKS6Z9Us6BntxMHR6dabW4fZp8dlSRd3KCSef o3phnajDS825yt5b1BZBEWjmrJU8jQ== =udGv -----END PGP SIGNATURE----- --Sig_/=bAM7VWtYYB_K1q_jFTOT.D--