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

[Corinna Vinschen <vinschen@redhat.com>]

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