On Sun, Mar 03, 2019 at 09:01:58PM +0100, Hiltjo Posthuma 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; > > } > > >
Another bump, any thoughts on the above comments and patch? -- Kind regards, Hiltjo
