[PATCH 0/2] add tzset/_r POSIX angle bracket <> support in TZ env var

[PATCH 0/2] add tzset/_r POSIX angle bracket <> support in TZ env var

[Brian Inglis]

Brian Inglis (2):
  newlib/libc/time/tzset.c: doc update POSIX angle bracket <> support
  newlib/libc/time/tzset_r.c(_tzset_unlocked_r): POSIX angle bracket <>
    support

 newlib/libc/time/tzset.c   | 110 ++++++++++++++++++++++++++-----------
 newlib/libc/time/tzset_r.c |  67 ++++++++++++++++++----
 2 files changed, 134 insertions(+), 43 deletions(-)

-- 
2.35.1

[PATCH 1/2] tzset.c: doc update POSIX angle bracket <> support

[Brian Inglis]

[-- Attachment #1: Type: text/plain, Size: 161 bytes --]


add format and structure to doc
---
 newlib/libc/time/tzset.c | 110 ++++++++++++++++++++++++++++-----------
 1 file changed, 79 insertions(+), 31 deletions(-)


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-tzset.c-doc-update-POSIX-angle-bracket-support.patch --]
[-- Type: text/x-patch; name="0001-tzset.c-doc-update-POSIX-angle-bracket-support.patch", Size: 5371 bytes --]

