Issue #22117: Write unit tests for _PyTime_AsTimeval()

* _PyTime_AsTimeval() now ensures that tv_usec is always positive
* _PyTime_AsTimespec() now ensures that tv_nsec is always positive
* _PyTime_AsTimeval() now returns an integer on overflow instead of raising an
  exception

5 files changed, 103 insertions, 15 deletions

diff --git a/Include/pytime.h b/Include/pytime.h
index 17d5ea1..72b6460 100644
--- a/Include/pytime.h
+++ b/Include/pytime.h
@@ -140,13 +140,15 @@ PyAPI_FUNC(_PyTime_t) _PyTime_AsMilliseconds(_PyTime_t t,
 PyAPI_FUNC(PyObject *) _PyTime_AsNanosecondsObject(_PyTime_t t);
 
 /* Convert a timestamp to a timeval structure (microsecond resolution).
-   Raise an exception and return -1 on error, return 0 on success. */
+   tv_usec is always positive.
+   Return -1 if the conversion overflowed, return 0 on success. */
 PyAPI_FUNC(int) _PyTime_AsTimeval(_PyTime_t t,
     struct timeval *tv,
     _PyTime_round_t round);
 
 #ifdef HAVE_CLOCK_GETTIME
 /* Convert a timestamp to a timespec structure (nanosecond resolution).
+   tv_nsec is always positive.
    Raise an exception and return -1 on error, return 0 on success. */
 PyAPI_FUNC(int) _PyTime_AsTimespec(_PyTime_t t, struct timespec *ts);
 #endif
diff --git a/Lib/test/test_time.py b/Lib/test/test_time.py
index cfec329..78314a7 100644
--- a/Lib/test/test_time.py
+++ b/Lib/test/test_time.py
@@ -902,6 +902,44 @@ class TestPyTime_t(unittest.TestCase):
                 self.assertEqual(PyTime_AsSecondsDouble(nanoseconds),
                                  seconds)
 
+    def test_timeval(self):
+        from _testcapi import PyTime_AsTimeval
+        for rnd in ALL_ROUNDING_METHODS:
+            for ns, tv in (
+                # microseconds
+                (0, (0, 0)),
+                (1000, (0, 1)),
+                (-1000, (-1, 999999)),
+
+                # seconds
+                (2 * SEC_TO_NS, (2, 0)),
+                (-3 * SEC_TO_NS, (-3, 0)),
+
+                # seconds + nanoseconds
+                (1234567000, (1, 234567)),
+                (-1234567000, (-2, 765433)),
+            ):
+                with self.subTest(nanoseconds=ns, timeval=tv, round=rnd):
+                    self.assertEqual(PyTime_AsTimeval(ns, rnd), tv)
+
+        UP = _PyTime.ROUND_UP
+        DOWN = _PyTime.ROUND_DOWN
+        for ns, tv, rnd in (
+            # nanoseconds
+            (1, (0, 1), UP),
+            (1, (0, 0), DOWN),
+            (-1, (0, 0), DOWN),
+            (-1, (-1, 999999), UP),
+
+            # seconds + nanoseconds
+            (1234567001, (1, 234568), UP),
+            (1234567001, (1, 234567), DOWN),
+            (-1234567001, (-2, 765433), DOWN),
+            (-1234567001, (-2, 765432), UP),
+        ):
+            with self.subTest(nanoseconds=ns, timeval=tv, round=rnd):
+                self.assertEqual(PyTime_AsTimeval(ns, rnd), tv)
+
     @unittest.skipUnless(hasattr(_testcapi, 'PyTime_AsTimespec'),
                          'need _testcapi.PyTime_AsTimespec')
     def test_timespec(self):
diff --git a/Modules/_testcapimodule.c b/Modules/_testcapimodule.c
index 5029105..4503dc3 100644
--- a/Modules/_testcapimodule.c
+++ b/Modules/_testcapimodule.c
@@ -14,6 +14,10 @@
 #include "marshal.h"
 #include <signal.h>
 
+#ifdef MS_WINDOWS
+#  include <winsock2.h>
+#endif
+
 #ifdef WITH_THREAD
 #include "pythread.h"
 #endif /* WITH_THREAD */
@@ -3408,6 +3412,32 @@ test_pytime_assecondsdouble(PyObject *self, PyObject *args)
     return PyFloat_FromDouble(d);
 }
 
+static PyObject *
+test_PyTime_AsTimeval(PyObject *self, PyObject *args)
+{
+    PY_LONG_LONG ns;
+    int round;
+    _PyTime_t t;
+    struct timeval tv;
+    PyObject *seconds;
+
+    if (!PyArg_ParseTuple(args, "Li", &ns, &round))
+        return NULL;
+    if (check_time_rounding(round) < 0)
+        return NULL;
+    t = _PyTime_FromNanoseconds(ns);
+    if (_PyTime_AsTimeval(t, &tv, round) < 0) {
+        PyErr_SetString(PyExc_OverflowError,
+                        "timeout doesn't fit into C timeval");
+        return NULL;
+    }
+
+    seconds = PyLong_FromLong((PY_LONG_LONG)tv.tv_sec);
+    if (seconds == NULL)
+        return NULL;
+    return Py_BuildValue("Nl", seconds, tv.tv_usec);
+}
+
 #ifdef HAVE_CLOCK_GETTIME
 static PyObject *
 test_PyTime_AsTimespec(PyObject *self, PyObject *args)
