public inbox for newlib@sourceware.org
 help / color / mirror / Atom feed
From: Brian Inglis <Brian.Inglis@SystematicSw.ab.ca>
To: newlib@sourceware.org
Subject: Re: [PATCH 2/2] newlib/libc/time/tzset_r.c(_tzset_unlocked_r): POSIX angle bracket <> support
Date: Mon, 28 Feb 2022 11:55:57 -0700	[thread overview]
Message-ID: <d71a5b05-531f-8028-7b06-6ee466053f5f@SystematicSw.ab.ca> (raw)
In-Reply-To: <Yhy6OKd/2o8VqIUH@calimero.vinschen.de>

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

  reply	other threads:[~2022-02-28 18:55 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
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 [this message]
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

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=d71a5b05-531f-8028-7b06-6ee466053f5f@SystematicSw.ab.ca \
    --to=brian.inglis@systematicsw.ab.ca \
    --cc=newlib@sourceware.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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).