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;
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Thu Jun 6 15:33:54 2002