@@ -3590,6 +3620,7 @@ static PyMethodDef TestMethods[] = {
         return_result_with_error, METH_NOARGS},
     {"PyTime_FromSecondsObject", test_pytime_fromsecondsobject,  METH_VARARGS},
     {"PyTime_AsSecondsDouble", test_pytime_assecondsdouble, METH_VARARGS},
+    {"PyTime_AsTimeval", test_PyTime_AsTimeval, METH_VARARGS},
 #ifdef HAVE_CLOCK_GETTIME
     {"PyTime_AsTimespec", test_PyTime_AsTimespec, METH_VARARGS},
 #endif
diff --git a/Modules/timemodule.c b/Modules/timemodule.c
index 880f3d2..21e6f43 100644
--- a/Modules/timemodule.c
+++ b/Modules/timemodule.c
@@ -1405,8 +1405,11 @@ pysleep(_PyTime_t secs)
 
     do {
 #ifndef MS_WINDOWS
-        if (_PyTime_AsTimeval(secs, &timeout, _PyTime_ROUND_UP) < 0)
+        if (_PyTime_AsTimeval(secs, &timeout, _PyTime_ROUND_UP) < 0) {
+            PyErr_SetString(PyExc_OverflowError,
+                            "delay doesn't fit into C timeval");
             return -1;
+        }
 
         Py_BEGIN_ALLOW_THREADS
         err = select(0, (fd_set *)0, (fd_set *)0, (fd_set *)0, &timeout);
diff --git a/Python/pytime.c b/Python/pytime.c
index bd94787..9893116 100644
--- a/Python/pytime.c
+++ b/Python/pytime.c
@@ -540,9 +540,14 @@ int
 _PyTime_AsTimeval(_PyTime_t t, struct timeval *tv, _PyTime_round_t round)
 {
     _PyTime_t secs, ns;
+    int res = 0;
 
     secs = t / SEC_TO_NS;
     ns = t % SEC_TO_NS;
+    if (ns < 0) {
+        ns += SEC_TO_NS;
+        secs -= 1;
+    }
 
 #ifdef MS_WINDOWS
     /* On Windows, timeval.tv_sec is a long (32 bit),
@@ -550,8 +555,12 @@ _PyTime_AsTimeval(_PyTime_t t, struct timeval *tv, _PyTime_round_t round)
     assert(sizeof(tv->tv_sec) == sizeof(long));
 #if SIZEOF_TIME_T > SIZEOF_LONG
     if (secs > LONG_MAX) {
-        _PyTime_overflow();
-        return -1;
+        secs = LONG_MAX;
+        res = -1;
+    }
+    else if (secs < LONG_MIN) {
+        secs = LONG_MIN;
+        res = -1;
     }
 #endif
     tv->tv_sec = (long)secs;
@@ -559,32 +568,37 @@ _PyTime_AsTimeval(_PyTime_t t, struct timeval *tv, _PyTime_round_t round)
     /* On OpenBSD 5.4, timeval.tv_sec is a long.
        Example: long is 64-bit, whereas time_t is 32-bit. */
     tv->tv_sec = secs;
-    if ((_PyTime_t)tv->tv_sec != secs) {
-        _PyTime_overflow();
-        return -1;
-    }
+    if ((_PyTime_t)tv->tv_sec != secs)
+        res = -1;
 #endif
 
-    if (round == _PyTime_ROUND_UP)
+    if ((round == _PyTime_ROUND_UP) ^ (tv->tv_sec < 0))
         tv->tv_usec = (int)((ns + US_TO_NS - 1) / US_TO_NS);
     else
         tv->tv_usec = (int)(ns / US_TO_NS);
-    return 0;
+
+    if (tv->tv_usec >= SEC_TO_US) {
+        tv->tv_usec -= SEC_TO_US;
+        tv->tv_sec += 1;
+    }
+
+    return res;
 }
 
 #ifdef HAVE_CLOCK_GETTIME
 int
 _PyTime_AsTimespec(_PyTime_t t, struct timespec *ts)
 {
-    _PyTime_t sec, nsec;
-    sec = t / SEC_TO_NS;
+    _PyTime_t secs, nsec;
+
+    secs = t / SEC_TO_NS;
     nsec = t % SEC_TO_NS;
     if (nsec < 0) {
         nsec += SEC_TO_NS;
-        sec -= 1;
+        secs -= 1;
     }
-    ts->tv_sec = (time_t)sec;
-    if ((_PyTime_t)ts->tv_sec != sec) {
+    ts->tv_sec = (time_t)secs;
+    if ((_PyTime_t)ts->tv_sec != secs) {
         _PyTime_overflow();
         return -1;
     }