From bbdb17d19bb1d5443ca4417254e014ad64c04540 Mon Sep 17 00:00:00 2001 From: Benjamin Peterson Date: Fri, 29 Dec 2017 13:13:06 -0800 Subject: return the new file descriptor from os.dup2 (closes bpo-32441) (#5041) --- Doc/library/os.rst | 10 +++-- Lib/test/test_os.py | 16 +++----- .../2017-12-28-21-30-40.bpo-32441.LqlboJ.rst | 2 + Modules/clinic/posixmodule.c.h | 11 ++++-- Modules/posixmodule.c | 43 ++++++++++++++-------- 5 files changed, 50 insertions(+), 32 deletions(-) create mode 100644 Misc/NEWS.d/next/Library/2017-12-28-21-30-40.bpo-32441.LqlboJ.rst diff --git a/Doc/library/os.rst b/Doc/library/os.rst index a24934c..ee08853 100644 --- a/Doc/library/os.rst +++ b/Doc/library/os.rst @@ -735,13 +735,17 @@ as internal buffering of data. .. function:: dup2(fd, fd2, inheritable=True) - Duplicate file descriptor *fd* to *fd2*, closing the latter first if necessary. - The file descriptor *fd2* is :ref:`inheritable ` by default, - or non-inheritable if *inheritable* is ``False``. + Duplicate file descriptor *fd* to *fd2*, closing the latter first if + necessary. Return *fd2*. The new file descriptor is :ref:`inheritable + ` by default or non-inheritable if *inheritable* + is ``False``. .. versionchanged:: 3.4 Add the optional *inheritable* parameter. + .. versionchanged:: 3.7 + Return *fd2* on success. Previously, ``None`` was always returned. + .. function:: fchmod(fd, mode) diff --git a/Lib/test/test_os.py b/Lib/test/test_os.py index f235f80..83e214d 100644 --- a/Lib/test/test_os.py +++ b/Lib/test/test_os.py @@ -3079,19 +3079,15 @@ class FDInheritanceTests(unittest.TestCase): # inheritable by default fd2 = os.open(__file__, os.O_RDONLY) - try: - os.dup2(fd, fd2) - self.assertEqual(os.get_inheritable(fd2), True) - finally: - os.close(fd2) + self.addCleanup(os.close, fd2) + self.assertEqual(os.dup2(fd, fd2), fd2) + self.assertTrue(os.get_inheritable(fd2)) # force non-inheritable fd3 = os.open(__file__, os.O_RDONLY) - try: - os.dup2(fd, fd3, inheritable=False) - self.assertEqual(os.get_inheritable(fd3), False) - finally: - os.close(fd3) + self.addCleanup(os.close, fd3) + self.assertEqual(os.dup2(fd, fd3, inheritable=False), fd3) + self.assertFalse(os.get_inheritable(fd3)) @unittest.skipUnless(hasattr(os, 'openpty'), "need os.openpty()") def test_openpty(self): diff --git a/Misc/NEWS.d/next/Library/2017-12-28-21-30-40.bpo-32441.LqlboJ.rst b/Misc/NEWS.d/next/Library/2017-12-28-21-30-40.bpo-32441.LqlboJ.rst new file mode 100644 index 0000000..a0fe4f3 --- /dev/null +++ b/Misc/NEWS.d/next/Library/2017-12-28-21-30-40.bpo-32441.LqlboJ.rst @@ -0,0 +1,2 @@ +Return the new file descriptor (i.e., the second argument) from ``os.dup2``. +Previously, ``None`` was always returned. diff --git a/Modules/clinic/posixmodule.c.h b/Modules/clinic/posixmodule.c.h index 9decf7a..6f4c028 100644 --- a/Modules/clinic/posixmodule.c.h +++ b/Modules/clinic/posixmodule.c.h @@ -3486,7 +3486,7 @@ PyDoc_STRVAR(os_dup2__doc__, #define OS_DUP2_METHODDEF \ {"dup2", (PyCFunction)os_dup2, METH_FASTCALL|METH_KEYWORDS, os_dup2__doc__}, -static PyObject * +static int os_dup2_impl(PyObject *module, int fd, int fd2, int inheritable); static PyObject * @@ -3498,12 +3498,17 @@ os_dup2(PyObject *module, PyObject *const *args, Py_ssize_t nargs, PyObject *kwn int fd; int fd2; int inheritable = 1; + int _return_value; if (!_PyArg_ParseStackAndKeywords(args, nargs, kwnames, &_parser, &fd, &fd2, &inheritable)) { goto exit; } - return_value = os_dup2_impl(module, fd, fd2, inheritable); + _return_value = os_dup2_impl(module, fd, fd2, inheritable); + if ((_return_value == -1) && PyErr_Occurred()) { + goto exit; + } + return_value = PyLong_FromLong((long)_return_value); exit: return return_value; @@ -6405,4 +6410,4 @@ exit: #ifndef OS_GETRANDOM_METHODDEF #define OS_GETRANDOM_METHODDEF #endif /* !defined(OS_GETRANDOM_METHODDEF) */ -/*[clinic end generated code: output=b6ade5f170d5a431 input=a9049054013a1b77]*/ +/*[clinic end generated code: output=6345053cd5992caf input=a9049054013a1b77]*/ diff --git a/Modules/posixmodule.c b/Modules/posixmodule.c index 38b6c80..47b79fc 100644 --- a/Modules/posixmodule.c +++ b/Modules/posixmodule.c @@ -7770,7 +7770,7 @@ os_dup_impl(PyObject *module, int fd) /*[clinic input] -os.dup2 +os.dup2 -> int fd: int fd2: int inheritable: bool=True @@ -7778,9 +7778,9 @@ os.dup2 Duplicate file descriptor. [clinic start generated code]*/ -static PyObject * +static int os_dup2_impl(PyObject *module, int fd, int fd2, int inheritable) -/*[clinic end generated code: output=db832a2d872ccc5f input=76e96f511be0352f]*/ +/*[clinic end generated code: output=bc059d34a73404d1 input=c3cddda8922b038d]*/ { int res; #if defined(HAVE_DUP3) && \ @@ -7789,8 +7789,10 @@ os_dup2_impl(PyObject *module, int fd, int fd2, int inheritable) int dup3_works = -1; #endif - if (fd < 0 || fd2 < 0) - return posix_error(); + 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 @@ -7802,13 +7804,16 @@ os_dup2_impl(PyObject *module, int fd, int fd2, int inheritable) res = dup2(fd, fd2); _Py_END_SUPPRESS_IPH Py_END_ALLOW_THREADS - if (res < 0) - return posix_error(); + if (res < 0) { + posix_error(); + return -1; + } + res = fd2; // msvcrt dup2 returns 0 on success. /* Character files like console cannot be make non-inheritable */ if (!inheritable && _Py_set_inheritable(fd2, 0, NULL) < 0) { close(fd2); - return NULL; + return -1; } #elif defined(HAVE_FCNTL_H) && defined(F_DUP2FD_CLOEXEC) @@ -7818,8 +7823,10 @@ os_dup2_impl(PyObject *module, int fd, int fd2, int inheritable) else res = dup2(fd, fd2); Py_END_ALLOW_THREADS - if (res < 0) - return posix_error(); + if (res < 0) { + posix_error(); + return -1; + } #else @@ -7831,8 +7838,10 @@ os_dup2_impl(PyObject *module, int fd, int fd2, int inheritable) if (res < 0) { if (dup3_works == -1) dup3_works = (errno != ENOSYS); - if (dup3_works) - return posix_error(); + if (dup3_works) { + posix_error(); + return -1; + } } } @@ -7842,12 +7851,14 @@ os_dup2_impl(PyObject *module, int fd, int fd2, int inheritable) Py_BEGIN_ALLOW_THREADS res = dup2(fd, fd2); Py_END_ALLOW_THREADS - if (res < 0) - return posix_error(); + if (res < 0) { + posix_error(); + return -1; + } if (!inheritable && _Py_set_inheritable(fd2, 0, NULL) < 0) { close(fd2); - return NULL; + return -1; } #ifdef HAVE_DUP3 } @@ -7855,7 +7866,7 @@ os_dup2_impl(PyObject *module, int fd, int fd2, int inheritable) #endif - Py_RETURN_NONE; + return res; } -- cgit v0.12