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

[Corinna Vinschen <vinschen@redhat.com>]

Hi Earle,

On Nov 14 15:03, Earle F. Philhower, III via Newlib wrote:
> Howdy all,
> 
> 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, 
> 
> > #define TZ_Africa_Casablanca	  "<+01>-1"
> 
> The existing tzset sscanf format string breaks at the first "+", assuming
> it's the
> beginning of the offset.  This patch special-cases names beginning with "<"
> to
> circumvent the issue.
> 
> Signed-off-by: Earle F. Philhower, III   <earlephilhower@yahoo.com>

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:

    __tzname_ptr[n - 1] = '>';
    __tzname_ptr[n] = '\0';


Thanks,
Corinna



> ---
>  newlib/libc/time/tzset_r.c | 46 ++++++++++++++++++++++++++++++--------
>  1 file changed, 37 insertions(+), 9 deletions(-)
> 
> diff --git a/newlib/libc/time/tzset_r.c b/newlib/libc/time/tzset_r.c
> index 9e0cf834b..5b8b692ee 100644
> --- a/newlib/libc/time/tzset_r.c
> +++ b/newlib/libc/time/tzset_r.c
> @@ -45,8 +45,19 @@ _tzset_unlocked_r (struct _reent *reent_ptr)
>    if (*tzenv == ':')
>      ++tzenv;
> 
> -  if (sscanf (tzenv, "%10[^0-9,+-]%n", __tzname_std, &n) <= 0)
> -    return;
> +  if (tzenv[0] == '<')
> +    {
> +      /* This is of the form "<[+-]nn>" so needs a different parsing */
> +      if (sscanf (tzenv, "%9[^>]>%n", __tzname_std, &n) <= 0)
> +        return;
> +      /* Include the final > */
> +      strcat (__tzname_std, ">");
> +    }
> +  else
> +    {
> +      if (sscanf (tzenv, "%10[^0-9,+-]%n", __tzname_std, &n) <= 0)
> +        return;
> +    }
> 
>    tzenv += n;
> 
> @@ -69,15 +80,32 @@ _tzset_unlocked_r (struct _reent *reent_ptr)
>    _tzname[0] = __tzname_std;
>    tzenv += n;
> 
> -  if (sscanf (tzenv, "%10[^0-9,+-]%n", __tzname_dst, &n) <= 0)
> -    { /* No dst */
> -      _tzname[1] = _tzname[0];
> -      _timezone = tz->__tzrule[0].offset;
> -      _daylight = 0;
> -      return;
> +  if (tzenv[0] == '<')
> +    {
> +      /* This is of the form "<[+-]nn>" so needs a different parsing */
> +      if (sscanf (tzenv, "%9[^>]>%n", __tzname_dst, &n) <= 0)
> +        { /* No dst */
> +          _tzname[1] = _tzname[0];
> +          _timezone = tz->__tzrule[0].offset;
> +          _daylight = 0;
> +          return;
> +        }
> +      /* Include the final > */
> +      strcat (__tzname_dst, ">");
> +      _tzname[1] = __tzname_dst;
>      }
>    else
> -    _tzname[1] = __tzname_dst;
> +    {
> +      if (sscanf (tzenv, "%10[^0-9,+-]%n", __tzname_dst, &n) <= 0)
> +        { /* No dst */
> +          _tzname[1] = _tzname[0];
> +          _timezone = tz->__tzrule[0].offset;
> +          _daylight = 0;
> +          return;
> +        }
> +      else
> +        _tzname[1] = __tzname_dst;
> +    }
> 
>    tzenv += n;
> 
> --
> 2.17.1
>