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=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 D075920248 for ; Tue, 12 Mar 2019 06:59: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=dZ/K0 CcDbM9dxjw4cziDqC46bno5c9r7MwCC53yD4zvipIu+SZ2K+A4stmu/Xxf2oGZOu pMEBon43uT6pbe8mTOUqoeG/V+X8vRBtoyWCiU+jEXvFk4VwAyTB9p3f8zt0bnyW sDkjKa3AS9vZE3o+WjOdshlO8QoMggfilsCwoM= 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=KuGIOBXNOx4 LiCGM4Qm4shij1EI=; b=mbmnMxoiUfs1dTm+3U3iE8UF4XdrNLPjWYTbNJNz5VD Z6hfgZ6oiLV0KgMyYBPU7fHMwL+5G3plqo5FX4hP6QnHc/ZWkdAGg38LFwqs0Mgr YkkkBjfcgY540dfc+aYicNHdKamUkL8acHeXzkoGd56e78h2F54JGiaVcHsGHi0E = Received: (qmail 34016 invoked by alias); 12 Mar 2019 06:59:15 -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 33914 invoked by uid 89); 12 Mar 2019 06:59:15 -0000 Authentication-Results: sourceware.org; auth=none X-HELO: mail-out.m-online.net Date: Tue, 12 Mar 2019 07:58: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: <20190312075856.33ac3c5b@jawa> In-Reply-To: References: <20190227112042.1794-1-lukma@denx.de> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha256; boundary="Sig_/ary8Oo4K=Ch42gL_OFI/xRk"; protocol="application/pgp-signature" --Sig_/ary8Oo4K=Ch42gL_OFI/xRk Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: quoted-printable Hi Paul, > On 2/27/19 3:20 AM, Lukasz Majewski wrote: > > +/* Another name for `__mktime64'. */ > > +extern __time64_t __timelocal64 (struct tm *__tp) __THROW; =20 >=20 > In hindsight the name 'timelocal' was a mistake: it's not a portable > name and its use has not caught on. Although we need to keep > 'timelocal' for backwards compatibility, there's no need to define > 'timelocal64', as the very few people who need such a function can > just call mktime64. So I suggest removing all traces of timelocal64, > __timelocal64, etc. from the patch. >=20 >=20 > > +/* Check whether a time64_t value fits in a time_t. */ > > +static inline bool > > +fits_in_time_t (__time64_t t64) > > +{ > > + time_t t =3D t64; > > + return t =3D=3D t64; > > +} > > + =20 >=20 > This function is used only in time/mktime.c, and so should be defined > there. This will help sharing with Gnulib, which doesn't have > include/time.h. Some patches on top of this one (y2038 related) use fits_in_time_t() to check if 64 bit wrapper on 32 bit version of function (like __clock_gettime()) shall proceed or return EOVERFLOW. 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 > The function's name should not end with "_t" since that's in the > reserved namespace (important for Gnulib). Ok. >=20 >=20 > > -verify (TYPE_IS_INTEGER (time_t)); > > +verify (TYPE_IS_INTEGER (__time64_t)); =20 >=20 > Please remove this line instead. It dates back to old POSIX, which > allowed time_t to be a floating-point type. POSIX no longer allows > this and we needn't worry about that possibility any more. >=20 >=20 > > + __time64_t t64; > > + time_t t; > > + struct tm tp0 =3D *tp; =20 > There is no need for both t and t64. Just declare "__time64_t t;" and > replace all uses of t64 with t. Also, please consistently rename tp0 > to tm0, since it's not a pointer. >=20 >=20 > > + t =3D t64; > > + if (t !=3D t64) =20 >=20 > Replace this with "if (! fits_in_time_t (t))" (but rename the > function). >=20 >=20 > > +/* The 32-bit-time wrapper. */ > > +time_t > > +mktime (struct tm *tp) > > +{ > > + __time64_t t64 =3D __mktime64 (tp); > > + if (fits_in_time_t (t64)) > > + return t64; > > + __set_errno (EOVERFLOW); > > + return -1; > > +} =20 >=20 > Fix this so that it doesn not modify *TP when failing due to > EOVERFLOW. The manual says mktime doesn't change *TP on failure. >=20 >=20 > Similar comments apply to the implementation of timegm. >=20 Thanks for your comments. 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_/ary8Oo4K=Ch42gL_OFI/xRk Content-Type: application/pgp-signature Content-Description: OpenPGP digital signature -----BEGIN PGP SIGNATURE----- iQEzBAEBCAAdFiEEgAyFJ+N6uu6+XupJAR8vZIA0zr0FAlyHWLAACgkQAR8vZIA0 zr3T6Qf/eOTyacFPBIPRTyMZVl4aElm8enYSj5B2nM3An7nuDn4vF8IxW8/SbqGv HrkpdvTZJHRAUD+Va2Sa6AWo4DYnkuoKnJxIMgR9uMe2IA6r9QhQRnYKz65QNJkz GHStah1v4Aejxkz8AbM8h7HCdUB5mA1jEG/z6FPsDDbevipNeVNSX6U5nY1KFRZd 6+zp08+PXzKAGFDMfN03iJU73BnCsEspdff9MEPyutT99g+vn5yzJoOb3JmVVGCO TQzAFik0Cfj/yCAdq/qwwrLrK8yv0kSt+jnuxlGezNXuUrAYdiLXY4gg0QQU9wDN piqfHF/9RM5KmdbuLXzBNC7/Cz06lg== =38eo -----END PGP SIGNATURE----- --Sig_/ary8Oo4K=Ch42gL_OFI/xRk--