From 02ab7a84ef86cfe043bbfbbd2912dcc8c8c67793 Mon Sep 17 00:00:00 2001 From: Benjamin Peterson Date: Wed, 9 Apr 2014 15:40:18 -0400 Subject: [PATCH] make sure fdopen always closes the fd in error cases (closes #21191) --- Doc/library/os.rst | 5 +++-- Lib/test/test_posix.py | 4 ++++ Misc/NEWS | 3 +++ Modules/posixmodule.c | 16 ++++++++++++---- 4 files changed, 22 insertions(+), 6 deletions(-) diff --git a/Doc/library/os.rst b/Doc/library/os.rst index a597a67d39..195a4a80fc 100644 --- a/Doc/library/os.rst +++ b/Doc/library/os.rst @@ -463,8 +463,9 @@ These functions create new file objects. (See also :func:`open`.) .. index:: single: I/O control; buffering Return an open file object connected to the file descriptor *fd*. The *mode* - and *bufsize* arguments have the same meaning as the corresponding arguments to - the built-in :func:`open` function. + and *bufsize* arguments have the same meaning as the corresponding arguments + to the built-in :func:`open` function. If :func:`fdopen` raises an + exception, it closes *fd*. Availability: Unix, Windows. diff --git a/Lib/test/test_posix.py b/Lib/test/test_posix.py index 76a74fb907..243fd5d6ed 100644 --- a/Lib/test/test_posix.py +++ b/Lib/test/test_posix.py @@ -194,6 +194,10 @@ class PosixTester(unittest.TestCase): self.fdopen_helper('r') self.fdopen_helper('r', 100) + fd = os.open(test_support.TESTFN, os.O_RDONLY) + self.assertRaises(OSError, posix.fdopen, fd, 'w') + self.assertRaises(OSError, os.close, fd) # fd should be closed. + @unittest.skipUnless(hasattr(posix, 'O_EXLOCK'), 'test needs posix.O_EXLOCK') def test_osexlock(self): diff --git a/Misc/NEWS b/Misc/NEWS index a6dfc7e752..873c7b6a5d 100644 --- a/Misc/NEWS +++ b/Misc/NEWS @@ -43,6 +43,9 @@ Core and Builtins Library ------- +- Issue #21191: In os.fdopen, alwyas close the file descriptor when an exception + happens. + - Issue #21149: Improved thread-safety in logging cleanup during interpreter shutdown. Thanks to Devin Jeanpierre for the patch. diff --git a/Modules/posixmodule.c b/Modules/posixmodule.c index 21a6cef3f7..168f7f4917 100644 --- a/Modules/posixmodule.c +++ b/Modules/posixmodule.c @@ -6841,16 +6841,21 @@ posix_fdopen(PyObject *self, PyObject *args) /* Sanitize mode. See fileobject.c */ mode = PyMem_MALLOC(strlen(orgmode)+3); if (!mode) { + close(fd); PyErr_NoMemory(); return NULL; } strcpy(mode, orgmode); if (_PyFile_SanitizeMode(mode)) { + close(fd); PyMem_FREE(mode); return NULL; } - if (!_PyVerify_fd(fd)) - return posix_error(); + if (!_PyVerify_fd(fd)) { + posix_error(); + close(fd); + return NULL; + } Py_BEGIN_ALLOW_THREADS #if !defined(MS_WINDOWS) && defined(HAVE_FCNTL_H) if (mode[0] == 'a') { @@ -6871,8 +6876,11 @@ posix_fdopen(PyObject *self, PyObject *args) #endif Py_END_ALLOW_THREADS PyMem_FREE(mode); - if (fp == NULL) - return posix_error(); + if (fp == NULL) { + posix_error(); + close(fd); + return NULL; + } /* The dummy filename used here must be kept in sync with the value tested against in gzip.GzipFile.__init__() - see issue #13781. */ f = PyFile_FromFile(fp, "", orgmode, fclose); -- 2.50.1