diff --git a/newlib/libc/time/tzset.c b/newlib/libc/time/tzset.c
index 3b4c01c660a4..77accbfb0485 100644
--- a/newlib/libc/time/tzset.c
+++ b/newlib/libc/time/tzset.c
@@ -1,6 +1,6 @@
 /*
 FUNCTION
-<<tzset>>---set timezone characteristics from TZ environment variable
+<<tzset>>---set timezone characteristics from <[TZ]> environment variable
 
 INDEX
 	tzset
@@ -13,36 +13,84 @@ SYNOPSIS
 	void _tzset_r (struct _reent *<[reent_ptr]>);
 
 DESCRIPTION
-<<tzset>> examines the TZ environment variable and sets up the three
-external variables: <<_timezone>>, <<_daylight>>, and <<tzname>>.  The
-value of <<_timezone>> shall be the offset from the current time zone
-to GMT.  The value of <<_daylight>> shall be 0 if there is no daylight
-savings time for the current time zone, otherwise it will be non-zero.
-The <<tzname>> array has two entries: the first is the name of the
-standard time zone, the second is the name of the daylight-savings time
-zone.
-
-The TZ environment variable is expected to be in the following POSIX
-format:
-
-  stdoffset1[dst[offset2][,start[/time1],end[/time2]]]
-
-where: std is the name of the standard time-zone (minimum 3 chars)
-       offset1 is the value to add to local time to arrive at Universal time
-         it has the form:  hh[:mm[:ss]]
-       dst is the name of the alternate (daylight-savings) time-zone (min 3 chars)
-       offset2 is the value to add to local time to arrive at Universal time
-         it has the same format as the std offset
-       start is the day that the alternate time-zone starts
-       time1 is the optional time that the alternate time-zone starts
-         (this is in local time and defaults to 02:00:00 if not specified)
-       end is the day that the alternate time-zone ends
-       time2 is the time that the alternate time-zone ends
-         (it is in local time and defaults to 02:00:00 if not specified)
-
-Note that there is no white-space padding between fields.  Also note that
-if TZ is null, the default is Universal GMT which has no daylight-savings
-time.  If TZ is empty, the default EST5EDT is used.
+<<tzset>> examines the <[TZ]> environment variable and sets up the three
+external variables: <<_timezone>>, <<_daylight>>, and <<tzname>>.
+The value of <<_timezone>> shall be the offset from the current time
+to Universal Time.
+The value of <<_daylight>> shall be 0 if there is no daylight savings
+time for the current time zone, otherwise it will be non-zero.
+The <<tzname>> array has two entries: the first is the designation of the
+standard time period, the second is the designation of the alternate time
+period.
+
+The <[TZ]> environment variable is expected to be in the following POSIX
+format (spaces inserted for clarity):
+
+    <[std]> <[offset1]> [<[dst]> [<[offset2]>] [,<[start]> [/<[time1]>], <[end]> [/<[time2]>]]]
+
+where:
+
+<[std]> is the designation for the standard time period (minimum 3,
+maximum <<TZNAME_MAX>> bytes) in one of two forms:
+
+*- quoted within angle bracket '<' '>' characters: portable numeric
+sign or alphanumeric characters in the current locale; the
+quoting characters are not included in the designation
+
+*- unquoted: portable alphabetic characters in the current locale
+
+<[offset1]> is the value to add to local standard time to get Universal Time;
+it has the format:
+
+    [<[S]>]<[hh]>[:<[mm]>[:<[ss]>]]
+
+    where:
+
+    <[S]> is an optional numeric sign character; if negative '-', the
+    time zone is East of the International Reference
+    Meridian; else it is positive and West, and '+' may be used
+
+    <[hh]> is the required numeric hour between 0 and 24
+
+    <[mm]> is the optional numeric minute between 0 and 59
+
+    <[ss]> is the optional numeric second between 0 and 59
+
+<[dst]> is the designation of the alternate (daylight saving or
+summer) time period, with the same limits and forms as
+the standard time period designation
+
+<[offset2]> is the value to add to local alternate time to get
+Universal Time; it has the same format as <[offset1]>
+
+<[start]> is the date in the year that alternate time starts;
+the form may be one of:
+(quotes "'" around characters below are used only to distinguish literals)
+
+    <[n]>	zero based Julian day (0-365), counting February 29 Leap days
+
+    'J'<[n]>	one based Julian day (1-365), not counting February 29 Leap
+    days; in all years day 59 is February 28 and day 60 is March 1
+
+    'M'<[m]>'.'<[w]>'.'<[d]>
+    month <[m]> (1-12) week <[w]> (1-5) day <[d]> (0-6) where week 1 is
+    the first week in month <[m]> with day <[d]>; week 5 is the last
+    week in the month; day 0 is Sunday
+
+<[time1]> is the optional local time that alternate time starts, in
+the same format as <[offset1]> without any sign, and defaults
+to 02:00:00
+
+<[end]> is the date in the year that alternate time ends, in the same
+forms as <[start]>
+
+<[time2]> is the optional local time that alternate time ends, in
+the same format as <[offset1]> without any sign, and
+defaults to 02:00:00
+
+Note that there is no white-space padding between fields. Also note that
+if <[TZ]> is null, the default is Universal Time which has no daylight saving
+time. If <[TZ]> is empty, the default EST5EDT is used.
 
 The function <<_tzset_r>> is identical to <<tzset>> only it is reentrant
 and is used for applications that use multiple threads.

[PATCH 2/2] tzset_r.c(_tzset_unlocked_r): POSIX angle bracket <> support

[Brian Inglis]

[-- Attachment #1: Type: text/plain, Size: 403 bytes --]


local defines for POSIX minimum TZ abbr size 3 TZNAME_MIN and
maximum TZ abbr size supported 10 TZNAME_MAX
allow POSIX angle bracket < > quoted signed alphanumeric tz abbr e.g. <MESZ+0330>
allow POSIX unquoted alphabetic tz abbr e.g. MESZ
allow same suuport for DST tz abbr
---
 newlib/libc/time/tzset_r.c | 67 +++++++++++++++++++++++++++++++-------
 1 file changed, 55 insertions(+), 12 deletions(-)


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0002-tzset_r.c-_tzset_unlocked_r-POSIX-angle-bracket-support.patch --]
[-- Type: text/x-patch; name="0002-tzset_r.c-_tzset_unlocked_r-POSIX-angle-bracket-support.patch", Size: 3041 bytes --]

diff --git a/newlib/libc/time/tzset_r.c b/newlib/libc/time/tzset_r.c
index 9e0cf834bd6b..bb64d21b0543 100644
--- a/newlib/libc/time/tzset_r.c
+++ b/newlib/libc/time/tzset_r.c
@@ -1,16 +1,21 @@
 #include <_ansi.h>
+#include <limits.h>	/* {,_POSIX_}TZNAME_MAX */
 #include <reent.h>
 #include <stdio.h>
 #include <stdlib.h>
 #include <string.h>
+#include <unistd.h>	/* sysconf(_SC_TZNAME_MAX) */
 #include <sys/types.h>
 #include <sys/time.h>
 #include "local.h"
 
 #define sscanf siscanf	/* avoid to pull in FP functions. */
 
-static char __tzname_std[11];
-static char __tzname_dst[11];
+#define TZNAME_MIN	3	/* POSIX min TZ abbr size local def */
+#define TZNAME_MAX	10	/* POSIX max TZ abbr size local def */
+
+static char __tzname_std[TZNAME_MAX + 1];
+static char __tzname_dst[TZNAME_MAX + 1];
 static char *prev_tzenv = NULL;
 
 void
@@ -45,8 +50,25 @@ _tzset_unlocked_r (struct _reent *reent_ptr)
   if (*tzenv == ':')
     ++tzenv;  
 
