My position is we should delete support.
Hiltjo Posthuma <[email protected]> wrote:
> On Sun, Feb 24, 2019 at 01:11:39PM +0100, Hiltjo Posthuma wrote:
> > Hi,
> >
> > I noticed some things in the strptime(3) implementing when parsing timezone
> > strings using the %z format string.
> >
> > 1. I noticed the tm_gmtoff value is not correctly set in some cases. It
> > should
> > set tm_gmtoff to the offset from UTC in seconds (as described in mktime(3)).
> >
> > 2. The military/nautical UTC offsets are also reversed. This was also
> > actually
> > a bug in RFC822:
> >
> > RFC5322 (referenced in strptime(3) man page):
> > https://tools.ietf.org/html/rfc5322
> > Section 4.3, page 34 says:
> > "
> > The 1 character military time zones were defined in a non-standard
> > way in [RFC0822] and are therefore unpredictable in their meaning.
> > The original definitions of the military zones "A" through "I" are
> > equivalent to "+0100" through "+0900", respectively; "K", "L", and
> > "M" are equivalent to "+1000", "+1100", and "+1200", respectively;
> > "N" through "Y" are equivalent to "-0100" through "-1200".
> > respectively; and "Z" is equivalent to "+0000". However, because of
> > the error in [RFC0822], they SHOULD all be considered equivalent to
> > "-0000" unless there is out-of-band information confirming their
> > meaning.
> > "
> >
> > While comparing the other BSD and Linux code-bases I noticed NetBSD has
> > fixed
> > this on 2017-08-24. It is not in NetBSD 8 stable yet, but in NetBSD-current:
> > http://cvsweb.netbsd.org/bsdweb.cgi/src/lib/libc/time/strptime.c
> >
> > 3. The military zone J is also ambiguous. Some standards define it as
> > unused,
> > while others define it as "local observed time". NetBSD handles it as local
> > observed time, but OpenBSD returns NULL in strptime(3). I left this as is.
> >
> > 4. While at it I also fixed some trailing whitespaces.
> >
> > Thanks for not falling asleep and reading the long text :)
> >
> > Patch and test program below:
> >
> > diff --git lib/libc/time/strptime.c lib/libc/time/strptime.c
> > index eaf182dc773..86a0d5353ee 100644
> > --- lib/libc/time/strptime.c
> > +++ lib/libc/time/strptime.c
> > @@ -116,7 +116,7 @@ _strptime(const char *buf, const char *fmt, struct tm
> > *tm, int initialize)
> > fmt++;
> > continue;
> > }
> > -
> > +
> > if ((c = *fmt++) != '%')
> > goto literal;
> >
> > @@ -142,7 +142,7 @@ literal:
> > _LEGAL_ALT(0);
> > alt_format |= _ALT_O;
> > goto again;
> > -
> > +
> > /*
> > * "Complex" conversion rules, implemented through recursion.
> > */
> > @@ -380,7 +380,7 @@ literal:
> > * number but without the century.
> > */
> > if (!(_conv_num(&bp, &i, 0, 99)))
> > - return (NULL);
> > + return (NULL);
> > continue;
> >
> > case 'G': /* The year corresponding to the ISO week
> > @@ -500,7 +500,7 @@ literal:
> > ep = _find_string(bp, &i, nast, NULL, 4);
> > if (ep != NULL) {
> > #ifdef TM_GMTOFF
> > - tm->TM_GMTOFF = -5 - i;
> > + tm->TM_GMTOFF = (-5 - i) * SECSPERHOUR;
> > #endif
> > #ifdef TM_ZONE
> > tm->TM_ZONE = (char *)nast[i];
> > @@ -512,7 +512,7 @@ literal:
> > if (ep != NULL) {
> > tm->tm_isdst = 1;
> > #ifdef TM_GMTOFF
> > - tm->TM_GMTOFF = -4 - i;
> > + tm->TM_GMTOFF = (-4 - i) * SECSPERHOUR;
> > #endif
> > #ifdef TM_ZONE
> > tm->TM_ZONE = (char *)nadt[i];
> > @@ -527,11 +527,12 @@ literal:
> > /* Argh! No 'J'! */
> > if (*bp >= 'A' && *bp <= 'I')
> > tm->TM_GMTOFF =
> > - ('A' - 1) - (int)*bp;
> > + (int)*bp - ('A' - 1);
> > else if (*bp >= 'L' && *bp <= 'M')
> > - tm->TM_GMTOFF = 'A' - (int)*bp;
> > + tm->TM_GMTOFF = (int)*bp - 'A';
> > else if (*bp >= 'N' && *bp <= 'Y')
> > - tm->TM_GMTOFF = (int)*bp - 'M';
> > + tm->TM_GMTOFF = 'M' - (int)*bp;
> > + tm->TM_GMTOFF *= SECSPERHOUR;
> > #endif
> > #ifdef TM_ZONE
> > tm->TM_ZONE = NULL; /* XXX */
> > @@ -556,14 +557,15 @@ literal:
> > }
> > switch (i) {
> > case 2:
> > - offs *= 100;
> > + offs *= SECSPERHOUR;
> > break;
> > case 4:
> > i = offs % 100;
> > - if (i >= 60)
> > + offs /= 100;
> > + if (i >= SECSPERMIN)
> > return NULL;
> > /* Convert minutes into decimal */
> > - offs = (offs / 100) * 100 + (i * 50) / 30;
> > + offs = offs * SECSPERHOUR + i * SECSPERMIN;
> > break;
> > default:
> > return NULL;
> > @@ -719,7 +721,7 @@ _find_string(const u_char *bp, int *tgt, const char *
> > const *n1,
> > return NULL;
> > }
> >
> > -static int
> > +static int
> > leaps_thru_end_of(const int y)
> > {
> > return (y >= 0) ? (y / 4 - y / 100 + y / 400) :
> >
> >
> > Quick & dirty test program below:
> >
> >
> > #include <sys/types.h>
> >
> > #include <err.h>
> > #include <stdio.h>
> > #include <stdlib.h>
> > #include <string.h>
> > #include <time.h>
> >
> > void
> > testtime(const char *s)
> > {
> > struct tm tm;
> >
> > memset(&tm, 0, sizeof(tm));
> > if (strptime(s, "%Y-%m-%d %H:%M:%S %z", &tm) == NULL)
> > errx(1, "strptime: s=%s", s);
> >
> > printf("s=%s, gmtoff=%ld, zone=%s\n", s, tm.tm_gmtoff,
> > tm.tm_zone ? tm.tm_zone : "(null)");
> > }
> >
> > int
> > main(void)
> > {
> > testtime("2000-01-01 00:00:01 Z");
> > testtime("2000-01-01 00:00:01 UT");
> > testtime("2000-01-01 00:00:01 GMT");
> > testtime("2000-01-01 00:00:01 -04:00");
> > testtime("2000-01-01 00:00:01 -0400");
> > testtime("2000-01-01 00:00:01 +04:00");
> > testtime("2000-01-01 00:00:01 +0400");
> > testtime("2000-01-01 00:00:01 EST");
> > testtime("2000-01-01 00:00:01 EDT");
> > testtime("2000-01-01 00:00:01 A");
> > testtime("2000-01-01 00:00:01 B");
> > testtime("2000-01-01 00:00:01 C");
> > //testtime("2000-01-01 00:00:01 J"); // strptime returns NULL.
> > testtime("2000-01-01 00:00:01 Q");
> > testtime("2000-01-01 00:00:01 R");
> >
> > return 0;
> > }
> >
>
> Last weekly bump,
>
> Any further comments or OK's to get this in or reject it?
>
> As discussed previously for point 2 I think it is fine to either return NULL
> and not handle military zones like most libc's do or handle them properly like
> NetBSD (reversed offsets).
>
> Thanks,
>
> --
> Kind regards,
> Hiltjo
>
