From 1ffb40e2ccf207936b1185d33d12b0d9e2103bb0 Mon Sep 17 00:00:00 2001 From: Serhiy Storchaka Date: Sun, 24 Sep 2017 21:06:31 +0300 Subject: [PATCH 1/2] bpo-31572: Get rid of PyObject_HasAttr() and _PyObject_HasAttrId() in io. --- Modules/_io/bufferedio.c | 35 +++++++------ Modules/_io/iobase.c | 107 +++++++++++++++++++++++++++------------ Modules/_io/textio.c | 13 ++++- 3 files changed, 105 insertions(+), 50 deletions(-) diff --git a/Modules/_io/bufferedio.c b/Modules/_io/bufferedio.c index edc4ba5a537632..fd2c71e2f69eb3 100644 --- a/Modules/_io/bufferedio.c +++ b/Modules/_io/bufferedio.c @@ -1541,7 +1541,7 @@ static PyObject * _bufferedreader_read_all(buffered *self) { Py_ssize_t current_size; - PyObject *res = NULL, *data = NULL, *tmp = NULL, *chunks = NULL; + PyObject *res = NULL, *data = NULL, *tmp = NULL, *chunks = NULL, *readall; /* First copy what we have in the current buffer. */ current_size = Py_SAFE_DOWNCAST(READAHEAD(self), Py_off_t, Py_ssize_t); @@ -1561,32 +1561,31 @@ _bufferedreader_read_all(buffered *self) } _bufferedreader_reset_buf(self); - if (PyObject_HasAttr(self->raw, _PyIO_str_readall)) { - tmp = PyObject_CallMethodObjArgs(self->raw, _PyIO_str_readall, NULL); + readall = PyObject_GetAttr(self->raw, _PyIO_str_readall); + if (readall) { + tmp = _PyObject_CallNoArg(readall); + Py_DECREF(readall); if (tmp == NULL) goto cleanup; if (tmp != Py_None && !PyBytes_Check(tmp)) { PyErr_SetString(PyExc_TypeError, "readall() should return bytes"); goto cleanup; } - if (tmp == Py_None) { - if (current_size == 0) { - res = Py_None; - goto cleanup; - } else { - res = data; - goto cleanup; + if (current_size == 0) { + res = tmp; + } else { + if (tmp != Py_None) { + PyBytes_Concat(&data, tmp); } - } - else if (current_size) { - PyBytes_Concat(&data, tmp); res = data; - goto cleanup; - } - else { - res = tmp; - goto cleanup; } + goto cleanup; + } + else if (PyErr_ExceptionMatches(PyExc_AttributeError)) { + PyErr_Clear(); + } + else { + goto cleanup; } chunks = PyList_New(0); diff --git a/Modules/_io/iobase.c b/Modules/_io/iobase.c index bcefb3862c3200..52504d57d98b32 100644 --- a/Modules/_io/iobase.c +++ b/Modules/_io/iobase.c @@ -68,11 +68,9 @@ PyDoc_STRVAR(iobase_doc, by whatever subclass. */ _Py_IDENTIFIER(__IOBase_closed); -#define IS_CLOSED(self) \ - _PyObject_HasAttrId(self, &PyId___IOBase_closed) - _Py_IDENTIFIER(read); + /* Internal methods */ static PyObject * iobase_unsupported(const char *message) @@ -131,6 +129,24 @@ iobase_truncate(PyObject *self, PyObject *args) return iobase_unsupported("truncate"); } +static int +iobase_is_closed(PyObject *self) +{ + PyObject *res; + /* This gets the derived attribute, which is *not* __IOBase_closed + in most cases! */ + res = _PyObject_GetAttrId(self, &PyId___IOBase_closed); + if (res == NULL) { + if (!PyErr_ExceptionMatches(PyExc_AttributeError)) { + return -1; + } + PyErr_Clear(); + return 0; + } + Py_DECREF(res); + return 1; +} + /* Flush and close methods */ /*[clinic input] @@ -146,45 +162,61 @@ _io__IOBase_flush_impl(PyObject *self) /*[clinic end generated code: output=7cef4b4d54656a3b input=773be121abe270aa]*/ { /* XXX Should this return the number of bytes written??? */ - if (IS_CLOSED(self)) { + int closed = iobase_is_closed(self); + + if (!closed) { + Py_RETURN_NONE; + } + if (closed > 0) { PyErr_SetString(PyExc_ValueError, "I/O operation on closed file."); + } + return NULL; +} + +static PyObject * +iobase_closed_get(PyObject *self, void *context) +{ + int closed = iobase_is_closed(self); + if (closed < 0) { return NULL; } - Py_RETURN_NONE; + return PyBool_FromLong(closed); } static int -iobase_closed(PyObject *self) +iobase_check_closed(PyObject *self) { PyObject *res; int closed; /* This gets the derived attribute, which is *not* __IOBase_closed in most cases! */ res = PyObject_GetAttr(self, _PyIO_str_closed); - if (res == NULL) + if (res == NULL) { + if (!PyErr_ExceptionMatches(PyExc_AttributeError)) { + return -1; + } + PyErr_Clear(); return 0; + } closed = PyObject_IsTrue(res); Py_DECREF(res); - return closed; -} - -static PyObject * -iobase_closed_get(PyObject *self, void *context) -{ - return PyBool_FromLong(IS_CLOSED(self)); + if (closed <= 0) { + return closed; + } + PyErr_SetString(PyExc_ValueError, "I/O operation on closed file."); + return -1; } PyObject * _PyIOBase_check_closed(PyObject *self, PyObject *args) { - if (iobase_closed(self)) { - PyErr_SetString(PyExc_ValueError, "I/O operation on closed file."); + if (iobase_check_closed(self)) { return NULL; } - if (args == Py_True) + if (args == Py_True) { return Py_None; - else - Py_RETURN_NONE; + } + Py_RETURN_NONE; } /* XXX: IOBase thinks it has to maintain its own internal state in @@ -204,9 +236,14 @@ _io__IOBase_close_impl(PyObject *self) /*[clinic end generated code: output=63c6a6f57d783d6d input=f4494d5c31dbc6b7]*/ { PyObject *res; + int closed = iobase_is_closed(self); - if (IS_CLOSED(self)) + if (closed < 0) { + return NULL; + } + if (closed) { Py_RETURN_NONE; + } res = PyObject_CallMethodObjArgs(self, _PyIO_str_flush, NULL); @@ -428,7 +465,7 @@ _PyIOBase_check_writable(PyObject *self, PyObject *args) static PyObject * iobase_enter(PyObject *self, PyObject *args) { - if (_PyIOBase_check_closed(self, Py_True) == NULL) + if (iobase_check_closed(self)) return NULL; Py_INCREF(self); @@ -472,7 +509,7 @@ static PyObject * _io__IOBase_isatty_impl(PyObject *self) /*[clinic end generated code: output=60cab77cede41cdd input=9ef76530d368458b]*/ { - if (_PyIOBase_check_closed(self, Py_True) == NULL) + if (iobase_check_closed(self)) return NULL; Py_RETURN_FALSE; } @@ -499,24 +536,30 @@ _io__IOBase_readline_impl(PyObject *self, Py_ssize_t limit) { /* For backwards compatibility, a (slowish) readline(). */ - int has_peek = 0; - PyObject *buffer, *result; + PyObject *peek, *buffer, *result; Py_ssize_t old_size = -1; _Py_IDENTIFIER(peek); - if (_PyObject_HasAttrId(self, &PyId_peek)) - has_peek = 1; + peek = _PyObject_GetAttrId(self, &PyId_peek); + if (peek == NULL) { + if (!PyErr_ExceptionMatches(PyExc_AttributeError)) { + return NULL; + } + PyErr_Clear(); + } buffer = PyByteArray_FromStringAndSize(NULL, 0); - if (buffer == NULL) + if (buffer == NULL) { + Py_XDECREF(peek); return NULL; + } while (limit < 0 || PyByteArray_GET_SIZE(buffer) < limit) { Py_ssize_t nreadahead = 1; PyObject *b; - if (has_peek) { - PyObject *readahead = _PyObject_CallMethodId(self, &PyId_peek, "i", 1); + if (peek != NULL) { + PyObject *readahead = PyObject_CallFunctionObjArgs(peek, _PyLong_One, NULL); if (readahead == NULL) { /* NOTE: PyErr_SetFromErrno() calls PyErr_CheckSignals() when EINTR occurs so we needn't do it ourselves. */ @@ -593,9 +636,11 @@ _io__IOBase_readline_impl(PyObject *self, Py_ssize_t limit) result = PyBytes_FromStringAndSize(PyByteArray_AS_STRING(buffer), PyByteArray_GET_SIZE(buffer)); + Py_XDECREF(peek); Py_DECREF(buffer); return result; fail: + Py_XDECREF(peek); Py_DECREF(buffer); return NULL; } @@ -603,7 +648,7 @@ _io__IOBase_readline_impl(PyObject *self, Py_ssize_t limit) static PyObject * iobase_iter(PyObject *self) { - if (_PyIOBase_check_closed(self, Py_True) == NULL) + if (iobase_check_closed(self)) return NULL; Py_INCREF(self); @@ -716,7 +761,7 @@ _io__IOBase_writelines(PyObject *self, PyObject *lines) { PyObject *iter, *res; - if (_PyIOBase_check_closed(self, Py_True) == NULL) + if (iobase_check_closed(self)) return NULL; iter = PyObject_GetIter(lines); diff --git a/Modules/_io/textio.c b/Modules/_io/textio.c index 5239e8518e6ae2..290a246254d5e5 100644 --- a/Modules/_io/textio.c +++ b/Modules/_io/textio.c @@ -1074,7 +1074,18 @@ _io_TextIOWrapper___init___impl(textio *self, PyObject *buffer, goto error; self->seekable = self->telling = r; - self->has_read1 = _PyObject_HasAttrId(buffer, &PyId_read1); + res = _PyObject_GetAttrId(buffer, &PyId_read1); + if (res != NULL) { + Py_DECREF(res); + self->has_read1 = 1; + } + else if (PyErr_ExceptionMatches(PyExc_AttributeError)) { + PyErr_Clear(); + self->has_read1 = 0; + } + else { + goto error; + } self->encoding_start_of_stream = 0; if (self->seekable && self->encoder) { From 88041520e594390d7c671b3240d277c9609e9ab4 Mon Sep 17 00:00:00 2001 From: Serhiy Storchaka Date: Tue, 16 Jan 2018 16:35:11 +0200 Subject: [PATCH 2/2] Use new API for simplicity. --- Modules/_io/_iomodule.c | 2 ++ Modules/_io/_iomodule.h | 1 + Modules/_io/bufferedio.c | 7 ++----- Modules/_io/iobase.c | 17 ++++++----------- Modules/_io/textio.c | 18 +++++------------- 5 files changed, 16 insertions(+), 29 deletions(-) diff --git a/Modules/_io/_iomodule.c b/Modules/_io/_iomodule.c index cfd78e2d04add9..11d7b01883113a 100644 --- a/Modules/_io/_iomodule.c +++ b/Modules/_io/_iomodule.c @@ -36,6 +36,7 @@ PyObject *_PyIO_str_getstate = NULL; PyObject *_PyIO_str_isatty = NULL; PyObject *_PyIO_str_newlines = NULL; PyObject *_PyIO_str_nl = NULL; +PyObject *_PyIO_str_peek = NULL; PyObject *_PyIO_str_read = NULL; PyObject *_PyIO_str_read1 = NULL; PyObject *_PyIO_str_readable = NULL; @@ -740,6 +741,7 @@ PyInit__io(void) ADD_INTERNED(getstate) ADD_INTERNED(isatty) ADD_INTERNED(newlines) + ADD_INTERNED(peek) ADD_INTERNED(read) ADD_INTERNED(read1) ADD_INTERNED(readable) diff --git a/Modules/_io/_iomodule.h b/Modules/_io/_iomodule.h index db8403774ead23..4d318acd0b3f8d 100644 --- a/Modules/_io/_iomodule.h +++ b/Modules/_io/_iomodule.h @@ -164,6 +164,7 @@ extern PyObject *_PyIO_str_getstate; extern PyObject *_PyIO_str_isatty; extern PyObject *_PyIO_str_newlines; extern PyObject *_PyIO_str_nl; +extern PyObject *_PyIO_str_peek; extern PyObject *_PyIO_str_read; extern PyObject *_PyIO_str_read1; extern PyObject *_PyIO_str_readable; diff --git a/Modules/_io/bufferedio.c b/Modules/_io/bufferedio.c index c4930028d7b07f..b81abde5c0a01d 100644 --- a/Modules/_io/bufferedio.c +++ b/Modules/_io/bufferedio.c @@ -1541,7 +1541,7 @@ _bufferedreader_read_all(buffered *self) } _bufferedreader_reset_buf(self); - readall = PyObject_GetAttr(self->raw, _PyIO_str_readall); + readall = _PyObject_GetAttrWithoutError(self->raw, _PyIO_str_readall); if (readall) { tmp = _PyObject_CallNoArg(readall); Py_DECREF(readall); @@ -1561,10 +1561,7 @@ _bufferedreader_read_all(buffered *self) } goto cleanup; } - else if (PyErr_ExceptionMatches(PyExc_AttributeError)) { - PyErr_Clear(); - } - else { + else if (PyErr_Occurred()) { goto cleanup; } diff --git a/Modules/_io/iobase.c b/Modules/_io/iobase.c index 52504d57d98b32..348d449a272729 100644 --- a/Modules/_io/iobase.c +++ b/Modules/_io/iobase.c @@ -190,12 +190,11 @@ iobase_check_closed(PyObject *self) int closed; /* This gets the derived attribute, which is *not* __IOBase_closed in most cases! */ - res = PyObject_GetAttr(self, _PyIO_str_closed); + res = _PyObject_GetAttrWithoutError(self, _PyIO_str_closed); if (res == NULL) { - if (!PyErr_ExceptionMatches(PyExc_AttributeError)) { + if (PyErr_Occurred()) { return -1; } - PyErr_Clear(); return 0; } closed = PyObject_IsTrue(res); @@ -274,7 +273,7 @@ iobase_finalize(PyObject *self) /* If `closed` doesn't exist or can't be evaluated as bool, then the object is probably in an unusable state, so ignore. */ - res = PyObject_GetAttr(self, _PyIO_str_closed); + res = _PyObject_GetAttrWithoutError(self, _PyIO_str_closed); if (res == NULL) { PyErr_Clear(); closed = -1; @@ -538,14 +537,10 @@ _io__IOBase_readline_impl(PyObject *self, Py_ssize_t limit) PyObject *peek, *buffer, *result; Py_ssize_t old_size = -1; - _Py_IDENTIFIER(peek); - peek = _PyObject_GetAttrId(self, &PyId_peek); - if (peek == NULL) { - if (!PyErr_ExceptionMatches(PyExc_AttributeError)) { - return NULL; - } - PyErr_Clear(); + peek = _PyObject_GetAttrWithoutError(self, _PyIO_str_peek); + if (peek == NULL && PyErr_Occurred()) { + return NULL; } buffer = PyByteArray_FromStringAndSize(NULL, 0); diff --git a/Modules/_io/textio.c b/Modules/_io/textio.c index 850624465b25ac..9f3fd2d2346a0c 100644 --- a/Modules/_io/textio.c +++ b/Modules/_io/textio.c @@ -29,7 +29,6 @@ _Py_IDENTIFIER(mode); _Py_IDENTIFIER(name); _Py_IDENTIFIER(raw); _Py_IDENTIFIER(read); -_Py_IDENTIFIER(read1); _Py_IDENTIFIER(readable); _Py_IDENTIFIER(replace); _Py_IDENTIFIER(reset); @@ -1202,13 +1201,12 @@ _io_TextIOWrapper___init___impl(textio *self, PyObject *buffer, goto error; self->seekable = self->telling = r; - res = _PyObject_GetAttrId(buffer, &PyId_read1); + res = _PyObject_GetAttrWithoutError(buffer, _PyIO_str_read1); if (res != NULL) { Py_DECREF(res); self->has_read1 = 1; } - else if (PyErr_ExceptionMatches(PyExc_AttributeError)) { - PyErr_Clear(); + else if (!PyErr_Occurred()) { self->has_read1 = 0; } else { @@ -3024,15 +3022,9 @@ textiowrapper_newlines_get(textio *self, void *context) CHECK_ATTACHED(self); if (self->decoder == NULL) Py_RETURN_NONE; - res = PyObject_GetAttr(self->decoder, _PyIO_str_newlines); - if (res == NULL) { - if (PyErr_ExceptionMatches(PyExc_AttributeError)) { - PyErr_Clear(); - Py_RETURN_NONE; - } - else { - return NULL; - } + res = _PyObject_GetAttrWithoutError(self->decoder, _PyIO_str_newlines); + if (res == NULL && !PyErr_Occurred()) { + Py_RETURN_NONE; } return res; }