[tz] zdump bug in Asia/Singapore
Paul Eggert
eggert at cs.ucla.edu
Thu Sep 11 07:17:57 UTC 2014
Tim Parenti wrote:
> Unfortunately, it seems your example doesn't match observed behavior.
Right you are. Thanks for catching that. Since it doesn't really
matter I'd rather leave the code alone, and fix the documentation to
describe it, as that'd be less disruptive (and also easier to implement
:-). Problem fixed in the first proposed patch attached.
> I also just noticed, above, that zdump returns "YST" for both timepoints in
> the transition above, when this should be a transition from XST to YST.
Thanks again. That's a regression from 2014g. It occurs only on
platforms that lack tm_zone in struct tm, which is why I didn't observe
it. Please see proposed patches 2 and 3, attached. This suggests that
we need a new code release soon.
I found some minor cleanups while looking into the above, and attach
them as proposed patch 4.
-------------- next part --------------
From 2e78bf08dce685478da73888d90fe2fb2fbfc6cd Mon Sep 17 00:00:00 2001
From: Paul Eggert <eggert at cs.ucla.edu>
Date: Wed, 10 Sep 2014 23:05:10 -0700
Subject: [PROPOSED PATCH 1/4] * zdump.8: Fix documentation of zdump cutoff
behavior.
(Thanks to Tim Parenti for reporting the problem.)
---
zdump.8 | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/zdump.8 b/zdump.8
index c4b92b0..c5f725d 100644
--- a/zdump.8
+++ b/zdump.8
@@ -48,11 +48,11 @@ implementations with different time representations.
Cut off verbose output at the given year(s).
Cutoff times are computed using the Gregorian calendar with year 0
and with Universal Time (UT) ignoring leap seconds.
-The lower bound is inclusive and the upper is exclusive; for example, a
+The lower bound is exclusive and the upper is inclusive; for example, a
.I loyear
-of 1970 includes a transition occurring at the very start of 1970 but a
+of 1970 excludes a transition occurring at the very start of 1970 but a
.I hiyear
-of 1970 excludes the transition.
+of 1970 includes the transition.
The default cutoff is
.BR \*-500,2500 .
.TP
@@ -65,7 +65,7 @@ The
determines whether the count includes leap seconds.
As with
.BR \*-c ,
-the cutoff's lower bound is inclusive and its upper bound is exclusive.
+the cutoff's lower bound is exclusive and its upper bound is inclusive.
.SH LIMITATIONS
Time discontinuities are found by sampling the results returned by localtime
at twelve-hour intervals.
--
1.9.3
-------------- next part --------------
From 9618efb826ed5a1efae98a6852f8a187791ea8f2 Mon Sep 17 00:00:00 2001
From: Paul Eggert <eggert at cs.ucla.edu>
Date: Wed, 10 Sep 2014 23:21:45 -0700
Subject: [PROPOSED PATCH 2/4] Fix 2014g regression: localtime and mktime
weren't setting tzname.
In 2014g, localtime and mktime set tzname[0] and tzname[1] as if by
tzset, regardless of the requested time stamp. In 2014f and earlier,
localtime and mktime then set tzname[0] or tzname[1] to a time zone
appropriate for the time stamp in question.
Although both behaviors conform to POSIX, the 2014f behavior is
better because it lets a single-threaded application discover the
time zone abbreviation appropriate for the just-queried time stamp
even on platforms that lack TM_ZONE. The 2014f-and-earlier zdump
relies on the 2014f library behavior on these platforms.
Problem reported by Tim Parenti in:
http://mm.icann.org/pipermail/tz/2014-September/021585.html
* localtime.c (localsub): Treat a nonzero offset value
as a request to set tzname on success. All callers changed.
(localtime_rz, mktime_z): Define only if NETBSD_INSPIRED,
as they're no longer needed otherwise.
(localtime_tzset): Invert sense of third arg from SKIP_TZSET
to SETTZ. New arg SETNAME. All callers changed.
(localtime): Request tzname to be set on success.
(mktime_tzname): New function, with most of the previous
contents of mktime_z.
(mktime_z, mktime): Use it.
* NEWS: Document the above.
---
NEWS | 4 ++++
localtime.c | 47 ++++++++++++++++++++++++++++++++++-------------
2 files changed, 38 insertions(+), 13 deletions(-)
diff --git a/NEWS b/NEWS
index b786be6..ab751e7 100644
--- a/NEWS
+++ b/NEWS
@@ -17,6 +17,10 @@ Unreleased, experimental changes
Changes affecting code
+ The tz library's localtime and mktime functions now set tzname to a value
+ appropriate for the requested time stamp, fixing a 2014g regression.
+ (Thanks to Tim Parenti for reporting the problem.)
+
zdump -c no longer mishandles transitions near year boundaries.
(Thanks to Tim Parenti for reporting the problem.)
diff --git a/localtime.c b/localtime.c
index 4b88d1e..354d0d7 100644
--- a/localtime.c
+++ b/localtime.c
@@ -1294,7 +1294,8 @@ tzfree(timezone_t sp)
** freely called. (And no, the PANS doesn't require the above behavior,
** but it *is* desirable.)
**
-** The unused offset argument is for the benefit of mktime variants.
+** If OFFSET is nonzero, set tzname if successful.
+** OFFSET's type is intfast32_t for compatibility with gmtsub.
*/
/*ARGSUSED*/
@@ -1307,8 +1308,12 @@ localsub(struct state const *sp, time_t const *timep, int_fast32_t offset,
register struct tm * result;
const time_t t = *timep;
- if (sp == NULL)
- return gmtsub(gmtptr, timep, offset, tmp);
+ if (sp == NULL) {
+ result = gmtsub(gmtptr, timep, 0, tmp);
+ if (result && offset)
+ tzname[0] = gmtptr ? gmtptr->chars : (char *) gmt;
+ return result;
+ }
if ((sp->goback && t < sp->ats[0]) ||
(sp->goahead && t > sp->ats[sp->timecnt - 1])) {
time_t newt = t;
@@ -1365,29 +1370,35 @@ localsub(struct state const *sp, time_t const *timep, int_fast32_t offset,
*/
result = timesub(&t, ttisp->tt_gmtoff, sp, tmp);
tmp->tm_isdst = ttisp->tt_isdst;
+ if (result && offset)
+ tzname[tmp->tm_isdst] = (char *) &sp->chars[ttisp->tt_abbrind];
#ifdef TM_ZONE
tmp->TM_ZONE = (char *) &sp->chars[ttisp->tt_abbrind];
#endif /* defined TM_ZONE */
return result;
}
-NETBSD_INSPIRED_EXTERN struct tm *
+#if NETBSD_INSPIRED
+
+struct tm *
localtime_rz(struct state *sp, time_t const *timep, struct tm *tmp)
{
return localsub(sp, timep, 0, tmp);
}
+#endif
+
static struct tm *
-localtime_tzset(time_t const *timep, struct tm *tmp, bool skip_tzset)
+localtime_tzset(time_t const *timep, struct tm *tmp, bool settz, bool setname)
{
int err = lock();
if (err) {
errno = err;
return NULL;
}
- if (!skip_tzset)
+ if (settz)
tzset_unlocked();
- tmp = localtime_rz(lclptr, timep, tmp);
+ tmp = localsub(lclptr, timep, setname, tmp);
unlock();
return tmp;
}
@@ -1395,13 +1406,13 @@ localtime_tzset(time_t const *timep, struct tm *tmp, bool skip_tzset)
struct tm *
localtime(const time_t *const timep)
{
- return localtime_tzset(timep, &tm, 0);
+ return localtime_tzset(timep, &tm, true, true);
}
struct tm *
localtime_r(const time_t *const timep, struct tm *tmp)
{
- return localtime_tzset(timep, tmp, lcl_is_set != 0);
+ return localtime_tzset(timep, tmp, lcl_is_set == 0, false);
}
/*
@@ -2015,17 +2026,27 @@ time1(struct tm *const tmp,
return WRONG;
}
-NETBSD_INSPIRED_EXTERN time_t
-mktime_z(struct state *sp, struct tm *tmp)
+static time_t
+mktime_tzname(struct state *sp, struct tm *tmp, bool setname)
{
if (sp)
- return time1(tmp, localsub, sp, 0);
+ return time1(tmp, localsub, sp, setname);
else {
gmtcheck();
return time1(tmp, gmtsub, gmtptr, 0);
}
}
+#if NETBSD_INSPIRED
+
+time_t
+mktime_z(struct state *sp, struct tm *tmp)
+{
+ return mktime_tzname(sp, tmp, false);
+}
+
+#endif
+
time_t
mktime(struct tm *const tmp)
{
@@ -2036,7 +2057,7 @@ mktime(struct tm *const tmp)
return -1;
}
tzset_unlocked();
- t = mktime_z(lclptr, tmp);
+ t = mktime_tzname(lclptr, tmp, true);
unlock();
return t;
}
--
1.9.3
-------------- next part --------------
From 50cafab5d86559b9a5c72f26817853784449b73a Mon Sep 17 00:00:00 2001
From: Paul Eggert <eggert at cs.ucla.edu>
Date: Wed, 10 Sep 2014 23:27:27 -0700
Subject: [PROPOSED PATCH 3/4] Fix 2014g zdump regression on platforms without
TM_ZONE.
Problem reported by Tim Parenti in:
http://mm.icann.org/pipermail/tz/2014-September/021585.html
* zdump.c (localtime_r, localtime_rz) [!TM_ZONE]:
Just call 'localtime', since this can set tzname in a
timestamp-dependent way, which is better for zdump.
* NEWS: Document this.
---
NEWS | 3 ++-
zdump.c | 7 +++++--
2 files changed, 7 insertions(+), 3 deletions(-)
diff --git a/NEWS b/NEWS
index ab751e7..0e58e97 100644
--- a/NEWS
+++ b/NEWS
@@ -18,7 +18,8 @@ Unreleased, experimental changes
Changes affecting code
The tz library's localtime and mktime functions now set tzname to a value
- appropriate for the requested time stamp, fixing a 2014g regression.
+ appropriate for the requested time stamp, and zdump now uses this
+ on platforms not defining TM_ZONE, fixing a 2014g regression.
(Thanks to Tim Parenti for reporting the problem.)
zdump -c no longer mishandles transitions near year boundaries.
diff --git a/zdump.c b/zdump.c
index 3dfa768..0735c2a 100644
--- a/zdump.c
+++ b/zdump.c
@@ -304,9 +304,12 @@ sumsize(size_t a, size_t b)
static void tzset(void) { }
#endif
-#if ! HAVE_LOCALTIME_RZ
+/* Platforms with TM_ZONE don't need tzname, so they can use the
+ faster localtime_rz or localtime_r if available. */
+
+#if !defined TM_ZONE || ! HAVE_LOCALTIME_RZ
-# if ! HAVE_LOCALTIME_R || ! HAVE_TZSET
+# if !defined TM_ZONE || ! HAVE_LOCALTIME_R || ! HAVE_TZSET
# undef localtime_r
# define localtime_r zdump_localtime_r
static struct tm *
--
1.9.3
-------------- next part --------------
From 51075b0f5c8bff2efd78c4f44f70270152991371 Mon Sep 17 00:00:00 2001
From: Paul Eggert <eggert at cs.ucla.edu>
Date: Thu, 11 Sep 2014 00:11:37 -0700
Subject: [PROPOSED PATCH 4/4] Minor localtime fixups.
* localtime.c (localsub): Tune slightly.
Don't set tzname unless successful.
(gmtime_r, offtime, timeoff): Simplify.
* NEWS: Document the tzname change.
---
NEWS | 2 ++
localtime.c | 27 ++++++++++++---------------
2 files changed, 14 insertions(+), 15 deletions(-)
diff --git a/NEWS b/NEWS
index 0e58e97..a2a7355 100644
--- a/NEWS
+++ b/NEWS
@@ -22,6 +22,8 @@ Unreleased, experimental changes
on platforms not defining TM_ZONE, fixing a 2014g regression.
(Thanks to Tim Parenti for reporting the problem.)
+ The tz library no longer sets tzname if localtime or mktime fails.
+
zdump -c no longer mishandles transitions near year boundaries.
(Thanks to Tim Parenti for reporting the problem.)
diff --git a/localtime.c b/localtime.c
index 354d0d7..62743e2 100644
--- a/localtime.c
+++ b/localtime.c
@@ -1333,16 +1333,16 @@ localsub(struct state const *sp, time_t const *timep, int_fast32_t offset,
newt > sp->ats[sp->timecnt - 1])
return NULL; /* "cannot happen" */
result = localsub(sp, &newt, offset, tmp);
- if (result == tmp) {
+ if (result) {
register int_fast64_t newy;
- newy = tmp->tm_year;
+ newy = result->tm_year;
if (t < sp->ats[0])
newy -= years;
else newy += years;
if (! (INT_MIN <= newy && newy <= INT_MAX))
return NULL;
- tmp->tm_year = newy;
+ result->tm_year = newy;
}
return result;
}
@@ -1369,12 +1369,14 @@ localsub(struct state const *sp, time_t const *timep, int_fast32_t offset,
** timesub(&t, 0L, sp, tmp);
*/
result = timesub(&t, ttisp->tt_gmtoff, sp, tmp);
- tmp->tm_isdst = ttisp->tt_isdst;
- if (result && offset)
- tzname[tmp->tm_isdst] = (char *) &sp->chars[ttisp->tt_abbrind];
+ if (result) {
+ result->tm_isdst = ttisp->tt_isdst;
+ if (offset)
+ tzname[result->tm_isdst] = (char *) &sp->chars[ttisp->tt_abbrind];
#ifdef TM_ZONE
- tmp->TM_ZONE = (char *) &sp->chars[ttisp->tt_abbrind];
+ result->TM_ZONE = (char *) &sp->chars[ttisp->tt_abbrind];
#endif /* defined TM_ZONE */
+ }
return result;
}
@@ -1452,8 +1454,7 @@ struct tm *
gmtime_r(const time_t *const timep, struct tm *tmp)
{
gmtcheck();
- tmp = gmtsub(gmtptr, timep, 0, tmp);
- return tmp;
+ return gmtsub(gmtptr, timep, 0, tmp);
}
#ifdef STD_INSPIRED
@@ -1461,10 +1462,8 @@ gmtime_r(const time_t *const timep, struct tm *tmp)
struct tm *
offtime(const time_t *const timep, const long offset)
{
- struct tm *tmp;
gmtcheck();
- tmp = gmtsub(gmtptr, timep, offset, &tm);
- return tmp;
+ return gmtsub(gmtptr, timep, offset, &tm);
}
#endif /* defined STD_INSPIRED */
@@ -2081,12 +2080,10 @@ timegm(struct tm *const tmp)
time_t
timeoff(struct tm *const tmp, const long offset)
{
- time_t t;
if (tmp)
tmp->tm_isdst = 0;
gmtcheck();
- t = time1(tmp, gmtsub, gmtptr, offset);
- return t;
+ return time1(tmp, gmtsub, gmtptr, offset);
}
#endif /* defined STD_INSPIRED */
--
1.9.3
More information about the tz
mailing list