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

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

[Brian Inglis]

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

Brian Inglis (2):
  newlib/libc/time/tzset.c: add 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 |  74 +++++++++++++++++++++----
 2 files changed, 143 insertions(+), 41 deletions(-)

-- 
2.35.1

[PATCH 1/2] newlib/libc/time/tzset.c: add POSIX angle bracket <> doc

[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-newlib-libc-time-tzset.c-add-POSIX-angle-bracket-sup.patch --]
[-- Type: text/x-patch; name="0001-newlib-libc-time-tzset.c-add-POSIX-angle-bracket-sup.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] newlib/libc/time/tzset_r.c(_tzset_unlocked_r): POSIX angle bracket <> support

[Brian Inglis]

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


define POSIX specified minimum TZ abbr size 3 TZNAME_MIN
use limits.h TZNAME_MAX, _POSIX_TZNAME_MAX, unistd.h sysconf(_SC_TZNAME_MAX)
issue error if no symbols defined (document fallback value in case required)
allow POSIX angle bracket < > quoted signed alphanumeric tz abbr e.g. <MESZ+0330>
allow POSIX unquoted alphabetic tz abbr e.g. MESZ
apply same changes for DST tz abbr
---
 newlib/libc/time/tzset_r.c | 74 ++++++++++++++++++++++++++++++++------
 1 file changed, 64 insertions(+), 10 deletions(-)


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0002-newlib-libc-time-tzset_r.c-_tzset_unlocked_r-POSIX-a.patch --]
[-- Type: text/x-patch; name="0002-newlib-libc-time-tzset_r.c-_tzset_unlocked_r-POSIX-a.patch", Size: 3449 bytes --]

diff --git a/newlib/libc/time/tzset_r.c b/newlib/libc/time/tzset_r.c
index 9e0cf834bd6b..6a5fd578c0be 100644
--- a/newlib/libc/time/tzset_r.c
+++ b/newlib/libc/time/tzset_r.c
@@ -1,14 +1,30 @@
 #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. */
 
+#define TZNAME_MIN	3	/* POSIX specified minimum TZ abbr size */
+/* TZNAME_MAX - POSIX specified maximum TZ abbr size */
+/* define TZNAME_MAX if undefined and available */
+#if	!defined(TZNAME_MAX)
+#if	 defined(_POSIX_TZNAME_MAX)
+#define TZNAME_MAX	_POSIX_TZNAME_MAX	/* use POSIX value */
+#elif	 defined(_SC_TZNAME_MAX)
+#define TZNAME_MAX	sysconf(_SC_TZNAME_MAX)	/* use sysconf value */
+#else
+#error	"None of TZNAME_MAX, _POSIX_TZNAME_MAX, _SC_TZNAME_MAX are defined"
+#define TZNAME_MAX	9			/* could use fallback value */
+#endif	/* defined _POSIX_TZNAME_MAX || _SC_TZNAME_MAX */
+#endif	/* !defined(TZNAME_MAX) */
+
 static char __tzname_std[11];
 static char __tzname_dst[11];
 static char *prev_tzenv = NULL;
@@ -45,8 +61,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 +101,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] newlib/libc/time/tzset_r.c(_tzset_unlocked_r): POSIX angle bracket <> support

[Corinna Vinschen]

Hi Brian,

On Feb 25 09:39, Brian Inglis wrote:
> 
> define POSIX specified minimum TZ abbr size 3 TZNAME_MIN
> use limits.h TZNAME_MAX, _POSIX_TZNAME_MAX, unistd.h sysconf(_SC_TZNAME_MAX)
> issue error if no symbols defined (document fallback value in case required)
> allow POSIX angle bracket < > quoted signed alphanumeric tz abbr e.g. <MESZ+0330>
> allow POSIX unquoted alphabetic tz abbr e.g. MESZ
> apply same changes for DST tz abbr
> ---
>  newlib/libc/time/tzset_r.c | 74 ++++++++++++++++++++++++++++++++------
>  1 file changed, 64 insertions(+), 10 deletions(-)
> 

