From 66aab0c4b5b6b328aebc115f96275e7dcd114f8b Mon Sep 17 00:00:00 2001 From: Victor Stinner Date: Thu, 19 Mar 2015 22:53:20 +0100 Subject: Issue #23708: Add _Py_read() and _Py_write() functions to factorize code handle EINTR error and special cases for Windows. These functions now truncate the length to PY_SSIZE_T_MAX to have a portable and reliable behaviour. For example, read() result is undefined if counter is greater than PY_SSIZE_T_MAX on Linux. --- Include/fileutils.h | 10 ++++ Modules/_io/fileio.c | 142 +++++++++++------------------------------------ Modules/posixmodule.c | 46 ++-------------- Objects/fileobject.c | 23 ++------ Python/fileutils.c | 149 ++++++++++++++++++++++++++++++++++++++++++++++++++ 5 files changed, 201 insertions(+), 169 deletions(-) diff --git a/Include/fileutils.h b/Include/fileutils.h index f5fd668..cfcc403 100644 --- a/Include/fileutils.h +++ b/Include/fileutils.h @@ -80,6 +80,16 @@ PyAPI_FUNC(FILE*) _Py_fopen_obj( PyObject *path, const char *mode); +PyAPI_FUNC(Py_ssize_t) _Py_read( + int fd, + void *buf, + size_t count); + +PyAPI_FUNC(Py_ssize_t) _Py_write( + int fd, + const void *buf, + size_t count); + #ifdef HAVE_READLINK PyAPI_FUNC(int) _Py_wreadlink( const wchar_t *path, diff --git a/Modules/_io/fileio.c b/Modules/_io/fileio.c index 0f226ea..6152cde 100644 --- a/Modules/_io/fileio.c +++ b/Modules/_io/fileio.c @@ -570,8 +570,8 @@ static PyObject * fileio_readinto(fileio *self, PyObject *args) { Py_buffer pbuf; - Py_ssize_t n, len; - int err, async_err = 0; + Py_ssize_t n; + int err; if (self->fd < 0) return err_closed(); @@ -581,36 +581,16 @@ fileio_readinto(fileio *self, PyObject *args) if (!PyArg_ParseTuple(args, "w*", &pbuf)) return NULL; - if (_PyVerify_fd(self->fd)) { - len = pbuf.len; -#ifdef MS_WINDOWS - if (len > INT_MAX) - len = INT_MAX; -#endif - - do { - Py_BEGIN_ALLOW_THREADS - errno = 0; -#ifdef MS_WINDOWS - n = read(self->fd, pbuf.buf, (int)len); -#else - n = read(self->fd, pbuf.buf, len); -#endif - Py_END_ALLOW_THREADS - } while (n < 0 && errno == EINTR && - !(async_err = PyErr_CheckSignals())); - - if (async_err) - return NULL; - } else - n = -1; + n = _Py_read(self->fd, pbuf.buf, pbuf.len); + /* copy errno because PyBuffer_Release() can indirectly modify it */ err = errno; PyBuffer_Release(&pbuf); - if (n < 0) { - if (err == EAGAIN) + + if (n == -1) { + if (err == EAGAIN) { + PyErr_Clear(); Py_RETURN_NONE; - errno = err; - PyErr_SetFromErrno(PyExc_IOError); + } return NULL; } @@ -645,9 +625,8 @@ fileio_readall(fileio *self) Py_off_t pos, end; PyObject *result; Py_ssize_t bytes_read = 0; - Py_ssize_t len, n; + Py_ssize_t n; size_t bufsize; - int async_err = 0; if (self->fd < 0) return err_closed(); @@ -695,36 +674,21 @@ fileio_readall(fileio *self) } } - len = bufsize - bytes_read; -#ifdef MS_WINDOWS - if (len > INT_MAX) - len = INT_MAX; -#endif - do { - Py_BEGIN_ALLOW_THREADS - errno = 0; -#ifdef MS_WINDOWS - n = read(self->fd, PyBytes_AS_STRING(result) + bytes_read, (int)len); -#else - n = read(self->fd, PyBytes_AS_STRING(result) + bytes_read, len); -#endif - Py_END_ALLOW_THREADS - } while (n < 0 && errno == EINTR && - !(async_err = PyErr_CheckSignals())); + n = _Py_read(self->fd, + PyBytes_AS_STRING(result) + bytes_read, + bufsize - bytes_read); - if (async_err) - return NULL; if (n == 0) break; - if (n < 0) { + if (n == -1) { if (errno == EAGAIN) { + PyErr_Clear(); if (bytes_read > 0) break; Py_DECREF(result); Py_RETURN_NONE; } Py_DECREF(result); - PyErr_SetFromErrno(PyExc_IOError); return NULL; } bytes_read += n; @@ -756,7 +720,6 @@ fileio_read(fileio *self, PyObject *args) char *ptr; Py_ssize_t n; Py_ssize_t size = -1; - int async_err = 0; PyObject *bytes; if (self->fd < 0) @@ -767,44 +730,29 @@ fileio_read(fileio *self, PyObject *args) if (!PyArg_ParseTuple(args, "|O&", &_PyIO_ConvertSsize_t, &size)) return NULL; - if (size < 0) { + if (size < 0) return fileio_readall(self); - } #ifdef MS_WINDOWS + /* On Windows, the count parameter of read() is an int */ if (size > INT_MAX) size = INT_MAX; #endif + bytes = PyBytes_FromStringAndSize(NULL, size); if (bytes == NULL) return NULL; ptr = PyBytes_AS_STRING(bytes); - if (_PyVerify_fd(self->fd)) { - do { - Py_BEGIN_ALLOW_THREADS - errno = 0; -#ifdef MS_WINDOWS - n = read(self->fd, ptr, (int)size); -#else - n = read(self->fd, ptr, size); -#endif - Py_END_ALLOW_THREADS - } while (n < 0 && errno == EINTR && - !(async_err = PyErr_CheckSignals())); - - if (async_err) - return NULL; - } else - n = -1; - - if (n < 0) { + n = _Py_read(self->fd, ptr, size); + if (n == -1) { + /* copy errno because Py_DECREF() can indirectly modify it */ int err = errno; Py_DECREF(bytes); - if (err == EAGAIN) + if (err == EAGAIN) { + PyErr_Clear(); Py_RETURN_NONE; - errno = err; - PyErr_SetFromErrno(PyExc_IOError); + } return NULL; } @@ -822,8 +770,8 @@ static PyObject * fileio_write(fileio *self, PyObject *args) { Py_buffer pbuf; - Py_ssize_t n, len; - int err, async_err = 0; + Py_ssize_t n; + int err; if (self->fd < 0) return err_closed(); @@ -833,44 +781,16 @@ fileio_write(fileio *self, PyObject *args) if (!PyArg_ParseTuple(args, "y*", &pbuf)) return NULL; - if (_PyVerify_fd(self->fd)) { - len = pbuf.len; -#ifdef MS_WINDOWS - if (len > 32767 && isatty(self->fd)) { - /* Issue #11395: the Windows console returns an error (12: not - enough space error) on writing into stdout if stdout mode is - binary and the length is greater than 66,000 bytes (or less, - depending on heap usage). */ - len = 32767; - } else if (len > INT_MAX) - len = INT_MAX; -#endif - - do { - Py_BEGIN_ALLOW_THREADS - errno = 0; -#ifdef MS_WINDOWS - n = write(self->fd, pbuf.buf, (int)len); -#else - n = write(self->fd, pbuf.buf, len); -#endif - Py_END_ALLOW_THREADS - } while (n < 0 && errno == EINTR && - !(async_err = PyErr_CheckSignals())); - - if (async_err) - return NULL; - } else - n = -1; + n = _Py_write(self->fd, pbuf.buf, pbuf.len); + /* copy errno because PyBuffer_Release() can indirectly modify it */ err = errno; - PyBuffer_Release(&pbuf); if (n < 0) { - if (err == EAGAIN) + if (err == EAGAIN) { + PyErr_Clear(); Py_RETURN_NONE; - errno = err; - PyErr_SetFromErrno(PyExc_IOError); + } return NULL; } diff --git a/Modules/posixmodule.c b/Modules/posixmodule.c index b8151c3..7aa8050 100644 --- a/Modules/posixmodule.c +++ b/Modules/posixmodule.c @@ -11207,37 +11207,27 @@ os_read_impl(PyModuleDef *module, int fd, Py_ssize_t length) /*[clinic end generated code: output=1f3bc27260a24968 input=1df2eaa27c0bf1d3]*/ { Py_ssize_t n; - int async_err = 0; PyObject *buffer; if (length < 0) { errno = EINVAL; return posix_error(); } - if (!_PyVerify_fd(fd)) - return posix_error(); #ifdef MS_WINDOWS - #define READ_CAST (int) + /* On Windows, the count parameter of read() is an int */ if (length > INT_MAX) length = INT_MAX; -#else - #define READ_CAST #endif buffer = PyBytes_FromStringAndSize((char *)NULL, length); if (buffer == NULL) return NULL; - do { - Py_BEGIN_ALLOW_THREADS - n = read(fd, PyBytes_AS_STRING(buffer), READ_CAST length); - Py_END_ALLOW_THREADS - } while (n < 0 && errno == EINTR && !(async_err = PyErr_CheckSignals())); - - if (n < 0) { + n = _Py_read(fd, PyBytes_AS_STRING(buffer), length); + if (n == -1) { Py_DECREF(buffer); - return (!async_err) ? posix_error() : NULL; + return NULL; } if (n != length) @@ -11541,33 +11531,7 @@ static Py_ssize_t os_write_impl(PyModuleDef *module, int fd, Py_buffer *data) /*[clinic end generated code: output=aeb96acfdd4d5112 input=3207e28963234f3c]*/ { - Py_ssize_t size; - int async_err = 0; - Py_ssize_t len = data->len; - - if (!_PyVerify_fd(fd)) { - posix_error(); - return -1; - } - - do { - Py_BEGIN_ALLOW_THREADS -#ifdef MS_WINDOWS - if (len > INT_MAX) - len = INT_MAX; - size = write(fd, data->buf, (int)len); -#else - size = write(fd, data->buf, len); -#endif - Py_END_ALLOW_THREADS - } while (size < 0 && errno == EINTR && !(async_err = PyErr_CheckSignals())); - - if (size < 0) { - if (!async_err) - posix_error(); - return -1; - } - return size; + return _Py_write(fd, data->buf, data->len); } #ifdef HAVE_SENDFILE diff --git a/Objects/fileobject.c b/Objects/fileobject.c index 596f909..6f2e351 100644 --- a/Objects/fileobject.c +++ b/Objects/fileobject.c @@ -383,26 +383,15 @@ stdprinter_write(PyStdPrinter_Object *self, PyObject *args) Py_RETURN_NONE; } - if (!PyArg_ParseTuple(args, "s", &c)) { + if (!PyArg_ParseTuple(args, "s", &c)) return NULL; - } - n = strlen(c); - Py_BEGIN_ALLOW_THREADS - errno = 0; -#ifdef MS_WINDOWS - if (n > INT_MAX) - n = INT_MAX; - n = write(self->fd, c, (int)n); -#else - n = write(self->fd, c, n); -#endif - Py_END_ALLOW_THREADS - - if (n < 0) { - if (errno == EAGAIN) + n = _Py_write(self->fd, c, strlen(c)); + if (n == -1) { + if (errno == EAGAIN) { + PyErr_Clear(); Py_RETURN_NONE; - PyErr_SetFromErrno(PyExc_IOError); + } return NULL; } diff --git a/Python/fileutils.c b/Python/fileutils.c index fa481ae..0948781 100644 --- a/Python/fileutils.c +++ b/Python/fileutils.c @@ -1120,6 +1120,155 @@ _Py_fopen_obj(PyObject *path, const char *mode) return f; } +/* Read count bytes from fd into buf. + * + * On success, return the number of read bytes, it can be lower than count. + * If the current file offset is at or past the end of file, no bytes are read, + * and read() returns zero. + * + * On error, raise an exception, set errno and return -1. + * + * When interrupted by a signal (read() fails with EINTR), retry the syscall. + * If the Python signal handler raises an exception, the function returns -1 + * (the syscall is not retried). + * + * The GIL must be held. */ +Py_ssize_t +_Py_read(int fd, void *buf, size_t count) +{ + Py_ssize_t n; + int async_err = 0; + + /* _Py_read() must not be called with an exception set, otherwise the + * caller may think that read() was interrupted by a signal and the signal + * handler raised an exception. */ + assert(!PyErr_Occurred()); + + if (!_PyVerify_fd(self->fd)) { + PyErr_SetFromErrno(PyExc_OSError); + assert(errno == EBADF); + return -1; + } + +#ifdef MS_WINDOWS + if (count > INT_MAX) { + /* On Windows, the count parameter of read() is an int */ + count = INT_MAX; + } +#else + if (count > PY_SSIZE_T_MAX) { + /* if count is greater than PY_SSIZE_T_MAX, + * read() result is undefined */ + count = PY_SSIZE_T_MAX; + } +#endif + + do { + Py_BEGIN_ALLOW_THREADS + errno = 0; +#ifdef MS_WINDOWS + n = read(fd, buf, (int)count); +#else + n = read(fd, buf, count); +#endif + Py_END_ALLOW_THREADS + } while (n < 0 && errno == EINTR && + !(async_err = PyErr_CheckSignals())); + + if (async_err) { + /* read() was interrupted by a signal (failed with EINTR) + * and the Python signal handler raised an exception */ + assert(errno == EINTR && PyErr_Occurred()); + return -1; + } + if (n < 0) { +#ifndef NDEBUG + int err = errno; +#endif + PyErr_SetFromErrno(PyExc_OSError); + assert(errno == err); + return -1; + } + + return n; +} + +/* Write count bytes of buf into fd. + * + * -On success, return the number of written bytes, it can be lower than count + * including 0 + * - On error, raise an exception, set errno and return -1. + * + * When interrupted by a signal (write() fails with EINTR), retry the syscall. + * If the Python signal handler raises an exception, the function returns -1 + * (the syscall is not retried). + * + * The GIL must be held. */ +Py_ssize_t +_Py_write(int fd, const void *buf, size_t count) +{ + Py_ssize_t n; + int async_err = 0; + + /* _Py_write() must not be called with an exception set, otherwise the + * caller may think that write() was interrupted by a signal and the signal + * handler raised an exception. */ + assert(!PyErr_Occurred()); + + if (!_PyVerify_fd(fd)) { + PyErr_SetFromErrno(PyExc_OSError); + assert(errno == EBADF); + return -1; + } + +#ifdef MS_WINDOWS + if (count > 32767 && isatty(fd)) { + /* Issue #11395: the Windows console returns an error (12: not + enough space error) on writing into stdout if stdout mode is + binary and the length is greater than 66,000 bytes (or less, + depending on heap usage). */ + count = 32767; + } + else if (count > INT_MAX) + count = INT_MAX; +#else + if (count > PY_SSIZE_T_MAX) { + /* write() should truncate count to PY_SSIZE_T_MAX, but it's safer + * to do it ourself to have a portable behaviour. */ + count = PY_SSIZE_T_MAX; + } +#endif + + do { + Py_BEGIN_ALLOW_THREADS + errno = 0; +#ifdef MS_WINDOWS + n = write(fd, buf, (int)count); +#else + n = write(fd, buf, count); +#endif + Py_END_ALLOW_THREADS + } while (n < 0 && errno == EINTR && + !(async_err = PyErr_CheckSignals())); + + if (async_err) { + /* write() was interrupted by a signal (failed with EINTR) + * and the Python signal handler raised an exception */ + assert(errno == EINTR && PyErr_Occurred()); + return -1; + } + if (n < 0) { +#ifndef NDEBUG + int err = errno; +#endif + PyErr_SetFromErrno(PyExc_OSError); + assert(errno == err); + return -1; + } + + return n; +} + #ifdef HAVE_READLINK /* Read value of symbolic link. Encode the path to the locale encoding, decode -- cgit v0.12