[Python-checkins] cpython (2.7): make sure fdopen always closes the fd in error cases (closes #21191)

[benjamin.peterson]

http://hg.python.org/cpython/rev/4a3b455abf76
changeset:   90198:4a3b455abf76
branch:      2.7
parent:      90195:aec35c0d308b
user:        Benjamin Peterson <benjamin at python.org>
date:        Wed Apr 09 15:40:18 2014 -0400
summary:
  make sure fdopen always closes the fd in error cases (closes #21191)

files:
  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
--- a/Doc/library/os.rst
+++ b/Doc/library/os.rst
@@ -463,8 +463,9 @@
    .. 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
--- a/Lib/test/test_posix.py
+++ b/Lib/test/test_posix.py
@@ -194,6 +194,10 @@
         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
--- a/Misc/NEWS
+++ b/Misc/NEWS
@@ -43,6 +43,9 @@
 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
--- a/Modules/posixmodule.c
+++ b/Modules/posixmodule.c
@@ -6841,16 +6841,21 @@
     /* 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 @@
 #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, "<fdopen>", orgmode, fclose);

-- 
Repository URL: http://hg.python.org/cpython