> diff --git a/newlib/libc/time/tzset_r.c b/newlib/libc/time/tzset_r.c
> index 9e0cf834bd6b..6a5fd578c0be 100644
> --- a/newlib/libc/time/tzset_r.c
> +++ b/newlib/libc/time/tzset_r.c
> @@ -1,14 +1,30 @@
>  #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. */
>  
> +#define TZNAME_MIN	3	/* POSIX specified minimum TZ abbr size */

Are you sure?

> +/* TZNAME_MAX - POSIX specified maximum TZ abbr size */
> +/* define TZNAME_MAX if undefined and available */
> +#if	!defined(TZNAME_MAX)
> +#if	 defined(_POSIX_TZNAME_MAX)
> +#define TZNAME_MAX	_POSIX_TZNAME_MAX	/* use POSIX value */
> +#elif	 defined(_SC_TZNAME_MAX)
> +#define TZNAME_MAX	sysconf(_SC_TZNAME_MAX)	/* use sysconf value */

This is not a safe bet.  _SC_TZNAME_MAX is defined in unistd.h
unconditionally, even for targets not providing sysconf().  And
given that _POSIX_TZNAME_MAX is only defined for RTEMS and Cygwin,
this will run into problems on most other targets.

Only Phoenix-RTOS, RISCV, RTEMS and Cygwin support sysconf, and none
of them actually supports _SC_TZNAME_MAX as parameter.  And even Cygwin
returns -1 and sets errno to EINVAL.

Given you're checking _POSIX_TZNAME_MAX first, which is defined on
Cygwin, the sysconf path will never be used there anyway.  But that's
actually a minor point.

The real problem is, you can't use sysconf(_SC_TZNAME_MAX) like this:
- You don't know at compile time if the function is really supported.
- You don't know if _SC_TZNAME_MAX returns a positive value or -1.

And then again, there's no good reason to check against _POSIX_TZNAME_MAX
either.  _POSIX_TZNAME_MAX is a system-independent definition, defining
a *minimum* value which has to be supported by the OS.  So any OS can
define TZNAME_MAX as _POSIX_TZNAME_MAX, or any bigger value, or not at
all, if there just is no limit.

So, either the system defines TZNAME_MAX, or the system provides
sysconf() *and* sysconf(_SC_TZNAME_MAX) returns a positive value.  In
these cases, a check against that value makes sense.  In all other
cases, there's just no limit to check for.


Corinna

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

[Brian Inglis]

On 2022-02-28 05:04, Corinna Vinschen wrote:
> Hi Brian,
> 
> On Feb 25 09:39, Brian Inglis wrote:
>>
>> define POSIX specified minimum TZ abbr size 3 TZNAME_MIN
>> use limits.h TZNAME_MAX, _POSIX_TZNAME_MAX, unistd.h sysconf(_SC_TZNAME_MAX)
>> issue error if no symbols defined (document fallback value in case required)
>> allow POSIX angle bracket < > quoted signed alphanumeric tz abbr e.g. <MESZ+0330>
>> allow POSIX unquoted alphabetic tz abbr e.g. MESZ
>> apply same changes for DST tz abbr
>> ---
>>   newlib/libc/time/tzset_r.c | 74 ++++++++++++++++++++++++++++++++------
>>   1 file changed, 64 insertions(+), 10 deletions(-)
>>
> 
>> diff --git a/newlib/libc/time/tzset_r.c b/newlib/libc/time/tzset_r.c
>> index 9e0cf834bd6b..6a5fd578c0be 100644
>> --- a/newlib/libc/time/tzset_r.c
>> +++ b/newlib/libc/time/tzset_r.c
>> @@ -1,14 +1,30 @@
>>   #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. */
>>   
>> +#define TZNAME_MIN	3	/* POSIX specified minimum TZ abbr size */
> 
> Are you sure?

Minimum is historical.

>> +/* TZNAME_MAX - POSIX specified maximum TZ abbr size */
>> +/* define TZNAME_MAX if undefined and available */
>> +#if	!defined(TZNAME_MAX)
>> +#if	 defined(_POSIX_TZNAME_MAX)
>> +#define TZNAME_MAX	_POSIX_TZNAME_MAX	/* use POSIX value */
>> +#elif	 defined(_SC_TZNAME_MAX)
>> +#define TZNAME_MAX	sysconf(_SC_TZNAME_MAX)	/* use sysconf value */
> 
> This is not a safe bet.  _SC_TZNAME_MAX is defined in unistd.h
> unconditionally, even for targets not providing sysconf().  And

