Issue #13847: time.localtime() and time.gmtime() now raise an OSError instead

of ValueError on failure. time.ctime() and time.asctime() now raises an
OSError if localtime() failed.

3 files changed, 81 insertions, 52 deletions

diff --git a/Lib/test/test_time.py b/Lib/test/test_time.py
index 685e821..0533895 100644
--- a/Lib/test/test_time.py
+++ b/Lib/test/test_time.py
@@ -343,6 +343,21 @@ class TimeTestCase(unittest.TestCase):
         dt = t2 - t1
         self.assertAlmostEqual(dt, 0.1, delta=0.2)
 
+    def test_localtime_failure(self):
+        # Issue #13847: check for localtime() failure
+        invalid_time_t = 2**60
+        try:
+            time.localtime(invalid_time_t)
+        except ValueError as err:
+            if str(err) == "timestamp out of range for platform time_t":
+                self.skipTest("need 64-bit time_t")
+            else:
+                raise
+        except OSError:
+            pass
+        self.assertRaises(OSError, time.localtime, invalid_time_t)
+        self.assertRaises(OSError, time.gmtime, invalid_time_t)
+        self.assertRaises(OSError, time.ctime, invalid_time_t)
 
 class TestLocale(unittest.TestCase):
     def setUp(self):
diff --git a/Misc/NEWS b/Misc/NEWS
index 70d8095..320e386 100644
--- a/Misc/NEWS
+++ b/Misc/NEWS
@@ -461,6 +461,10 @@ Core and Builtins
 Library
 -------
 
+- Issue #13847: time.localtime() and time.gmtime() now raise an OSError instead
+  of ValueError on failure. time.ctime() and time.asctime() now raises an
+  OSError if localtime() failed.
+
 - Issue #13862: Fix spurious failure in test_zlib due to runtime/compile time
   minor versions not matching.
 
diff --git a/Modules/timemodule.c b/Modules/timemodule.c
index f7dac5b..d2cc62f 100644
--- a/Modules/timemodule.c
+++ b/Modules/timemodule.c
@@ -247,55 +247,53 @@ tmtotuple(struct tm *p)
     return v;
 }
 
-static PyObject *
-time_convert(double when, struct tm * (*function)(const time_t *))
-{
-    struct tm *p;
-    time_t whent = _PyTime_DoubleToTimet(when);
-
-    if (whent == (time_t)-1 && PyErr_Occurred())
-        return NULL;
-    errno = 0;
-    p = function(&whent);
-    if (p == NULL) {
-#ifdef EINVAL
-        if (errno == 0)
-            errno = EINVAL;
-#endif
-        return PyErr_SetFromErrno(PyExc_ValueError);
-    }
-    return tmtotuple(p);
-}
-
 /* Parse arg tuple that can contain an optional float-or-None value;
    format needs to be "|O:name".
    Returns non-zero on success (parallels PyArg_ParseTuple).
 */
 static int
