6 Jul 2012 01:52
[tz] localtime.c bugs
Doug Bailey <DBailey <at> qnx.com>
2012-07-05 23:52:05 GMT
2012-07-05 23:52:05 GMT
I've started working with tzcode2012b and I've run into a couple of issues.
The first: in testing, I found that if I set a timezone like "CST6", the tzname[] array would not be initialized. Digging into it, I found that settzname[] depends on having transition rules for the zone, but a zone like "CST6" will only be created with a single type and no transitions.
To work around that one I've added the following code to initialize the tzname[] array from the ttis array before running through the types array:
+ for (i=0; i<sp->typecnt; i++) {
+ const struct ttinfo * const ttisp = &sp->ttis[i];
+ tzname[ttisp->tt_isdst] = &sp->chars[ttisp->tt_abbrind];
+ }
/*
** And to get the latest zone names into tzname. . .
*/
for (i = 0; i < sp->timecnt; ++i) {
register const struct ttinfo * const ttisp =
&sp->ttis[
sp->types[i]];
The second has to do with our environment. We have an unfortunate time_t definition: an unsigned 32-bit number. The tzload code attempts to take unsigned time_t values into account, but if I'm not making a mistake understanding the way it's supposed to work, I think it has a few problems.
First, the loop over (i < sp->timecnt - 2) means that in comparing ats[i] > ats[i+1], you never compare ats[sp->timecnt-2] > ats[sp->timecnt-1]. This means that if all the transitions except the last are pre-epoch (negative number), the list doesn't get trimmed down.
Second, with that fixed, there are still problems with zones that only have pre-epoch transitions (e.g. America/Regina). In such a case, the ats[i] > ats[i+1] case never triggers, so you're left with a list of ats[] values that aren't valid -- they're negative numbers in a system with an unsigned time_t.
This is rather uglier. I've included my fix for what it's worth, but you can probably work out something better. One thing to note: I try to preserve the last pre-epoch transition, to handle situations where there's a large gap in transitions between the start of the epoch the the first post-epoch transition.
/*
** Out-of-sort ats should mean we're running on a
** signed time_t system but using a data file with
** unsigned values (or vice versa).
*/
#if 1
for (i = 0; i <= sp->timecnt - 1; ++i)
if ( (i < sp->timecnt-1 && sp->ats[i] > sp->ats[i + 1] ) ||
(i == sp->timecnt-1 && !TYPE_SIGNED(time_t) && sp->ats[i] > INT32_MAX) ) {
if (TYPE_SIGNED(time_t)) {
/*
** Ignore the end (easy).
*/
sp->timecnt = i+1;
} else {
/*
** Ignore the beginning (harder).
*/
register int j;
// keep the record right before the epoch boundary, but
// tweak it so that it starts right with the epoch
sp->ats[i] = 0;
// move everything back...
if (i > 0) {
for (j = 0; j + i < sp->timecnt; ++j) {
sp->ats[j] = sp->ats[j + i];
sp->types[j] = sp->types[j + i];
}
sp->timecnt = j;
}
}
break;
}
#else
for (i = 0; i < sp->timecnt - 2; ++i)
if (sp->ats[i] > sp->ats[i + 1]) {
++i;
if (TYPE_SIGNED(time_t)) {
/*
** Ignore the end (easy).
*/
sp->timecnt = i;
} else {
/*
** Ignore the beginning (harder).
*/
register int j;
for (j = 0; j + i < sp->timecnt; ++j) {
sp->ats[j] = sp->ats[j + i];
sp->types[j] = sp->types[j + i];
}
sp->timecnt = j;
}
break;
}
#endif
I appreciate any assistance with this.
Doug
The first: in testing, I found that if I set a timezone like "CST6", the tzname[] array would not be initialized. Digging into it, I found that settzname[] depends on having transition rules for the zone, but a zone like "CST6" will only be created with a single type and no transitions.
To work around that one I've added the following code to initialize the tzname[] array from the ttis array before running through the types array:
+ for (i=0; i<sp->typecnt; i++) {
+ const struct ttinfo * const ttisp = &sp->ttis[i];
+ tzname[ttisp->tt_isdst] = &sp->chars[ttisp->tt_abbrind];
+ }
/*
** And to get the latest zone names into tzname. . .
*/
for (i = 0; i < sp->timecnt; ++i) {
register const struct ttinfo * const ttisp =
&sp->ttis[
sp->types[i]];
The second has to do with our environment. We have an unfortunate time_t definition: an unsigned 32-bit number. The tzload code attempts to take unsigned time_t values into account, but if I'm not making a mistake understanding the way it's supposed to work, I think it has a few problems.
First, the loop over (i < sp->timecnt - 2) means that in comparing ats[i] > ats[i+1], you never compare ats[sp->timecnt-2] > ats[sp->timecnt-1]. This means that if all the transitions except the last are pre-epoch (negative number), the list doesn't get trimmed down.
Second, with that fixed, there are still problems with zones that only have pre-epoch transitions (e.g. America/Regina). In such a case, the ats[i] > ats[i+1] case never triggers, so you're left with a list of ats[] values that aren't valid -- they're negative numbers in a system with an unsigned time_t.
This is rather uglier. I've included my fix for what it's worth, but you can probably work out something better. One thing to note: I try to preserve the last pre-epoch transition, to handle situations where there's a large gap in transitions between the start of the epoch the the first post-epoch transition.
/*
** Out-of-sort ats should mean we're running on a
** signed time_t system but using a data file with
** unsigned values (or vice versa).
*/
#if 1
for (i = 0; i <= sp->timecnt - 1; ++i)
if ( (i < sp->timecnt-1 && sp->ats[i] > sp->ats[i + 1] ) ||
(i == sp->timecnt-1 && !TYPE_SIGNED(time_t) && sp->ats[i] > INT32_MAX) ) {
if (TYPE_SIGNED(time_t)) {
/*
** Ignore the end (easy).
*/
sp->timecnt = i+1;
} else {
/*
** Ignore the beginning (harder).
*/
register int j;
// keep the record right before the epoch boundary, but
// tweak it so that it starts right with the epoch
sp->ats[i] = 0;
// move everything back...
if (i > 0) {
for (j = 0; j + i < sp->timecnt; ++j) {
sp->ats[j] = sp->ats[j + i];
sp->types[j] = sp->types[j + i];
}
sp->timecnt = j;
}
}
break;
}
#else
for (i = 0; i < sp->timecnt - 2; ++i)
if (sp->ats[i] > sp->ats[i + 1]) {
++i;
if (TYPE_SIGNED(time_t)) {
/*
** Ignore the end (easy).
*/
sp->timecnt = i;
} else {
/*
** Ignore the beginning (harder).
*/
register int j;
for (j = 0; j + i < sp->timecnt; ++j) {
sp->ats[j] = sp->ats[j + i];
sp->types[j] = sp->types[j + i];
}
sp->timecnt = j;
}
break;
}
#endif
I appreciate any assistance with this.
Doug
RSS Feed