gh-102179: Fix os.dup2() error reporting for negative fds

A remnant of old code manually checks the sign of fds, but doesn't set errno before raising OSError: ``` >>> os.dup2(-1, 0) Traceback (most recent call last): File "<stdin>", line 1, in <module> OSError: [Errno 0] Error ``` Remove the manual check altogether: the C library functions used to implement os.dup2() check fd validity anyway.
python · kumaraditya303 · Mar 4, 2023 · Feb 23, 2023 · Mar 3, 2023 · Mar 4, 2023
commit 411107d5a8df5fa11a2cd89d89811c8811d77cd4
diff --git a/Lib/test/test_os.py b/Lib/test/test_os.py
@@ -2221,6 +2221,22 @@ def test_closerange(self):
    def test_dup2(self):
        self.check(os.dup2, 20)

+    @unittest.skipUnless(hasattr(os, 'dup2'), 'test needs os.dup2()')
+    def test_dup2_negative_fd(self):
+        valid_fd = os.open(__file__, os.O_RDONLY)
+        self.addCleanup(os.close, valid_fd)
+        fds = [
+            valid_fd,
+            -1,
+            -2**31,
+        ]
+        for fd, fd2 in itertools.product(fds, repeat=2):
+            if fd != fd2:
+                with self.subTest(fd=fd, fd2=fd2):
+                    with self.assertRaises(OSError) as ctx:
+                        os.dup2(fd, fd2)
+                    self.assertEqual(ctx.exception.errno, errno.EBADF)
+
    @unittest.skipUnless(hasattr(os, 'fchmod'), 'test needs os.fchmod()')
    def test_fchmod(self):
        self.check(os.fchmod, 0)

diff --git a/Misc/NEWS.d/next/Library/2023-02-23-15-06-01.gh-issue-102179.P6KQ4c.rst b/Misc/NEWS.d/next/Library/2023-02-23-15-06-01.gh-issue-102179.P6KQ4c.rst
@@ -0,0 +1 @@
+Fix ``os.dup2()`` error reporting for negative fds.
diff --git a/Modules/posixmodule.c b/Modules/posixmodule.c
@@ -9829,11 +9829,6 @@ os_dup2_impl(PyObject *module, int fd, int fd2, int inheritable)
    static int dup3_works = -1;
 #endif

-    if (fd < 0 || fd2 < 0) {
-        posix_error();
-        return -1;
-    }
-
    /* dup2() can fail with EINTR if the target FD is already open, because it
     * then has to be closed. See os_close_impl() for why we don't handle EINTR
     * upon close(), and therefore below.
Original file line number	Diff line number	Diff line change
		@@ -0,0 +1 @@
		Fix ``os.dup2()`` error reporting for negative fds.
kumaraditya303 marked this conversation as resolved. Show resolved Hide resolved