On 11/7/18 6:48 AM, Albert ARIBAUD wrote:
1. If the for-loop reaches remaining_probes==0, then it really should
set errno = EOVERFLOW before returning -1, because remaining_probes
is only decremented in the else clause inside the for-loop, and that
only happens (or should only happen) when there were no failures so
far, so if we fail then, we have to set errno.
Thanks for the diagnosis. Revised patches attached, which set errno in
that case as you suggested.
2. It is not normal that t, gt, t1 and t2 remain the same for all six
iterations of the for-loop. That should be investigated and fixed.
Long ago I came up with weird scenarios involving odd combinations of
leap seconds and DST transitions all near the maximum convertible time_t
values that could involve that many iterations. None of these scenarios
will ever happen, really; the number is that large just to be safe. It
could be that I overestimated the number, but that's no big deal.
I don't know why ranged_convert alters an argument which should be
a pure imput. In fact, I don't know why it does not just copy this
argument into a local time_t. Any known reason?
Because it communicates back to the caller the nearest long_int value
that is in range. This value is not obvious because it can depend on
timezone and leap second information.
After looking at the mktime implementation again I see some other things
that need fixing. These are mostly for Gnulib (when we can't assume that
localtime_r fails only due to EOVERFLOW), but there are some code
cleanups and fixes for very unlikely bugs. Proposed glibc patches attached.
From d7b1a2c6fda647672fd7774fc70cbe0b797af4e5 Mon Sep 17 00:00:00 2001
From: Paul Eggert <[email protected]>
Date: Wed, 7 Nov 2018 07:52:17 -0800
Subject: [PATCH 1/7] mktime: fix EOVERFLOW bug
[BZ#23789]
* time/mktime.c [!_LIBC && !DEBUG_MKTIME]:
Include libc-config.h, not config.h, for __set_errno.
(guess_time_tm, __mktime_internal): Set errno to EOVERFLOW on overflow.
---
ChangeLog | 8 ++++++++
time/mktime.c | 26 +++++++++++++++++++-------
2 files changed, 27 insertions(+), 7 deletions(-)
diff --git a/ChangeLog b/ChangeLog
index e43fd3e987..9f6d1d4edd 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,11 @@
+2018-11-09 Paul Eggert <[email protected]>
+
+ mktime: fix EOVERFLOW bug
+ [BZ#23789]
+ * time/mktime.c [!_LIBC && !DEBUG_MKTIME]:
+ Include libc-config.h, not config.h, for __set_errno.
+ (guess_time_tm, __mktime_internal): Set errno to EOVERFLOW on overflow.
+
2018-11-09 Martin Sebor <[email protected]>
* include/libc-symbols.h (__attribute_copy__): Define macro unless
diff --git a/time/mktime.c b/time/mktime.c
index 00f0dec6b4..106b4eac26 100644
--- a/time/mktime.c
+++ b/time/mktime.c
@@ -39,7 +39,7 @@
*/
#if !defined _LIBC && !DEBUG_MKTIME
-# include <config.h>
+# include <libc-config.h>
#endif
/* Assume that leap seconds are possible, unless told otherwise.
@@ -51,6 +51,7 @@
#include <time.h>
+#include <errno.h>
#include <limits.h>
#include <stdbool.h>
#include <stdlib.h>
@@ -255,8 +256,9 @@ long_int_avg (long_int a, long_int b)
If TP is null, return a value not equal to T; this avoids false matches.
YEAR and YDAY must not be so large that multiplying them by three times the
number of seconds in a year (or day, respectively) would overflow long_int.
- If the returned value would be out of range, yield the minimal or
- maximal in-range value, except do not yield a value equal to T. */
+ If TP is non-null and the returned value would be out of range, set
+ errno to EOVERFLOW and yield a minimal or maximal in-range value
+ that is not equal to T. */
static long_int
guess_time_tm (long_int year, long_int yday, int hour, int min, int sec,
long_int t, const struct tm *tp)
@@ -269,9 +271,10 @@ guess_time_tm (long_int year, long_int yday, int hour, int
min, int sec,
tp->tm_hour, tp->tm_min, tp->tm_sec);
if (! INT_ADD_WRAPV (t, d, &result))
return result;
+ __set_errno (EOVERFLOW);
}
- /* Overflow occurred one way or another. Return the nearest result
+ /* An error occurred, probably overflow. Return the nearest result
that is actually in range, except don't report a zero difference
if the actual difference is nonzero, as that would cause a false
match; and don't oscillate between two values, as that would
@@ -344,6 +347,8 @@ ranged_convert (struct tm *(*convert) (const time_t *,
struct tm *),
Use *OFFSET to keep track of a guess at the offset of the result,
compared to what the result would be for UTC without leap seconds.
If *OFFSET's guess is correct, only one CONVERT call is needed.
+ If successful, set *TP to the canonicalized struct tm;
+ otherwise leave *TP alone, return ((time_t) -1) and set errno.
This function is external because it is used also by timegm.c. */
time_t
__mktime_internal (struct tm *tp,
@@ -435,7 +440,10 @@ __mktime_internal (struct tm *tp,
useful than returning -1. */
goto offset_found;
else if (--remaining_probes == 0)
- return -1;
+ {
+ __set_errno (EOVERFLOW);
+ return -1;
+ }
/* We have a match. Check whether tm.tm_isdst has the requested
value, if any. */
@@ -505,8 +513,12 @@ __mktime_internal (struct tm *tp,
sec_adjustment -= sec;
sec_adjustment += sec_requested;
if (INT_ADD_WRAPV (t, sec_adjustment, &t)
- || ! (mktime_min <= t && t <= mktime_max)
- || ! convert_time (convert, t, &tm))
+ || ! (mktime_min <= t && t <= mktime_max))
+ {
+ __set_errno (EOVERFLOW);
+ return -1;
+ }
+ if (! convert_time (convert, t, &tm))
return -1;
}
--
2.19.1
From 423a60057ac9b775f964f7686c0f61000e0a4b03 Mon Sep 17 00:00:00 2001
From: Paul Eggert <[email protected]>
Date: Wed, 7 Nov 2018 07:52:18 -0800
Subject: [PATCH 2/7] mktime: new test for mktime failure
[BZ#23789]
Based on a test suggested by Albert Aribaud in:
https://www.sourceware.org/ml/libc-alpha/2018-10/msg00662.html
* time/Makefile (tests): Add bug-mktime4.
* time/bug-mktime4.c: New file.
---
ChangeLog | 7 ++++
time/Makefile | 2 +-
time/bug-mktime4.c | 92 ++++++++++++++++++++++++++++++++++++++++++++++
3 files changed, 100 insertions(+), 1 deletion(-)
create mode 100644 time/bug-mktime4.c
diff --git a/ChangeLog b/ChangeLog
index 9f6d1d4edd..e92ce16df9 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,5 +1,12 @@
2018-11-09 Paul Eggert <[email protected]>
+ mktime: new test for mktime failure
+ [BZ#23789]
+ Based on a test suggested by Albert Aribaud in:
+ https://www.sourceware.org/ml/libc-alpha/2018-10/msg00662.html
+ * time/Makefile (tests): Add bug-mktime4.
+ * time/bug-mktime4.c: New file.
+
mktime: fix EOVERFLOW bug
[BZ#23789]
* time/mktime.c [!_LIBC && !DEBUG_MKTIME]:
diff --git a/time/Makefile b/time/Makefile
index ec3e39dcea..743bd99f18 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-y2039
+ tst-tzname tst-y2039 bug-mktime4
include ../Rules
diff --git a/time/bug-mktime4.c b/time/bug-mktime4.c
new file mode 100644
index 0000000000..9c076eb623
--- /dev/null
+++ b/time/bug-mktime4.c
@@ -0,0 +1,92 @@
+#include <time.h>
+#include <errno.h>
+#include <limits.h>
+#include <stdbool.h>
+#include <stdio.h>
+#include <string.h>
+
+static bool
+equal_tm (struct tm const *t, struct tm const *u)
+{
+ return (t->tm_sec == u->tm_sec && t->tm_min == u->tm_min
+ && t->tm_hour == u->tm_hour && t->tm_mday == u->tm_mday
+ && t->tm_mon == u->tm_mon && t->tm_year == u->tm_year
+ && t->tm_wday == u->tm_wday && t->tm_yday == u->tm_yday
+ && t->tm_isdst == u->tm_isdst && t->tm_gmtoff == u->tm_gmtoff
+ && t->tm_zone == u->tm_zone);
+}
+
+static int
+do_test (void)
+{
+ /* Calculate minimum time_t value. This would be simpler with C11,
+ which has _Generic, but we cannot assume C11. It would also
+ be simpler with intprops.h, which has TYPE_MINIMUM, but it's
+ better not to use glibc internals. */
+ time_t time_t_min = -1;
+ time_t_min = (0 < time_t_min ? 0
+ : sizeof time_t_min == sizeof (long int) ? LONG_MIN
+ : sizeof time_t_min == sizeof (long long int) ? LLONG_MIN
+ : 1);
+ if (time_t_min == 1)
+ {
+ printf ("unknown time type\n");
+ return 1;
+ }
+ time_t ymin = time_t_min / 60 / 60 / 24 / 366;
+ bool mktime_should_fail = ymin == 0 || INT_MIN + 1900 < ymin + 1970;
+
+ struct tm tm0 = { .tm_year = INT_MIN, .tm_mday = 1, .tm_wday = -1 };
+ struct tm tm = tm0;
+ errno = 0;
+ time_t t = mktime (&tm);
+ long long int llt = t;
+ bool mktime_failed = tm.tm_wday == tm0.tm_wday;
+
+ if (mktime_failed)
+ {
+ if (! mktime_should_fail)
+ {
+ printf ("mktime failed but should have succeeded\n");
+ return 1;
+ }
+ if (errno == 0)
+ {
+ printf ("mktime failed without setting errno");
+ return 1;
+ }
+ if (t != (time_t) -1)
+ {
+ printf ("mktime returned %lld but did not set tm_wday\n", llt);
+ return 1;
+ }
+ if (! equal_tm (&tm, &tm0))
+ {
+ printf ("mktime (P) failed but modified *P\n");
+ return 1;
+ }
+ }
+ else
+ {
+ if (mktime_should_fail)
+ {
+ printf ("mktime succeeded but should have failed\n");
+ return 1;
+ }
+ struct tm *lt = localtime (&t);
+ if (lt == NULL)
+ {
+ printf ("mktime returned a value rejected by localtime\n");
+ return 1;
+ }
+ if (! equal_tm (lt, &tm))
+ {
+ printf ("mktime result does not match localtime result\n");
+ return 1;
+ }
+ }
+ return 0;
+}
+
+#define TIMEOUT 1000
+#include "support/test-driver.c"
--
2.19.1
From 48c1f7c67e85c9a23f8ae365627b6210426399fb Mon Sep 17 00:00:00 2001
From: Paul Eggert <[email protected]>
Date: Thu, 8 Nov 2018 08:26:21 -0800
Subject: [PATCH 3/7] mktime: simplify offset guess
[BZ#23789]
* time/mktime.c (__mktime_internal): Omit excess precision.
---
ChangeLog | 4 ++++
time/mktime.c | 6 +++---
2 files changed, 7 insertions(+), 3 deletions(-)
diff --git a/ChangeLog b/ChangeLog
index e92ce16df9..b7008a8972 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,5 +1,9 @@
2018-11-09 Paul Eggert <[email protected]>
+ mktime: simplify offset guess
+ [BZ#23789]
+ * time/mktime.c (__mktime_internal): Omit excess precision.
+
mktime: new test for mktime failure
[BZ#23789]
Based on a test suggested by Albert Aribaud in:
diff --git a/time/mktime.c b/time/mktime.c
index 106b4eac26..0f905eb8fe 100644
--- a/time/mktime.c
+++ b/time/mktime.c
@@ -355,7 +355,7 @@ __mktime_internal (struct tm *tp,
struct tm *(*convert) (const time_t *, struct tm *),
mktime_offset_t *offset)
{
- long_int t, gt, t0, t1, t2, dt;
+ long_int t, gt, t0, t1, t2;
struct tm tm;
/* The maximum number of probes (calls to CONVERT) should be enough
@@ -502,8 +502,8 @@ __mktime_internal (struct tm *tp,
/* Set *OFFSET to the low-order bits of T - T0 - NEGATIVE_OFFSET_GUESS.
This is just a heuristic to speed up the next mktime call, and
correctness is unaffected if integer overflow occurs here. */
- INT_SUBTRACT_WRAPV (t, t0, &dt);
- INT_SUBTRACT_WRAPV (dt, negative_offset_guess, offset);
+ INT_SUBTRACT_WRAPV (t, t0, offset);
+ INT_SUBTRACT_WRAPV (*offset, negative_offset_guess, offset);
if (LEAP_SECONDS_POSSIBLE && sec_requested != tm.tm_sec)
{
--
2.19.1
From 7678c7757fdf133db5edf17c7b3f6097b76e6ade Mon Sep 17 00:00:00 2001
From: Paul Eggert <[email protected]>
Date: Thu, 8 Nov 2018 10:20:50 -0800
Subject: [PATCH 4/7] mktime: make more room for overflow
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
[BZ#23789]
* time/mktime.c (long_int): Now 4⨯ int, not just 3⨯.
This is so that we can add tm_diff results to a previous guess,
which will be useful in a later patch.
---
ChangeLog | 6 ++++++
time/mktime.c | 20 +++++++++++---------
2 files changed, 17 insertions(+), 9 deletions(-)
diff --git a/ChangeLog b/ChangeLog
index b7008a8972..82cdd795ab 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,5 +1,11 @@
2018-11-09 Paul Eggert <[email protected]>
+ mktime: make more room for overflow
+ [BZ#23789]
+ * time/mktime.c (long_int): Now 4⨯ int, not just 3⨯.
+ This is so that we can add tm_diff results to a previous guess,
+ which will be useful in a later patch.
+
mktime: simplify offset guess
[BZ#23789]
* time/mktime.c (__mktime_internal): Omit excess precision.
diff --git a/time/mktime.c b/time/mktime.c
index 0f905eb8fe..ffbb5ea171 100644
--- a/time/mktime.c
+++ b/time/mktime.c
@@ -120,11 +120,12 @@ my_tzset (void)
#if defined _LIBC || NEED_MKTIME_WORKING || NEED_MKTIME_INTERNAL
/* A signed type that can represent an integer number of years
- multiplied by three times the number of seconds in a year. It is
+ multiplied by four times the number of seconds in a year. It is
needed when converting a tm_year value times the number of seconds
- in a year. The factor of three comes because these products need
+ in a year. The factor of four comes because these products need
to be subtracted from each other, and sometimes with an offset
- added to them, without worrying about overflow.
+ added to them, and then with another timestamp added, without
+ worrying about overflow.
Much of the code uses long_int to represent time_t values, to
lessen the hassle of dealing with platforms where time_t is
@@ -132,12 +133,12 @@ my_tzset (void)
time_t values that mktime can generate even on platforms where
time_t is excessively wide. */
-#if INT_MAX <= LONG_MAX / 3 / 366 / 24 / 60 / 60
+#if INT_MAX <= LONG_MAX / 4 / 366 / 24 / 60 / 60
typedef long int long_int;
#else
typedef long long int long_int;
#endif
-verify (INT_MAX <= TYPE_MAXIMUM (long_int) / 3 / 366 / 24 / 60 / 60);
+verify (INT_MAX <= TYPE_MAXIMUM (long_int) / 4 / 366 / 24 / 60 / 60);
/* Shift A right by B bits portably, by dividing A by 2**B and
truncating towards minus infinity. B should be in the range 0 <= B
@@ -211,9 +212,10 @@ isdst_differ (int a, int b)
were not adjusted between the timestamps.
The YEAR values uses the same numbering as TP->tm_year. Values
- need not be in the usual range. However, YEAR1 must not overflow
- when multiplied by three times the number of seconds in a year, and
- likewise for YDAY1 and three times the number of seconds in a day. */
+ need not be in the usual range. However, YEAR1 - YEAR0 must not
+ overflow even when multiplied by three times the number of seconds
+ in a year, and likewise for YDAY1 - YDAY0 and three times the
+ number of seconds in a day. */
static long_int
ydhms_diff (long_int year1, long_int yday1, int hour1, int min1, int sec1,
@@ -403,7 +405,7 @@ __mktime_internal (struct tm *tp,
if (LEAP_SECONDS_POSSIBLE)
{
/* Handle out-of-range seconds specially,
- since ydhms_tm_diff assumes every minute has 60 seconds. */
+ since ydhms_diff assumes every minute has 60 seconds. */
if (sec < 0)
sec = 0;
if (59 < sec)
--
2.19.1
From 19a49e00d5fab2ac24d36900dc810407fd05d12a Mon Sep 17 00:00:00 2001
From: Paul Eggert <[email protected]>
Date: Thu, 8 Nov 2018 10:30:36 -0800
Subject: [PATCH 5/7] mktime: fix bug with Y2038 DST transition
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
[BZ#23789]
* time/mktime.c (ranged_convert): On 32-bit platforms, don’t
mishandle a DST transition that jumps over the Y2038 boundary.
No such DST transitions are known so this is only a theoretical
bug, but we might as well do things right.
---
ChangeLog | 7 +++++++
time/mktime.c | 4 +++-
2 files changed, 10 insertions(+), 1 deletion(-)
diff --git a/ChangeLog b/ChangeLog
index 82cdd795ab..939ca140ef 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,5 +1,12 @@
2018-11-09 Paul Eggert <[email protected]>
+ mktime: fix bug with Y2038 DST transition
+ [BZ#23789]
+ * time/mktime.c (ranged_convert): On 32-bit platforms, don’t
+ mishandle a DST transition that jumps over the Y2038 boundary.
+ No such DST transitions are known so this is only a theoretical
+ bug, but we might as well do things right.
+
mktime: make more room for overflow
[BZ#23789]
* time/mktime.c (long_int): Now 4⨯ int, not just 3⨯.
diff --git a/time/mktime.c b/time/mktime.c
index ffbb5ea171..6d5b8cf838 100644
--- a/time/mktime.c
+++ b/time/mktime.c
@@ -323,7 +323,7 @@ ranged_convert (struct tm *(*convert) (const time_t *,
struct tm *),
while (true)
{
long_int mid = long_int_avg (ok, bad);
- if (mid != ok && mid != bad)
+ if (mid == ok || mid == bad)
break;
r = convert_time (convert, mid, tp);
if (r)
@@ -332,6 +332,8 @@ ranged_convert (struct tm *(*convert) (const time_t *,
struct tm *),
bad = mid;
}
+ *t = ok;
+
if (!r && ok)
{
/* The last conversion attempt failed;
--
2.19.1
From 67ece14d68a5b086f2b4a04d27b0d48f3f47bea3 Mon Sep 17 00:00:00 2001
From: Paul Eggert <[email protected]>
Date: Fri, 9 Nov 2018 17:33:24 -0800
Subject: [PATCH 6/7] mktime: fix non-EOVERFLOW errno handling
[BZ#23789]
mktime was not properly reporting failures when the underlying
localtime_r fails with errno != EOVERFLOW; it incorrectly treated
them like EOVERFLOW failures, and set errno to EOVERFLOW.
The problem could happen on non-glibc platforms, with Gnulib.
* time/mktime.c (guess_time_tm): Remove, replacing with ...
(tm_diff): ... this simpler function, which does not change errno.
All callers changed to deal with errno themselves.
(ranged_convert, __mktime_internal): Return failure immediately if
the underlying function reports any failure other than EOVERFLOW.
(__mktime_internal): Set errno to EOVERFLOW if the spring-forward
gap code fails.
---
ChangeLog | 14 ++++
time/mktime.c | 197 +++++++++++++++++++++++++-------------------------
2 files changed, 113 insertions(+), 98 deletions(-)
diff --git a/ChangeLog b/ChangeLog
index 939ca140ef..9f62ab865e 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,5 +1,19 @@
2018-11-09 Paul Eggert <[email protected]>
+ mktime: fix non-EOVERFLOW errno handling
+ [BZ#23789]
+ mktime was not properly reporting failures when the underlying
+ localtime_r fails with errno != EOVERFLOW; it incorrectly treated
+ them like EOVERFLOW failures, and set errno to EOVERFLOW.
+ The problem could happen on non-glibc platforms, with Gnulib.
+ * time/mktime.c (guess_time_tm): Remove, replacing with ...
+ (tm_diff): ... this simpler function, which does not change errno.
+ All callers changed to deal with errno themselves.
+ (ranged_convert, __mktime_internal): Return failure immediately if
+ the underlying function reports any failure other than EOVERFLOW.
+ (__mktime_internal): Set errno to EOVERFLOW if the spring-forward
+ gap code fails.
+
mktime: fix bug with Y2038 DST transition
[BZ#23789]
* time/mktime.c (ranged_convert): On 32-bit platforms, don’t
diff --git a/time/mktime.c b/time/mktime.c
index 6d5b8cf838..dc83985bbd 100644
--- a/time/mktime.c
+++ b/time/mktime.c
@@ -250,45 +250,25 @@ long_int_avg (long_int a, long_int b)
return shr (a, 1) + shr (b, 1) + ((a | b) & 1);
}
-/* Return a time_t value corresponding to (YEAR-YDAY HOUR:MIN:SEC),
- assuming that T corresponds to *TP and that no clock adjustments
- occurred between *TP and the desired time.
- Although T and the returned value are of type long_int,
- they represent time_t values and must be in time_t range.
- If TP is null, return a value not equal to T; this avoids false matches.
+/* Return a long_int value corresponding to (YEAR-YDAY HOUR:MIN:SEC)
+ minus *TP seconds, assuming no clock adjustments occurred between
+ the two timestamps.
+
YEAR and YDAY must not be so large that multiplying them by three times the
number of seconds in a year (or day, respectively) would overflow long_int.
- If TP is non-null and the returned value would be out of range, set
- errno to EOVERFLOW and yield a minimal or maximal in-range value
- that is not equal to T. */
+ *TP should be in the usual range. */
static long_int
-guess_time_tm (long_int year, long_int yday, int hour, int min, int sec,
- long_int t, const struct tm *tp)
+tm_diff (long_int year, long_int yday, int hour, int min, int sec,
+ struct tm const *tp)
{
- if (tp)
- {
- long_int result;
- long_int d = ydhms_diff (year, yday, hour, min, sec,
- tp->tm_year, tp->tm_yday,
- tp->tm_hour, tp->tm_min, tp->tm_sec);
- if (! INT_ADD_WRAPV (t, d, &result))
- return result;
- __set_errno (EOVERFLOW);
- }
-
- /* An error occurred, probably overflow. Return the nearest result
- that is actually in range, except don't report a zero difference
- if the actual difference is nonzero, as that would cause a false
- match; and don't oscillate between two values, as that would
- confuse the spring-forward gap detector. */
- return (t < long_int_avg (mktime_min, mktime_max)
- ? (t <= mktime_min + 1 ? t + 1 : mktime_min)
- : (mktime_max - 1 <= t ? t - 1 : mktime_max));
+ return ydhms_diff (year, yday, hour, min, sec,
+ tp->tm_year, tp->tm_yday,
+ tp->tm_hour, tp->tm_min, tp->tm_sec);
}
/* Use CONVERT to convert T to a struct tm value in *TM. T must be in
- range for time_t. Return TM if successful, NULL if T is out of
- range for CONVERT. */
+ range for time_t. Return TM if successful, NULL (setting errno) on
+ failure. */
static struct tm *
convert_time (struct tm *(*convert) (const time_t *, struct tm *),
long_int t, struct tm *tm)
@@ -300,49 +280,48 @@ convert_time (struct tm *(*convert) (const time_t *,
struct tm *),
/* Use CONVERT to convert *T to a broken down time in *TP.
If *T is out of range for conversion, adjust it so that
it is the nearest in-range value and then convert that.
- A value is in range if it fits in both time_t and long_int. */
+ A value is in range if it fits in both time_t and long_int.
+ Return TP on success, NULL (setting errno) on failure. */
static struct tm *
ranged_convert (struct tm *(*convert) (const time_t *, struct tm *),
long_int *t, struct tm *tp)
{
- struct tm *r;
- if (*t < mktime_min)
- *t = mktime_min;
- else if (mktime_max < *t)
- *t = mktime_max;
- r = convert_time (convert, *t, tp);
-
- if (!r && *t)
+ long_int t1 = (*t < mktime_min ? mktime_min
+ : *t <= mktime_max ? *t : mktime_max);
+ struct tm *r = convert_time (convert, t1, tp);
+ if (r)
{
- long_int bad = *t;
- long_int ok = 0;
-
- /* BAD is a known unconvertible value, and OK is a known good one.
- Use binary search to narrow the range between BAD and OK until
- they differ by 1. */
- while (true)
- {
- long_int mid = long_int_avg (ok, bad);
- if (mid == ok || mid == bad)
- break;
- r = convert_time (convert, mid, tp);
- if (r)
- ok = mid;
- else
- bad = mid;
- }
+ *t = t1;
+ return r;
+ }
+ if (errno != EOVERFLOW)
+ return NULL;
- *t = ok;
+ long_int bad = t1;
+ long_int ok = 0;
+ struct tm oktm; oktm.tm_sec = -1;
- if (!r && ok)
- {
- /* The last conversion attempt failed;
- revert to the most recent successful attempt. */
- r = convert_time (convert, ok, tp);
- }
+ /* BAD is a known out-of-range value, and OK is a known in-range one.
+ Use binary search to narrow the range between BAD and OK until
+ they differ by 1. */
+ while (true)
+ {
+ long_int mid = long_int_avg (ok, bad);
+ if (mid == ok || mid == bad)
+ break;
+ if (convert_time (convert, mid, tp))
+ ok = mid, oktm = *tp;
+ else if (errno != EOVERFLOW)
+ return NULL;
+ else
+ bad = mid;
}
- return r;
+ if (oktm.tm_sec < 0)
+ return NULL;
+ *t = ok;
+ *tp = oktm;
+ return tp;
}
@@ -359,7 +338,6 @@ __mktime_internal (struct tm *tp,
struct tm *(*convert) (const time_t *, struct tm *),
mktime_offset_t *offset)
{
- long_int t, gt, t0, t1, t2;
struct tm tm;
/* The maximum number of probes (calls to CONVERT) should be enough
@@ -379,7 +357,7 @@ __mktime_internal (struct tm *tp,
int isdst = tp->tm_isdst;
/* 1 if the previous probe was DST. */
- int dst2;
+ int dst2 = 0;
/* Ensure that mon is in range, and set year accordingly. */
int mon_remainder = mon % 12;
@@ -418,36 +396,46 @@ __mktime_internal (struct tm *tp,
time. */
INT_SUBTRACT_WRAPV (0, off, &negative_offset_guess);
- t0 = ydhms_diff (year, yday, hour, min, sec,
- EPOCH_YEAR - TM_YEAR_BASE, 0, 0, 0, negative_offset_guess);
+ long_int t0 = ydhms_diff (year, yday, hour, min, sec,
+ EPOCH_YEAR - TM_YEAR_BASE, 0, 0, 0,
+ negative_offset_guess);
+ long_int t = t0, t1 = t0, t2 = t0;
/* Repeatedly use the error to improve the guess. */
- for (t = t1 = t2 = t0, dst2 = 0;
- (gt = guess_time_tm (year, yday, hour, min, sec, t,
- ranged_convert (convert, &t, &tm)),
- t != gt);
- t1 = t2, t2 = t, t = gt, dst2 = tm.tm_isdst != 0)
- if (t == t1 && t != t2
- && (tm.tm_isdst < 0
- || (isdst < 0
- ? dst2 <= (tm.tm_isdst != 0)
- : (isdst != 0) != (tm.tm_isdst != 0))))
- /* We can't possibly find a match, as we are oscillating
- between two values. The requested time probably falls
- within a spring-forward gap of size GT - T. Follow the common
- practice in this case, which is to return a time that is GT - T
- away from the requested time, preferring a time whose
- tm_isdst differs from the requested value. (If no tm_isdst
- was requested and only one of the two values has a nonzero
- tm_isdst, prefer that value.) In practice, this is more
- useful than returning -1. */
- goto offset_found;
- else if (--remaining_probes == 0)
- {
- __set_errno (EOVERFLOW);
+ while (true)
+ {
+ if (! ranged_convert (convert, &t, &tm))
return -1;
- }
+ long_int dt = tm_diff (year, yday, hour, min, sec, &tm);
+ if (dt == 0)
+ break;
+
+ if (t == t1 && t != t2
+ && (tm.tm_isdst < 0
+ || (isdst < 0
+ ? dst2 <= (tm.tm_isdst != 0)
+ : (isdst != 0) != (tm.tm_isdst != 0))))
+ /* We can't possibly find a match, as we are oscillating
+ between two values. The requested time probably falls
+ within a spring-forward gap of size DT. Follow the common
+ practice in this case, which is to return a time that is DT
+ away from the requested time, preferring a time whose
+ tm_isdst differs from the requested value. (If no tm_isdst
+ was requested and only one of the two values has a nonzero
+ tm_isdst, prefer that value.) In practice, this is more
+ useful than returning -1. */
+ goto offset_found;
+
+ remaining_probes--;
+ if (remaining_probes == 0)
+ {
+ __set_errno (EOVERFLOW);
+ return -1;
+ }
+
+ t1 = t2, t2 = t, t += dt, dst2 = tm.tm_isdst != 0;
+ }
/* We have a match. Check whether tm.tm_isdst has the requested
value, if any. */
@@ -489,17 +477,30 @@ __mktime_internal (struct tm *tp,
if (! INT_ADD_WRAPV (t, delta * direction, &ot))
{
struct tm otm;
- ranged_convert (convert, &ot, &otm);
+ if (! ranged_convert (convert, &ot, &otm))
+ return -1;
if (! isdst_differ (isdst, otm.tm_isdst))
{
/* We found the desired tm_isdst.
Extrapolate back to the desired time. */
- t = guess_time_tm (year, yday, hour, min, sec, ot, &otm);
- ranged_convert (convert, &t, &tm);
- goto offset_found;
+ long_int gt = ot + tm_diff (year, yday, hour, min, sec,
+ &otm);
+ if (mktime_min <= gt && gt <= mktime_max)
+ {
+ if (convert_time (convert, gt, &tm))
+ {
+ t = gt;
+ goto offset_found;
+ }
+ if (errno != EOVERFLOW)
+ return -1;
+ }
}
}
}
+
+ __set_errno (EOVERFLOW);
+ return -1;
}
offset_found:
--
2.19.1
From c1d348099f2253e18437e711c4de3d886f9bb8ca Mon Sep 17 00:00:00 2001
From: Paul Eggert <[email protected]>
Date: Fri, 9 Nov 2018 17:43:48 -0800
Subject: [PATCH 7/7] mktime: DEBUG_MKTIME cleanup
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
The DEBUG_MKTIME code no longer works in glibc or in Gnulib.
And it’s no longer needed now that glibc and Gnulib both have
their own testing mechanisms for mktime.
* time/mktime.c (DEBUG_MKTIME): Remove. All uses removed.
---
ChangeLog | 6 ++
time/mktime.c | 162 +-------------------------------------------------
2 files changed, 8 insertions(+), 160 deletions(-)
diff --git a/ChangeLog b/ChangeLog
index 9f62ab865e..e039728000 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,5 +1,11 @@
2018-11-09 Paul Eggert <[email protected]>
+ mktime: DEBUG_MKTIME cleanup
+ The DEBUG_MKTIME code no longer works in glibc or in Gnulib.
+ And it’s no longer needed now that glibc and Gnulib both have
+ their own testing mechanisms for mktime.
+ * time/mktime.c (DEBUG_MKTIME): Remove. All uses removed.
+
mktime: fix non-EOVERFLOW errno handling
[BZ#23789]
mktime was not properly reporting failures when the underlying
diff --git a/time/mktime.c b/time/mktime.c
index dc83985bbd..8faa9bc93d 100644
--- a/time/mktime.c
+++ b/time/mktime.c
@@ -17,12 +17,6 @@
License along with the GNU C Library; if not, see
<https://www.gnu.org/licenses/>. */
-/* Define this to 1 to have a standalone program to test this implementation of
- mktime. */
-#ifndef DEBUG_MKTIME
-# define DEBUG_MKTIME 0
-#endif
-
/* The following macros influence what gets defined when this file is compiled:
Macro/expression Which gnulib module This compilation unit
@@ -34,11 +28,9 @@
|| NEED_MKTIME_WINDOWS
NEED_MKTIME_INTERNAL mktime-internal mktime_internal
-
- DEBUG_MKTIME (defined manually) my_mktime, main
*/
-#if !defined _LIBC && !DEBUG_MKTIME
+#ifndef _LIBC
# include <libc-config.h>
#endif
@@ -60,13 +52,6 @@
#include <intprops.h>
#include <verify.h>
-#if DEBUG_MKTIME
-# include <stdio.h>
-/* Make it work even if the system's libc has its own mktime routine. */
-# undef mktime
-# define mktime my_mktime
-#endif /* DEBUG_MKTIME */
-
#ifndef NEED_MKTIME_INTERNAL
# define NEED_MKTIME_INTERNAL 0
#endif
@@ -74,7 +59,7 @@
# define NEED_MKTIME_WINDOWS 0
#endif
#ifndef NEED_MKTIME_WORKING
-# define NEED_MKTIME_WORKING DEBUG_MKTIME
+# define NEED_MKTIME_WORKING 0
#endif
#include "mktime-internal.h"
@@ -562,146 +547,3 @@ weak_alias (mktime, timelocal)
libc_hidden_def (mktime)
libc_hidden_weak (timelocal)
#endif
-
-#if DEBUG_MKTIME
-
-static int
-not_equal_tm (const struct tm *a, const struct tm *b)
-{
- return ((a->tm_sec ^ b->tm_sec)
- | (a->tm_min ^ b->tm_min)
- | (a->tm_hour ^ b->tm_hour)
- | (a->tm_mday ^ b->tm_mday)
- | (a->tm_mon ^ b->tm_mon)
- | (a->tm_year ^ b->tm_year)
- | (a->tm_yday ^ b->tm_yday)
- | isdst_differ (a->tm_isdst, b->tm_isdst));
-}
-
-static void
-print_tm (const struct tm *tp)
-{
- if (tp)
- printf ("%04d-%02d-%02d %02d:%02d:%02d yday %03d wday %d isdst %d",
- tp->tm_year + TM_YEAR_BASE, tp->tm_mon + 1, tp->tm_mday,
- tp->tm_hour, tp->tm_min, tp->tm_sec,
- tp->tm_yday, tp->tm_wday, tp->tm_isdst);
- else
- printf ("0");
-}
-
-static int
-check_result (time_t tk, struct tm tmk, time_t tl, const struct tm *lt)
-{
- if (tk != tl || !lt || not_equal_tm (&tmk, lt))
- {
- printf ("mktime (");
- print_tm (lt);
- printf (")\nyields (");
- print_tm (&tmk);
- printf (") == %ld, should be %ld\n", (long int) tk, (long int) tl);
- return 1;
- }
-
- return 0;
-}
-
-int
-main (int argc, char **argv)
-{
- int status = 0;
- struct tm tm, tmk, tml;
- struct tm *lt;
- time_t tk, tl, tl1;
- char trailer;
-
- /* Sanity check, plus call tzset. */
- tl = 0;
- if (! localtime (&tl))
- {
- printf ("localtime (0) fails\n");
- status = 1;
- }
-
- if ((argc == 3 || argc == 4)
- && (sscanf (argv[1], "%d-%d-%d%c",
- &tm.tm_year, &tm.tm_mon, &tm.tm_mday, &trailer)
- == 3)
- && (sscanf (argv[2], "%d:%d:%d%c",
- &tm.tm_hour, &tm.tm_min, &tm.tm_sec, &trailer)
- == 3))
- {
- tm.tm_year -= TM_YEAR_BASE;
- tm.tm_mon--;
- tm.tm_isdst = argc == 3 ? -1 : atoi (argv[3]);
- tmk = tm;
- tl = mktime (&tmk);
- lt = localtime_r (&tl, &tml);
- printf ("mktime returns %ld == ", (long int) tl);
- print_tm (&tmk);
- printf ("\n");
- status = check_result (tl, tmk, tl, lt);
- }
- else if (argc == 4 || (argc == 5 && strcmp (argv[4], "-") == 0))
- {
- time_t from = atol (argv[1]);
- time_t by = atol (argv[2]);
- time_t to = atol (argv[3]);
-
- if (argc == 4)
- for (tl = from; by < 0 ? to <= tl : tl <= to; tl = tl1)
- {
- lt = localtime_r (&tl, &tml);
- if (lt)
- {
- tmk = tml;
- tk = mktime (&tmk);
- status |= check_result (tk, tmk, tl, &tml);
- }
- else
- {
- printf ("localtime_r (%ld) yields 0\n", (long int) tl);
- status = 1;
- }
- tl1 = tl + by;
- if ((tl1 < tl) != (by < 0))
- break;
- }
- else
- for (tl = from; by < 0 ? to <= tl : tl <= to; tl = tl1)
- {
- /* Null benchmark. */
- lt = localtime_r (&tl, &tml);
- if (lt)
- {
- tmk = tml;
- tk = tl;
- status |= check_result (tk, tmk, tl, &tml);
- }
- else
- {
- printf ("localtime_r (%ld) yields 0\n", (long int) tl);
- status = 1;
- }
- tl1 = tl + by;
- if ((tl1 < tl) != (by < 0))
- break;
- }
- }
- else
- printf ("Usage:\
-\t%s YYYY-MM-DD HH:MM:SS [ISDST] # Test given time.\n\
-\t%s FROM BY TO # Test values FROM, FROM+BY, ..., TO.\n\
-\t%s FROM BY TO - # Do not test those values (for benchmark).\n",
- argv[0], argv[0], argv[0]);
-
- return status;
-}
-
-#endif /* DEBUG_MKTIME */
-
-/*
-Local Variables:
-compile-command: "gcc -DDEBUG_MKTIME -I. -Wall -W -O2 -g mktime.c -o mktime"
-End:
-*/
--
2.19.1
