From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [63.128.21.124]) by sourceware.org (Postfix) with ESMTP id 63D233865474 for ; Mon, 14 Dec 2020 09:30:59 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org 63D233865474 Received: from mimecast-mx01.redhat.com (mimecast-mx01.redhat.com [209.132.183.4]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-98-Pe67chSUMo-xeShe4M5BHg-1; Mon, 14 Dec 2020 04:30:55 -0500 X-MC-Unique: Pe67chSUMo-xeShe4M5BHg-1 Received: from smtp.corp.redhat.com (int-mx01.intmail.prod.int.phx2.redhat.com [10.5.11.11]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx01.redhat.com (Postfix) with ESMTPS id F32CB10054FF; Mon, 14 Dec 2020 09:30:54 +0000 (UTC) Received: from calimero.vinschen.de (ovpn-113-243.ams2.redhat.com [10.36.113.243]) by smtp.corp.redhat.com (Postfix) with ESMTPS id C4C325B4A4; Mon, 14 Dec 2020 09:30:54 +0000 (UTC) Received: by calimero.vinschen.de (Postfix, from userid 500) id 521F4A829BB; Mon, 14 Dec 2020 10:30:53 +0100 (CET) Date: Mon, 14 Dec 2020 10:30:53 +0100 From: Corinna Vinschen To: "Earle F. Philhower, III" Cc: "newlib@sourceware.org" Subject: Re: [PATCH] Add support for TZ names with <> in tzset Message-ID: <20201214093053.GF4102@calimero.vinschen.de> Reply-To: newlib@sourceware.org Mail-Followup-To: "Earle F. Philhower, III" , "newlib@sourceware.org" References: <080401d6bada$6c52f060$44f8d120$.ref@yahoo.com> <080401d6bada$6c52f060$44f8d120$@yahoo.com> <20201116151359.GD41926@calimero.vinschen.de> <3a5f706a-f1c0-595f-ff96-e674cb72e233@SystematicSw.ab.ca> <76c91228-465f-1c99-9a8e-6269063b04bd@SystematicSw.ab.ca> <1223838587.4607472.1607795805086@mail.yahoo.com> MIME-Version: 1.0 In-Reply-To: <1223838587.4607472.1607795805086@mail.yahoo.com> X-Scanned-By: MIMEDefang 2.79 on 10.5.11.11 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: 8bit X-Spam-Status: No, score=-6.9 required=5.0 tests=BAYES_00, DKIMWL_WL_HIGH, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, RCVD_IN_DNSWL_NONE, RCVD_IN_MSPIKE_H4, RCVD_IN_MSPIKE_WL, SPF_HELO_NONE, SPF_PASS, TXREP autolearn=ham autolearn_force=no version=3.4.2 X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on server2.sourceware.org X-BeenThere: newlib@sourceware.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Newlib mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 14 Dec 2020 09:31:00 -0000 Hi Earle, On Dec 12 17:56, Earle F. Philhower, III via Newlib wrote: > > > On 2020-11-16 18:43, C Howland via Newlib wrote:>> On Monday, November 16, 2020 5:30 PM, Brian Inglis wrote: > >>> On 2020-11-16 08:13, Corinna Vinschen via Newlib wrote: > >>>> On Nov 14 15:03, Earle F. Philhower, III via Newlib wrote: > >>>>> Attached is a patch which extends the tzset() function to support a > >>>>> format for "unnamed" TZ environment timezones which use "<+/-nn>" as > >>>>> the timezone name instead of an alphabetic name.  These are supported > >>>>> in glibc and are present in several major TZ databases that we use on > >>>>> the ESP8266 Arduino core.  For example, > ...>>>> Basically this looks ok.  I have two nits, though.>>>> - Now that the scanning got more complicated than a single sscanf call, > >>>>    this crys out for a helper function doing the actual scanning for > >>>>    both, std and dst strings.  This could be an inline function which is > >>>>    only inlined > >>>>    #if !defined(PREFER_SIZE_OVER_SPEED) && !defined(__OPTIMIZE_SIZE__)>>>> - The strcat call seems a bit heavy.  What about sth like this instead: > > Thanks Corinna.  I've factored out the common name scanner to its own > static function, removed the strcat in favor of your suggestion, and > added documentation in the code about its function. > > However, I'm not sure it makes sense to worry about size/speed > optimizations here.  This is tzset_r and the normal case is to call > it ~1 time per application startup to set the localtime offsets > using the TZ environment variable, and never again. The size optimization would be the point here in the first place, the speed optimization by inlining secondary. However, adding the aforementioned #if is really not much of a problem to handle both preferences alike. Other than that, the patch looks ok. Can you please send it in `git format-patch' format when you're ready? Thanks, Corinna