From a555cfcb73cf677a99d29af6fa0bcfe4c35a2aeb Mon Sep 17 00:00:00 2001 From: Victor Stinner Date: Wed, 18 Mar 2015 00:22:14 +0100 Subject: Issue #23694: Enhance _Py_open(), it now raises exceptions * _Py_open() now raises exceptions on error. If open() fails, it raises an OSError with the filename. * _Py_open() now releases the GIL while calling open() * Add _Py_open_noraise() when _Py_open() cannot be used because the GIL is not held --- Include/fileutils.h | 4 +++ Modules/_posixsubprocess.c | 1 + Modules/mmapmodule.c | 1 - Modules/ossaudiodev.c | 9 ++---- Modules/posixmodule.c | 8 ++---- Modules/selectmodule.c | 11 ++----- Python/fileutils.c | 72 +++++++++++++++++++++++++++++++++++----------- Python/random.c | 10 ++----- 8 files changed, 71 insertions(+), 45 deletions(-) diff --git a/Include/fileutils.h b/Include/fileutils.h index 95632ed..f5fd668 100644 --- a/Include/fileutils.h +++ b/Include/fileutils.h @@ -62,6 +62,10 @@ PyAPI_FUNC(int) _Py_stat( PyAPI_FUNC(int) _Py_open( const char *pathname, int flags); + +PyAPI_FUNC(int) _Py_open_noraise( + const char *pathname, + int flags); #endif PyAPI_FUNC(FILE *) _Py_wfopen( diff --git a/Modules/_posixsubprocess.c b/Modules/_posixsubprocess.c index a8c2be2..a33df21 100644 --- a/Modules/_posixsubprocess.c +++ b/Modules/_posixsubprocess.c @@ -257,6 +257,7 @@ _close_open_fds_safe(int start_fd, PyObject* py_fds_to_keep) fd_dir_fd = _Py_open(FD_DIR, O_RDONLY); if (fd_dir_fd == -1) { /* No way to get a list of open fds. */ + PyErr_Clear(); _close_fds_by_brute_force(start_fd, py_fds_to_keep); return; } else { diff --git a/Modules/mmapmodule.c b/Modules/mmapmodule.c index ac134b8..7c4d17f 100644 --- a/Modules/mmapmodule.c +++ b/Modules/mmapmodule.c @@ -1221,7 +1221,6 @@ new_mmap_object(PyTypeObject *type, PyObject *args, PyObject *kwdict) fd = devzero = _Py_open("/dev/zero", O_RDWR); if (devzero == -1) { Py_DECREF(m_obj); - PyErr_SetFromErrno(PyExc_OSError); return NULL; } #endif diff --git a/Modules/ossaudiodev.c b/Modules/ossaudiodev.c index 2d0dd09..34e68c7 100644 --- a/Modules/ossaudiodev.c +++ b/Modules/ossaudiodev.c @@ -116,11 +116,8 @@ newossobject(PyObject *arg) provides a special ioctl() for non-blocking read/write, which is exposed via oss_nonblock() below. */ fd = _Py_open(devicename, imode|O_NONBLOCK); - - if (fd == -1) { - PyErr_SetFromErrnoWithFilename(PyExc_IOError, devicename); + if (fd == -1) return NULL; - } /* And (try to) put it back in blocking mode so we get the expected write() semantics. */ @@ -180,10 +177,8 @@ newossmixerobject(PyObject *arg) } fd = _Py_open(devicename, O_RDWR); - if (fd == -1) { - PyErr_SetFromErrnoWithFilename(PyExc_IOError, devicename); + if (fd == -1) return NULL; - } if ((self = PyObject_New(oss_mixer_t, &OSSMixerType)) == NULL) { close(fd); diff --git a/Modules/posixmodule.c b/Modules/posixmodule.c index e47bd84..b8151c3 100644 --- a/Modules/posixmodule.c +++ b/Modules/posixmodule.c @@ -7930,7 +7930,7 @@ os_openpty_impl(PyModuleDef *module) slave_fd = _Py_open(slave_name, O_RDWR); if (slave_fd < 0) - goto posix_error; + goto error; #else master_fd = open(DEV_PTY_FILE, O_RDWR | O_NOCTTY); /* open master */ @@ -7958,8 +7958,8 @@ os_openpty_impl(PyModuleDef *module) goto posix_error; slave_fd = _Py_open(slave_name, O_RDWR | O_NOCTTY); /* open slave */ - if (slave_fd < 0) - goto posix_error; + if (slave_fd == -1) + goto error; if (_Py_set_inheritable(master_fd, 0, NULL) < 0) goto posix_error; @@ -7977,9 +7977,7 @@ os_openpty_impl(PyModuleDef *module) posix_error: posix_error(); -#if defined(HAVE_OPENPTY) || defined(HAVE__GETPTY) error: -#endif if (master_fd != -1) close(master_fd); if (slave_fd != -1) diff --git a/Modules/selectmodule.c b/Modules/selectmodule.c index ffaf865..ef53067 100644 --- a/Modules/selectmodule.c +++ b/Modules/selectmodule.c @@ -1013,7 +1013,6 @@ newDevPollObject(void) struct pollfd *fds; struct rlimit limit; - Py_BEGIN_ALLOW_THREADS /* ** If we try to process more that getrlimit() ** fds, the kernel will give an error, so @@ -1021,18 +1020,14 @@ newDevPollObject(void) ** value, because we can change rlimit() anytime. */ limit_result = getrlimit(RLIMIT_NOFILE, &limit); - if (limit_result != -1) - fd_devpoll = _Py_open("/dev/poll", O_RDWR); - Py_END_ALLOW_THREADS - if (limit_result == -1) { PyErr_SetFromErrno(PyExc_OSError); return NULL; } - if (fd_devpoll == -1) { - PyErr_SetFromErrnoWithFilename(PyExc_IOError, "/dev/poll"); + + fd_devpoll = _Py_open("/dev/poll", O_RDWR); + if (fd_devpoll == -1) return NULL; - } fds = PyMem_NEW(struct pollfd, limit.rlim_cur); if (fds == NULL) { diff --git a/Python/fileutils.c b/Python/fileutils.c index c6cdb19..7686040 100644 --- a/Python/fileutils.c +++ b/Python/fileutils.c @@ -30,7 +30,8 @@ extern wchar_t* _Py_DecodeUTF8_surrogateescape(const char *s, Py_ssize_t size); 0: open() ignores O_CLOEXEC flag, ex: Linux kernel older than 2.6.23 1: open() supports O_CLOEXEC flag, close-on-exec is set - The flag is used by _Py_open(), io.FileIO and os.open() */ + The flag is used by _Py_open(), _Py_open_noraise(), io.FileIO + and os.open(). */ int _Py_open_cloexec_works = -1; #endif @@ -907,37 +908,74 @@ _Py_set_inheritable(int fd, int inheritable, int *atomic_flag_works) return set_inheritable(fd, inheritable, 1, atomic_flag_works); } -/* Open a file with the specified flags (wrapper to open() function). - The file descriptor is created non-inheritable. */ -int -_Py_open(const char *pathname, int flags) +static int +_Py_open_impl(const char *pathname, int flags, int gil_held) { int fd; -#ifdef MS_WINDOWS - fd = open(pathname, flags | O_NOINHERIT); - if (fd < 0) - return fd; -#else - +#ifndef MS_WINDOWS int *atomic_flag_works; -#ifdef O_CLOEXEC +#endif + +#ifdef MS_WINDOWS + flags |= O_NOINHERIT; +#elif defined(O_CLOEXEC) atomic_flag_works = &_Py_open_cloexec_works; flags |= O_CLOEXEC; #else atomic_flag_works = NULL; #endif - fd = open(pathname, flags); - if (fd < 0) - return fd; - if (set_inheritable(fd, 0, 0, atomic_flag_works) < 0) { + if (gil_held) { + Py_BEGIN_ALLOW_THREADS + fd = open(pathname, flags); + Py_END_ALLOW_THREADS + + if (fd < 0) { + PyErr_SetFromErrnoWithFilename(PyExc_OSError, pathname); + return -1; + } + } + else { + fd = open(pathname, flags); + if (fd < 0) + return -1; + } + +#ifndef MS_WINDOWS + if (set_inheritable(fd, 0, gil_held, atomic_flag_works) < 0) { close(fd); return -1; } -#endif /* !MS_WINDOWS */ +#endif + return fd; } +/* Open a file with the specified flags (wrapper to open() function). + Return a file descriptor on success. Raise an exception and return -1 on + error. + + The file descriptor is created non-inheritable. + + The GIL must be held. Use _Py_open_noraise() if the GIL cannot be held. */ +int +_Py_open(const char *pathname, int flags) +{ + /* _Py_open() must be called with the GIL held. */ + assert(PyGILState_Check()); + return _Py_open_impl(pathname, flags, 1); +} + +/* Open a file with the specified flags (wrapper to open() function). + Return a file descriptor on success. Set errno and return -1 on error. + + The file descriptor is created non-inheritable. */ +int +_Py_open_noraise(const char *pathname, int flags) +{ + return _Py_open_impl(pathname, flags, 0); +} + /* 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. */ diff --git a/Python/random.c b/Python/random.c index 031d887..39a389c 100644 --- a/Python/random.c +++ b/Python/random.c @@ -111,7 +111,7 @@ dev_urandom_noraise(unsigned char *buffer, Py_ssize_t size) assert (0 < size); - fd = _Py_open("/dev/urandom", O_RDONLY); + fd = _Py_open_noraise("/dev/urandom", O_RDONLY); if (fd < 0) Py_FatalError("Failed to open /dev/urandom"); @@ -158,17 +158,13 @@ dev_urandom_python(char *buffer, Py_ssize_t size) if (urandom_cache.fd >= 0) fd = urandom_cache.fd; else { - Py_BEGIN_ALLOW_THREADS fd = _Py_open("/dev/urandom", O_RDONLY); - Py_END_ALLOW_THREADS - if (fd < 0) - { + if (fd < 0) { if (errno == ENOENT || errno == ENXIO || errno == ENODEV || errno == EACCES) PyErr_SetString(PyExc_NotImplementedError, "/dev/urandom (or equivalent) not found"); - else - PyErr_SetFromErrno(PyExc_OSError); + /* otherwise, keep the OSError exception raised by _Py_open() */ return -1; } if (urandom_cache.fd >= 0) { -- cgit v0.12