Why define indices if they are unusable on that platform?

> given that _POSIX_TZNAME_MAX is only defined for RTEMS and Cygwin,
> this will run into problems on most other targets.
> 
> Only Phoenix-RTOS, RISCV, RTEMS and Cygwin support sysconf, and none
> of them actually supports _SC_TZNAME_MAX as parameter.  And even Cygwin
> returns -1 and sets errno to EINVAL.
> 
> Given you're checking _POSIX_TZNAME_MAX first, which is defined on
> Cygwin, the sysconf path will never be used there anyway.  But that's
> actually a minor point.
> 
> The real problem is, you can't use sysconf(_SC_TZNAME_MAX) like this:
> - You don't know at compile time if the function is really supported.
> - You don't know if _SC_TZNAME_MAX returns a positive value or -1.

The docs appeared unclear whether the index or return value could be -1.
POSIX sysconf(3p) man page says -1/EINVAL means invalid argument value 
which should not apply if _SC_TZNAME_MAX is defined, otherwise -1/errno 
is unchanged means no definite limit?!
I can just enclose it in HAVE_SYSCONF, else define a macro returning -1, 
  and leave it at that for our purposes, while allowing for any platform 
that decides to support it.

> And then again, there's no good reason to check against _POSIX_TZNAME_MAX
> either.  _POSIX_TZNAME_MAX is a system-independent definition, defining
> a *minimum* value which has to be supported by the OS.  So any OS can
> define TZNAME_MAX as _POSIX_TZNAME_MAX, or any bigger value, or not at
> all, if there just is no limit.
> 
> So, either the system defines TZNAME_MAX, or the system provides
> sysconf() *and* sysconf(_SC_TZNAME_MAX) returns a positive value.  In
> these cases, a check against that value makes sense.  In all other
> cases, there's just no limit to check for.

So I may as well default to using the hardwired limit of 10, which is 
the most sensible value allowing for five letters, sign, four digits.

-- 
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] newlib/libc/time/tzset_r.c(_tzset_unlocked_r): POSIX angle bracket <> support

[Brian Inglis]

