Return-Path: Delivered-To: apmail-apr-dev-archive@apr.apache.org Received: (qmail 70087 invoked by uid 500); 6 Jun 2002 15:04:52 -0000 Mailing-List: contact dev-help@apr.apache.org; run by ezmlm Precedence: bulk List-Post: List-Help: List-Unsubscribe: List-Subscribe: Delivered-To: mailing list dev@apr.apache.org Delivered-To: moderator for dev@apr.apache.org Received: (qmail 66419 invoked from network); 6 Jun 2002 13:33:09 -0000 Message-ID: <3CFF6486.8070709@hermes.si> Date: Thu, 06 Jun 2002 15:32:54 +0200 From: =?UTF-8?B?QnJhbmtvIMSMaWJlag==?= Reply-To: brane@xbc.nu Organization: HERMES SoftLab User-Agent: Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.0rc3) Gecko/20020523 X-Accept-Language: sl, en-gb, en MIME-Version: 1.0 To: dev@apr.apache.org Cc: dev@subversion.tigris.org Subject: Fixing tm_gmtoff behaviour on Win32 (again) Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit X-Virus-Scanned: by AMaViS snapshot-20010714 X-Spam-Rating: daedalus.apache.org 1.6.2 0/1000/N Subversion got bitten by the APR time functions on Windows again. The culprit, as usual, was incorrect semantics of the tm_gmtoff field in apr_time_exp_t in the presence of DST. I decided that enough was enough, and it was time to do something about it. It turned out that none of the tests in testtime.c tickled that particular problem, so I added a test case (see patch below). The test passed on Unix with and withoug DST, and failed on Windows with DST enabled; again, as expected. I tracked down the provblem to the way tm_gmtoff was computed from the system's timezone info. Reading the docs for GetTimezoneInformation suggested that we should be using the StandardBias and DaylightBias fields compute tm_gmtoff, so that's what I did. The test now passes on Win2k (and presumably NT and XP) both with and without DST, and the relevant Subversion test passes, too. Now, before I commit this patch, I'd like to ask people to test it on Win95/98/Me (and possibly WinCE). I don't have access to any of these platforms, and the docs suggest there may be some difference on the Win9x series at least. Thanks, Brane Index: time/win32/time.c =================================================================== RCS file: /home/cvspublic/apr/time/win32/time.c,v retrieving revision 1.33 diff -u -r1.33 time.c --- time/win32/time.c 22 May 2002 00:57:24 -0000 1.33 +++ time/win32/time.c 6 Jun 2002 12:45:28 -0000 @@ -105,16 +105,19 @@ rc = GetTimeZoneInformation(&tz); switch (rc) { case TIME_ZONE_ID_UNKNOWN: - case TIME_ZONE_ID_STANDARD: xt->tm_isdst = 0; /* Bias = UTC - local time in minutes * tm_gmtoff is seconds east of UTC */ xt->tm_gmtoff = tz.Bias * -60; break; + case TIME_ZONE_ID_STANDARD: + xt->tm_isdst = 0; + xt->tm_gmtoff = (tz.Bias + tz.StandardBias) * -60; + break; case TIME_ZONE_ID_DAYLIGHT: xt->tm_isdst = 1; - xt->tm_gmtoff = tz.Bias * -60; + xt->tm_gmtoff = (tz.Bias + tz.DaylightBias) * -60; break; default: xt->tm_isdst = 0; @@ -224,8 +227,7 @@ { apr_status_t status = apr_time_exp_get(t, xt); if (status == APR_SUCCESS) - *t -= (apr_time_t) (xt->tm_isdst * 3600 - + xt->tm_gmtoff) * APR_USEC_PER_SEC; + *t -= (apr_time_t) xt->tm_gmtoff * APR_USEC_PER_SEC; return status; } Index: test/testtime.c =================================================================== RCS file: /home/cvspublic/apr/test/testtime.c,v retrieving revision 1.32 diff -u -r1.32 testtime.c --- test/testtime.c 15 Apr 2002 00:01:09 -0000 1.32 +++ test/testtime.c 6 Jun 2002 12:45:27 -0000 @@ -62,6 +62,23 @@ #define STR_SIZE 45 +static const char* print_time (apr_pool_t *pool, const apr_time_exp_t *xt) +{ + return apr_psprintf (pool, + "%04d-%02d-%02d %02d:%02d:%02d.%06d %+05d [%d %s]%s", + xt->tm_year + 1900, + xt->tm_mon, + xt->tm_mday, + xt->tm_hour, + xt->tm_min, + xt->tm_sec, + xt->tm_usec, + xt->tm_gmtoff, + xt->tm_yday + 1, + apr_day_snames[xt->tm_wday], + (xt->tm_isdst ? " DST" : "")); +} + int main(void) { apr_time_t now; @@ -84,19 +101,38 @@ printf("OK\n"); STD_TEST_NEQ(" apr_time_exp_gmt", apr_time_exp_gmt(&xt, now)) + printf(" (%s)\n", print_time(p, &xt)); STD_TEST_NEQ(" apr_time_exp_lt", apr_time_exp_lt(&xt2, now)) + printf(" (%s)\n", print_time(p, &xt2)); STD_TEST_NEQ(" apr_time_exp_get (GMT)", apr_time_exp_get(&imp, &xt)) printf("%-60s", " checking GMT explode == implode"); - if (imp != now) { + hr_off_64 = (apr_int64_t) xt.tm_gmtoff * APR_USEC_PER_SEC; + if (imp != now + hr_off_64) { printf("mismatch\n" "\t\tapr_now() %" APR_INT64_T_FMT "\n" "\t\tapr_implode() returned %" APR_INT64_T_FMT "\n" "\t\terror delta was %" APR_TIME_T_FMT "\n" - "\t\tshould have been 0\n", - now, imp, imp-now); + "\t\tshould have been %" APR_INT64_T_FMT "\n", + now, imp, imp-now, hr_off_64); + exit(-1); + } + printf("OK\n"); + + STD_TEST_NEQ(" apr_time_exp_get (localtime)", + apr_time_exp_get(&imp, &xt2)) + + printf("%-60s", " checking localtime explode == implode"); + hr_off_64 = (apr_int64_t) xt2.tm_gmtoff * APR_USEC_PER_SEC; + if (imp != now + hr_off_64) { + printf("mismatch\n" + "\t\tapr_now() %" APR_INT64_T_FMT "\n" + "\t\tapr_implode() returned %" APR_INT64_T_FMT "\n" + "\t\terror delta was %" APR_TIME_T_FMT "\n" + "\t\tshould have been %" APR_INT64_T_FMT "\n", + now, imp, imp-now, hr_off_64); exit(-1); } printf("OK\n"); @@ -178,7 +214,8 @@ exit(-1); } printf("OK\n"); - printf(" ( %lld - %lld = %lld )\n", imp, now, imp - now); + printf(" ( %" APR_TIME_T_FMT " - %" APR_TIME_T_FMT + " = %" APR_INT64_T_FMT " )\n", imp, now, imp - now); printf("\nTest Complete.\n"); return 0;