* [PATCH] Use 64-bit time_t for time zone file parsing
@ 2018-05-08 13:16 Florian Weimer
2018-05-08 14:05 ` Andreas Schwab
0 siblings, 1 reply; 7+ messages in thread
From: Florian Weimer @ 2018-05-08 13:16 UTC (permalink / raw)
To: GNU C Library; +Cc: Albert ARIBAUD
[-- Attachment #1: Type: text/plain, Size: 285 bytes --]
This patch is intended as a precursor for time64_t support. Some
further conversions of internal interfaces will be required. It also
enables future cleanups in the parser itself.
It does not change much, but I believe it is a small step in the right
direction.
Thanks,
Florian
[-- Attachment #2: internal_time_t.patch --]
[-- Type: text/x-patch, Size: 9625 bytes --]
Subject: [PATCH] time: Use 64-bit time values for time zone parsing
To: libc-alpha@sourceware.org
2018-05-08 Florian Weimer <fweimer@redhat.com>
Use 64-bit epoch values in the time zone file parser.
* include/time.h (internal_time_t): Define.
(__tzfile_compute): Use it.
* time/tzfile.c (struct leap): Use internal_time_t for epoch
member.
(transitions): Switch to internal_time_t.
(__tzfile_read): Likewise. Remove code dealing with 4-byte time_t
types.
(__tzfile_compute): Use internal_time_t for timer argument. Check
for truncation before calling __offtime.
diff --git a/include/time.h b/include/time.h
index aab26d7768..23d2580528 100644
--- a/include/time.h
+++ b/include/time.h
@@ -26,6 +26,10 @@ extern __typeof (clock_getcpuclockid) __clock_getcpuclockid;
/* Now define the internal interfaces. */
struct tm;
+/* time_t variant for representing time zone data, independent of
+ time_t. */
+typedef __int64_t internal_time_t;
+
/* Defined in mktime.c. */
extern const unsigned short int __mon_yday[2][13] attribute_hidden;
@@ -39,7 +43,7 @@ extern int __use_tzfile attribute_hidden;
extern void __tzfile_read (const char *file, size_t extra,
char **extrap) attribute_hidden;
-extern void __tzfile_compute (time_t timer, int use_localtime,
+extern void __tzfile_compute (internal_time_t timer, int use_localtime,
long int *leap_correct, int *leap_hit,
struct tm *tp) attribute_hidden;
extern void __tzfile_default (const char *std, const char *dst,
diff --git a/time/tzfile.c b/time/tzfile.c
index 3e39723148..d317db951e 100644
--- a/time/tzfile.c
+++ b/time/tzfile.c
@@ -44,12 +44,12 @@ struct ttinfo
struct leap
{
- time_t transition; /* Time the transition takes effect. */
+ internal_time_t transition; /* Time the transition takes effect. */
long int change; /* Seconds of correction to apply. */
};
static size_t num_transitions;
-libc_freeres_ptr (static time_t *transitions);
+libc_freeres_ptr (static internal_time_t *transitions);
static unsigned char *type_idxs;
static size_t num_types;
static struct ttinfo *types;
@@ -113,8 +113,8 @@ __tzfile_read (const char *file, size_t extra, char **extrap)
size_t tzspec_len;
char *new = NULL;
- if (sizeof (time_t) != 4 && sizeof (time_t) != 8)
- abort ();
+ _Static_assert (sizeof (internal_time_t) == 8,
+ "internal_time_t must be eight bytes");
__use_tzfile = 0;
@@ -200,9 +200,7 @@ __tzfile_read (const char *file, size_t extra, char **extrap)
if (__glibc_unlikely (num_isstd > num_types || num_isgmt > num_types))
goto lose;
- /* For platforms with 64-bit time_t we use the new format if available. */
- if (sizeof (time_t) == 8 && trans_width == 4
- && tzhead.tzh_version[0] != '\0')
+ if (trans_width == 4 && tzhead.tzh_version[0] != '\0')
{
/* We use the 8-byte format. */
trans_width = 8;
@@ -222,9 +220,9 @@ __tzfile_read (const char *file, size_t extra, char **extrap)
if (__builtin_expect (num_transitions
> ((SIZE_MAX - (__alignof__ (struct ttinfo) - 1))
- / (sizeof (time_t) + 1)), 0))
+ / (sizeof (internal_time_t) + 1)), 0))
goto lose;
- total_size = num_transitions * (sizeof (time_t) + 1);
+ total_size = num_transitions * (sizeof (internal_time_t) + 1);
total_size = ((total_size + __alignof__ (struct ttinfo) - 1)
& ~(__alignof__ (struct ttinfo) - 1));
types_idx = total_size;
@@ -246,7 +244,7 @@ __tzfile_read (const char *file, size_t extra, char **extrap)
goto lose;
total_size += num_leaps * sizeof (struct leap);
tzspec_len = 0;
- if (sizeof (time_t) == 8 && trans_width == 8)
+ if (trans_width == 8)
{
off_t rem = st.st_size - __ftello (f);
if (__builtin_expect (rem < 0
@@ -276,23 +274,23 @@ __tzfile_read (const char *file, size_t extra, char **extrap)
/* Allocate enough memory including the extra block requested by the
caller. */
- transitions = (time_t *) malloc (total_size + tzspec_len + extra);
+ transitions = malloc (total_size + tzspec_len + extra);
if (transitions == NULL)
goto lose;
type_idxs = (unsigned char *) transitions + (num_transitions
- * sizeof (time_t));
+ * sizeof (internal_time_t));
types = (struct ttinfo *) ((char *) transitions + types_idx);
zone_names = (char *) types + num_types * sizeof (struct ttinfo);
leaps = (struct leap *) ((char *) transitions + leaps_idx);
- if (sizeof (time_t) == 8 && trans_width == 8)
+ if (trans_width == 8)
tzspec = (char *) leaps + num_leaps * sizeof (struct leap) + extra;
else
tzspec = NULL;
if (extra > 0)
*extrap = (char *) &leaps[num_leaps];
- if (sizeof (time_t) == 4 || __builtin_expect (trans_width == 8, 1))
+ if (__builtin_expect (trans_width == 8, 1))
{
if (__builtin_expect (__fread_unlocked (transitions, trans_width + 1,
num_transitions, f)
@@ -315,19 +313,18 @@ __tzfile_read (const char *file, size_t extra, char **extrap)
if (__glibc_unlikely (type_idxs[i] >= num_types))
goto lose;
- if ((BYTE_ORDER != BIG_ENDIAN && (sizeof (time_t) == 4 || trans_width == 4))
- || (BYTE_ORDER == BIG_ENDIAN && sizeof (time_t) == 8
- && trans_width == 4))
+ if ((BYTE_ORDER != BIG_ENDIAN && trans_width == 4)
+ || (BYTE_ORDER == BIG_ENDIAN && trans_width == 4))
{
/* Decode the transition times, stored as 4-byte integers in
- network (big-endian) byte order. We work from the end of
- the array so as not to clobber the next element to be
- processed when sizeof (time_t) > 4. */
+ network (big-endian) byte order. We work from the end of the
+ array so as not to clobber the next element to be
+ processed. */
i = num_transitions;
while (i-- > 0)
transitions[i] = decode ((char *) transitions + i * 4);
}
- else if (BYTE_ORDER != BIG_ENDIAN && sizeof (time_t) == 8)
+ else if (BYTE_ORDER != BIG_ENDIAN)
{
/* Decode the transition times, stored as 8-byte integers in
network (big-endian) byte order. */
@@ -364,10 +361,10 @@ __tzfile_read (const char *file, size_t extra, char **extrap)
if (__builtin_expect (__fread_unlocked (x, 1, trans_width, f)
!= trans_width, 0))
goto lose;
- if (sizeof (time_t) == 4 || trans_width == 4)
- leaps[i].transition = (time_t) decode (x);
+ if (trans_width == 4)
+ leaps[i].transition = decode (x);
else
- leaps[i].transition = (time_t) decode64 (x);
+ leaps[i].transition = decode64 (x);
if (__glibc_unlikely (__fread_unlocked (x, 1, 4, f) != 4))
goto lose;
@@ -395,7 +392,7 @@ __tzfile_read (const char *file, size_t extra, char **extrap)
types[i++].isgmt = 0;
/* Read the POSIX TZ-style information if possible. */
- if (sizeof (time_t) == 8 && tzspec != NULL)
+ if (tzspec != NULL)
{
/* Skip over the newline first. */
if (__getc_unlocked (f) != '\n'
@@ -405,52 +402,6 @@ __tzfile_read (const char *file, size_t extra, char **extrap)
else
tzspec[tzspec_len - 1] = '\0';
}
- else if (sizeof (time_t) == 4 && tzhead.tzh_version[0] != '\0')
- {
- /* Get the TZ string. */
- if (__builtin_expect (__fread_unlocked ((void *) &tzhead,
- sizeof (tzhead), 1, f) != 1, 0)
- || (memcmp (tzhead.tzh_magic, TZ_MAGIC, sizeof (tzhead.tzh_magic))
- != 0))
- goto lose;
-
- size_t num_transitions2 = (size_t) decode (tzhead.tzh_timecnt);
- size_t num_types2 = (size_t) decode (tzhead.tzh_typecnt);
- size_t chars2 = (size_t) decode (tzhead.tzh_charcnt);
- size_t num_leaps2 = (size_t) decode (tzhead.tzh_leapcnt);
- size_t num_isstd2 = (size_t) decode (tzhead.tzh_ttisstdcnt);
- size_t num_isgmt2 = (size_t) decode (tzhead.tzh_ttisgmtcnt);
-
- /* Position the stream before the second header. */
- size_t to_skip = (num_transitions2 * (8 + 1)
- + num_types2 * 6
- + chars2
- + num_leaps2 * 12
- + num_isstd2
- + num_isgmt2);
- off_t off;
- if (fseek (f, to_skip, SEEK_CUR) != 0
- || (off = __ftello (f)) < 0
- || st.st_size < off + 2)
- goto lose;
-
- tzspec_len = st.st_size - off - 1;
- if (tzspec_len == 0)
- goto lose;
- char *tzstr = malloc (tzspec_len);
- if (tzstr == NULL)
- goto lose;
- if (__getc_unlocked (f) != '\n'
- || (__fread_unlocked (tzstr, 1, tzspec_len - 1, f)
- != tzspec_len - 1))
- {
- free (tzstr);
- goto lose;
- }
- tzstr[tzspec_len - 1] = '\0';
- tzspec = __tzstring (tzstr);
- free (tzstr);
- }
/* Don't use an empty TZ string. */
if (tzspec != NULL && tzspec[0] == '\0')
@@ -630,7 +581,7 @@ __tzfile_default (const char *std, const char *dst,
}
\f
void
-__tzfile_compute (time_t timer, int use_localtime,
+__tzfile_compute (internal_time_t timer, int use_localtime,
long int *leap_correct, int *leap_hit,
struct tm *tp)
{
@@ -685,10 +636,16 @@ __tzfile_compute (time_t timer, int use_localtime,
/* Convert to broken down structure. If this fails do not
use the string. */
- if (__glibc_unlikely (! __offtime (&timer, 0, tp)))
- goto use_last;
+ {
+ time_t truncated = timer;
+ if (__glibc_unlikely (truncated != timer
+ || ! __offtime (&truncated, 0, tp)))
+ goto use_last;
+ }
- /* Use the rules from the TZ string to compute the change. */
+ /* Use the rules from the TZ string to compute the change.
+ timer fits into time_t due to the truncation check
+ above. */
__tz_compute (timer, tp, 1);
/* If tzspec comes from posixrules loaded by __tzfile_default,
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] Use 64-bit time_t for time zone file parsing
2018-05-08 13:16 [PATCH] Use 64-bit time_t for time zone file parsing Florian Weimer
@ 2018-05-08 14:05 ` Andreas Schwab
2018-05-08 14:29 ` Florian Weimer
0 siblings, 1 reply; 7+ messages in thread
From: Andreas Schwab @ 2018-05-08 14:05 UTC (permalink / raw)
To: Florian Weimer; +Cc: GNU C Library, Albert ARIBAUD
On Mai 08 2018, Florian Weimer <fweimer@redhat.com> wrote:
> + if ((BYTE_ORDER != BIG_ENDIAN && trans_width == 4)
> + || (BYTE_ORDER == BIG_ENDIAN && trans_width == 4))
Redundant condition.
Andreas.
--
Andreas Schwab, SUSE Labs, schwab@suse.de
GPG Key fingerprint = 0196 BAD8 1CE9 1970 F4BE 1748 E4D4 88E3 0EEA B9D7
"And now for something completely different."
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] Use 64-bit time_t for time zone file parsing
2018-05-08 14:05 ` Andreas Schwab
@ 2018-05-08 14:29 ` Florian Weimer
2018-05-10 2:04 ` Paul Eggert
0 siblings, 1 reply; 7+ messages in thread
From: Florian Weimer @ 2018-05-08 14:29 UTC (permalink / raw)
To: Andreas Schwab; +Cc: GNU C Library, Albert ARIBAUD
[-- Attachment #1: Type: text/plain, Size: 315 bytes --]
On 05/08/2018 04:05 PM, Andreas Schwab wrote:
> On Mai 08 2018, Florian Weimer <fweimer@redhat.com> wrote:
>
>> + if ((BYTE_ORDER != BIG_ENDIAN && trans_width == 4)
>> + || (BYTE_ORDER == BIG_ENDIAN && trans_width == 4))
>
> Redundant condition.
Ah. I'm dropping the check of BYTE_ORDER.
Thanks,
Florian
[-- Attachment #2: internal_time_t-2.patch --]
[-- Type: text/x-patch, Size: 9538 bytes --]
Subject: [PATCH] time: Use 64-bit time values for time zone parsing
To: libc-alpha@sourceware.org
2018-05-08 Florian Weimer <fweimer@redhat.com>
Use 64-bit epoch values in the time zone file parser.
* include/time.h (internal_time_t): Define.
(__tzfile_compute): Use it.
* time/tzfile.c (struct leap): Use internal_time_t for epoch
member.
(transitions): Switch to internal_time_t.
(__tzfile_read): Likewise. Remove code dealing with 4-byte time_t
types.
(__tzfile_compute): Use internal_time_t for timer argument. Check
for truncation before calling __offtime.
diff --git a/include/time.h b/include/time.h
index aab26d7768..23d2580528 100644
--- a/include/time.h
+++ b/include/time.h
@@ -26,6 +26,10 @@ extern __typeof (clock_getcpuclockid) __clock_getcpuclockid;
/* Now define the internal interfaces. */
struct tm;
+/* time_t variant for representing time zone data, independent of
+ time_t. */
+typedef __int64_t internal_time_t;
+
/* Defined in mktime.c. */
extern const unsigned short int __mon_yday[2][13] attribute_hidden;
@@ -39,7 +43,7 @@ extern int __use_tzfile attribute_hidden;
extern void __tzfile_read (const char *file, size_t extra,
char **extrap) attribute_hidden;
-extern void __tzfile_compute (time_t timer, int use_localtime,
+extern void __tzfile_compute (internal_time_t timer, int use_localtime,
long int *leap_correct, int *leap_hit,
struct tm *tp) attribute_hidden;
extern void __tzfile_default (const char *std, const char *dst,
diff --git a/time/tzfile.c b/time/tzfile.c
index 3e39723148..0ffc1a224a 100644
--- a/time/tzfile.c
+++ b/time/tzfile.c
@@ -44,12 +44,12 @@ struct ttinfo
struct leap
{
- time_t transition; /* Time the transition takes effect. */
+ internal_time_t transition; /* Time the transition takes effect. */
long int change; /* Seconds of correction to apply. */
};
static size_t num_transitions;
-libc_freeres_ptr (static time_t *transitions);
+libc_freeres_ptr (static internal_time_t *transitions);
static unsigned char *type_idxs;
static size_t num_types;
static struct ttinfo *types;
@@ -113,8 +113,8 @@ __tzfile_read (const char *file, size_t extra, char **extrap)
size_t tzspec_len;
char *new = NULL;
- if (sizeof (time_t) != 4 && sizeof (time_t) != 8)
- abort ();
+ _Static_assert (sizeof (internal_time_t) == 8,
+ "internal_time_t must be eight bytes");
__use_tzfile = 0;
@@ -200,9 +200,7 @@ __tzfile_read (const char *file, size_t extra, char **extrap)
if (__glibc_unlikely (num_isstd > num_types || num_isgmt > num_types))
goto lose;
- /* For platforms with 64-bit time_t we use the new format if available. */
- if (sizeof (time_t) == 8 && trans_width == 4
- && tzhead.tzh_version[0] != '\0')
+ if (trans_width == 4 && tzhead.tzh_version[0] != '\0')
{
/* We use the 8-byte format. */
trans_width = 8;
@@ -222,9 +220,9 @@ __tzfile_read (const char *file, size_t extra, char **extrap)
if (__builtin_expect (num_transitions
> ((SIZE_MAX - (__alignof__ (struct ttinfo) - 1))
- / (sizeof (time_t) + 1)), 0))
+ / (sizeof (internal_time_t) + 1)), 0))
goto lose;
- total_size = num_transitions * (sizeof (time_t) + 1);
+ total_size = num_transitions * (sizeof (internal_time_t) + 1);
total_size = ((total_size + __alignof__ (struct ttinfo) - 1)
& ~(__alignof__ (struct ttinfo) - 1));
types_idx = total_size;
@@ -246,7 +244,7 @@ __tzfile_read (const char *file, size_t extra, char **extrap)
goto lose;
total_size += num_leaps * sizeof (struct leap);
tzspec_len = 0;
- if (sizeof (time_t) == 8 && trans_width == 8)
+ if (trans_width == 8)
{
off_t rem = st.st_size - __ftello (f);
if (__builtin_expect (rem < 0
@@ -276,23 +274,23 @@ __tzfile_read (const char *file, size_t extra, char **extrap)
/* Allocate enough memory including the extra block requested by the
caller. */
- transitions = (time_t *) malloc (total_size + tzspec_len + extra);
+ transitions = malloc (total_size + tzspec_len + extra);
if (transitions == NULL)
goto lose;
type_idxs = (unsigned char *) transitions + (num_transitions
- * sizeof (time_t));
+ * sizeof (internal_time_t));
types = (struct ttinfo *) ((char *) transitions + types_idx);
zone_names = (char *) types + num_types * sizeof (struct ttinfo);
leaps = (struct leap *) ((char *) transitions + leaps_idx);
- if (sizeof (time_t) == 8 && trans_width == 8)
+ if (trans_width == 8)
tzspec = (char *) leaps + num_leaps * sizeof (struct leap) + extra;
else
tzspec = NULL;
if (extra > 0)
*extrap = (char *) &leaps[num_leaps];
- if (sizeof (time_t) == 4 || __builtin_expect (trans_width == 8, 1))
+ if (__builtin_expect (trans_width == 8, 1))
{
if (__builtin_expect (__fread_unlocked (transitions, trans_width + 1,
num_transitions, f)
@@ -315,19 +313,17 @@ __tzfile_read (const char *file, size_t extra, char **extrap)
if (__glibc_unlikely (type_idxs[i] >= num_types))
goto lose;
- if ((BYTE_ORDER != BIG_ENDIAN && (sizeof (time_t) == 4 || trans_width == 4))
- || (BYTE_ORDER == BIG_ENDIAN && sizeof (time_t) == 8
- && trans_width == 4))
+ if (trans_width == 4)
{
/* Decode the transition times, stored as 4-byte integers in
- network (big-endian) byte order. We work from the end of
- the array so as not to clobber the next element to be
- processed when sizeof (time_t) > 4. */
+ network (big-endian) byte order. We work from the end of the
+ array so as not to clobber the next element to be
+ processed. */
i = num_transitions;
while (i-- > 0)
transitions[i] = decode ((char *) transitions + i * 4);
}
- else if (BYTE_ORDER != BIG_ENDIAN && sizeof (time_t) == 8)
+ else if (BYTE_ORDER != BIG_ENDIAN)
{
/* Decode the transition times, stored as 8-byte integers in
network (big-endian) byte order. */
@@ -364,10 +360,10 @@ __tzfile_read (const char *file, size_t extra, char **extrap)
if (__builtin_expect (__fread_unlocked (x, 1, trans_width, f)
!= trans_width, 0))
goto lose;
- if (sizeof (time_t) == 4 || trans_width == 4)
- leaps[i].transition = (time_t) decode (x);
+ if (trans_width == 4)
+ leaps[i].transition = decode (x);
else
- leaps[i].transition = (time_t) decode64 (x);
+ leaps[i].transition = decode64 (x);
if (__glibc_unlikely (__fread_unlocked (x, 1, 4, f) != 4))
goto lose;
@@ -395,7 +391,7 @@ __tzfile_read (const char *file, size_t extra, char **extrap)
types[i++].isgmt = 0;
/* Read the POSIX TZ-style information if possible. */
- if (sizeof (time_t) == 8 && tzspec != NULL)
+ if (tzspec != NULL)
{
/* Skip over the newline first. */
if (__getc_unlocked (f) != '\n'
@@ -405,52 +401,6 @@ __tzfile_read (const char *file, size_t extra, char **extrap)
else
tzspec[tzspec_len - 1] = '\0';
}
- else if (sizeof (time_t) == 4 && tzhead.tzh_version[0] != '\0')
- {
- /* Get the TZ string. */
- if (__builtin_expect (__fread_unlocked ((void *) &tzhead,
- sizeof (tzhead), 1, f) != 1, 0)
- || (memcmp (tzhead.tzh_magic, TZ_MAGIC, sizeof (tzhead.tzh_magic))
- != 0))
- goto lose;
-
- size_t num_transitions2 = (size_t) decode (tzhead.tzh_timecnt);
- size_t num_types2 = (size_t) decode (tzhead.tzh_typecnt);
- size_t chars2 = (size_t) decode (tzhead.tzh_charcnt);
- size_t num_leaps2 = (size_t) decode (tzhead.tzh_leapcnt);
- size_t num_isstd2 = (size_t) decode (tzhead.tzh_ttisstdcnt);
- size_t num_isgmt2 = (size_t) decode (tzhead.tzh_ttisgmtcnt);
-
- /* Position the stream before the second header. */
- size_t to_skip = (num_transitions2 * (8 + 1)
- + num_types2 * 6
- + chars2
- + num_leaps2 * 12
- + num_isstd2
- + num_isgmt2);
- off_t off;
- if (fseek (f, to_skip, SEEK_CUR) != 0
- || (off = __ftello (f)) < 0
- || st.st_size < off + 2)
- goto lose;
-
- tzspec_len = st.st_size - off - 1;
- if (tzspec_len == 0)
- goto lose;
- char *tzstr = malloc (tzspec_len);
- if (tzstr == NULL)
- goto lose;
- if (__getc_unlocked (f) != '\n'
- || (__fread_unlocked (tzstr, 1, tzspec_len - 1, f)
- != tzspec_len - 1))
- {
- free (tzstr);
- goto lose;
- }
- tzstr[tzspec_len - 1] = '\0';
- tzspec = __tzstring (tzstr);
- free (tzstr);
- }
/* Don't use an empty TZ string. */
if (tzspec != NULL && tzspec[0] == '\0')
@@ -630,7 +580,7 @@ __tzfile_default (const char *std, const char *dst,
}
\f
void
-__tzfile_compute (time_t timer, int use_localtime,
+__tzfile_compute (internal_time_t timer, int use_localtime,
long int *leap_correct, int *leap_hit,
struct tm *tp)
{
@@ -685,10 +635,16 @@ __tzfile_compute (time_t timer, int use_localtime,
/* Convert to broken down structure. If this fails do not
use the string. */
- if (__glibc_unlikely (! __offtime (&timer, 0, tp)))
- goto use_last;
+ {
+ time_t truncated = timer;
+ if (__glibc_unlikely (truncated != timer
+ || ! __offtime (&truncated, 0, tp)))
+ goto use_last;
+ }
- /* Use the rules from the TZ string to compute the change. */
+ /* Use the rules from the TZ string to compute the change.
+ timer fits into time_t due to the truncation check
+ above. */
__tz_compute (timer, tp, 1);
/* If tzspec comes from posixrules loaded by __tzfile_default,
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] Use 64-bit time_t for time zone file parsing
2018-05-08 14:29 ` Florian Weimer
@ 2018-05-10 2:04 ` Paul Eggert
2018-05-11 14:34 ` Florian Weimer
2018-05-17 21:06 ` Joseph Myers
0 siblings, 2 replies; 7+ messages in thread
From: Paul Eggert @ 2018-05-10 2:04 UTC (permalink / raw)
To: Florian Weimer, Andreas Schwab; +Cc: GNU C Library, Albert ARIBAUD
On 05/08/2018 07:29 AM, Florian Weimer wrote:
> Ah. I'm dropping the check of BYTE_ORDER.
Thanks, this looks good to me. One minor suggestion. This line:
i = (transitions[num_transitions - 1] - timer) / 15778476;
is now trickier than it used to be. Although the code still works since
it's just computing a guess, this new behavior might be worth a comment,
something along the lines "If size_t is 32 bits the assignment to I
could in theory overflow and wrap around, but this is harmless."
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] Use 64-bit time_t for time zone file parsing
2018-05-10 2:04 ` Paul Eggert
@ 2018-05-11 14:34 ` Florian Weimer
2018-05-17 21:06 ` Joseph Myers
1 sibling, 0 replies; 7+ messages in thread
From: Florian Weimer @ 2018-05-11 14:34 UTC (permalink / raw)
To: Paul Eggert, Andreas Schwab; +Cc: GNU C Library, Albert ARIBAUD
On 05/10/2018 04:04 AM, Paul Eggert wrote:
> On 05/08/2018 07:29 AM, Florian Weimer wrote:
>> Ah. I'm dropping the check of BYTE_ORDER.
>
> Thanks, this looks good to me. One minor suggestion. This line:
>
> Â Â Â Â i = (transitions[num_transitions - 1] - timer) / 15778476;
>
> is now trickier than it used to be. Although the code still works since
> it's just computing a guess, this new behavior might be worth a comment,
> something along the lines "If size_t is 32 bits the assignment to I
> could in theory overflow and wrap around, but this is harmless."
Thanks, I added a comment and pushed thechange.
Florian
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] Use 64-bit time_t for time zone file parsing
2018-05-10 2:04 ` Paul Eggert
2018-05-11 14:34 ` Florian Weimer
@ 2018-05-17 21:06 ` Joseph Myers
2018-05-18 12:12 ` Florian Weimer
1 sibling, 1 reply; 7+ messages in thread
From: Joseph Myers @ 2018-05-17 21:06 UTC (permalink / raw)
To: Paul Eggert; +Cc: Florian Weimer, Andreas Schwab, GNU C Library, Albert ARIBAUD
Does this use of 64-bit time_t fix bug 19738 ("__tzfile_default 32-bit
time_t overflow")? If it does, we should add a testcase for that bug then
resolve it as FIXED in 2.28.
--
Joseph S. Myers
joseph@codesourcery.com
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] Use 64-bit time_t for time zone file parsing
2018-05-17 21:06 ` Joseph Myers
@ 2018-05-18 12:12 ` Florian Weimer
0 siblings, 0 replies; 7+ messages in thread
From: Florian Weimer @ 2018-05-18 12:12 UTC (permalink / raw)
To: Joseph Myers, Paul Eggert; +Cc: Andreas Schwab, GNU C Library, Albert ARIBAUD
[-- Attachment #1: Type: text/plain, Size: 599 bytes --]
On 05/17/2018 11:06 PM, Joseph Myers wrote:
> Does this use of 64-bit time_t fix bug 19738 ("__tzfile_default 32-bit
> time_t overflow")? If it does, we should add a testcase for that bug then
> resolve it as FIXED in 2.28.
Looks like it. Here's a test case which fails before on i386 (assuming
that the system posixrules file as the required contents) and passes
afterwards.
I would call the bug fixed because dates where the overflow occurs with
a 64-bit (internal) time_t are after the expected life-time of the sun,
where daylight savings time is certainly meaningless.
Thanks,
Florian
[-- Attachment #2: bug19738.patch --]
[-- Type: text/x-patch, Size: 2421 bytes --]
Subject: [PATCH] time: Add test case for bug 19738
To: libc-alpha@sourceware.org
2018-05-18 Florian Weimer <fweimer@redhat.com>
[BZ #19738]
* time/tst-posixrules-overflow.c (do_test): New file.
* time/Makefile (tests): Add it.
diff --git a/time/Makefile b/time/Makefile
index 0db1206820..5454fe37e3 100644
--- a/time/Makefile
+++ b/time/Makefile
@@ -43,7 +43,7 @@ tests := test_time clocktest tst-posixtz tst-strptime tst_wcsftime \
tst-getdate tst-mktime tst-mktime2 tst-ftime_l tst-strftime \
tst-mktime3 tst-strptime2 bug-asctime bug-asctime_r bug-mktime1 \
tst-strptime3 bug-getdate1 tst-strptime-whitespace tst-ftime \
- tst-tzname
+ tst-tzname tst-posixrules-overflow
include ../Rules
diff --git a/time/tst-posixrules-overflow.c b/time/tst-posixrules-overflow.c
new file mode 100644
index 0000000000..13d0789a93
--- /dev/null
+++ b/time/tst-posixrules-overflow.c
@@ -0,0 +1,43 @@
+/* Check processing of posixrules/TZDEFRULES (bug 19738).
+ Copyright (C) 2018 Free Software Foundation, Inc.
+ This file is part of the GNU C Library.
+
+ The GNU C Library is free software; you can redistribute it and/or
+ modify it under the terms of the GNU Lesser General Public
+ License as published by the Free Software Foundation; either
+ version 2.1 of the License, or (at your option) any later version.
+
+ The GNU C Library is distributed in the hope that it will be useful,
+ but WITHOUT ANY WARRANTY; without even the implied warranty of
+ MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ Lesser General Public License for more details.
+
+ You should have received a copy of the GNU Lesser General Public
+ License along with the GNU C Library; if not, see
+ <http://www.gnu.org/licenses/>. */
+
+#include <stdlib.h>
+#include <support/check.h>
+#include <time.h>
+
+static int
+do_test (void)
+{
+ TEST_COMPARE (setenv ("TZ", "STD+8DST", 1), 0);
+ tzset ();
+ time_t epoch = 1467414184;
+ struct tm *tm = localtime (&epoch);
+ TEST_VERIFY_EXIT (tm != NULL);
+ TEST_COMPARE (tm->tm_year, 2016 - 1900);
+ TEST_COMPARE (tm->tm_mon, 7 - 1);
+ TEST_COMPARE (tm->tm_mday, 1);
+ TEST_COMPARE (tm->tm_hour, 16);
+ TEST_COMPARE (tm->tm_min, 3);
+ TEST_COMPARE (tm->tm_sec, 4);
+ TEST_COMPARE (tm->tm_wday, 5);
+ TEST_COMPARE (tm->tm_yday, 183 - 1);
+ TEST_VERIFY (tm->tm_isdst);
+ return 0;
+}
+
+#include <support/test-driver.c>
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2018-05-18 12:12 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-05-08 13:16 [PATCH] Use 64-bit time_t for time zone file parsing Florian Weimer
2018-05-08 14:05 ` Andreas Schwab
2018-05-08 14:29 ` Florian Weimer
2018-05-10 2:04 ` Paul Eggert
2018-05-11 14:34 ` Florian Weimer
2018-05-17 21:06 ` Joseph Myers
2018-05-18 12:12 ` Florian Weimer
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).