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.1 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 80B1720248 for ; Tue, 12 Mar 2019 00:11:30 +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:subject:to:cc:references:from:message-id:date :mime-version:in-reply-to:content-type :content-transfer-encoding; q=dns; s=default; b=IK5+KzuyC6Y80600 VrSUtlN3+Ap46ANs2fEX7oY4nOT+PxZuxoxWLhtnxEPp5lPWp4IMngyV6MdfkDj0 VrUskyyZ4e8m3MkrDvqND1euByj2i6fT7P5g0PJh3T5v8YMh6fk5AV9CNdzq6nB4 wI3O63LxBE0RjdBDB9FM9gAwwIQ= 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:subject:to:cc:references:from:message-id:date :mime-version:in-reply-to:content-type :content-transfer-encoding; s=default; bh=U2/3iARSqztUBt3uj0hHTp FurgM=; b=wzpeurabsTT68aVVik+TamKT1xHCOkHaOYSKxTaEt7wGGlFkg8AyxR Kx9bMhUsYnkQ7QXqrrbuUgps0AnVr0wiQPEf4vJVxidAqkSv6NI7cqTv2n2Uxnbq ivwUCV/P43OszZaaE93LYUnb3THQDTcm3/JcyjdYtnwW30jCzTjgk= Received: (qmail 74678 invoked by alias); 12 Mar 2019 00:11:27 -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 74670 invoked by uid 89); 12 Mar 2019 00:11:27 -0000 Authentication-Results: sourceware.org; auth=none X-HELO: zimbra.cs.ucla.edu Subject: Re: [PATCH v2 1/2] Y2038: make __mktime_internal compatible with __time64_t To: Lukasz Majewski , libc-alpha@sourceware.org Cc: Joseph Myers References: <20190227112042.1794-1-lukma@denx.de> From: Paul Eggert Openpgp: preference=signencrypt Message-ID: Date: Mon, 11 Mar 2019 17:11:23 -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: <20190227112042.1794-1-lukma@denx.de> Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit On 2/27/19 3:20 AM, Lukasz Majewski wrote: > +/* Another name for `__mktime64'. */ > +extern __time64_t __timelocal64 (struct tm *__tp) __THROW; 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. > +/* Check whether a time64_t value fits in a time_t. */ > +static inline bool > +fits_in_time_t (__time64_t t64) > +{ > + time_t t = t64; > + return t == t64; > +} > + 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. The function's name should not end with "_t" since that's in the reserved namespace (important for Gnulib). > -verify (TYPE_IS_INTEGER (time_t)); > +verify (TYPE_IS_INTEGER (__time64_t)); 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. > + __time64_t t64; > + time_t t; > + struct tm tp0 = *tp; 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. > + t = t64; > + if (t != t64) Replace this with "if (! fits_in_time_t (t))" (but rename the function). > +/* The 32-bit-time wrapper. */ > +time_t > +mktime (struct tm *tp) > +{ > + __time64_t t64 = __mktime64 (tp); > + if (fits_in_time_t (t64)) > + return t64; > + __set_errno (EOVERFLOW); > + return -1; > +} Fix this so that it doesn not modify *TP when failing due to EOVERFLOW. The manual says mktime doesn't change *TP on failure. Similar comments apply to the implementation of timegm.