From 940f33a50f3877cbde7adb59ba6e1a0a0acd3d11 Mon Sep 17 00:00:00 2001 From: Steve Dower Date: Thu, 8 Sep 2016 11:21:54 -0700 Subject: [PATCH] Issue #23524: Finish removing _PyVerify_fd from sources --- Include/fileutils.h | 12 ---- Modules/_io/fileio.c | 39 +++++-------- Modules/faulthandler.c | 2 +- Modules/mmapmodule.c | 4 -- Modules/posixmodule.c | 75 +----------------------- Modules/signalmodule.c | 7 +-- PC/msvcrtmodule.c | 11 +--- Parser/myreadline.c | 5 +- Python/fileutils.c | 128 ++--------------------------------------- Python/pylifecycle.c | 2 +- 10 files changed, 27 insertions(+), 258 deletions(-) diff --git a/Include/fileutils.h b/Include/fileutils.h index b4a683c176c..63ff80e62f3 100644 --- a/Include/fileutils.h +++ b/Include/fileutils.h @@ -121,18 +121,6 @@ PyAPI_FUNC(int) _Py_get_blocking(int fd); PyAPI_FUNC(int) _Py_set_blocking(int fd, int blocking); #endif /* !MS_WINDOWS */ -#if defined _MSC_VER && _MSC_VER >= 1400 && _MSC_VER < 1900 -/* A routine to check if a file descriptor is valid on Windows. Returns 0 - * and sets errno to EBADF if it isn't. This is to avoid Assertions - * from various functions in the Windows CRT beginning with - * Visual Studio 2005 - */ -int _PyVerify_fd(int fd); - -#else -#define _PyVerify_fd(A) (1) /* dummy */ -#endif - #endif /* Py_LIMITED_API */ #ifdef __cplusplus diff --git a/Modules/_io/fileio.c b/Modules/_io/fileio.c index 12e5156fbb7..54cfb7fa7d6 100644 --- a/Modules/_io/fileio.c +++ b/Modules/_io/fileio.c @@ -117,18 +117,13 @@ internal_close(fileio *self) int fd = self->fd; self->fd = -1; /* fd is accessible and someone else may have closed it */ - if (_PyVerify_fd(fd)) { - Py_BEGIN_ALLOW_THREADS - _Py_BEGIN_SUPPRESS_IPH - err = close(fd); - if (err < 0) - save_errno = errno; - _Py_END_SUPPRESS_IPH - Py_END_ALLOW_THREADS - } else { + Py_BEGIN_ALLOW_THREADS + _Py_BEGIN_SUPPRESS_IPH + err = close(fd); + if (err < 0) save_errno = errno; - err = -1; - } + _Py_END_SUPPRESS_IPH + Py_END_ALLOW_THREADS } if (err < 0) { errno = save_errno; @@ -700,8 +695,6 @@ _io_FileIO_readall_impl(fileio *self) if (self->fd < 0) return err_closed(); - if (!_PyVerify_fd(self->fd)) - return PyErr_SetFromErrno(PyExc_IOError); _Py_BEGIN_SUPPRESS_IPH #ifdef MS_WINDOWS @@ -914,18 +907,15 @@ portable_lseek(int fd, PyObject *posobj, int whence) return NULL; } - if (_PyVerify_fd(fd)) { - Py_BEGIN_ALLOW_THREADS - _Py_BEGIN_SUPPRESS_IPH + Py_BEGIN_ALLOW_THREADS + _Py_BEGIN_SUPPRESS_IPH #ifdef MS_WINDOWS - res = _lseeki64(fd, pos, whence); + res = _lseeki64(fd, pos, whence); #else - res = lseek(fd, pos, whence); + res = lseek(fd, pos, whence); #endif - _Py_END_SUPPRESS_IPH - Py_END_ALLOW_THREADS - } else - res = -1; + _Py_END_SUPPRESS_IPH + Py_END_ALLOW_THREADS if (res < 0) return PyErr_SetFromErrno(PyExc_IOError); @@ -1116,10 +1106,7 @@ _io_FileIO_isatty_impl(fileio *self) return err_closed(); Py_BEGIN_ALLOW_THREADS _Py_BEGIN_SUPPRESS_IPH - if (_PyVerify_fd(self->fd)) - res = isatty(self->fd); - else - res = 0; + res = isatty(self->fd); _Py_END_SUPPRESS_IPH Py_END_ALLOW_THREADS return PyBool_FromLong(res); diff --git a/Modules/faulthandler.c b/Modules/faulthandler.c index c2d30008fc7..1c1e4fb7d17 100644 --- a/Modules/faulthandler.c +++ b/Modules/faulthandler.c @@ -159,7 +159,7 @@ faulthandler_get_fileno(PyObject **file_ptr) fd = _PyLong_AsInt(file); if (fd == -1 && PyErr_Occurred()) return -1; - if (fd < 0 || !_PyVerify_fd(fd)) { + if (fd < 0) { PyErr_SetString(PyExc_ValueError, "file is not a valid file descripter"); return -1; diff --git a/Modules/mmapmodule.c b/Modules/mmapmodule.c index b0015a5756f..73c37d07121 100644 --- a/Modules/mmapmodule.c +++ b/Modules/mmapmodule.c @@ -1326,10 +1326,6 @@ new_mmap_object(PyTypeObject *type, PyObject *args, PyObject *kwdict) */ if (fileno != -1 && fileno != 0) { /* Ensure that fileno is within the CRT's valid range */ - if (!_PyVerify_fd(fileno)) { - PyErr_SetFromErrno(PyExc_OSError); - return NULL; - } _Py_BEGIN_SUPPRESS_IPH fh = (HANDLE)_get_osfhandle(fileno); _Py_END_SUPPRESS_IPH diff --git a/Modules/posixmodule.c b/Modules/posixmodule.c index 4f5ec99e4ad..957a5f2b7db 100644 --- a/Modules/posixmodule.c +++ b/Modules/posixmodule.c @@ -1178,34 +1178,6 @@ PyLong_FromPy_off_t(Py_off_t offset) #endif } - -#if defined _MSC_VER && _MSC_VER >= 1400 && _MSC_VER < 1900 -/* Legacy implementation of _PyVerify_fd_dup2 while transitioning to - * MSVC 14.0. This should eventually be removed. (issue23524) - */ -#define IOINFO_L2E 5 -#define IOINFO_ARRAYS 64 -#define IOINFO_ARRAY_ELTS (1 << IOINFO_L2E) -#define _NHANDLE_ (IOINFO_ARRAYS * IOINFO_ARRAY_ELTS) -#define _NO_CONSOLE_FILENO (intptr_t)-2 - -/* the special case of checking dup2. The target fd must be in a sensible range */ -static int -_PyVerify_fd_dup2(int fd1, int fd2) -{ - if (!_PyVerify_fd(fd1)) - return 0; - if (fd2 == _NO_CONSOLE_FILENO) - return 0; - if ((unsigned)fd2 < _NHANDLE_) - return 1; - else - return 0; -} -#else -#define _PyVerify_fd_dup2(fd1, fd2) (_PyVerify_fd(fd1) && (fd2) >= 0) -#endif - #ifdef MS_WINDOWS static int @@ -1409,9 +1381,6 @@ posix_fildes_fd(int fd, int (*func)(int)) int res; int async_err = 0; - if (!_PyVerify_fd(fd)) - return posix_error(); - do { Py_BEGIN_ALLOW_THREADS _Py_BEGIN_SUPPRESS_IPH @@ -7549,8 +7518,6 @@ os_close_impl(PyObject *module, int fd) /*[clinic end generated code: output=2fe4e93602822c14 input=2bc42451ca5c3223]*/ { int res; - if (!_PyVerify_fd(fd)) - return posix_error(); /* We do not want to retry upon EINTR: see http://lwn.net/Articles/576478/ * and http://linux.derkeiler.com/Mailing-Lists/Kernel/2005-09/3000.html * for more details. @@ -7583,9 +7550,8 @@ os_closerange_impl(PyObject *module, int fd_low, int fd_high) int i; Py_BEGIN_ALLOW_THREADS _Py_BEGIN_SUPPRESS_IPH - for (i = fd_low; i < fd_high; i++) - if (_PyVerify_fd(i)) - close(i); + for (i = max(fd_low, 0); i < fd_high; i++) + close(i); _Py_END_SUPPRESS_IPH Py_END_ALLOW_THREADS Py_RETURN_NONE; @@ -7629,7 +7595,7 @@ os_dup2_impl(PyObject *module, int fd, int fd2, int inheritable) int dup3_works = -1; #endif - if (!_PyVerify_fd_dup2(fd, fd2)) + if (fd < 0 || fd2 < 0) return posix_error(); /* dup2() can fail with EINTR if the target FD is already open, because it @@ -7753,10 +7719,6 @@ os_lseek_impl(PyObject *module, int fd, Py_off_t position, int how) { Py_off_t result; - if (!_PyVerify_fd(fd)) { - posix_error(); - return -1; - } #ifdef SEEK_SET /* Turn 0, 1, 2 into SEEK_{SET,CUR,END} */ switch (how) { @@ -7769,10 +7731,6 @@ os_lseek_impl(PyObject *module, int fd, Py_off_t position, int how) if (PyErr_Occurred()) return -1; - if (!_PyVerify_fd(fd)) { - posix_error(); - return -1; - } Py_BEGIN_ALLOW_THREADS _Py_BEGIN_SUPPRESS_IPH #ifdef MS_WINDOWS @@ -7980,10 +7938,6 @@ os_pread_impl(PyObject *module, int fd, int length, Py_off_t offset) buffer = PyBytes_FromStringAndSize((char *)NULL, length); if (buffer == NULL) return NULL; - if (!_PyVerify_fd(fd)) { - Py_DECREF(buffer); - return posix_error(); - } do { Py_BEGIN_ALLOW_THREADS @@ -8226,8 +8180,6 @@ os_isatty_impl(PyObject *module, int fd) /*[clinic end generated code: output=6a48c8b4e644ca00 input=08ce94aa1eaf7b5e]*/ { int return_value; - if (!_PyVerify_fd(fd)) - return 0; _Py_BEGIN_SUPPRESS_IPH return_value = isatty(fd); _Py_END_SUPPRESS_IPH @@ -8419,11 +8371,6 @@ os_pwrite_impl(PyObject *module, int fd, Py_buffer *buffer, Py_off_t offset) Py_ssize_t size; int async_err = 0; - if (!_PyVerify_fd(fd)) { - posix_error(); - return -1; - } - do { Py_BEGIN_ALLOW_THREADS _Py_BEGIN_SUPPRESS_IPH @@ -8606,9 +8553,6 @@ os_ftruncate_impl(PyObject *module, int fd, Py_off_t length) int result; int async_err = 0; - if (!_PyVerify_fd(fd)) - return posix_error(); - do { Py_BEGIN_ALLOW_THREADS _Py_BEGIN_SUPPRESS_IPH @@ -10979,11 +10923,6 @@ os_get_inheritable_impl(PyObject *module, int fd) /*[clinic end generated code: output=0445e20e149aa5b8 input=89ac008dc9ab6b95]*/ { int return_value; - if (!_PyVerify_fd(fd)) { - posix_error(); - return -1; - } - _Py_BEGIN_SUPPRESS_IPH return_value = _Py_get_inheritable(fd); _Py_END_SUPPRESS_IPH @@ -11005,8 +10944,6 @@ os_set_inheritable_impl(PyObject *module, int fd, int inheritable) /*[clinic end generated code: output=f1b1918a2f3c38c2 input=9ceaead87a1e2402]*/ { int result; - if (!_PyVerify_fd(fd)) - return posix_error(); _Py_BEGIN_SUPPRESS_IPH result = _Py_set_inheritable(fd, inheritable, NULL); @@ -11080,9 +11017,6 @@ posix_get_blocking(PyObject *self, PyObject *args) if (!PyArg_ParseTuple(args, "i:get_blocking", &fd)) return NULL; - if (!_PyVerify_fd(fd)) - return posix_error(); - _Py_BEGIN_SUPPRESS_IPH blocking = _Py_get_blocking(fd); _Py_END_SUPPRESS_IPH @@ -11106,9 +11040,6 @@ posix_set_blocking(PyObject *self, PyObject *args) if (!PyArg_ParseTuple(args, "ii:set_blocking", &fd, &blocking)) return NULL; - if (!_PyVerify_fd(fd)) - return posix_error(); - _Py_BEGIN_SUPPRESS_IPH result = _Py_set_blocking(fd, blocking); _Py_END_SUPPRESS_IPH diff --git a/Modules/signalmodule.c b/Modules/signalmodule.c index e0780917c87..7eef0b5e59b 100644 --- a/Modules/signalmodule.c +++ b/Modules/signalmodule.c @@ -579,7 +579,7 @@ signal_set_wakeup_fd(PyObject *self, PyObject *args) } fd = (int)sockfd; - if ((SOCKET_T)fd != sockfd || !_PyVerify_fd(fd)) { + if ((SOCKET_T)fd != sockfd) { PyErr_SetString(PyExc_ValueError, "invalid fd"); return NULL; } @@ -609,11 +609,6 @@ signal_set_wakeup_fd(PyObject *self, PyObject *args) if (fd != -1) { int blocking; - if (!_PyVerify_fd(fd)) { - PyErr_SetString(PyExc_ValueError, "invalid fd"); - return NULL; - } - if (_Py_fstat(fd, &status) != 0) return NULL; diff --git a/PC/msvcrtmodule.c b/PC/msvcrtmodule.c index 02927416348..c9d1e6cec60 100644 --- a/PC/msvcrtmodule.c +++ b/PC/msvcrtmodule.c @@ -189,16 +189,11 @@ msvcrt_get_osfhandle_impl(PyObject *module, int fd) { intptr_t handle = -1; - if (!_PyVerify_fd(fd)) { - PyErr_SetFromErrno(PyExc_IOError); - } - else { _Py_BEGIN_SUPPRESS_IPH - handle = _get_osfhandle(fd); + handle = _get_osfhandle(fd); _Py_END_SUPPRESS_IPH - if (handle == -1) - PyErr_SetFromErrno(PyExc_IOError); - } + if (handle == -1) + PyErr_SetFromErrno(PyExc_IOError); return handle; } diff --git a/Parser/myreadline.c b/Parser/myreadline.c index 28c7b6d7fff..9522c794e87 100644 --- a/Parser/myreadline.c +++ b/Parser/myreadline.c @@ -41,10 +41,7 @@ my_fgets(char *buf, int len, FILE *fp) (void)(PyOS_InputHook)(); errno = 0; clearerr(fp); - if (_PyVerify_fd(fileno(fp))) - p = fgets(buf, len, fp); - else - p = NULL; + p = fgets(buf, len, fp); if (p != NULL) return 0; /* No error */ err = errno; diff --git a/Python/fileutils.c b/Python/fileutils.c index 06531d97260..7ce3b63306c 100644 --- a/Python/fileutils.c +++ b/Python/fileutils.c @@ -44,7 +44,7 @@ _Py_device_encoding(int fd) #endif int valid; _Py_BEGIN_SUPPRESS_IPH - valid = _PyVerify_fd(fd) && isatty(fd); + valid = isatty(fd); _Py_END_SUPPRESS_IPH if (!valid) Py_RETURN_NONE; @@ -613,13 +613,9 @@ _Py_fstat_noraise(int fd, struct _Py_stat_struct *status) HANDLE h; int type; - if (!_PyVerify_fd(fd)) - h = INVALID_HANDLE_VALUE; - else { - _Py_BEGIN_SUPPRESS_IPH - h = (HANDLE)_get_osfhandle(fd); - _Py_END_SUPPRESS_IPH - } + _Py_BEGIN_SUPPRESS_IPH + h = (HANDLE)_get_osfhandle(fd); + _Py_END_SUPPRESS_IPH if (h == INVALID_HANDLE_VALUE) { /* errno is already set by _get_osfhandle, but we also set @@ -742,12 +738,6 @@ get_inheritable(int fd, int raise) HANDLE handle; DWORD flags; - if (!_PyVerify_fd(fd)) { - if (raise) - PyErr_SetFromErrno(PyExc_OSError); - return -1; - } - _Py_BEGIN_SUPPRESS_IPH handle = (HANDLE)_get_osfhandle(fd); _Py_END_SUPPRESS_IPH @@ -819,12 +809,6 @@ set_inheritable(int fd, int inheritable, int raise, int *atomic_flag_works) } #ifdef MS_WINDOWS - if (!_PyVerify_fd(fd)) { - if (raise) - PyErr_SetFromErrno(PyExc_OSError); - return -1; - } - _Py_BEGIN_SUPPRESS_IPH handle = (HANDLE)_get_osfhandle(fd); _Py_END_SUPPRESS_IPH @@ -1190,14 +1174,6 @@ _Py_read(int fd, void *buf, size_t count) * handler raised an exception. */ assert(!PyErr_Occurred()); - if (!_PyVerify_fd(fd)) { - /* save/restore errno because PyErr_SetFromErrno() can modify it */ - err = errno; - PyErr_SetFromErrno(PyExc_OSError); - errno = err; - return -1; - } - #ifdef MS_WINDOWS if (count > INT_MAX) { /* On Windows, the count parameter of read() is an int */ @@ -1251,16 +1227,6 @@ _Py_write_impl(int fd, const void *buf, size_t count, int gil_held) int err; int async_err = 0; - if (!_PyVerify_fd(fd)) { - if (gil_held) { - /* save/restore errno because PyErr_SetFromErrno() can modify it */ - err = errno; - PyErr_SetFromErrno(PyExc_OSError); - errno = err; - } - return -1; - } - _Py_BEGIN_SUPPRESS_IPH #ifdef MS_WINDOWS if (count > 32767 && isatty(fd)) { @@ -1497,11 +1463,6 @@ _Py_dup(int fd) assert(PyGILState_Check()); #endif - if (!_PyVerify_fd(fd)) { - PyErr_SetFromErrno(PyExc_OSError); - return -1; - } - #ifdef MS_WINDOWS _Py_BEGIN_SUPPRESS_IPH handle = (HANDLE)_get_osfhandle(fd); @@ -1624,84 +1585,3 @@ error: return -1; } #endif - -#if defined _MSC_VER && _MSC_VER >= 1400 && _MSC_VER < 1900 -/* Legacy implementation of _PyVerify_fd while transitioning to - * MSVC 14.0. This should eventually be removed. (issue23524) - */ - -/* Microsoft CRT in VS2005 and higher will verify that a filehandle is - * valid and raise an assertion if it isn't. - * Normally, an invalid fd is likely to be a C program error and therefore - * an assertion can be useful, but it does contradict the POSIX standard - * which for write(2) states: - * "Otherwise, -1 shall be returned and errno set to indicate the error." - * "[EBADF] The fildes argument is not a valid file descriptor open for - * writing." - * Furthermore, python allows the user to enter any old integer - * as a fd and should merely raise a python exception on error. - * The Microsoft CRT doesn't provide an official way to check for the - * validity of a file descriptor, but we can emulate its internal behaviour - * by using the exported __pinfo data member and knowledge of the - * internal structures involved. - * The structures below must be updated for each version of visual studio - * according to the file internal.h in the CRT source, until MS comes - * up with a less hacky way to do this. - * (all of this is to avoid globally modifying the CRT behaviour using - * _set_invalid_parameter_handler() and _CrtSetReportMode()) - */ -/* The actual size of the structure is determined at runtime. - * Only the first items must be present. - */ -typedef struct { - intptr_t osfhnd; - char osfile; -} my_ioinfo; - -extern __declspec(dllimport) char * __pioinfo[]; -#define IOINFO_L2E 5 -#define IOINFO_ARRAYS 64 -#define IOINFO_ARRAY_ELTS (1 << IOINFO_L2E) -#define _NHANDLE_ (IOINFO_ARRAYS * IOINFO_ARRAY_ELTS) -#define FOPEN 0x01 -#define _NO_CONSOLE_FILENO (intptr_t)-2 - -/* This function emulates what the windows CRT does to validate file handles */ -int -_PyVerify_fd(int fd) -{ - const int i1 = fd >> IOINFO_L2E; - const int i2 = fd & ((1 << IOINFO_L2E) - 1); - - static size_t sizeof_ioinfo = 0; - - /* Determine the actual size of the ioinfo structure, - * as used by the CRT loaded in memory - */ - if (sizeof_ioinfo == 0 && __pioinfo[0] != NULL) { - sizeof_ioinfo = _msize(__pioinfo[0]) / IOINFO_ARRAY_ELTS; - } - if (sizeof_ioinfo == 0) { - /* This should not happen... */ - goto fail; - } - - /* See that it isn't a special CLEAR fileno */ - if (fd != _NO_CONSOLE_FILENO) { - /* Microsoft CRT would check that 0<=fd<_nhandle but we can't do that. Instead - * we check pointer validity and other info - */ - if (0 <= i1 && i1 < IOINFO_ARRAYS && __pioinfo[i1] != NULL) { - /* finally, check that the file is open */ - my_ioinfo* info = (my_ioinfo*)(__pioinfo[i1] + i2 * sizeof_ioinfo); - if (info->osfile & FOPEN) { - return 1; - } - } - } - fail: - errno = EBADF; - return 0; -} - -#endif /* defined _MSC_VER && _MSC_VER >= 1400 && _MSC_VER < 1900 */ diff --git a/Python/pylifecycle.c b/Python/pylifecycle.c index 3f3b614a47a..dab5c3c1ab6 100644 --- a/Python/pylifecycle.c +++ b/Python/pylifecycle.c @@ -1026,7 +1026,7 @@ static int is_valid_fd(int fd) { int fd2; - if (fd < 0 || !_PyVerify_fd(fd)) + if (fd < 0) return 0; _Py_BEGIN_SUPPRESS_IPH /* Prefer dup() over fstat(). fstat() can require input/output whereas