From b02408d5d930a765b2f32991a8468e5bc2c0b948 Mon Sep 17 00:00:00 2001 From: Segev Finer Date: Sat, 3 Jun 2017 15:29:08 +0300 Subject: [PATCH 01/12] bpo-30555: Fix WindowsConsoleIO failure in the presence of fd redirection This works by not caching the handle and instead getting the handle from the file descriptor each time, so that if the actual handle changes by fd redirection closing/opening the console handle beneath our feet, we will keep working correctly. I think I also fixed some resource handling issues along the way since ucrt/msvcrt fds *own* the HANDLEs they use and this module tried to close them itself. --- Modules/_io/clinic/winconsoleio.c.h | 13 +-- Modules/_io/winconsoleio.c | 143 ++++++++++++++++------------ 2 files changed, 88 insertions(+), 68 deletions(-) diff --git a/Modules/_io/clinic/winconsoleio.c.h b/Modules/_io/clinic/winconsoleio.c.h index f694cd8cc88e38..3abd21ed82010b 100644 --- a/Modules/_io/clinic/winconsoleio.c.h +++ b/Modules/_io/clinic/winconsoleio.c.h @@ -8,10 +8,10 @@ PyDoc_STRVAR(_io__WindowsConsoleIO_close__doc__, "close($self, /)\n" "--\n" "\n" -"Close the handle.\n" +"Close the console object.\n" "\n" -"A closed handle cannot be used for further I/O operations. close() may be\n" -"called more than once without error."); +"A closed console object cannot be used for further I/O operations.\n" +"close() may be called more than once without error."); #define _IO__WINDOWSCONSOLEIO_CLOSE_METHODDEF \ {"close", (PyCFunction)_io__WindowsConsoleIO_close, METH_NOARGS, _io__WindowsConsoleIO_close__doc__}, @@ -73,10 +73,7 @@ PyDoc_STRVAR(_io__WindowsConsoleIO_fileno__doc__, "fileno($self, /)\n" "--\n" "\n" -"Return the underlying file descriptor (an integer).\n" -"\n" -"fileno is only set when a file descriptor is used to open\n" -"one of the standard streams."); +"Return the underlying file descriptor (an integer)."); #define _IO__WINDOWSCONSOLEIO_FILENO_METHODDEF \ {"fileno", (PyCFunction)_io__WindowsConsoleIO_fileno, METH_NOARGS, _io__WindowsConsoleIO_fileno__doc__}, @@ -332,4 +329,4 @@ _io__WindowsConsoleIO_isatty(winconsoleio *self, PyObject *Py_UNUSED(ignored)) #ifndef _IO__WINDOWSCONSOLEIO_ISATTY_METHODDEF #define _IO__WINDOWSCONSOLEIO_ISATTY_METHODDEF #endif /* !defined(_IO__WINDOWSCONSOLEIO_ISATTY_METHODDEF) */ -/*[clinic end generated code: output=f2a240ec6af12a20 input=a9049054013a1b77]*/ +/*[clinic end generated code: output=91d7de6d9241470a input=a9049054013a1b77]*/ diff --git a/Modules/_io/winconsoleio.c b/Modules/_io/winconsoleio.c index d51df7e8887cee..dd2aee2ec85907 100644 --- a/Modules/_io/winconsoleio.c +++ b/Modules/_io/winconsoleio.c @@ -133,6 +133,21 @@ char _PyIO_get_console_type(PyObject *path_or_fd) { return m; } +static HANDLE py_get_osfhandle(int fd) +{ + HANDLE handle; + + _Py_BEGIN_SUPPRESS_IPH + handle = (HANDLE)_get_osfhandle(fd); + _Py_END_SUPPRESS_IPH + if (handle == INVALID_HANDLE_VALUE) { + PyErr_SetFromErrno(PyExc_OSError); + return INVALID_HANDLE_VALUE; + } + + return handle; +} + /*[clinic input] module _io @@ -142,12 +157,11 @@ class _io._WindowsConsoleIO "winconsoleio *" "&PyWindowsConsoleIO_Type" typedef struct { PyObject_HEAD - HANDLE handle; int fd; unsigned int created : 1; unsigned int readable : 1; unsigned int writable : 1; - unsigned int closehandle : 1; + unsigned int closefd : 1; char finalizing; unsigned int blksize; PyObject *weakreflist; @@ -163,7 +177,7 @@ _Py_IDENTIFIER(name); int _PyWindowsConsoleIO_closed(PyObject *self) { - return ((winconsoleio *)self)->handle == INVALID_HANDLE_VALUE; + return ((winconsoleio *)self)->fd == -1; } @@ -171,16 +185,12 @@ _PyWindowsConsoleIO_closed(PyObject *self) static int internal_close(winconsoleio *self) { - if (self->handle != INVALID_HANDLE_VALUE) { - if (self->closehandle) { - if (self->fd >= 0) { - _Py_BEGIN_SUPPRESS_IPH - close(self->fd); - _Py_END_SUPPRESS_IPH - } - CloseHandle(self->handle); + if (self->fd != -1) { + if (self->closefd) { + _Py_BEGIN_SUPPRESS_IPH + close(self->fd); + _Py_END_SUPPRESS_IPH } - self->handle = INVALID_HANDLE_VALUE; self->fd = -1; } return 0; @@ -189,15 +199,15 @@ internal_close(winconsoleio *self) /*[clinic input] _io._WindowsConsoleIO.close -Close the handle. +Close the console object. -A closed handle cannot be used for further I/O operations. close() may be -called more than once without error. +A closed console object cannot be used for further I/O operations. +close() may be called more than once without error. [clinic start generated code]*/ static PyObject * _io__WindowsConsoleIO_close_impl(winconsoleio *self) -/*[clinic end generated code: output=27ef95b66c29057b input=185617e349ae4c7b]*/ +/*[clinic end generated code: output=27ef95b66c29057b input=68c4e5754f8136c2]*/ { PyObject *res; PyObject *exc, *val, *tb; @@ -205,8 +215,8 @@ _io__WindowsConsoleIO_close_impl(winconsoleio *self) _Py_IDENTIFIER(close); res = _PyObject_CallMethodIdObjArgs((PyObject*)&PyRawIOBase_Type, &PyId_close, self, NULL); - if (!self->closehandle) { - self->handle = INVALID_HANDLE_VALUE; + if (!self->closefd) { + self->fd = -1; return res; } if (res == NULL) @@ -228,12 +238,11 @@ winconsoleio_new(PyTypeObject *type, PyObject *args, PyObject *kwds) self = (winconsoleio *) type->tp_alloc(type, 0); if (self != NULL) { - self->handle = INVALID_HANDLE_VALUE; self->fd = -1; self->created = 0; self->readable = 0; self->writable = 0; - self->closehandle = 0; + self->closefd = 0; self->blksize = 0; self->weakreflist = NULL; } @@ -268,16 +277,17 @@ _io__WindowsConsoleIO___init___impl(winconsoleio *self, PyObject *nameobj, int rwa = 0; int fd = -1; int fd_is_own = 0; + HANDLE handle = NULL; assert(PyWindowsConsoleIO_Check(self)); - if (self->handle >= 0) { - if (self->closehandle) { + if (self->fd >= 0) { + if (self->closefd) { /* Have to close the existing file first. */ if (internal_close(self) < 0) return -1; } else - self->handle = INVALID_HANDLE_VALUE; + self->fd = -1; } if (PyFloat_Check(nameobj)) { @@ -355,13 +365,13 @@ _io__WindowsConsoleIO___init___impl(winconsoleio *self, PyObject *nameobj, if (fd >= 0) { _Py_BEGIN_SUPPRESS_IPH - self->handle = (HANDLE)_get_osfhandle(fd); + handle = (HANDLE)_get_osfhandle(fd); _Py_END_SUPPRESS_IPH - self->closehandle = 0; + self->closefd = 0; } else { DWORD access = GENERIC_READ; - self->closehandle = 1; + self->closefd = 1; if (!closefd) { PyErr_SetString(PyExc_ValueError, "Cannot use closefd=False with file name"); @@ -376,21 +386,33 @@ _io__WindowsConsoleIO___init___impl(winconsoleio *self, PyObject *nameobj, on the specific access. This is required for modern names CONIN$ and CONOUT$, which allow reading/writing state as well as reading/writing content. */ - self->handle = CreateFileW(name, GENERIC_READ | GENERIC_WRITE, + handle = CreateFileW(name, GENERIC_READ | GENERIC_WRITE, FILE_SHARE_READ | FILE_SHARE_WRITE, NULL, OPEN_EXISTING, 0, NULL); - if (self->handle == INVALID_HANDLE_VALUE) - self->handle = CreateFileW(name, access, + if (handle == INVALID_HANDLE_VALUE) + handle = CreateFileW(name, access, FILE_SHARE_READ | FILE_SHARE_WRITE, NULL, OPEN_EXISTING, 0, NULL); Py_END_ALLOW_THREADS - if (self->handle == INVALID_HANDLE_VALUE) { + if (handle == INVALID_HANDLE_VALUE) { PyErr_SetExcFromWindowsErrWithFilenameObject(PyExc_OSError, GetLastError(), nameobj); goto error; } + + _Py_BEGIN_SUPPRESS_IPH + if (self->writable) + self->fd = _open_osfhandle((intptr_t)handle, _O_WRONLY | _O_BINARY); + else + self->fd = _open_osfhandle((intptr_t)handle, _O_RDONLY | _O_BINARY); + _Py_END_SUPPRESS_IPH + if (self->fd < 0) { + CloseHandle(handle); + PyErr_SetFromErrnoWithFilenameObject(PyExc_OSError, nameobj); + goto error; + } } if (console_type == '\0') - console_type = _get_console_type(self->handle); + console_type = _get_console_type(handle); if (self->writable && console_type != 'w') { PyErr_SetString(PyExc_ValueError, @@ -473,25 +495,12 @@ _io._WindowsConsoleIO.fileno Return the underlying file descriptor (an integer). -fileno is only set when a file descriptor is used to open -one of the standard streams. - [clinic start generated code]*/ static PyObject * _io__WindowsConsoleIO_fileno_impl(winconsoleio *self) -/*[clinic end generated code: output=006fa74ce3b5cfbf input=079adc330ddaabe6]*/ +/*[clinic end generated code: output=006fa74ce3b5cfbf input=845c47ebbc3a2f67]*/ { - if (self->fd < 0 && self->handle != INVALID_HANDLE_VALUE) { - _Py_BEGIN_SUPPRESS_IPH - if (self->writable) - self->fd = _open_osfhandle((intptr_t)self->handle, _O_WRONLY | _O_BINARY); - else - self->fd = _open_osfhandle((intptr_t)self->handle, _O_RDONLY | _O_BINARY); - _Py_END_SUPPRESS_IPH - } - if (self->fd < 0) - return err_mode("fileno"); return PyLong_FromLong(self->fd); } @@ -505,7 +514,7 @@ static PyObject * _io__WindowsConsoleIO_readable_impl(winconsoleio *self) /*[clinic end generated code: output=daf9cef2743becf0 input=6be9defb5302daae]*/ { - if (self->handle == INVALID_HANDLE_VALUE) + if (self->fd == -1) return err_closed(); return PyBool_FromLong((long) self->readable); } @@ -520,7 +529,7 @@ static PyObject * _io__WindowsConsoleIO_writable_impl(winconsoleio *self) /*[clinic end generated code: output=e0a2ad7eae5abf67 input=cefbd8abc24df6a0]*/ { - if (self->handle == INVALID_HANDLE_VALUE) + if (self->fd == -1) return err_closed(); return PyBool_FromLong((long) self->writable); } @@ -651,7 +660,7 @@ read_console_w(HANDLE handle, DWORD maxlen, DWORD *readlen) { static Py_ssize_t readinto(winconsoleio *self, char *buf, Py_ssize_t len) { - if (self->handle == INVALID_HANDLE_VALUE) { + if (self->fd == -1) { err_closed(); return -1; } @@ -666,6 +675,10 @@ readinto(winconsoleio *self, char *buf, Py_ssize_t len) return -1; } + HANDLE handle = py_get_osfhandle(self->fd); + if (handle == INVALID_HANDLE_VALUE) + return -1; + /* Each character may take up to 4 bytes in the final buffer. This is highly conservative, but necessary to avoid failure for any given Unicode input (e.g. \U0010ffff). @@ -687,7 +700,7 @@ readinto(winconsoleio *self, char *buf, Py_ssize_t len) return read_len; DWORD n; - wchar_t *wbuf = read_console_w(self->handle, wlen, &n); + wchar_t *wbuf = read_console_w(handle, wlen, &n); if (wbuf == NULL) return -1; if (n == 0) { @@ -793,10 +806,15 @@ _io__WindowsConsoleIO_readall_impl(winconsoleio *self) DWORD bufsize, n, len = 0; PyObject *bytes; DWORD bytes_size, rn; + HANDLE handle; - if (self->handle == INVALID_HANDLE_VALUE) + if (self->fd == -1) return err_closed(); + handle = py_get_osfhandle(self->fd); + if (handle == INVALID_HANDLE_VALUE) + return NULL; + bufsize = BUFSIZ; buf = (wchar_t*)PyMem_Malloc((bufsize + 1) * sizeof(wchar_t)); @@ -826,7 +844,7 @@ _io__WindowsConsoleIO_readall_impl(winconsoleio *self) } } - subbuf = read_console_w(self->handle, bufsize - len, &n); + subbuf = read_console_w(handle, bufsize - len, &n); if (subbuf == NULL) { PyMem_Free(buf); @@ -916,7 +934,7 @@ _io__WindowsConsoleIO_read_impl(winconsoleio *self, Py_ssize_t size) PyObject *bytes; Py_ssize_t bytes_size; - if (self->handle == INVALID_HANDLE_VALUE) + if (self->fd == -1) return err_closed(); if (!self->readable) return err_mode("reading"); @@ -966,12 +984,17 @@ _io__WindowsConsoleIO_write_impl(winconsoleio *self, Py_buffer *b) BOOL res = TRUE; wchar_t *wbuf; DWORD len, wlen, n = 0; + HANDLE handle; - if (self->handle == INVALID_HANDLE_VALUE) + if (self->fd == -1) return err_closed(); if (!self->writable) return err_mode("writing"); + handle = py_get_osfhandle(self->fd); + if (handle == INVALID_HANDLE_VALUE) + return NULL; + if (b->len > BUFMAX) len = BUFMAX; else @@ -999,7 +1022,7 @@ _io__WindowsConsoleIO_write_impl(winconsoleio *self, Py_buffer *b) Py_BEGIN_ALLOW_THREADS wlen = MultiByteToWideChar(CP_UTF8, 0, b->buf, len, wbuf, wlen); if (wlen) { - res = WriteConsoleW(self->handle, wbuf, wlen, &n, NULL); + res = WriteConsoleW(handle, wbuf, wlen, &n, NULL); if (res && n < wlen) { /* Wrote fewer characters than expected, which means our * len value may be wrong. So recalculate it from the @@ -1031,15 +1054,15 @@ _io__WindowsConsoleIO_write_impl(winconsoleio *self, Py_buffer *b) static PyObject * winconsoleio_repr(winconsoleio *self) { - if (self->handle == INVALID_HANDLE_VALUE) + if (self->fd == -1) return PyUnicode_FromFormat("<_io._WindowsConsoleIO [closed]>"); if (self->readable) return PyUnicode_FromFormat("<_io._WindowsConsoleIO mode='rb' closefd=%s>", - self->closehandle ? "True" : "False"); + self->closefd ? "True" : "False"); if (self->writable) return PyUnicode_FromFormat("<_io._WindowsConsoleIO mode='wb' closefd=%s>", - self->closehandle ? "True" : "False"); + self->closefd ? "True" : "False"); PyErr_SetString(PyExc_SystemError, "_WindowsConsoleIO has invalid mode"); return NULL; @@ -1055,7 +1078,7 @@ static PyObject * _io__WindowsConsoleIO_isatty_impl(winconsoleio *self) /*[clinic end generated code: output=9eac09d287c11bd7 input=9b91591dbe356f86]*/ { - if (self->handle == INVALID_HANDLE_VALUE) + if (self->fd == -1) return err_closed(); Py_RETURN_TRUE; @@ -1090,13 +1113,13 @@ static PyMethodDef winconsoleio_methods[] = { static PyObject * get_closed(winconsoleio *self, void *closure) { - return PyBool_FromLong((long)(self->handle == INVALID_HANDLE_VALUE)); + return PyBool_FromLong((long)(self->fd == -1)); } static PyObject * get_closefd(winconsoleio *self, void *closure) { - return PyBool_FromLong((long)(self->closehandle)); + return PyBool_FromLong((long)(self->closefd)); } static PyObject * From ac242ebd35ebc8946c3f9bd67fab786266059661 Mon Sep 17 00:00:00 2001 From: Segev Finer Date: Sat, 3 Jun 2017 16:04:19 +0300 Subject: [PATCH 02/12] bpo-30555: Fixed the kludgey PC/_testconsole.c, grumble grumble... --- PC/_testconsole.c | 54 ++++++++++++++++++++++++++++++++++++++++++++--- 1 file changed, 51 insertions(+), 3 deletions(-) diff --git a/PC/_testconsole.c b/PC/_testconsole.c index 1c93679e435422..f744a57f45ff7c 100644 --- a/PC/_testconsole.c +++ b/PC/_testconsole.c @@ -12,11 +12,56 @@ #include #include +/* + * Macros to protect CRT calls against instant termination when passed an + * invalid parameter (issue23524). + * + * It does not need to be defined when building using MSVC + * earlier than 14.0 (_MSC_VER == 1900). + * + * Copied here since it's required to call _get_osfhandle safely and we don't + * want to export py_get_osfhandle only because this test module needs it. + */ +#if defined _MSC_VER && _MSC_VER >= 1900 + +static void __cdecl _silent_invalid_parameter_handler( + wchar_t const* expression, + wchar_t const* function, + wchar_t const* file, + unsigned int line, + uintptr_t pReserved) { } + +#define _Py_BEGIN_SUPPRESS_IPH { _invalid_parameter_handler _Py_old_handler = \ + _set_thread_local_invalid_parameter_handler(_silent_invalid_parameter_handler); +#define _Py_END_SUPPRESS_IPH _set_thread_local_invalid_parameter_handler(_Py_old_handler); } + +#else + +#define _Py_BEGIN_SUPPRESS_IPH +#define _Py_END_SUPPRESS_IPH + +#endif /* _MSC_VER >= 1900 */ + +static HANDLE py_get_osfhandle(int fd) +{ + HANDLE handle; + + _Py_BEGIN_SUPPRESS_IPH + handle = (HANDLE)_get_osfhandle(fd); + _Py_END_SUPPRESS_IPH + if (handle == INVALID_HANDLE_VALUE) { + PyErr_SetFromErrno(PyExc_OSError); + return INVALID_HANDLE_VALUE; + } + + return handle; +} + /* The full definition is in iomodule. We reproduce - enough here to get the handle, which is all we want. */ + enough here to get the fd, which is all we want. */ typedef struct { PyObject_HEAD - HANDLE handle; + int fd; } winconsoleio; @@ -68,7 +113,10 @@ _testconsole_write_input_impl(PyObject *module, PyObject *file, prec->Event.KeyEvent.uChar.UnicodeChar = *p; } - HANDLE hInput = ((winconsoleio*)file)->handle; + HANDLE hInput = py_get_osfhandle(((winconsoleio*)file)->fd); + if (hInput == INVALID_HANDLE_VALUE) + goto error; + DWORD total = 0; while (total < size) { DWORD wrote; From 645dfa33cc43640dc2a294a477a63f8175948190 Mon Sep 17 00:00:00 2001 From: Segev Finer Date: Sun, 4 Jun 2017 19:40:01 +0300 Subject: [PATCH 03/12] bpo-30555: Fix refleaks The change in test_winconsoleio.py is only to avoid the ref leak in `sys.getwindowsversion`. --- Modules/_io/winconsoleio.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/Modules/_io/winconsoleio.c b/Modules/_io/winconsoleio.c index dd2aee2ec85907..4a9df1df671cc7 100644 --- a/Modules/_io/winconsoleio.c +++ b/Modules/_io/winconsoleio.c @@ -308,8 +308,7 @@ _io__WindowsConsoleIO___init___impl(winconsoleio *self, PyObject *nameobj, self->fd = fd; if (fd < 0) { - PyObject *decodedname = Py_None; - Py_INCREF(decodedname); + PyObject *decodedname = NULL; int d = PyUnicode_FSDecoder(nameobj, (void*)&decodedname); if (!d) From 013d8ea6aaf4643b99701aeda73590fbad42a926 Mon Sep 17 00:00:00 2001 From: Segev Finer Date: Sun, 4 Jun 2017 20:52:20 +0300 Subject: [PATCH 04/12] bpo-30555: Add _Py_{get,open}_osfhandle{,_noraise} and use them instead --- Include/fileutils.h | 10 +++++++- Modules/_io/winconsoleio.c | 21 +++-------------- PC/_testconsole.c | 47 +------------------------------------- Python/fileutils.c | 38 +++++++++++++++++++++++++++++- 4 files changed, 50 insertions(+), 66 deletions(-) diff --git a/Include/fileutils.h b/Include/fileutils.h index 900c70faad719f..99dff326fa4b20 100644 --- a/Include/fileutils.h +++ b/Include/fileutils.h @@ -117,7 +117,15 @@ PyAPI_FUNC(int) _Py_dup(int fd); PyAPI_FUNC(int) _Py_get_blocking(int fd); PyAPI_FUNC(int) _Py_set_blocking(int fd, int blocking); -#endif /* !MS_WINDOWS */ +#else /* MS_WINDOWS */ +PyAPI_FUNC(intptr_t) _Py_get_osfhandle_noraise(int fd); + +PyAPI_FUNC(intptr_t) _Py_get_osfhandle(int fd); + +PyAPI_FUNC(int) _Py_open_osfhandle_noraise(intptr_t handle, int flags); + +PyAPI_FUNC(int) _Py_open_osfhandle(intptr_t handle, int flags); +#endif /* MS_WINDOWS */ #endif /* Py_LIMITED_API */ diff --git a/Modules/_io/winconsoleio.c b/Modules/_io/winconsoleio.c index 4a9df1df671cc7..8ff1aa6678d5f3 100644 --- a/Modules/_io/winconsoleio.c +++ b/Modules/_io/winconsoleio.c @@ -133,21 +133,6 @@ char _PyIO_get_console_type(PyObject *path_or_fd) { return m; } -static HANDLE py_get_osfhandle(int fd) -{ - HANDLE handle; - - _Py_BEGIN_SUPPRESS_IPH - handle = (HANDLE)_get_osfhandle(fd); - _Py_END_SUPPRESS_IPH - if (handle == INVALID_HANDLE_VALUE) { - PyErr_SetFromErrno(PyExc_OSError); - return INVALID_HANDLE_VALUE; - } - - return handle; -} - /*[clinic input] module _io @@ -674,7 +659,7 @@ readinto(winconsoleio *self, char *buf, Py_ssize_t len) return -1; } - HANDLE handle = py_get_osfhandle(self->fd); + HANDLE handle = (HANDLE)_Py_get_osfhandle(self->fd); if (handle == INVALID_HANDLE_VALUE) return -1; @@ -810,7 +795,7 @@ _io__WindowsConsoleIO_readall_impl(winconsoleio *self) if (self->fd == -1) return err_closed(); - handle = py_get_osfhandle(self->fd); + handle = (HANDLE)_Py_get_osfhandle(self->fd); if (handle == INVALID_HANDLE_VALUE) return NULL; @@ -990,7 +975,7 @@ _io__WindowsConsoleIO_write_impl(winconsoleio *self, Py_buffer *b) if (!self->writable) return err_mode("writing"); - handle = py_get_osfhandle(self->fd); + handle = (HANDLE)_Py_get_osfhandle(self->fd); if (handle == INVALID_HANDLE_VALUE) return NULL; diff --git a/PC/_testconsole.c b/PC/_testconsole.c index f744a57f45ff7c..ea74e47fe58c26 100644 --- a/PC/_testconsole.c +++ b/PC/_testconsole.c @@ -12,51 +12,6 @@ #include #include -/* - * Macros to protect CRT calls against instant termination when passed an - * invalid parameter (issue23524). - * - * It does not need to be defined when building using MSVC - * earlier than 14.0 (_MSC_VER == 1900). - * - * Copied here since it's required to call _get_osfhandle safely and we don't - * want to export py_get_osfhandle only because this test module needs it. - */ -#if defined _MSC_VER && _MSC_VER >= 1900 - -static void __cdecl _silent_invalid_parameter_handler( - wchar_t const* expression, - wchar_t const* function, - wchar_t const* file, - unsigned int line, - uintptr_t pReserved) { } - -#define _Py_BEGIN_SUPPRESS_IPH { _invalid_parameter_handler _Py_old_handler = \ - _set_thread_local_invalid_parameter_handler(_silent_invalid_parameter_handler); -#define _Py_END_SUPPRESS_IPH _set_thread_local_invalid_parameter_handler(_Py_old_handler); } - -#else - -#define _Py_BEGIN_SUPPRESS_IPH -#define _Py_END_SUPPRESS_IPH - -#endif /* _MSC_VER >= 1900 */ - -static HANDLE py_get_osfhandle(int fd) -{ - HANDLE handle; - - _Py_BEGIN_SUPPRESS_IPH - handle = (HANDLE)_get_osfhandle(fd); - _Py_END_SUPPRESS_IPH - if (handle == INVALID_HANDLE_VALUE) { - PyErr_SetFromErrno(PyExc_OSError); - return INVALID_HANDLE_VALUE; - } - - return handle; -} - /* The full definition is in iomodule. We reproduce enough here to get the fd, which is all we want. */ typedef struct { @@ -113,7 +68,7 @@ _testconsole_write_input_impl(PyObject *module, PyObject *file, prec->Event.KeyEvent.uChar.UnicodeChar = *p; } - HANDLE hInput = py_get_osfhandle(((winconsoleio*)file)->fd); + HANDLE hInput = (HANDLE)_Py_get_osfhandle(((winconsoleio*)file)->fd); if (hInput == INVALID_HANDLE_VALUE) goto error; diff --git a/Python/fileutils.c b/Python/fileutils.c index f3764e4b3c5a2d..65eb9ec725f62e 100644 --- a/Python/fileutils.c +++ b/Python/fileutils.c @@ -1585,4 +1585,40 @@ _Py_set_blocking(int fd, int blocking) PyErr_SetFromErrno(PyExc_OSError); return -1; } -#endif +#else /* MS_WINDOWS */ +intptr_t +_Py_get_osfhandle_noraise(int fd) +{ + _Py_BEGIN_SUPPRESS_IPH + return _get_osfhandle(fd); + _Py_END_SUPPRESS_IPH +} + +intptr_t +_Py_get_osfhandle(int fd) +{ + intptr_t handle = _Py_get_osfhandle_noraise(fd); + if (handle == (intptr_t)INVALID_HANDLE_VALUE) + PyErr_SetFromErrno(PyExc_OSError); + + return handle; +} + +int +_Py_open_osfhandle_noraise(intptr_t handle, int flags) +{ + _Py_BEGIN_SUPPRESS_IPH + return _open_osfhandle(handle, flags); + _Py_END_SUPPRESS_IPH +} + +int +_Py_open_osfhandle(intptr_t handle, int flags) +{ + int fd = _Py_open_osfhandle_noraise(handle, flags); + if (fd == -1) + PyErr_SetFromErrno(PyExc_OSError); + + return fd; +} +#endif /* MS_WINDOWS */ From aaf6020ccbeb3b1e9cb1dc9299247d270e3ffa11 Mon Sep 17 00:00:00 2001 From: Segev Finer Date: Sun, 4 Jun 2017 21:26:22 +0300 Subject: [PATCH 05/12] bpo-30555: Swapped the rest of the uses of _{get,open}_osfhandle --- Modules/_io/winconsoleio.c | 15 ++++----------- Modules/mmapmodule.c | 9 +++------ Modules/posixmodule.c | 6 ++---- PC/msvcrtmodule.c | 20 ++------------------ Parser/myreadline.c | 6 ++---- Python/fileutils.c | 20 +++++--------------- 6 files changed, 18 insertions(+), 58 deletions(-) diff --git a/Modules/_io/winconsoleio.c b/Modules/_io/winconsoleio.c index 8ff1aa6678d5f3..1f884b8f0dcdc1 100644 --- a/Modules/_io/winconsoleio.c +++ b/Modules/_io/winconsoleio.c @@ -63,10 +63,7 @@ char _PyIO_get_console_type(PyObject *path_or_fd) { int fd = PyLong_AsLong(path_or_fd); PyErr_Clear(); if (fd >= 0) { - HANDLE handle; - _Py_BEGIN_SUPPRESS_IPH - handle = (HANDLE)_get_osfhandle(fd); - _Py_END_SUPPRESS_IPH + HANDLE handle = (HANDLE)_Py_get_osfhandle_noraise(fd); if (handle == INVALID_HANDLE_VALUE) return '\0'; return _get_console_type(handle); @@ -348,9 +345,7 @@ _io__WindowsConsoleIO___init___impl(winconsoleio *self, PyObject *nameobj, goto bad_mode; if (fd >= 0) { - _Py_BEGIN_SUPPRESS_IPH - handle = (HANDLE)_get_osfhandle(fd); - _Py_END_SUPPRESS_IPH + handle = (HANDLE)_Py_get_osfhandle_noraise(fd); self->closefd = 0; } else { DWORD access = GENERIC_READ; @@ -382,12 +377,10 @@ _io__WindowsConsoleIO___init___impl(winconsoleio *self, PyObject *nameobj, goto error; } - _Py_BEGIN_SUPPRESS_IPH if (self->writable) - self->fd = _open_osfhandle((intptr_t)handle, _O_WRONLY | _O_BINARY); + self->fd = _Py_open_osfhandle_noraise((intptr_t)handle, _O_WRONLY | _O_BINARY); else - self->fd = _open_osfhandle((intptr_t)handle, _O_RDONLY | _O_BINARY); - _Py_END_SUPPRESS_IPH + self->fd = _Py_open_osfhandle_noraise((intptr_t)handle, _O_RDONLY | _O_BINARY); if (self->fd < 0) { CloseHandle(handle); PyErr_SetFromErrnoWithFilenameObject(PyExc_OSError, nameobj); diff --git a/Modules/mmapmodule.c b/Modules/mmapmodule.c index 49214a1defce89..a0649927eb8a9f 100644 --- a/Modules/mmapmodule.c +++ b/Modules/mmapmodule.c @@ -1266,13 +1266,10 @@ new_mmap_object(PyTypeObject *type, PyObject *args, PyObject *kwdict) */ if (fileno != -1 && fileno != 0) { /* Ensure that fileno is within the CRT's valid range */ - _Py_BEGIN_SUPPRESS_IPH - fh = (HANDLE)_get_osfhandle(fileno); - _Py_END_SUPPRESS_IPH - if (fh==(HANDLE)-1) { - PyErr_SetFromErrno(PyExc_OSError); + fh = (HANDLE)_Py_get_osfhandle(fileno); + if (fh == (HANDLE)INVALID_HANDLE_VALUE) return NULL; - } + /* Win9x appears to need us seeked to zero */ lseek(fileno, 0, SEEK_SET); } diff --git a/Modules/posixmodule.c b/Modules/posixmodule.c index f4a21679d0b202..8352724f38decd 100644 --- a/Modules/posixmodule.c +++ b/Modules/posixmodule.c @@ -8452,18 +8452,16 @@ os_pipe_impl(PyObject *module) attr.bInheritHandle = FALSE; Py_BEGIN_ALLOW_THREADS - _Py_BEGIN_SUPPRESS_IPH ok = CreatePipe(&read, &write, &attr, 0); if (ok) { - fds[0] = _open_osfhandle((intptr_t)read, _O_RDONLY); - fds[1] = _open_osfhandle((intptr_t)write, _O_WRONLY); + fds[0] = _Py_open_osfhandle_noraise((intptr_t)read, _O_RDONLY); + fds[1] = _Py_open_osfhandle_noraise((intptr_t)write, _O_WRONLY); if (fds[0] == -1 || fds[1] == -1) { CloseHandle(read); CloseHandle(write); ok = 0; } } - _Py_END_SUPPRESS_IPH Py_END_ALLOW_THREADS if (!ok) diff --git a/PC/msvcrtmodule.c b/PC/msvcrtmodule.c index 59bf54fa0b7743..0b25aa431d4460 100644 --- a/PC/msvcrtmodule.c +++ b/PC/msvcrtmodule.c @@ -167,15 +167,7 @@ static long msvcrt_open_osfhandle_impl(PyObject *module, intptr_t handle, int flags) /*[clinic end generated code: output=cede871bf939d6e3 input=cb2108bbea84514e]*/ { - int fd; - - _Py_BEGIN_SUPPRESS_IPH - fd = _open_osfhandle(handle, flags); - _Py_END_SUPPRESS_IPH - if (fd == -1) - PyErr_SetFromErrno(PyExc_OSError); - - return fd; + return _Py_open_osfhandle(handle, flags); } /*[clinic input] @@ -193,15 +185,7 @@ static intptr_t msvcrt_get_osfhandle_impl(PyObject *module, int fd) /*[clinic end generated code: output=7ce761dd0de2b503 input=305900f4bfab76c7]*/ { - intptr_t handle = -1; - - _Py_BEGIN_SUPPRESS_IPH - handle = _get_osfhandle(fd); - _Py_END_SUPPRESS_IPH - if (handle == -1) - PyErr_SetFromErrno(PyExc_OSError); - - return handle; + return _Py_get_osfhandle(fd); } /* Console I/O */ diff --git a/Parser/myreadline.c b/Parser/myreadline.c index 9f3c2e343c032c..15597a774a410d 100644 --- a/Parser/myreadline.c +++ b/Parser/myreadline.c @@ -205,10 +205,8 @@ PyOS_StdioReadline(FILE *sys_stdin, FILE *sys_stdout, const char *prompt) if (!Py_LegacyWindowsStdioFlag && sys_stdin == stdin) { HANDLE hStdIn, hStdErr; - _Py_BEGIN_SUPPRESS_IPH - hStdIn = (HANDLE)_get_osfhandle(fileno(sys_stdin)); - hStdErr = (HANDLE)_get_osfhandle(fileno(stderr)); - _Py_END_SUPPRESS_IPH + hStdIn = (HANDLE)_Py_get_osfhandle_noraise(fileno(sys_stdin)); + hStdErr = (HANDLE)_Py_get_osfhandle_noraise(fileno(stderr)); if (_get_console_type(hStdIn) == 'r') { fflush(sys_stdout); diff --git a/Python/fileutils.c b/Python/fileutils.c index 65eb9ec725f62e..c634468db87116 100644 --- a/Python/fileutils.c +++ b/Python/fileutils.c @@ -614,9 +614,7 @@ _Py_fstat_noraise(int fd, struct _Py_stat_struct *status) HANDLE h; int type; - _Py_BEGIN_SUPPRESS_IPH - h = (HANDLE)_get_osfhandle(fd); - _Py_END_SUPPRESS_IPH + h = (HANDLE)_Py_get_osfhandle_noraise(fd); if (h == INVALID_HANDLE_VALUE) { /* errno is already set by _get_osfhandle, but we also set @@ -739,9 +737,7 @@ get_inheritable(int fd, int raise) HANDLE handle; DWORD flags; - _Py_BEGIN_SUPPRESS_IPH - handle = (HANDLE)_get_osfhandle(fd); - _Py_END_SUPPRESS_IPH + handle = (HANDLE)_Py_get_osfhandle_noraise(fd); if (handle == INVALID_HANDLE_VALUE) { if (raise) PyErr_SetFromErrno(PyExc_OSError); @@ -810,9 +806,7 @@ set_inheritable(int fd, int inheritable, int raise, int *atomic_flag_works) } #ifdef MS_WINDOWS - _Py_BEGIN_SUPPRESS_IPH - handle = (HANDLE)_get_osfhandle(fd); - _Py_END_SUPPRESS_IPH + handle = (HANDLE)_Py_get_osfhandle_noraise(fd); if (handle == INVALID_HANDLE_VALUE) { if (raise) PyErr_SetFromErrno(PyExc_OSError); @@ -1465,13 +1459,9 @@ _Py_dup(int fd) #endif #ifdef MS_WINDOWS - _Py_BEGIN_SUPPRESS_IPH - handle = (HANDLE)_get_osfhandle(fd); - _Py_END_SUPPRESS_IPH - if (handle == INVALID_HANDLE_VALUE) { - PyErr_SetFromErrno(PyExc_OSError); + handle = (HANDLE)_Py_get_osfhandle(fd); + if (handle == INVALID_HANDLE_VALUE) return -1; - } /* get the file type, ignore the error if it failed */ ftype = GetFileType(handle); From 7727bc558bf37047228271deb2ddf89f75ac4988 Mon Sep 17 00:00:00 2001 From: Segev Finer Date: Mon, 5 Jun 2017 19:41:08 +0300 Subject: [PATCH 06/12] bpo-30555: Swap intptr_t to void* in _Py_{get,open}_osfhandle{,_noraise} --- Include/fileutils.h | 8 ++++---- Modules/_io/winconsoleio.c | 14 +++++++------- Modules/mmapmodule.c | 4 ++-- Modules/posixmodule.c | 4 ++-- PC/_testconsole.c | 2 +- PC/msvcrtmodule.c | 4 ++-- Parser/myreadline.c | 4 ++-- Python/fileutils.c | 24 ++++++++++++------------ 8 files changed, 32 insertions(+), 32 deletions(-) diff --git a/Include/fileutils.h b/Include/fileutils.h index 99dff326fa4b20..dfe3e3292c4802 100644 --- a/Include/fileutils.h +++ b/Include/fileutils.h @@ -118,13 +118,13 @@ PyAPI_FUNC(int) _Py_get_blocking(int fd); PyAPI_FUNC(int) _Py_set_blocking(int fd, int blocking); #else /* MS_WINDOWS */ -PyAPI_FUNC(intptr_t) _Py_get_osfhandle_noraise(int fd); +PyAPI_FUNC(void*) _Py_get_osfhandle_noraise(int fd); -PyAPI_FUNC(intptr_t) _Py_get_osfhandle(int fd); +PyAPI_FUNC(void*) _Py_get_osfhandle(int fd); -PyAPI_FUNC(int) _Py_open_osfhandle_noraise(intptr_t handle, int flags); +PyAPI_FUNC(int) _Py_open_osfhandle_noraise(void *handle, int flags); -PyAPI_FUNC(int) _Py_open_osfhandle(intptr_t handle, int flags); +PyAPI_FUNC(int) _Py_open_osfhandle(void *handle, int flags); #endif /* MS_WINDOWS */ #endif /* Py_LIMITED_API */ diff --git a/Modules/_io/winconsoleio.c b/Modules/_io/winconsoleio.c index 1f884b8f0dcdc1..488aa70680ab6d 100644 --- a/Modules/_io/winconsoleio.c +++ b/Modules/_io/winconsoleio.c @@ -63,7 +63,7 @@ char _PyIO_get_console_type(PyObject *path_or_fd) { int fd = PyLong_AsLong(path_or_fd); PyErr_Clear(); if (fd >= 0) { - HANDLE handle = (HANDLE)_Py_get_osfhandle_noraise(fd); + HANDLE handle = _Py_get_osfhandle_noraise(fd); if (handle == INVALID_HANDLE_VALUE) return '\0'; return _get_console_type(handle); @@ -345,7 +345,7 @@ _io__WindowsConsoleIO___init___impl(winconsoleio *self, PyObject *nameobj, goto bad_mode; if (fd >= 0) { - handle = (HANDLE)_Py_get_osfhandle_noraise(fd); + handle = _Py_get_osfhandle_noraise(fd); self->closefd = 0; } else { DWORD access = GENERIC_READ; @@ -378,9 +378,9 @@ _io__WindowsConsoleIO___init___impl(winconsoleio *self, PyObject *nameobj, } if (self->writable) - self->fd = _Py_open_osfhandle_noraise((intptr_t)handle, _O_WRONLY | _O_BINARY); + self->fd = _Py_open_osfhandle_noraise(handle, _O_WRONLY | _O_BINARY); else - self->fd = _Py_open_osfhandle_noraise((intptr_t)handle, _O_RDONLY | _O_BINARY); + self->fd = _Py_open_osfhandle_noraise(handle, _O_RDONLY | _O_BINARY); if (self->fd < 0) { CloseHandle(handle); PyErr_SetFromErrnoWithFilenameObject(PyExc_OSError, nameobj); @@ -652,7 +652,7 @@ readinto(winconsoleio *self, char *buf, Py_ssize_t len) return -1; } - HANDLE handle = (HANDLE)_Py_get_osfhandle(self->fd); + HANDLE handle = _Py_get_osfhandle(self->fd); if (handle == INVALID_HANDLE_VALUE) return -1; @@ -788,7 +788,7 @@ _io__WindowsConsoleIO_readall_impl(winconsoleio *self) if (self->fd == -1) return err_closed(); - handle = (HANDLE)_Py_get_osfhandle(self->fd); + handle = _Py_get_osfhandle(self->fd); if (handle == INVALID_HANDLE_VALUE) return NULL; @@ -968,7 +968,7 @@ _io__WindowsConsoleIO_write_impl(winconsoleio *self, Py_buffer *b) if (!self->writable) return err_mode("writing"); - handle = (HANDLE)_Py_get_osfhandle(self->fd); + handle = _Py_get_osfhandle(self->fd); if (handle == INVALID_HANDLE_VALUE) return NULL; diff --git a/Modules/mmapmodule.c b/Modules/mmapmodule.c index a0649927eb8a9f..888680cad4c01f 100644 --- a/Modules/mmapmodule.c +++ b/Modules/mmapmodule.c @@ -1266,8 +1266,8 @@ new_mmap_object(PyTypeObject *type, PyObject *args, PyObject *kwdict) */ if (fileno != -1 && fileno != 0) { /* Ensure that fileno is within the CRT's valid range */ - fh = (HANDLE)_Py_get_osfhandle(fileno); - if (fh == (HANDLE)INVALID_HANDLE_VALUE) + fh = _Py_get_osfhandle(fileno); + if (fh == INVALID_HANDLE_VALUE) return NULL; /* Win9x appears to need us seeked to zero */ diff --git a/Modules/posixmodule.c b/Modules/posixmodule.c index 8352724f38decd..9031397227c647 100644 --- a/Modules/posixmodule.c +++ b/Modules/posixmodule.c @@ -8454,8 +8454,8 @@ os_pipe_impl(PyObject *module) Py_BEGIN_ALLOW_THREADS ok = CreatePipe(&read, &write, &attr, 0); if (ok) { - fds[0] = _Py_open_osfhandle_noraise((intptr_t)read, _O_RDONLY); - fds[1] = _Py_open_osfhandle_noraise((intptr_t)write, _O_WRONLY); + fds[0] = _Py_open_osfhandle_noraise(read, _O_RDONLY); + fds[1] = _Py_open_osfhandle_noraise(write, _O_WRONLY); if (fds[0] == -1 || fds[1] == -1) { CloseHandle(read); CloseHandle(write); diff --git a/PC/_testconsole.c b/PC/_testconsole.c index ea74e47fe58c26..ba45af7208b625 100644 --- a/PC/_testconsole.c +++ b/PC/_testconsole.c @@ -68,7 +68,7 @@ _testconsole_write_input_impl(PyObject *module, PyObject *file, prec->Event.KeyEvent.uChar.UnicodeChar = *p; } - HANDLE hInput = (HANDLE)_Py_get_osfhandle(((winconsoleio*)file)->fd); + HANDLE hInput = _Py_get_osfhandle(((winconsoleio*)file)->fd); if (hInput == INVALID_HANDLE_VALUE) goto error; diff --git a/PC/msvcrtmodule.c b/PC/msvcrtmodule.c index 0b25aa431d4460..e33d869cef2372 100644 --- a/PC/msvcrtmodule.c +++ b/PC/msvcrtmodule.c @@ -167,7 +167,7 @@ static long msvcrt_open_osfhandle_impl(PyObject *module, intptr_t handle, int flags) /*[clinic end generated code: output=cede871bf939d6e3 input=cb2108bbea84514e]*/ { - return _Py_open_osfhandle(handle, flags); + return _Py_open_osfhandle((void*)handle, flags); } /*[clinic input] @@ -185,7 +185,7 @@ static intptr_t msvcrt_get_osfhandle_impl(PyObject *module, int fd) /*[clinic end generated code: output=7ce761dd0de2b503 input=305900f4bfab76c7]*/ { - return _Py_get_osfhandle(fd); + return (intptr_t)_Py_get_osfhandle(fd); } /* Console I/O */ diff --git a/Parser/myreadline.c b/Parser/myreadline.c index 15597a774a410d..129a465613fa7f 100644 --- a/Parser/myreadline.c +++ b/Parser/myreadline.c @@ -205,8 +205,8 @@ PyOS_StdioReadline(FILE *sys_stdin, FILE *sys_stdout, const char *prompt) if (!Py_LegacyWindowsStdioFlag && sys_stdin == stdin) { HANDLE hStdIn, hStdErr; - hStdIn = (HANDLE)_Py_get_osfhandle_noraise(fileno(sys_stdin)); - hStdErr = (HANDLE)_Py_get_osfhandle_noraise(fileno(stderr)); + hStdIn = _Py_get_osfhandle_noraise(fileno(sys_stdin)); + hStdErr = _Py_get_osfhandle_noraise(fileno(stderr)); if (_get_console_type(hStdIn) == 'r') { fflush(sys_stdout); diff --git a/Python/fileutils.c b/Python/fileutils.c index c634468db87116..fa4f744f6ee8b3 100644 --- a/Python/fileutils.c +++ b/Python/fileutils.c @@ -614,7 +614,7 @@ _Py_fstat_noraise(int fd, struct _Py_stat_struct *status) HANDLE h; int type; - h = (HANDLE)_Py_get_osfhandle_noraise(fd); + h = _Py_get_osfhandle_noraise(fd); if (h == INVALID_HANDLE_VALUE) { /* errno is already set by _get_osfhandle, but we also set @@ -737,7 +737,7 @@ get_inheritable(int fd, int raise) HANDLE handle; DWORD flags; - handle = (HANDLE)_Py_get_osfhandle_noraise(fd); + handle = _Py_get_osfhandle_noraise(fd); if (handle == INVALID_HANDLE_VALUE) { if (raise) PyErr_SetFromErrno(PyExc_OSError); @@ -806,7 +806,7 @@ set_inheritable(int fd, int inheritable, int raise, int *atomic_flag_works) } #ifdef MS_WINDOWS - handle = (HANDLE)_Py_get_osfhandle_noraise(fd); + handle = _Py_get_osfhandle_noraise(fd); if (handle == INVALID_HANDLE_VALUE) { if (raise) PyErr_SetFromErrno(PyExc_OSError); @@ -1459,7 +1459,7 @@ _Py_dup(int fd) #endif #ifdef MS_WINDOWS - handle = (HANDLE)_Py_get_osfhandle(fd); + handle = _Py_get_osfhandle(fd); if (handle == INVALID_HANDLE_VALUE) return -1; @@ -1576,34 +1576,34 @@ _Py_set_blocking(int fd, int blocking) return -1; } #else /* MS_WINDOWS */ -intptr_t +void* _Py_get_osfhandle_noraise(int fd) { _Py_BEGIN_SUPPRESS_IPH - return _get_osfhandle(fd); + return (void*)_get_osfhandle(fd); _Py_END_SUPPRESS_IPH } -intptr_t +void* _Py_get_osfhandle(int fd) { - intptr_t handle = _Py_get_osfhandle_noraise(fd); - if (handle == (intptr_t)INVALID_HANDLE_VALUE) + void *handle = _Py_get_osfhandle_noraise(fd); + if (handle == INVALID_HANDLE_VALUE) PyErr_SetFromErrno(PyExc_OSError); return handle; } int -_Py_open_osfhandle_noraise(intptr_t handle, int flags) +_Py_open_osfhandle_noraise(void *handle, int flags) { _Py_BEGIN_SUPPRESS_IPH - return _open_osfhandle(handle, flags); + return _open_osfhandle((intptr_t)handle, flags); _Py_END_SUPPRESS_IPH } int -_Py_open_osfhandle(intptr_t handle, int flags) +_Py_open_osfhandle(void *handle, int flags) { int fd = _Py_open_osfhandle_noraise(handle, flags); if (fd == -1) From 06446349ccdb17a1ff4347a053435472987b4906 Mon Sep 17 00:00:00 2001 From: Segev Finer Date: Tue, 6 Jun 2017 01:47:34 +0300 Subject: [PATCH 07/12] bpo-30555: msvcrtmodule.c: Use an argument clinic convertor instead of a cast --- PC/clinic/msvcrtmodule.c.h | 14 +++++++------- PC/msvcrtmodule.c | 34 +++++++++++++++++++--------------- 2 files changed, 26 insertions(+), 22 deletions(-) diff --git a/PC/clinic/msvcrtmodule.c.h b/PC/clinic/msvcrtmodule.c.h index 2c8303b0232405..a393eda22e0310 100644 --- a/PC/clinic/msvcrtmodule.c.h +++ b/PC/clinic/msvcrtmodule.c.h @@ -121,13 +121,13 @@ PyDoc_STRVAR(msvcrt_open_osfhandle__doc__, {"open_osfhandle", (PyCFunction)msvcrt_open_osfhandle, METH_FASTCALL, msvcrt_open_osfhandle__doc__}, static long -msvcrt_open_osfhandle_impl(PyObject *module, intptr_t handle, int flags); +msvcrt_open_osfhandle_impl(PyObject *module, void *handle, int flags); static PyObject * msvcrt_open_osfhandle(PyObject *module, PyObject **args, Py_ssize_t nargs, PyObject *kwnames) { PyObject *return_value = NULL; - intptr_t handle; + void *handle; int flags; long _return_value; @@ -160,7 +160,7 @@ PyDoc_STRVAR(msvcrt_get_osfhandle__doc__, #define MSVCRT_GET_OSFHANDLE_METHODDEF \ {"get_osfhandle", (PyCFunction)msvcrt_get_osfhandle, METH_O, msvcrt_get_osfhandle__doc__}, -static intptr_t +static void * msvcrt_get_osfhandle_impl(PyObject *module, int fd); static PyObject * @@ -168,16 +168,16 @@ msvcrt_get_osfhandle(PyObject *module, PyObject *arg) { PyObject *return_value = NULL; int fd; - intptr_t _return_value; + void *_return_value; if (!PyArg_Parse(arg, "i:get_osfhandle", &fd)) { goto exit; } _return_value = msvcrt_get_osfhandle_impl(module, fd); - if ((_return_value == -1) && PyErr_Occurred()) { + if ((_return_value == INVALID_HANDLE_VALUE) && PyErr_Occurred()) { goto exit; } - return_value = PyLong_FromVoidPtr((void *)_return_value); + return_value = PyLong_FromVoidPtr(_return_value); exit: return return_value; @@ -589,4 +589,4 @@ msvcrt_SetErrorMode(PyObject *module, PyObject *arg) #ifndef MSVCRT_SET_ERROR_MODE_METHODDEF #define MSVCRT_SET_ERROR_MODE_METHODDEF #endif /* !defined(MSVCRT_SET_ERROR_MODE_METHODDEF) */ -/*[clinic end generated code: output=be516d0e78532ba3 input=a9049054013a1b77]*/ +/*[clinic end generated code: output=e00da195932d9190 input=a9049054013a1b77]*/ diff --git a/PC/msvcrtmodule.c b/PC/msvcrtmodule.c index e33d869cef2372..2cdfbde86044b6 100644 --- a/PC/msvcrtmodule.c +++ b/PC/msvcrtmodule.c @@ -33,14 +33,18 @@ #endif /*[python input] -class intptr_t_converter(CConverter): - type = 'intptr_t' +class HANDLE_converter(CConverter): + type = 'void *' format_unit = '"_Py_PARSE_INTPTR"' -class handle_return_converter(long_return_converter): - type = 'intptr_t' - cast = '(void *)' - conversion_fn = 'PyLong_FromVoidPtr' +class HANDLE_return_converter(CReturnConverter): + type = 'void *' + + def render(self, function, data): + self.declare(data) + self.err_occurred_if("_return_value == INVALID_HANDLE_VALUE", data) + data.return_conversion.append( + 'return_value = PyLong_FromVoidPtr(_return_value);\n') class byte_char_return_converter(CReturnConverter): type = 'int' @@ -59,7 +63,7 @@ class wchar_t_return_converter(CReturnConverter): data.return_conversion.append( 'return_value = PyUnicode_FromOrdinal(_return_value);\n') [python start generated code]*/ -/*[python end generated code: output=da39a3ee5e6b4b0d input=b59f1663dba11997]*/ +/*[python end generated code: output=da39a3ee5e6b4b0d input=dab543102cf6345d]*/ /*[clinic input] module msvcrt @@ -152,7 +156,7 @@ msvcrt_setmode_impl(PyObject *module, int fd, int flags) /*[clinic input] msvcrt.open_osfhandle -> long - handle: intptr_t + handle: HANDLE flags: int / @@ -164,14 +168,14 @@ to os.fdopen() to create a file object. [clinic start generated code]*/ static long -msvcrt_open_osfhandle_impl(PyObject *module, intptr_t handle, int flags) -/*[clinic end generated code: output=cede871bf939d6e3 input=cb2108bbea84514e]*/ +msvcrt_open_osfhandle_impl(PyObject *module, void *handle, int flags) +/*[clinic end generated code: output=b2fb97c4b515e4e6 input=d5db190a307cf4bb]*/ { - return _Py_open_osfhandle((void*)handle, flags); + return _Py_open_osfhandle(handle, flags); } /*[clinic input] -msvcrt.get_osfhandle -> handle +msvcrt.get_osfhandle -> HANDLE fd: int / @@ -181,11 +185,11 @@ Return the file handle for the file descriptor fd. Raises OSError if fd is not recognized. [clinic start generated code]*/ -static intptr_t +static void * msvcrt_get_osfhandle_impl(PyObject *module, int fd) -/*[clinic end generated code: output=7ce761dd0de2b503 input=305900f4bfab76c7]*/ +/*[clinic end generated code: output=aca01dfe24637374 input=5fcfde9b17136aa2]*/ { - return (intptr_t)_Py_get_osfhandle(fd); + return _Py_get_osfhandle(fd); } /* Console I/O */ From 9dd4ea958e91ddf92850cfbba49d0e38b52d57a5 Mon Sep 17 00:00:00 2001 From: Segev Finer Date: Fri, 30 Jun 2017 17:37:26 +0300 Subject: [PATCH 08/12] bpo-30555: Also consider NULL as a possible error in HANDLE_return_converter --- PC/clinic/msvcrtmodule.c.h | 4 ++-- PC/msvcrtmodule.c | 6 ++++-- 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/PC/clinic/msvcrtmodule.c.h b/PC/clinic/msvcrtmodule.c.h index 4a2e793c8e8d5f..f7ffa6fdfb54de 100644 --- a/PC/clinic/msvcrtmodule.c.h +++ b/PC/clinic/msvcrtmodule.c.h @@ -174,7 +174,7 @@ msvcrt_get_osfhandle(PyObject *module, PyObject *arg) goto exit; } _return_value = msvcrt_get_osfhandle_impl(module, fd); - if ((_return_value == INVALID_HANDLE_VALUE) && PyErr_Occurred()) { + if ((_return_value == NULL || _return_value == INVALID_HANDLE_VALUE) && PyErr_Occurred()) { goto exit; } return_value = PyLong_FromVoidPtr(_return_value); @@ -589,4 +589,4 @@ msvcrt_SetErrorMode(PyObject *module, PyObject *arg) #ifndef MSVCRT_SET_ERROR_MODE_METHODDEF #define MSVCRT_SET_ERROR_MODE_METHODDEF #endif /* !defined(MSVCRT_SET_ERROR_MODE_METHODDEF) */ -/*[clinic end generated code: output=14ab3d3d39ce9deb input=a9049054013a1b77]*/ +/*[clinic end generated code: output=b7829f02d3bc2a9f input=a9049054013a1b77]*/ diff --git a/PC/msvcrtmodule.c b/PC/msvcrtmodule.c index 2cdfbde86044b6..30af487c853048 100644 --- a/PC/msvcrtmodule.c +++ b/PC/msvcrtmodule.c @@ -42,7 +42,9 @@ class HANDLE_return_converter(CReturnConverter): def render(self, function, data): self.declare(data) - self.err_occurred_if("_return_value == INVALID_HANDLE_VALUE", data) + self.err_occurred_if( + "_return_value == NULL || _return_value == INVALID_HANDLE_VALUE", + data) data.return_conversion.append( 'return_value = PyLong_FromVoidPtr(_return_value);\n') @@ -63,7 +65,7 @@ class wchar_t_return_converter(CReturnConverter): data.return_conversion.append( 'return_value = PyUnicode_FromOrdinal(_return_value);\n') [python start generated code]*/ -/*[python end generated code: output=da39a3ee5e6b4b0d input=dab543102cf6345d]*/ +/*[python end generated code: output=da39a3ee5e6b4b0d input=2b25dc89e9e59534]*/ /*[clinic input] module msvcrt From 2d252aba6d61624e1d5f278e5d9a970bfc1442eb Mon Sep 17 00:00:00 2001 From: Segev Finer Date: Sat, 16 Dec 2017 12:24:56 +0200 Subject: [PATCH 09/12] Add NEWS.d entry for bpo-30555 --- Misc/ACKS | 1 + .../next/Windows/2017-12-16-12-23-51.bpo-30555.3ybjly.rst | 2 ++ 2 files changed, 3 insertions(+) create mode 100644 Misc/NEWS.d/next/Windows/2017-12-16-12-23-51.bpo-30555.3ybjly.rst diff --git a/Misc/ACKS b/Misc/ACKS index 8303ce80050d2e..e5343899640fe6 100644 --- a/Misc/ACKS +++ b/Misc/ACKS @@ -466,6 +466,7 @@ Carl Feynman Vincent Fiack Anastasia Filatova Tomer Filiba +Segev Finer Jeffrey Finkelstein Russell Finn Dan Finnie diff --git a/Misc/NEWS.d/next/Windows/2017-12-16-12-23-51.bpo-30555.3ybjly.rst b/Misc/NEWS.d/next/Windows/2017-12-16-12-23-51.bpo-30555.3ybjly.rst new file mode 100644 index 00000000000000..2b0c2219539e9e --- /dev/null +++ b/Misc/NEWS.d/next/Windows/2017-12-16-12-23-51.bpo-30555.3ybjly.rst @@ -0,0 +1,2 @@ +Fix ``WindowsConsoleIO`` errors in the presence of fd redirection. Patch by +Segev Finer. From 36fccb4042896e77e7cb63e41b310cbc35beaddb Mon Sep 17 00:00:00 2001 From: Segev Finer Date: Sun, 14 Apr 2019 20:00:56 +0300 Subject: [PATCH 10/12] Review fix --- Python/fileutils.c | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/Python/fileutils.c b/Python/fileutils.c index 03efec2f08aae7..735b35e71ffad7 100644 --- a/Python/fileutils.c +++ b/Python/fileutils.c @@ -1871,9 +1871,11 @@ _Py_set_blocking(int fd, int blocking) void* _Py_get_osfhandle_noraise(int fd) { + void *handle; _Py_BEGIN_SUPPRESS_IPH - return (void*)_get_osfhandle(fd); + handle = (void*)_get_osfhandle(fd); _Py_END_SUPPRESS_IPH + return handle; } void* @@ -1889,9 +1891,11 @@ _Py_get_osfhandle(int fd) int _Py_open_osfhandle_noraise(void *handle, int flags) { + int fd; _Py_BEGIN_SUPPRESS_IPH - return _open_osfhandle((intptr_t)handle, flags); + fd = _open_osfhandle((intptr_t)handle, flags); _Py_END_SUPPRESS_IPH + return fd; } int From bd8233ec9837e4179538e7a0596d8207f32f6e71 Mon Sep 17 00:00:00 2001 From: Segev Finer Date: Tue, 16 Mar 2021 16:24:15 +0200 Subject: [PATCH 11/12] fileno should error when closed Co-authored-by: Eryk Sun --- Modules/_io/winconsoleio.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/Modules/_io/winconsoleio.c b/Modules/_io/winconsoleio.c index 367c0a678371f9..f4d543cc5233e1 100644 --- a/Modules/_io/winconsoleio.c +++ b/Modules/_io/winconsoleio.c @@ -466,6 +466,8 @@ static PyObject * _io__WindowsConsoleIO_fileno_impl(winconsoleio *self) /*[clinic end generated code: output=006fa74ce3b5cfbf input=845c47ebbc3a2f67]*/ { + if (self->fd < 0) + return err_closed(); return PyLong_FromLong(self->fd); } From b76da3ef759208ce431f851595701cb5da640d2d Mon Sep 17 00:00:00 2001 From: Segev Finer Date: Tue, 16 Mar 2021 16:54:29 +0200 Subject: [PATCH 12/12] Fix faulty call to IOBase.close introduced by automatic merge --- Modules/_io/winconsoleio.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Modules/_io/winconsoleio.c b/Modules/_io/winconsoleio.c index f4d543cc5233e1..460f2d3fa071a8 100644 --- a/Modules/_io/winconsoleio.c +++ b/Modules/_io/winconsoleio.c @@ -196,8 +196,8 @@ _io__WindowsConsoleIO_close_impl(winconsoleio *self) PyObject *exc, *val, *tb; int rc; _Py_IDENTIFIER(close); - res = _PyObject_CallMethodIdObjArgs((PyObject*)&PyRawIOBase_Type, - &PyId_close, (PyObject*)self); + res = _PyObject_CallMethodIdOneArg((PyObject*)&PyRawIOBase_Type, + &PyId_close, (PyObject*)self); if (!self->closefd) { self->fd = -1; return res;