-  if (sscanf (tzenv, "%10[^0-9,+-]%n", __tzname_std, &n) <= 0)
-    return;
+  /* allow POSIX angle bracket < > quoted signed alphanumeric tz abbr e.g. <MESZ+0330> */
+  if (*tzenv == '<')
+    {
+      ++tzenv;
+
+      /* quit if no items, too few or too many chars, or no close quote '>' */
+      if (sscanf (tzenv, "%10[-+0-9A-Za-z]%n", __tzname_std, &n) <= 0
+		|| n < TZNAME_MIN || TZNAME_MAX < n || '>' != tzenv[n])
+        return;
+
+      ++tzenv;	/* bump for close quote '>' */
+    }
+  else
+    {
+      /* allow POSIX unquoted alphabetic tz abbr e.g. MESZ */
+      if (sscanf (tzenv, "%10[A-Za-z]%n", __tzname_std, &n) <= 0
+				|| n < TZNAME_MIN || TZNAME_MAX < n)
+        return;
+    }
  
   tzenv += n;
 
@@ -68,17 +90,38 @@ _tzset_unlocked_r (struct _reent *reent_ptr)
   tz->__tzrule[0].offset = sign * (ss + SECSPERMIN * mm + SECSPERHOUR * hh);
   _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;
+
+  /* allow POSIX angle bracket < > quoted signed alphanumeric tz abbr e.g. <MESZ+0330> */
+  if (*tzenv == '<')
+    {
+      ++tzenv;
+
+      /* quit if no items, too few or too many chars, or no close quote '>' */
+      if (sscanf (tzenv, "%10[-+0-9A-Za-z]%n", __tzname_dst, &n) <= 0
+		|| n < TZNAME_MIN || TZNAME_MAX < n || '>' != tzenv[n])
+	{ /* No dst */
+	  _tzname[1] = _tzname[0];
+	  _timezone = tz->__tzrule[0].offset;
+	  _daylight = 0;
+	  return;
+	}
+
+      ++tzenv;	/* bump for close quote '>' */
     }
   else
-    _tzname[1] = __tzname_dst;
+    {
+      /* allow POSIX unquoted alphabetic tz abbr e.g. MESZ */
+      if (sscanf (tzenv, "%10[A-Za-z]%n", __tzname_dst, &n) <= 0
+				|| n < TZNAME_MIN || TZNAME_MAX < n)
+	{ /* No dst */
+	  _tzname[1] = _tzname[0];
+	  _timezone = tz->__tzrule[0].offset;
+	  _daylight = 0;
+	  return;
+	}
+    }
 
+  _tzname[1] = __tzname_dst;
   tzenv += n;
 
   /* otherwise we have a dst name, look for the offset */

Re: [PATCH 2/2] tzset_r.c(_tzset_unlocked_r): POSIX angle bracket <> support

[jdoubleu]

Hi Brian

> +#include <limits.h>	/* {,_POSIX_}TZNAME_MAX */
> ... > +#define TZNAME_MIN	3	/* POSIX min TZ abbr size local def */
> +#define TZNAME_MAX	10	/* POSIX max TZ abbr size local def */

The comment suggests, that `TZNAME_MAX` is coming from `limits.h`, isn't it?

Everything else looks good to me.

