* [PATCH 0/2] add tzset POSIX tz abbr angle bracket <> support in TZ env var
@ 2022-02-25 16:39 Brian Inglis
2022-02-25 16:39 ` [PATCH 1/2] newlib/libc/time/tzset.c: add POSIX angle bracket <> doc Brian Inglis
2022-02-25 16:39 ` [PATCH 2/2] newlib/libc/time/tzset_r.c(_tzset_unlocked_r): POSIX angle bracket <> support Brian Inglis
0 siblings, 2 replies; 9+ messages in thread
From: Brian Inglis @ 2022-02-25 16:39 UTC (permalink / raw)
To: newlib
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
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH 1/2] newlib/libc/time/tzset.c: add POSIX angle bracket <> doc
2022-02-25 16:39 [PATCH 0/2] add tzset POSIX tz abbr angle bracket <> support in TZ env var Brian Inglis
@ 2022-02-25 16:39 ` Brian Inglis
2022-02-25 16:39 ` [PATCH 2/2] newlib/libc/time/tzset_r.c(_tzset_unlocked_r): POSIX angle bracket <> support Brian Inglis
1 sibling, 0 replies; 9+ messages in thread
From: Brian Inglis @ 2022-02-25 16:39 UTC (permalink / raw)
To: newlib
[-- 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.
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH 2/2] newlib/libc/time/tzset_r.c(_tzset_unlocked_r): POSIX angle bracket <> support
2022-02-25 16:39 [PATCH 0/2] add tzset POSIX tz abbr angle bracket <> support in TZ env var Brian Inglis
2022-02-25 16:39 ` [PATCH 1/2] newlib/libc/time/tzset.c: add POSIX angle bracket <> doc Brian Inglis
@ 2022-02-25 16:39 ` Brian Inglis
2022-02-28 12:04 ` Corinna Vinschen
2022-03-21 16:07 ` jdoubleu
1 sibling, 2 replies; 9+ messages in thread
From: Brian Inglis @ 2022-02-25 16:39 UTC (permalink / raw)
To: newlib
[-- 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 */
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 2/2] newlib/libc/time/tzset_r.c(_tzset_unlocked_r): POSIX angle bracket <> support
2022-02-25 16:39 ` [PATCH 2/2] newlib/libc/time/tzset_r.c(_tzset_unlocked_r): POSIX angle bracket <> support Brian Inglis
@ 2022-02-28 12:04 ` Corinna Vinschen
2022-02-28 18:55 ` Brian Inglis
2022-03-21 16:07 ` jdoubleu
1 sibling, 1 reply; 9+ messages in thread
From: Corinna Vinschen @ 2022-02-28 12:04 UTC (permalink / raw)
To: newlib
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
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 2/2] newlib/libc/time/tzset_r.c(_tzset_unlocked_r): POSIX angle bracket <> support
2022-02-28 12:04 ` Corinna Vinschen
@ 2022-02-28 18:55 ` Brian Inglis
2022-02-28 19:26 ` Brian Inglis
2022-03-01 11:03 ` Corinna Vinschen
0 siblings, 2 replies; 9+ messages in thread
From: Brian Inglis @ 2022-02-28 18:55 UTC (permalink / raw)
To: newlib
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.]
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 2/2] newlib/libc/time/tzset_r.c(_tzset_unlocked_r): POSIX angle bracket <> support
2022-02-28 18:55 ` Brian Inglis
@ 2022-02-28 19:26 ` Brian Inglis
2022-03-01 11:03 ` Corinna Vinschen
1 sibling, 0 replies; 9+ messages in thread
From: Brian Inglis @ 2022-02-28 19:26 UTC (permalink / raw)
To: newlib
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.]
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 2/2] newlib/libc/time/tzset_r.c(_tzset_unlocked_r): POSIX angle bracket <> support
2022-02-28 18:55 ` Brian Inglis
2022-02-28 19:26 ` Brian Inglis
@ 2022-03-01 11:03 ` Corinna Vinschen
1 sibling, 0 replies; 9+ messages in thread
From: Corinna Vinschen @ 2022-03-01 11:03 UTC (permalink / raw)
To: newlib
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
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 2/2] newlib/libc/time/tzset_r.c(_tzset_unlocked_r): POSIX angle bracket <> support
2022-02-25 16:39 ` [PATCH 2/2] newlib/libc/time/tzset_r.c(_tzset_unlocked_r): POSIX angle bracket <> support Brian Inglis
2022-02-28 12:04 ` Corinna Vinschen
@ 2022-03-21 16:07 ` jdoubleu
2022-03-22 17:35 ` Brian Inglis
1 sibling, 1 reply; 9+ messages in thread
From: jdoubleu @ 2022-03-21 16:07 UTC (permalink / raw)
To: newlib
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
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 2/2] newlib/libc/time/tzset_r.c(_tzset_unlocked_r): POSIX angle bracket <> support
2022-03-21 16:07 ` jdoubleu
@ 2022-03-22 17:35 ` Brian Inglis
0 siblings, 0 replies; 9+ messages in thread
From: Brian Inglis @ 2022-03-22 17:35 UTC (permalink / raw)
To: newlib
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.]
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2022-03-22 17:35 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-02-25 16:39 [PATCH 0/2] add tzset POSIX tz abbr angle bracket <> support in TZ env var Brian Inglis
2022-02-25 16:39 ` [PATCH 1/2] newlib/libc/time/tzset.c: add POSIX angle bracket <> doc Brian Inglis
2022-02-25 16:39 ` [PATCH 2/2] newlib/libc/time/tzset_r.c(_tzset_unlocked_r): POSIX angle bracket <> support Brian Inglis
2022-02-28 12:04 ` Corinna Vinschen
2022-02-28 18:55 ` Brian Inglis
2022-02-28 19:26 ` Brian Inglis
2022-03-01 11:03 ` Corinna Vinschen
2022-03-21 16:07 ` jdoubleu
2022-03-22 17:35 ` Brian Inglis
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).