On 2022-02-28 11:55, Brian Inglis wrote:
> On 2022-02-28 05:04, Corinna Vinschen wrote:
>> Hi Brian,
>>
>> On Feb 25 09:39, Brian Inglis wrote:
>>>
>>> define POSIX specified minimum TZ abbr size 3 TZNAME_MIN
>>> use limits.h TZNAME_MAX, _POSIX_TZNAME_MAX, unistd.h 
>>> sysconf(_SC_TZNAME_MAX)
>>> issue error if no symbols defined (document fallback value in case 
>>> required)
>>> allow POSIX angle bracket < > quoted signed alphanumeric tz abbr e.g. 
>>> <MESZ+0330>
>>> allow POSIX unquoted alphabetic tz abbr e.g. MESZ
>>> apply same changes for DST tz abbr
>>> ---
>>>   newlib/libc/time/tzset_r.c | 74 ++++++++++++++++++++++++++++++++------
>>>   1 file changed, 64 insertions(+), 10 deletions(-)
>>>
>>
>>> diff --git a/newlib/libc/time/tzset_r.c b/newlib/libc/time/tzset_r.c
>>> index 9e0cf834bd6b..6a5fd578c0be 100644
>>> --- a/newlib/libc/time/tzset_r.c
>>> +++ b/newlib/libc/time/tzset_r.c
>>> @@ -1,14 +1,30 @@
>>>   #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. */
>>> +#define TZNAME_MIN    3    /* POSIX specified minimum TZ abbr size */
>>
>> Are you sure?
> 
> Minimum is historical.
> 
>>> +/* TZNAME_MAX - POSIX specified maximum TZ abbr size */
>>> +/* define TZNAME_MAX if undefined and available */
>>> +#if    !defined(TZNAME_MAX)
>>> +#if     defined(_POSIX_TZNAME_MAX)
>>> +#define TZNAME_MAX    _POSIX_TZNAME_MAX    /* use POSIX value */
>>> +#elif     defined(_SC_TZNAME_MAX)
>>> +#define TZNAME_MAX    sysconf(_SC_TZNAME_MAX)    /* use sysconf 
>>> value */
>>
>> This is not a safe bet.  _SC_TZNAME_MAX is defined in unistd.h
>> unconditionally, even for targets not providing sysconf().  And
> 
> Why define indices if they are unusable on that platform?
> 
>> given that _POSIX_TZNAME_MAX is only defined for RTEMS and Cygwin,
>> this will run into problems on most other targets.
>>
>> Only Phoenix-RTOS, RISCV, RTEMS and Cygwin support sysconf, and none
>> of them actually supports _SC_TZNAME_MAX as parameter.  And even Cygwin
>> returns -1 and sets errno to EINVAL.
>>
>> Given you're checking _POSIX_TZNAME_MAX first, which is defined on
>> Cygwin, the sysconf path will never be used there anyway.  But that's
>> actually a minor point.
>>
>> The real problem is, you can't use sysconf(_SC_TZNAME_MAX) like this:
>> - You don't know at compile time if the function is really supported.
>> - You don't know if _SC_TZNAME_MAX returns a positive value or -1.
> 
> The docs appeared unclear whether the index or return value could be -1.
> POSIX sysconf(3p) man page says -1/EINVAL means invalid argument value 
> which should not apply if _SC_TZNAME_MAX is defined, otherwise -1/errno 
> is unchanged means no definite limit?!
> I can just enclose it in HAVE_SYSCONF, else define a macro returning -1, 
>   and leave it at that for our purposes, while allowing for any platform 
> that decides to support it.

Looks like I should define _SC_TZNAME_MAX == TZNAME_MAX and 
_SC_MONOTONIC_CLOCK == _POSIX_MONOTONIC_CLOCK in sysconf.cc?

-- 
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] newlib/libc/time/tzset_r.c(_tzset_unlocked_r): POSIX angle bracket <> support

[Corinna Vinschen]

On Feb 28 11:55, Brian Inglis wrote:
> On 2022-02-28 05:04, Corinna Vinschen wrote:
> > On Feb 25 09:39, Brian Inglis wrote:
> > > +#define TZNAME_MIN	3	/* POSIX specified minimum TZ abbr size */
> > Are you sure?
> 
> Minimum is historical.

It's no POSIX value, so the comment is misleading.  The value of 3
is fine, after all it's part of the POSIX description for TZ.

> > > +#elif	 defined(_SC_TZNAME_MAX)
> > > +#define TZNAME_MAX	sysconf(_SC_TZNAME_MAX)	/* use sysconf value */
> > 
> > This is not a safe bet.  _SC_TZNAME_MAX is defined in unistd.h
> > unconditionally, even for targets not providing sysconf().  And
> 
> Why define indices if they are unusable on that platform?

They are never unusable!  They have to be defined as minimum set of
index values requested by POSIX.  A platform can easily return -1 for
all of them to indicate it doesn't care for limits.  Or, by also
setting errno to EINVAL, indicate that a value is entirly unavailable
in the current environment (which may differ on the same system).

> > The real problem is, you can't use sysconf(_SC_TZNAME_MAX) like this:
> > - You don't know at compile time if the function is really supported.
> > - You don't know if _SC_TZNAME_MAX returns a positive value or -1.
> 
> The docs appeared unclear whether the index or return value could be -1.
> POSIX sysconf(3p) man page says -1/EINVAL means invalid argument value which
> should not apply if _SC_TZNAME_MAX is defined, otherwise -1/errno is
> unchanged means no definite limit?!

Right.  The required usage of sysconf goes like this:

  #include <errno.h>
  #include <stdio.h>
  #include <unistd.h>

  errno = 0;
  int ret = sysconf (_SC_foo);
  if (ret != -1)
    printf ("We have a limit %d\n", ret);
  else if (errno)
    printf ("We have an error %d\n", errno);
  else
    printf ("We have no limit\n");

