Hi Ted, Ted Unangst wrote on Thu, May 09, 2019 at 04:16:40PM -0400: > Ingo Schwarze wrote:
>> I'm not mixing anything else into this diff. The other bugs should >> be handled separately. > Works for me. (with additional comment removal) Thanks for checking, committed. Now let's get to the more serious part. Hiltjo observed that %Z and %z produce wrong results. First of all, neither POSIX nor XPG define tm_gmtoff nor %Z nor %z: https://pubs.opengroup.org/onlinepubs/9699919799/basedefs/time.h.html https://pubs.opengroup.org/onlinepubs/9699919799/functions/strptime.html So i think the best way to find out what tm_gmtoff should be is to understand how programs in our own tree use it. Here is a (hopefully) complete list of the users in OpenBSD base. The following files expect it to contain seconds: - lib/libc/time/strftime.c - lib/libc/time/wcsftime.c - usr.sbin/smtpd/to.c - usr.sbin/snmpd/mib.c - usr.sbin/cron/cron.c - usr.bin/ftp/util.c - usr.bin/cvs/entries.c - usr.bin/rcs/rcstime.c - gnu/usr.bin/perl/time64.c I failed to find any users that do *not* expect seconds. So my conclusion is that the documentation is right and what the code in strptime.c does is wrong. Here is a patch to fix the code. OK? The change to %Z is exactly what Hiltjo sent. The current code for %z is unnecessarily complicated. Rather than fixing it, i simply rewrote it from scratch. I like it when a bugfix results in -28 +11 LOC and better readability. Yours, Ingo Index: strptime.c =================================================================== RCS file: /cvs/src/lib/libc/time/strptime.c,v retrieving revision 1.26 diff -u -p -r1.26 strptime.c --- strptime.c 10 May 2019 12:49:16 -0000 1.26 +++ strptime.c 10 May 2019 14:40:49 -0000 @@ -497,7 +497,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]; @@ -509,7 +509,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]; @@ -519,33 +519,16 @@ literal: } return NULL; } - offs = 0; - for (i = 0; i < 4; ) { - if (isdigit(*bp)) { - offs = offs * 10 + (*bp++ - '0'); - i++; - continue; - } - if (i == 2 && *bp == ':') { - bp++; - continue; - } - break; - } - switch (i) { - case 2: - offs *= 100; - break; - case 4: - i = offs % 100; - if (i >= 60) - return NULL; - /* Convert minutes into decimal */ - offs = (offs / 100) * 100 + (i * 50) / 30; - break; - default: + if (!isdigit(bp[0]) || !isdigit(bp[1])) return NULL; - } + offs = ((bp[0]-'0') * 10 + (bp[1]-'0')) * SECSPERHOUR; + bp += 2; + if (*bp == ':') + bp++; + if (!isdigit(bp[0]) || !isdigit(bp[1])) + return NULL; + offs += ((bp[0]-'0') * 10 + (bp[1]-'0')) * SECSPERMIN; + bp += 2; if (neg) offs = -offs; tm->tm_isdst = 0; /* XXX */
