Re: [PATCH] Add support for TZ names with <> in tzset
Corinna Vinschen via Newlib <[email protected]>
| Newsgroups | gmane.comp.lib.newlib |
|---|---|
| Message-ID | <[email protected]> |
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