Incidentally, Cygwin sets errno for _SC_TZNAME_MAX, rather than just
returning -1.  I just fixed that.

> I can just enclose it in HAVE_SYSCONF,

Newlib configury does not run this kind of test macros.  After all, they
would only test if that function is available on the build system.  If
the function is available on the target system, it would be defined by
newlib itself, or be part of libgloss.

> > So, either the system defines TZNAME_MAX, or the system provides
> > sysconf() *and* sysconf(_SC_TZNAME_MAX) returns a positive value.  In
> > these cases, a check against that value makes sense.  In all other
> > cases, there's just no limit to check for.
> 
> So I may as well default to using the hardwired limit of 10, which is the
> most sensible value allowing for five letters, sign, four digits.

Yeah, we might keep it at that.  Cygwin isn't affected anyway, since it
runs its own tzset code, see tzparse() in winsup/cygwin/localtime.cc.
So we're doing this mostely for small targets.  10 should be fine.


Thanks,
Corinna

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

[jdoubleu]

Hi,

On 2/25/2022 5:39 PM, Brian Inglis wrote:
> 
> define POSIX specified minimum TZ abbr size 3 TZNAME_MIN
> use limits.h TZNAME_MAX, _POSIX_TZNAME_MAX, unistd.h sysconf(_SC_TZNAME_MAX)
> issue error if no symbols defined (document fallback value in case required)
> allow POSIX angle bracket < > quoted signed alphanumeric tz abbr e.g. <MESZ+0330>
> allow POSIX unquoted alphabetic tz abbr e.g. MESZ
> apply same changes for DST tz abbr
> ---
>   newlib/libc/time/tzset_r.c | 74 ++++++++++++++++++++++++++++++++------
>   1 file changed, 64 insertions(+), 10 deletions(-)
> 

> +      /* 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;

Is it safe to use the minus (-) as first char in the sscanf format set: 
"%10[-+0-9A-Za-z]%n"?

Newlib's sscanf docs states 
(https://sourceware.org/newlib/libc.html#sscanf):
> There is also a range facility which you can use as a shortcut. %[0-9] matches all decimal digits. The hyphen must not be the first or last character in the set.

Cheers
---
🙎🏻‍♂️ jdoubleu

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

[Brian Inglis]

On 2022-03-21 10:07, jdoubleu wrote:
> Hi,
> 
> On 2/25/2022 5:39 PM, Brian Inglis wrote:
>>
>> define POSIX specified minimum TZ abbr size 3 TZNAME_MIN
>> use limits.h TZNAME_MAX, _POSIX_TZNAME_MAX, unistd.h 
>> sysconf(_SC_TZNAME_MAX)
>> issue error if no symbols defined (document fallback value in case 
>> required)
>> allow POSIX angle bracket < > quoted signed alphanumeric tz abbr e.g. 
>> <MESZ+0330>
>> allow POSIX unquoted alphabetic tz abbr e.g. MESZ
>> apply same changes for DST tz abbr
>> ---
>>   newlib/libc/time/tzset_r.c | 74 ++++++++++++++++++++++++++++++++------
>>   1 file changed, 64 insertions(+), 10 deletions(-)
>>
> 
>> +      /* 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;
> 
> Is it safe to use the minus (-) as first char in the sscanf format set: 
> "%10[-+0-9A-Za-z]%n"?

The first and last positions are the only generally safe places to 
specify the hyphen/minus as part of the set and not part of a range.

> Newlib's sscanf docs states 
> (https://sourceware.org/newlib/libc.html#sscanf):
>> There is also a range facility which you can use as a shortcut. %[0-9] 
>> matches all decimal digits. The hyphen must not be the first or last 
>> character in the set.

I read that as saying that the first or last character within the 
brackets can not be the hyphen a la [--@] or [!--], as it then stands 
alone, and is not treated as part of a range, allowing the hyphen/minus 
to be one of the characters matched, as is also the case when the first 
and last characters are not lexically ordered e.g. [z-a] == z|-|a.
I presume this is a lexer limitation to allow safe and efficient range 
and set scanning.

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