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; } -- Kind regards, Hiltjo
