Re: [PATCH] Add support for TZ names with <> in tzset

["Earle F. Philhower, III" <earlephilhower@yahoo.com>]

> 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.

>>> Should consider modifying the PD TZ project tzcode reference src >>> localtime.c tzparse()
>>>    https://github.com/eggert/tz/blob/master/localtime.c#L1069
...
Brian, thanks for the suggestion but I'd be very worried about opening
up this function so completely.  There's unfortunately not a set of unit
tests for the time routines in newlib, and there's not any particular
issue that's seen other than this timezone naming scheme.
Updated patch follows, I hope it addresses the issues above.Anything missing, please do let me know.
----Extends the tzset() function to support <quoted> TZ environment timezones
names.

For example, "<+01>-1" is the timezone environment for TZ_Africa_Casablanca

Signed-off-by: Earle F. Philhower, III <earlephilhower@yahoo.com>
---
 newlib/libc/time/tzset_r.c | 28 ++++++++++++++++++++++++++--
 1 file changed, 26 insertions(+), 2 deletions(-)

diff --git a/newlib/libc/time/tzset_r.c b/newlib/libc/time/tzset_r.c
index 9e0cf834b..61228ab72 100644
--- a/newlib/libc/time/tzset_r.c
+++ b/newlib/libc/time/tzset_r.c
@@ -13,6 +13,30 @@ static char __tzname_std[11];
 static char __tzname_dst[11];
 static char *prev_tzenv = NULL;
 
+/* Handle parsing of both unquoted and <quoted> forms of the timzeone name. */
+/* Returns 0 on failure, 1 on successful parsing, placing name and count in */
+/* passed-in __tzname, n variables. */
+static int
+__parse_tzname_string(const char *tzenv, char *__tzname, int *n)
+{
+  if (tzenv[0] == '<')
+    {
+      /* This is of the quoted form "<[+-]nn>" so needs a different parsing */
+      if (sscanf (tzenv, "%9[^>]>%n", __tzname, n) <= 0)
+        return 0;
+      /* Include the final > in the timezone name */
+      __tzname[*n - 1] = '>';
+      __tzname[*n] = 0;
+    }
+  else
+    {
+      /* Unquoted timezone format parser */
+      if (sscanf (tzenv, "%10[^0-9,+-]%n", __tzname, n) <= 0)
+        return 0;
+    }
+  return 1;
+}
+
 void
 _tzset_unlocked_r (struct _reent *reent_ptr)
 {
@@ -45,7 +69,7 @@ _tzset_unlocked_r (struct _reent *reent_ptr)
   if (*tzenv == ':')
     ++tzenv;  
 
-  if (sscanf (tzenv, "%10[^0-9,+-]%n", __tzname_std, &n) <= 0)
+  if (!__parse_tzname_string (tzenv, __tzname_std, &n))
     return;
  
   tzenv += n;
@@ -69,7 +93,7 @@ _tzset_unlocked_r (struct _reent *reent_ptr)
   _tzname[0] = __tzname_std;
   tzenv += n;
   
-  if (sscanf (tzenv, "%10[^0-9,+-]%n", __tzname_dst, &n) <= 0)
+  if (!__parse_tzname_string (tzenv, __tzname_dst, &n))
     { /* No dst */
       _tzname[1] = _tzname[0];
       _timezone = tz->__tzrule[0].offset;
-- 
2.17.1


Thanks,-EFP3