-parse_time_double_args(PyObject *args, char *format, double *pwhen)
+parse_time_t_args(PyObject *args, char *format, time_t *pwhen)
 {
     PyObject *ot = NULL;
+    time_t whent;
 
     if (!PyArg_ParseTuple(args, format, &ot))
         return 0;
-    if (ot == NULL || ot == Py_None)
-        *pwhen = floattime();
+    if (ot == NULL || ot == Py_None) {
+        whent = time(NULL);
+    }
     else {
-        double when = PyFloat_AsDouble(ot);
+        double d = PyFloat_AsDouble(ot);
         if (PyErr_Occurred())
             return 0;
-        *pwhen = when;
+        whent = _PyTime_DoubleToTimet(d);
+        if (whent == (time_t)-1 && PyErr_Occurred())
+            return 0;
     }
+    *pwhen = whent;
     return 1;
 }
 
 static PyObject *
 time_gmtime(PyObject *self, PyObject *args)
 {
-    double when;
-    if (!parse_time_double_args(args, "|O:gmtime", &when))
+    time_t when;
+    struct tm buf, *local;
+
+    if (!parse_time_t_args(args, "|O:gmtime", &when))
         return NULL;
-    return time_convert(when, gmtime);
+
+    errno = 0;
+    local = gmtime(&when);
+    if (local == NULL) {
+#ifdef EINVAL
+        if (errno == 0)
+            errno = EINVAL;
+#endif
+        return PyErr_SetFromErrno(PyExc_OSError);
+    }
+    buf = *local;
+    return tmtotuple(&buf);
 }
 
 PyDoc_STRVAR(gmtime_doc,
@@ -305,13 +303,37 @@ PyDoc_STRVAR(gmtime_doc,
 Convert seconds since the Epoch to a time tuple expressing UTC (a.k.a.\n\
 GMT).  When 'seconds' is not passed in, convert the current time instead.");
 
+static int
+pylocaltime(time_t *timep, struct tm *result)
+{
+    struct tm *local;
+
+    assert (timep != NULL);
+    local = localtime(timep);
+    if (local == NULL) {
+        /* unconvertible time */
+#ifdef EINVAL
+        if (errno == 0)
+            errno = EINVAL;
+#endif
+        PyErr_SetFromErrno(PyExc_OSError);
+        return -1;
+    }
+    *result = *local;
+    return 0;
+}
+
 static PyObject *
 time_localtime(PyObject *self, PyObject *args)
 {
-    double when;
-    if (!parse_time_double_args(args, "|O:localtime", &when))
+    time_t when;
+    struct tm buf;
+
+    if (!parse_time_t_args(args, "|O:localtime", &when))
+        return NULL;
+    if (pylocaltime(&when, &buf) == 1)
         return NULL;
-    return time_convert(when, localtime);
+    return tmtotuple(&buf);
 }
 
 PyDoc_STRVAR(localtime_doc,
@@ -462,7 +484,8 @@ time_strftime(PyObject *self, PyObject *args)
 
     if (tup == NULL) {
         time_t tt = time(NULL);
-        buf = *localtime(&tt);
+        if (pylocaltime(&tt, &buf) == -1)
+            return NULL;
     }
     else if (!gettmarg(tup, &buf) || !checktm(&buf))
         return NULL;
@@ -627,7 +650,9 @@ time_asctime(PyObject *self, PyObject *args)
         return NULL;
     if (tup == NULL) {
         time_t tt = time(NULL);
-        buf = *localtime(&tt);
+        if (pylocaltime(&tt, &buf) == -1)
+            return NULL;
+
     } else if (!gettmarg(tup, &buf) || !checktm(&buf))
         return NULL;
     return _asctime(&buf);
@@ -643,28 +668,13 @@ is used.");
 static PyObject *
 time_ctime(PyObject *self, PyObject *args)
 {
-    PyObject *ot = NULL;
     time_t tt;
-    struct tm *timeptr;
-
-    if (!PyArg_UnpackTuple(args, "ctime", 0, 1, &ot))
+    struct tm buf;
+    if (!parse_time_t_args(args, "|O:ctime", &tt))
         return NULL;
-    if (ot == NULL || ot == Py_None)
-        tt = time(NULL);
-    else {
-        double dt = PyFloat_AsDouble(ot);
-        if (PyErr_Occurred())
-            return NULL;
-        tt = _PyTime_DoubleToTimet(dt);
-        if (tt == (time_t)-1 && PyErr_Occurred())
-            return NULL;
-    }
-    timeptr = localtime(&tt);
-    if (timeptr == NULL) {
-        PyErr_SetString(PyExc_ValueError, "unconvertible time");
+    if (pylocaltime(&tt, &buf) == -1)
         return NULL;
-    }
-    return _asctime(timeptr);
+    return _asctime(&buf);
 }
 
 PyDoc_STRVAR(ctime_doc,