From 4d9aec022063dcfc4cf40ae46b1c4a968297e664 Mon Sep 17 00:00:00 2001 From: Serhiy Storchaka Date: Tue, 16 Jan 2018 18:34:21 +0200 Subject: bpo-31572: Get rid of PyObject_HasAttr() and _PyObject_HasAttrId() in the _io module. (#3726) --- Modules/_io/_iomodule.c | 2 + Modules/_io/_iomodule.h | 1 + Modules/_io/bufferedio.c | 32 ++++++-------- Modules/_io/iobase.c | 108 ++++++++++++++++++++++++++++++++--------------- Modules/_io/textio.c | 25 ++++++----- 5 files changed, 105 insertions(+), 63 deletions(-) diff --git a/Modules/_io/_iomodule.c b/Modules/_io/_iomodule.c index cfd78e2..11d7b01 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 db84037..4d318ac 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 d7e82b9..b81abde 100644 --- a/Modules/_io/bufferedio.c +++ b/Modules/_io/bufferedio.c @@ -1521,7 +1521,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); @@ -1541,32 +1541,28 @@ _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_GetAttrWithoutError(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_Occurred()) { + goto cleanup; } chunks = PyList_New(0); diff --git a/Modules/_io/iobase.c b/Modules/_io/iobase.c index bcefb38..348d449 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,60 @@ _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) + res = _PyObject_GetAttrWithoutError(self, _PyIO_str_closed); + if (res == NULL) { + if (PyErr_Occurred()) { + return -1; + } 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 +235,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); @@ -237,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; @@ -428,7 +464,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 +508,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 +535,26 @@ _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_GetAttrWithoutError(self, _PyIO_str_peek); + if (peek == NULL && PyErr_Occurred()) { + return NULL; + } 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 +631,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 +643,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 +756,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 d776b5d..9f3fd2d 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,7 +1201,17 @@ _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_GetAttrWithoutError(buffer, _PyIO_str_read1); + if (res != NULL) { + Py_DECREF(res); + self->has_read1 = 1; + } + else if (!PyErr_Occurred()) { + self->has_read1 = 0; + } + else { + goto error; + } self->encoding_start_of_stream = 0; if (_textiowrapper_fix_encoder_state(self) < 0) { @@ -3013,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; } -- cgit v0.12