> From: Holger Kipp <[EMAIL PROTECTED]>
>
> 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 <stdio.h>
#include <string.h>
#include <time.h>

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;
}
_______________________________________________
freebsd-stable@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-stable
To unsubscribe, send any mail to "[EMAIL PROTECTED]"

Reply via email to