|
|
Message-ID: <20260323151509.GT3520958@port70.net>
Date: Mon, 23 Mar 2026 16:15:09 +0100
From: Szabolcs Nagy <nsz@...t70.net>
To: Rich Felker <dalias@...c.org>
Cc: Hannu Nyman <hannu.nyman@....fi>, musl@...ts.openwall.com
Subject: Re: strptime in 1.2.6 - is tzname[0/1] guaranteed to be set?
* Rich Felker <dalias@...c.org> [2026-03-23 11:04:37 -0400]:
> On Mon, Mar 23, 2026 at 11:00:02AM -0400, Rich Felker wrote:
> > On Mon, Mar 23, 2026 at 08:19:43AM +0100, Szabolcs Nagy wrote:
> > > * Rich Felker <dalias@...c.org> [2026-03-22 21:39:01 -0400]:
> > > > char *strptime(const char *restrict s, const char *restrict f, struct tm *restrict tm)
> > > > {
> > > > int i, w, neg, adj, min, range, *dest, dummy;
> > > > - const char *ex;
> > > > + const char *ex, *s1;
> > > > size_t len;
> > > > int want_century = 0, century = 0, relyear = 0;
> > > > while (*f) {
> > > > @@ -207,16 +208,10 @@ char *strptime(const char *restrict s, const char *restrict f, struct tm *restri
> > > > s += 5;
> > > > break;
> > > > case 'Z':
> > > > - if (!strncmp(s, tzname[0], len = strlen(tzname[0]))) {
> > > > - tm->tm_isdst = 0;
> > > > - s += len;
> > > > - } else if (!strncmp(s, tzname[1], len=strlen(tzname[1]))) {
> > > > - tm->tm_isdst = 1;
> > > > - s += len;
> > > > - } else {
> > > > - /* FIXME: is this supposed to be an error? */
> > > > - while ((*s|32)-'a' <= 'z'-'a') s++;
> > > > - }
> > > > + s1 = s;
> > > > + i = __tzname_to_isdst(&s1);
> > > > + if (i>=0) tm->tm_isdst = i;
> > > > + s = s1;
> > > > break;
> > >
> > > what is the point of
> > >
> > > s1 = s;
> > > foo(&s1);
> > > s = s1;
> > >
> > > seems equivalent to
> > >
> > > foo(&s);
> >
> > If you try that you'll see why it fails. But I think C admits a clean
> > version that works..
>
> Yep, the attached is good. I thought it would be bad to put a
> requirement that the caller's pointer be restrict-qualified into the
> signature of __tzname_to_isdst, but C allows the pointed-to type to be
> more qualified than the addressed object, so passing a pointer to a
> const char * would still be valid even with this change.
...
> +int __tzname_to_isdst(const char *restrict *s)
ah, restrict is ugly, but it is what it is.
> +{
> + size_t len;
> + int isdst = -1;
> + LOCK(lock);
> + if (tzname[0] && !strncmp(*s, tzname[0], len = strlen(tzname[0]))) {
> + isdst = 0;
> + s += len;
i'd expect
s += len
to become
*s += len
after the code move.
> + } else if (tzname[1] && !strncmp(*s, tzname[1], len=strlen(tzname[1]))) {
> + isdst = 1;
> + s += len;
likewise.
> + } else {
> + /* FIXME: is this supposed to be an error? */
> + while ((**s|32)-'a' <= 'z'-'a') ++*s;
> + }
> + UNLOCK(lock);
> + return isdst;
> +}
> diff --git a/src/time/strptime.c b/src/time/strptime.c
> index b1147242..40bb37af 100644
> --- a/src/time/strptime.c
> +++ b/src/time/strptime.c
> @@ -5,6 +5,7 @@
> #include <stddef.h>
> #include <string.h>
> #include <strings.h>
> +#include "time_impl.h"
>
> char *strptime(const char *restrict s, const char *restrict f, struct tm *restrict tm)
> {
> @@ -207,16 +208,8 @@ char *strptime(const char *restrict s, const char *restrict f, struct tm *restri
> s += 5;
> break;
> case 'Z':
> - if (!strncmp(s, tzname[0], len = strlen(tzname[0]))) {
> - tm->tm_isdst = 0;
> - s += len;
> - } else if (!strncmp(s, tzname[1], len=strlen(tzname[1]))) {
> - tm->tm_isdst = 1;
> - s += len;
> - } else {
> - /* FIXME: is this supposed to be an error? */
> - while ((*s|32)-'a' <= 'z'-'a') s++;
> - }
> + i = __tzname_to_isdst(&s);
> + if (i>=0) tm->tm_isdst = i;
> break;
> case '%':
> if (*s++ != '%') return 0;
> diff --git a/src/time/time_impl.h b/src/time/time_impl.h
> index f26d8005..ffe5050b 100644
> --- a/src/time/time_impl.h
> +++ b/src/time/time_impl.h
> @@ -5,6 +5,7 @@ hidden int __month_to_secs(int, int);
> hidden long long __year_to_secs(long long, int *);
> hidden long long __tm_to_secs(const struct tm *);
> hidden const char *__tm_to_tzname(const struct tm *);
> +hidden int __tzname_to_isdst(const char *restrict *);
> hidden int __secs_to_tm(long long, struct tm *);
> hidden void __secs_to_zone(long long, int, int *, long *, long *, const char **);
> hidden const char *__strftime_fmt_1(char (*)[100], size_t *, int, const struct tm *, locale_t, int);
> --
> 2.21.0
>
Powered by blists - more mailing lists
Confused about mailing lists and their use? Read about mailing lists on Wikipedia and check out these guidelines on proper formatting of your messages.