Unfortunately, I wasn't able to test these changes, yet. I cannot easily 
build newlib for my project. However, I've prepared some tests vectors 
(see 
https://github.com/jdoubleu/newlib-posix-tzset-tests/blob/main/timezones.h), 
which I plan to integrate with the newlib testsuite.

Cheers
---
🙎🏻‍♂️ jdoubleu
On 4/5/2022 6:03 AM, Brian Inglis wrote:
> 
> local defines for POSIX minimum TZ abbr size 3 TZNAME_MIN and
> maximum TZ abbr size supported 10 TZNAME_MAX
> allow POSIX angle bracket < > quoted signed alphanumeric tz abbr e.g. <MESZ+0330>
> allow POSIX unquoted alphabetic tz abbr e.g. MESZ
> allow same suuport for DST tz abbr
> ---
>   newlib/libc/time/tzset_r.c | 67 +++++++++++++++++++++++++++++++-------
>   1 file changed, 55 insertions(+), 12 deletions(-)
> 

Re: [PATCH 2/2] tzset_r.c(_tzset_unlocked_r): POSIX angle bracket <> support

[Brian Inglis]

On 2022-04-07 03:56, jdoubleu wrote:
>> +#include <limits.h>    /* {,_POSIX_}TZNAME_MAX */
>> ... > +#define TZNAME_MIN    3    /* POSIX min TZ abbr size local def */
>> +#define TZNAME_MAX    10    /* POSIX max TZ abbr size local def */
> 
> The comment suggests, that `TZNAME_MAX` is coming from `limits.h`, isn't 
> it?

 From previous comments, we are no longer using those predefined values, 
just those used locally in the code, and the minimum from the POSIX spec.

> Everything else looks good to me.
> Unfortunately, I wasn't able to test these changes, yet. I cannot easily 
> build newlib for my project. However, I've prepared some tests vectors 
> (see 
> https://github.com/jdoubleu/newlib-posix-tzset-tests/blob/main/timezones.h), 
> which I plan to integrate with the newlib testsuite.

Feel free to look at tzset_t.log attached to my ...testing post:

	tzset/_r POSIX tz abbr angle bracket <> support testing
	https://sourceware.org/pipermail/newlib/2022/019529.html

and add the 103 unique TZ=... values used there, extracted from the end 
of the latest tzdb tzdata /usr/share/zoneinfo/ files, as they include 
actual exceptional times and offsets to achieve the required transitions 
and challenge implementation parsers and calculations.
See the tzcode/tzdata project zic(8)/zdump(8)/tzfile(5) implementation 
limits for how wide those limits need to extend.

> 🙎🏻‍♂️ jdoubleu
> On 4/5/2022 6:03 AM, Brian Inglis wrote:
>>
>> local defines for POSIX minimum TZ abbr size 3 TZNAME_MIN and
>> maximum TZ abbr size supported 10 TZNAME_MAX
>> allow POSIX angle bracket < > quoted signed alphanumeric tz abbr e.g. 
>> <MESZ+0330>
>> allow POSIX unquoted alphabetic tz abbr e.g. MESZ
>> allow same suuport for DST tz abbr
>> ---
>>   newlib/libc/time/tzset_r.c | 67 +++++++++++++++++++++++++++++++-------
>>   1 file changed, 55 insertions(+), 12 deletions(-)

-- 
Take care. Thanks, Brian Inglis, Calgary, Alberta, Canada

This email may be disturbing to some readers as it contains
too much technical detail. Reader discretion is advised.
[Data in binary units and prefixes, physical quantities in SI.]

Re: [PATCH 2/2] tzset_r.c(_tzset_unlocked_r): POSIX angle bracket <> support

[jdoubleu]

> [..] we are no longer using those predefined values, just those used locally in the code

Yes, got that. The comment just confused me. Can it be removed?

> Feel free to look at tzset_t.log attached to my ...testing post

Great work! Should we actually generate these values with glibc and 
check whether they match?

In the meantime, I've also created some test vectors:

	[PATCH] add tests for tzset(3)
	https://sourceware.org/pipermail/newlib/2022/019536.html

All tests pass so far.


Cheers
---
🙎🏻‍♂️ jdoubleu
On 4/7/2022 6:07 PM, Brian Inglis wrote:
> On 2022-04-07 03:56, jdoubleu wrote:
>>> +#include <limits.h>    /* {,_POSIX_}TZNAME_MAX */
>>> ... > +#define TZNAME_MIN    3    /* POSIX min TZ abbr size local def */
>>> +#define TZNAME_MAX    10    /* POSIX max TZ abbr size local def */
>>
>> The comment suggests, that `TZNAME_MAX` is coming from `limits.h`, 
>> isn't it?
> 
>  From previous comments, we are no longer using those predefined values, 
> just those used locally in the code, and the minimum from the POSIX spec.
> 
>> Everything else looks good to me.
>> Unfortunately, I wasn't able to test these changes, yet. I cannot 
>> easily build newlib for my project. However, I've prepared some tests 
>> vectors (see 
>> https://github.com/jdoubleu/newlib-posix-tzset-tests/blob/main/timezones.h), 
>> which I plan to integrate with the newlib testsuite.
> 
> Feel free to look at tzset_t.log attached to my ...testing post:
> 
>      tzset/_r POSIX tz abbr angle bracket <> support testing
>      https://sourceware.org/pipermail/newlib/2022/019529.html
> 
> and add the 103 unique TZ=... values used there, extracted from the end 
> of the latest tzdb tzdata /usr/share/zoneinfo/ files, as they include 
> actual exceptional times and offsets to achieve the required transitions 
> and challenge implementation parsers and calculations.
> See the tzcode/tzdata project zic(8)/zdump(8)/tzfile(5) implementation 
> limits for how wide those limits need to extend.
> 
>> 🙎🏻‍♂️ jdoubleu
>> On 4/5/2022 6:03 AM, Brian Inglis wrote:
>>>
>>> local defines for POSIX minimum TZ abbr size 3 TZNAME_MIN and
>>> maximum TZ abbr size supported 10 TZNAME_MAX
>>> allow POSIX angle bracket < > quoted signed alphanumeric tz abbr e.g. 
>>> <MESZ+0330>
>>> allow POSIX unquoted alphabetic tz abbr e.g. MESZ
>>> allow same suuport for DST tz abbr
>>> ---
>>>   newlib/libc/time/tzset_r.c | 67 +++++++++++++++++++++++++++++++-------
>>>   1 file changed, 55 insertions(+), 12 deletions(-)
> 

Re: [PATCH 2/2] tzset_r.c(_tzset_unlocked_r): POSIX angle bracket <> support

[jdoubleu]

Hi,

> +      if (sscanf (tzenv, "%10[-+0-9A-Za-z]%n", __tzname_std, &n) <= 0
> +		|| n < TZNAME_MIN || TZNAME_MAX < n || '>' != tzenv[n])

during testing I noticed, that the condition `TZNAME_MAX < n` can never 
be true. The format to sscanf explicitly parses 10 characters at maximum.

Longer input strings will be truncated. That is also the case with the 
glibc implementation.


Cheers
---
🙎🏻‍♂️ jdoubleu
On 4/5/2022 6:03 AM, Brian Inglis wrote:
> 
> local defines for POSIX minimum TZ abbr size 3 TZNAME_MIN and
> maximum TZ abbr size supported 10 TZNAME_MAX
> allow POSIX angle bracket < > quoted signed alphanumeric tz abbr e.g. <MESZ+0330>
> allow POSIX unquoted alphabetic tz abbr e.g. MESZ
> allow same suuport for DST tz abbr
> ---
>   newlib/libc/time/tzset_r.c | 67 +++++++++++++++++++++++++++++++-------
>   1 file changed, 55 insertions(+), 12 deletions(-)
> 

Re: [PATCH 2/2] tzset_r.c(_tzset_unlocked_r): POSIX angle bracket <> support

[Jeff Johnston]

Hi Brian,

Looks good.  The only issue I have are the added includes (limits.h and
unistd.h) which don't appear to be needed.  Is there a reason for this?

-- Jeff J.

On Tue, Apr 5, 2022 at 12:04 AM Brian Inglis <
Brian.Inglis@systematicsw.ab.ca> wrote:

>
> local defines for POSIX minimum TZ abbr size 3 TZNAME_MIN and
> maximum TZ abbr size supported 10 TZNAME_MAX
> allow POSIX angle bracket < > quoted signed alphanumeric tz abbr e.g.
> <MESZ+0330>
> allow POSIX unquoted alphabetic tz abbr e.g. MESZ
> allow same suuport for DST tz abbr
> ---
>  newlib/libc/time/tzset_r.c | 67 +++++++++++++++++++++++++++++++-------
>  1 file changed, 55 insertions(+), 12 deletions(-)
>
>

Re: [PATCH 2/2] tzset_r.c(_tzset_unlocked_r): POSIX angle bracket <> support

[Brian Inglis]

Forgot they had been added for earlier iteration.
Removed in v2 revision submitted.

On 2022-04-07 15:35, Jeff Johnston wrote:
> Hi Brian,
> 
> Looks good.  The only issue I have are the added includes (limits.h and 
> unistd.h) which don't appear to be needed.  Is there a reason for this?
> 
> -- Jeff J.
> 
> On Tue, Apr 5, 2022 at 12:04 AM Brian Inglis 
> <Brian.Inglis@systematicsw.ab.ca 
> <mailto:Brian.Inglis@systematicsw.ab.ca>> wrote:
> 
> 
>     local defines for POSIX minimum TZ abbr size 3 TZNAME_MIN and
>     maximum TZ abbr size supported 10 TZNAME_MAX
>     allow POSIX angle bracket < > quoted signed alphanumeric tz abbr
>     e.g. <MESZ+0330>
>     allow POSIX unquoted alphabetic tz abbr e.g. MESZ
>     allow same suuport for DST tz abbr
>     ---
>       newlib/libc/time/tzset_r.c | 67 +++++++++++++++++++++++++++++++-------
>       1 file changed, 55 insertions(+), 12 deletions(-)
> 


-- 
Take care. Thanks, Brian Inglis, Calgary, Alberta, Canada

This email may be disturbing to some readers as it contains
too much technical detail. Reader discretion is advised.
[Data in binary units and prefixes, physical quantities in SI.]