[issue11757] test_subprocess.test_communicate_timeout_large_ouput failure on select(): negative timeout?

[Charles-Francois Natali]

Charles-Francois Natali <neologix at free.fr> added the comment:

It seems to have fixed the failure, no ?
I don't know what's the policy regarding syscall parameters check, but
I think it'd be better to check that the timeout passed to select is
not negative, and raise an exception otherwise, instead of silently
storing it into struct timeval (with an overflow) before passing it to
select.
Attached is a patch + test that does just that.

----------
keywords: +patch
Added file: http://bugs.python.org/file21566/select_negative_timeout.diff

_______________________________________
Python tracker <report at bugs.python.org>
<http://bugs.python.org/issue11757>
_______________________________________
-------------- next part --------------
diff -r bbfc65d05588 Lib/test/test_select.py
--- a/Lib/test/test_select.py	Thu Apr 07 10:48:29 2011 -0400
+++ b/Lib/test/test_select.py	Thu Apr 07 21:06:59 2011 +0200
@@ -20,6 +20,7 @@
         self.assertRaises(TypeError, select.select, [self.Nope()], [], [])
         self.assertRaises(TypeError, select.select, [self.Almost()], [], [])
         self.assertRaises(TypeError, select.select, [], [], [], "not a number")
+        self.assertRaises(ValueError, select.select, [], [], [], -1)
 
     def test_returned_list_identity(self):
         # See issue #8329
diff -r bbfc65d05588 Modules/selectmodule.c
--- a/Modules/selectmodule.c	Thu Apr 07 10:48:29 2011 -0400
+++ b/Modules/selectmodule.c	Thu Apr 07 21:06:59 2011 +0200
@@ -234,6 +234,11 @@
                             "timeout period too long");
             return NULL;
         }
+        if (timeout < 0) {
+            PyErr_SetString(PyExc_ValueError,
+                        "timeout must be non-negative");
+            return NULL;
+        }
         seconds = (long)timeout;
         timeout = timeout - (double)seconds;
         tv.tv_sec = seconds;