From owner-freebsd-stable@FreeBSD.ORG Mon Oct 29 01:19:58 2007 Return-Path: Delivered-To: freebsd-stable@FreeBSD.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id 51AE116A469 for ; Mon, 29 Oct 2007 01:19:58 +0000 (UTC) (envelope-from miguel@anjos.strangled.net) Received: from mailrly02.isp.novis.pt (mailrly02.isp.novis.pt [195.23.133.212]) by mx1.freebsd.org (Postfix) with ESMTP id 7C2FA13C4B2 for ; Mon, 29 Oct 2007 01:19:56 +0000 (UTC) (envelope-from miguel@anjos.strangled.net) Received: (qmail 23981 invoked from network); 29 Oct 2007 01:19:55 -0000 Received: from unknown (HELO mailfrt02.isp.novis.pt) ([195.23.133.194]) (envelope-sender ) by mailrly02.isp.novis.pt with compressed SMTP; 29 Oct 2007 01:19:55 -0000 Received: (qmail 22660 invoked from network); 29 Oct 2007 01:19:54 -0000 Received: from unknown (HELO satan.anjos.strangled.net) ([89.181.82.188]) (envelope-sender ) by mailfrt02.isp.novis.pt with SMTP; 29 Oct 2007 01:19:54 -0000 Received: from satan.anjos.strangled.net (localhost [127.0.0.1]) by satan.anjos.strangled.net (8.14.1/8.14.1) with ESMTP id l9T1JpKY009150; Mon, 29 Oct 2007 01:19:51 GMT (envelope-from miguel@satan.anjos.strangled.net) Received: (from miguel@localhost) by satan.anjos.strangled.net (8.14.1/8.14.1/Submit) id l9T1Jpvs009149; Mon, 29 Oct 2007 01:19:51 GMT (envelope-from miguel) Date: Mon, 29 Oct 2007 01:19:51 GMT From: Miguel Lopes Santos Ramos Message-Id: <200710290119.l9T1Jpvs009149@satan.anjos.strangled.net> To: eugen@kuzbass.ru, hk@alogis.com In-Reply-To: <20071028211538.GA92424@intserv.int1.b.intern> Cc: stable@FreeBSD.org, freebsd-stable@FreeBSD.org Subject: Re: date manupulation strangeness X-BeenThere: freebsd-stable@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: Production branch of FreeBSD source code List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 29 Oct 2007 01:19:58 -0000 > From: Holger Kipp > > On Mon, Oct 29, 2007 at 01:35:08AM +0700, Eugene Grosbein wrote: > > On Sun, Oct 28, 2007 at 07:20:11PM +0100, Holger Kipp wrote: > > > > > > # unixtime=1193511599 > > > > # LC_ALL=C TZ=Asia/Krasnoyarsk date -jr $unixtime > > > > Sun Oct 28 02:59:59 KRAT 2007 > > > > Here it shows 'Sun Oct 28 02:59:59 KRAST 2007' really > > (cut-n-paste error, mea culpa). Take a note of zone name, > > KRAST stands for 'KRAsnoyarsk Summer Time' and > > KRAT stands for 'KRAsnoyarsk Time' (winter one). > > ah, I see. I can reproduce it here as well: > > %setenv LC_ALL C > %setenv TZ Asia/Krasnoyarsk > %setenv unixtime 1193511599 > > %date -jr $unixtime > Sun Oct 28 02:59:59 KRAST 2007 > %date -jf $s $unixtime > Sun Oct 28 02:59:59 KRAT 2007 > %date -juf %s $unixtime > Sat Oct 27 18:59:59 UTC 2007 > %date -jur $unixtime > Sat Oct 27 18:59:59 UTC 2007 This is a lot of fun! Bugs like this go unnoticed for years... It is also very exciting finding people at GMT+8. Mind you, we programmers who live at GMT+0 do a lot of timezone errors because we look at a time and often don't know whether it's localtime or gmt. At least I do. Then we only find out when we change to summer time. The problem is of course in src/bin/date.c, line 268. Someone added the following on revision 1.32.2.4: 267: /* Let mktime() decide whether summer time is in effect. */ 268: lt->tm_isdst = -1; Now, who's mktime() to know? This line is erasing the output of strptime(), and strptime() knows better than anyone what the user might have wanted! See my test source code bellow. date first fills up a tm structure using localtime(time(0)) so that the data which the user does not supply is extracted from the current time. Then, it calls strptime to parse the user time using these defaults. It's summarized in function test1() of my test code. Historically, the behaviour was just this (actually, like test2()). The person who added line 268, wanted to solve the problem of setting a date across DST, but the way he/she did it is not, in my opinion, the best. As we have discouvered, it does not work when the user supplies good DST data, because the line tm_isdst = -1 erases it. Now, what should perhaps be erased is the default dst data. So the line tm_isdst = -1 should be moved up to line 191, between localtime() and strptime(). The code would behave like my test3() function. See the test output (using LC_ALL=C and TZ=Asia/Krasnoyarsk): Test1 using %s: case a Sun Oct 28 01:59:59 KRAST 2007 == Sun Oct 28 01:59:59 KRAST 2007 case b Sun Oct 28 02:59:59 KRAT 2007 != Sun Oct 28 02:59:59 KRAST 2007 case c Sun Oct 28 02:59:59 KRAT 2007 == Sun Oct 28 02:59:59 KRAT 2007 Test1 using %Y-%m-%d %H:%M:%S: case a Sun Oct 28 01:59:59 KRAST 2007 == Sun Oct 28 01:59:59 KRAST 2007 case b Sun Oct 28 02:59:59 KRAT 2007 != Sun Oct 28 02:59:59 KRAST 2007 case c Sun Oct 28 02:59:59 KRAT 2007 == Sun Oct 28 02:59:59 KRAT 2007 Test2 using %s: case a Sun Oct 28 01:59:59 KRAST 2007 == Sun Oct 28 01:59:59 KRAST 2007 case b Sun Oct 28 02:59:59 KRAST 2007 == Sun Oct 28 02:59:59 KRAST 2007 case c Sun Oct 28 02:59:59 KRAT 2007 == Sun Oct 28 02:59:59 KRAT 2007 Test2 using %Y-%m-%d %H:%M:%S: case a Sun Oct 28 02:59:59 KRAST 2007 != Sun Oct 28 01:59:59 KRAST 2007 case b Sun Oct 28 02:59:59 KRAT 2007 != Sun Oct 28 02:59:59 KRAST 2007 case c Sun Oct 28 02:59:59 KRAT 2007 == Sun Oct 28 02:59:59 KRAT 2007 Test3 using %s: case a Sun Oct 28 01:59:59 KRAST 2007 == Sun Oct 28 01:59:59 KRAST 2007 case b Sun Oct 28 02:59:59 KRAST 2007 == Sun Oct 28 02:59:59 KRAST 2007 case c Sun Oct 28 02:59:59 KRAT 2007 == Sun Oct 28 02:59:59 KRAT 2007 Test3 using %Y-%m-%d %H:%M:%S: case a Sun Oct 28 01:59:59 KRAST 2007 == Sun Oct 28 01:59:59 KRAST 2007 case b Sun Oct 28 02:59:59 KRAT 2007 != Sun Oct 28 02:59:59 KRAST 2007 case c Sun Oct 28 02:59:59 KRAT 2007 == Sun Oct 28 02:59:59 KRAT 2007 Historical behaviour is test2. It didn't work across DST when the user didn't supply DST data, but it did work fine for %s. The "correction", test1, introduced the error you've spotted, and that would only have been spotted by someone concerned with those two hours of each year... The solution I think its best works always that the user supplies DST data. Of course, it fails to guess dst status for test case b, but that's understandable, it can't guess. > Looks like one of the conversions is getting the > summertime-flag wrong here. > > I have tested this here with 6.2-STABLE from May 20... There's no point in naming the revision... the problematic line was introduced 6 years and 5 months ago by ru... Complete reference: src/bin/date.c Revision 1.32.2.4 Is ru around? Who fills in a pr for this? In summary, I think we should move line 268 to line 191 and erase the comment on line 267 (mktime() is no one to decide, strptime() must have the final word). Miguel Ramos Lisboa, Portugal ---- #include #include #include time_t test1(const char* datestr, const char* format) { time_t t = time(0); struct tm tm; localtime_r(&t, &tm); strptime(datestr, format, &tm); tm.tm_isdst = -1; return mktime(&tm); } time_t test2(const char* datestr, const char* format) { time_t t = time(0); struct tm tm; localtime_r(&t, &tm); strptime(datestr, format, &tm); return mktime(&tm); } time_t test3(const char* datestr, const char* format) { time_t t = time(0); struct tm tm; localtime_r(&t, &tm); tm.tm_isdst = -1; strptime(datestr, format, &tm); return mktime(&tm); } int main() { const char* fmt1 = "%s"; const char* fmt2 = "%Y-%m-%d %H:%M:%S"; const char* fmt3 = "%+"; const time_t a = 1193507999; const time_t b = 1193511599; const time_t c = 1193515199; time_t ta, tb, tc; char as[16], bs[16], cs[16]; char af[32], bf[32], cf[32]; char ad[32], bd[32], cd[32]; char at[32], bt[32], ct[32]; strftime(as, sizeof(as), fmt1, localtime(&a)); strftime(bs, sizeof(bs), fmt1, localtime(&b)); strftime(cs, sizeof(cs), fmt1, localtime(&c)); strftime(af, sizeof(af), fmt2, localtime(&a)); strftime(bf, sizeof(bf), fmt2, localtime(&b)); strftime(cf, sizeof(cf), fmt2, localtime(&c)); strftime(ad, sizeof(ad), fmt3, localtime(&a)); strftime(bd, sizeof(bd), fmt3, localtime(&b)); strftime(cd, sizeof(cd), fmt3, localtime(&c)); printf("Test1 using %s:\n", fmt1); ta = test1(as, fmt1); strftime(at, sizeof(at), fmt3, localtime(&ta)); tb = test1(bs, fmt1); strftime(bt, sizeof(bt), fmt3, localtime(&tb)); tc = test1(cs, fmt1); strftime(ct, sizeof(ct), fmt3, localtime(&tc)); printf("case a %s == %s\n", at, ad); printf("case b %s != %s\n", bt, bd); printf("case c %s == %s\n", ct, cd); printf("\nTest1 using %s:\n", fmt2); ta = test1(af, fmt2); strftime(at, sizeof(at), fmt3, localtime(&ta)); tb = test1(bf, fmt2); strftime(bt, sizeof(bt), fmt3, localtime(&tb)); tc = test1(cf, fmt2); strftime(ct, sizeof(ct), fmt3, localtime(&tc)); printf("case a %s == %s\n", at, ad); printf("case b %s != %s\n", bt, bd); printf("case c %s == %s\n", ct, cd); printf("\nTest2 using %s:\n", fmt1); ta = test2(as, fmt1); strftime(at, sizeof(at), fmt3, localtime(&ta)); tb = test2(bs, fmt1); strftime(bt, sizeof(bt), fmt3, localtime(&tb)); tc = test2(cs, fmt1); strftime(ct, sizeof(ct), fmt3, localtime(&tc)); printf("case a %s == %s\n", at, ad); printf("case b %s == %s\n", bt, bd); printf("case c %s == %s\n", ct, cd); printf("\nTest2 using %s:\n", fmt2); ta = test2(af, fmt2); strftime(at, sizeof(at), fmt3, localtime(&ta)); tb = test2(bf, fmt2); strftime(bt, sizeof(bt), fmt3, localtime(&tb)); tc = test2(cf, fmt2); strftime(ct, sizeof(ct), fmt3, localtime(&tc)); printf("case a %s != %s\n", at, ad); printf("case b %s != %s\n", bt, bd); printf("case c %s == %s\n", ct, cd); printf("\nTest3 using %s:\n", fmt1); ta = test3(as, fmt1); strftime(at, sizeof(at), fmt3, localtime(&ta)); tb = test3(bs, fmt1); strftime(bt, sizeof(bt), fmt3, localtime(&tb)); tc = test3(cs, fmt1); strftime(ct, sizeof(ct), fmt3, localtime(&tc)); printf("case a %s == %s\n", at, ad); printf("case b %s == %s\n", bt, bd); printf("case c %s == %s\n", ct, cd); printf("\nTest3 using %s:\n", fmt2); ta = test3(af, fmt2); strftime(at, sizeof(at), fmt3, localtime(&ta)); tb = test3(bf, fmt2); strftime(bt, sizeof(bt), fmt3, localtime(&tb)); tc = test3(cf, fmt2); strftime(ct, sizeof(ct), fmt3, localtime(&tc)); printf("case a %s == %s\n", at, ad); printf("case b %s != %s\n", bt, bd); printf("case c %s == %s\n", ct, cd); return 0; }