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

[Brian Inglis <Brian.Inglis@SystematicSw.ab.ca>]

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