[Python-checkins] cpython: Issue #23694: Enhance _Py_fopen(), it now raises an exception on error

[victor.stinner]

https://hg.python.org/cpython/rev/cfe541c694f3
changeset:   95026:cfe541c694f3
user:        Victor Stinner <victor.stinner at gmail.com>
date:        Wed Mar 18 01:39:23 2015 +0100
summary:
  Issue #23694: Enhance _Py_fopen(), it now raises an exception on error

* If fopen() fails, OSError is raised with the original filename object.
* The GIL is now released while calling fopen()

files:
  Modules/_ssl.c      |   6 +--
  Modules/zipimport.c |   8 +----
  Python/errors.c     |   4 ++
  Python/fileutils.c  |  48 ++++++++++++++++++++++++--------
  Python/import.c     |   2 -
  5 files changed, 44 insertions(+), 24 deletions(-)


diff --git a/Modules/_ssl.c b/Modules/_ssl.c
--- a/Modules/_ssl.c
+++ b/Modules/_ssl.c
@@ -2941,11 +2941,9 @@
     DH *dh;
 
     f = _Py_fopen_obj(filepath, "rb");
-    if (f == NULL) {
-        if (!PyErr_Occurred())
-            PyErr_SetFromErrnoWithFilenameObject(PyExc_OSError, filepath);
+    if (f == NULL)
         return NULL;
-    }
+
     errno = 0;
     PySSL_BEGIN_ALLOW_THREADS
     dh = PEM_read_DHparams(f, NULL, NULL, NULL);
diff --git a/Modules/zipimport.c b/Modules/zipimport.c
--- a/Modules/zipimport.c
+++ b/Modules/zipimport.c
@@ -875,7 +875,7 @@
 
     fp = _Py_fopen_obj(archive, "rb");
     if (fp == NULL) {
-        if (!PyErr_Occurred())
+        if (PyErr_ExceptionMatches(PyExc_OSError))
             PyErr_Format(ZipImportError, "can't open Zip file: %R", archive);
         return NULL;
     }
@@ -1073,12 +1073,8 @@
     }
 
     fp = _Py_fopen_obj(archive, "rb");
-    if (!fp) {
-        if (!PyErr_Occurred())
-            PyErr_Format(PyExc_IOError,
-               "zipimport: can not open file %U", archive);
+    if (!fp)
         return NULL;
-    }
 
     /* Check to make sure the local file header is correct */
     if (fseek(fp, file_offset, 0) == -1) {
diff --git a/Python/errors.c b/Python/errors.c
--- a/Python/errors.c
+++ b/Python/errors.c
@@ -1126,6 +1126,10 @@
     if (filename == NULL || lineno <= 0)
         return NULL;
     fp = _Py_fopen_obj(filename, "r" PY_STDIOTEXTMODE);
+    if (fp == NULL) {
+        PyErr_Clear();
+        return NULL;
+    }
     return err_programtext(fp, lineno);
 }
 
diff --git a/Python/fileutils.c b/Python/fileutils.c
--- a/Python/fileutils.c
+++ b/Python/fileutils.c
@@ -957,7 +957,7 @@
 
    The file descriptor is created non-inheritable.
 
-   The GIL must be held. Use _Py_open_noraise() if the GIL cannot be held. */
+   The GIL must be held. */
 int
 _Py_open(const char *pathname, int flags)
 {
@@ -977,8 +977,9 @@
 }
 
 /* Open a file. Use _wfopen() on Windows, encode the path to the locale
-   encoding and use fopen() otherwise. The file descriptor is created
-   non-inheritable. */
+   encoding and use fopen() otherwise.
+
+   The file descriptor is created non-inheritable). */
 FILE *
 _Py_wfopen(const wchar_t *path, const wchar_t *mode)
 {
@@ -1009,7 +1010,9 @@
     return f;
 }
 
-/* Wrapper to fopen(). The file descriptor is created non-inheritable. */
+/* Wrapper to fopen().
+
+   The file descriptor is created non-inheritable). */
 FILE*
 _Py_fopen(const char *pathname, const char *mode)
 {
@@ -1024,11 +1027,14 @@
 }
 
 /* Open a file. Call _wfopen() on Windows, or encode the path to the filesystem
-   encoding and call fopen() otherwise. The file descriptor is created
-   non-inheritable.
+   encoding and call fopen() otherwise.
 
-   Return the new file object on success, or NULL if the file cannot be open or
-   (if PyErr_Occurred()) on unicode error. */
+   Return the new file object on success. Raise an exception and return NULL
+   on error.
+
+   The file descriptor is created non-inheritable.
+
+   The GIL must be held. */
 FILE*
 _Py_fopen_obj(PyObject *path, const char *mode)
 {
@@ -1038,6 +1044,8 @@
     wchar_t wmode[10];
     int usize;
 
+    assert(PyGILState_Check());
+
     if (!PyUnicode_Check(path)) {
         PyErr_Format(PyExc_TypeError,
                      "str file path expected under Windows, got %R",
@@ -1049,20 +1057,36 @@
         return NULL;
 
     usize = MultiByteToWideChar(CP_ACP, 0, mode, -1, wmode, sizeof(wmode));
-    if (usize == 0)
+    if (usize == 0) {
+        PyErr_SetFromWindowsErr(0);
         return NULL;
+    }
 
+    Py_BEGIN_ALLOW_THREADS
     f = _wfopen(wpath, wmode);
+    Py_END_ALLOW_THREADS
 #else
     PyObject *bytes;
+    char *path_bytes;
+
+    assert(PyGILState_Check());
+
     if (!PyUnicode_FSConverter(path, &bytes))
         return NULL;
-    f = fopen(PyBytes_AS_STRING(bytes), mode);
+    path_bytes = PyBytes_AS_STRING(bytes);
+
+    Py_BEGIN_ALLOW_THREADS
+    f = fopen(path_bytes, mode);
+    Py_END_ALLOW_THREADS
+
     Py_DECREF(bytes);
 #endif
-    if (f == NULL)
+    if (f == NULL) {
+        PyErr_SetFromErrnoWithFilenameObject(PyExc_OSError, path);
         return NULL;
-    if (make_non_inheritable(fileno(f)) < 0) {
+    }
+
+    if (set_inheritable(fileno(f), 0, 1, NULL) < 0) {
         fclose(f);
         return NULL;
     }
diff --git a/Python/import.c b/Python/import.c
--- a/Python/import.c
+++ b/Python/import.c
@@ -1945,8 +1945,6 @@
         fp = _Py_fopen_obj(path, "r");
         if (fp == NULL) {
             Py_DECREF(path);
-            if (!PyErr_Occurred())
-                PyErr_SetFromErrno(PyExc_IOError);
             return NULL;
         }
     }

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