Skip to content

Commit

Permalink
bpo-32571: Avoid raising unneeded AttributeError and silencing it in…
Browse files Browse the repository at this point in the history
… C code (GH-5222)

Add two new private APIs: _PyObject_LookupAttr() and _PyObject_LookupAttrId()
  • Loading branch information
serhiy-storchaka authored and methane committed Jan 25, 2018
1 parent 2b822a0 commit f320be7
Show file tree
Hide file tree
Showing 22 changed files with 1,443 additions and 1,430 deletions.
13 changes: 11 additions & 2 deletions Include/object.h
Original file line number Diff line number Diff line change
Expand Up @@ -536,11 +536,20 @@ PyAPI_FUNC(int) PyObject_SetAttr(PyObject *, PyObject *, PyObject *);
PyAPI_FUNC(int) PyObject_HasAttr(PyObject *, PyObject *);
#ifndef Py_LIMITED_API
PyAPI_FUNC(int) _PyObject_IsAbstract(PyObject *);
/* Same as PyObject_GetAttr(), but don't raise AttributeError. */
PyAPI_FUNC(PyObject *) _PyObject_GetAttrWithoutError(PyObject *, PyObject *);
PyAPI_FUNC(PyObject *) _PyObject_GetAttrId(PyObject *, struct _Py_Identifier *);
PyAPI_FUNC(int) _PyObject_SetAttrId(PyObject *, struct _Py_Identifier *, PyObject *);
PyAPI_FUNC(int) _PyObject_HasAttrId(PyObject *, struct _Py_Identifier *);
/* Replacements of PyObject_GetAttr() and _PyObject_GetAttrId() which
don't raise AttributeError.
Return 1 and set *result != NULL if an attribute is found.
Return 0 and set *result == NULL if an attribute is not found;
an AttributeError is silenced.
Return -1 and set *result == NULL if an error other than AttributeError
is raised.
*/
PyAPI_FUNC(int) _PyObject_LookupAttr(PyObject *, PyObject *, PyObject **);
PyAPI_FUNC(int) _PyObject_LookupAttrId(PyObject *, struct _Py_Identifier *, PyObject **);
PyAPI_FUNC(PyObject **) _PyObject_GetDictPtr(PyObject *);
#endif
PyAPI_FUNC(PyObject *) PyObject_SelfIter(PyObject *);
Expand Down
8 changes: 3 additions & 5 deletions Modules/_collectionsmodule.c
Original file line number Diff line number Diff line change
Expand Up @@ -1339,12 +1339,10 @@ deque_reduce(dequeobject *deque)
PyObject *dict, *it;
_Py_IDENTIFIER(__dict__);

dict = _PyObject_GetAttrId((PyObject *)deque, &PyId___dict__);
if (_PyObject_LookupAttrId((PyObject *)deque, &PyId___dict__, &dict) < 0) {
return NULL;
}
if (dict == NULL) {
if (!PyErr_ExceptionMatches(PyExc_AttributeError)) {
return NULL;
}
PyErr_Clear();
dict = Py_None;
Py_INCREF(dict);
}
Expand Down
7 changes: 3 additions & 4 deletions Modules/_io/bufferedio.c
Original file line number Diff line number Diff line change
Expand Up @@ -1541,7 +1541,9 @@ _bufferedreader_read_all(buffered *self)
}
_bufferedreader_reset_buf(self);

readall = _PyObject_GetAttrWithoutError(self->raw, _PyIO_str_readall);
if (_PyObject_LookupAttr(self->raw, _PyIO_str_readall, &readall) < 0) {
goto cleanup;
}
if (readall) {
tmp = _PyObject_CallNoArg(readall);
Py_DECREF(readall);
Expand All @@ -1561,9 +1563,6 @@ _bufferedreader_read_all(buffered *self)
}
goto cleanup;
}
else if (PyErr_Occurred()) {
goto cleanup;
}

chunks = PyList_New(0);
if (chunks == NULL)
Expand Down
8 changes: 3 additions & 5 deletions Modules/_io/fileio.c
Original file line number Diff line number Diff line change
Expand Up @@ -1073,12 +1073,10 @@ fileio_repr(fileio *self)
if (self->fd < 0)
return PyUnicode_FromFormat("<_io.FileIO [closed]>");

nameobj = _PyObject_GetAttrId((PyObject *) self, &PyId_name);
if (_PyObject_LookupAttrId((PyObject *) self, &PyId_name, &nameobj) < 0) {
return NULL;
}
if (nameobj == NULL) {
if (PyErr_ExceptionMatches(PyExc_AttributeError))
PyErr_Clear();
else
return NULL;
res = PyUnicode_FromFormat(
"<_io.FileIO fd=%d mode='%s' closefd=%s>",
self->fd, mode_string(self), self->closefd ? "True" : "False");
Expand Down
38 changes: 13 additions & 25 deletions Modules/_io/iobase.c
Original file line number Diff line number Diff line change
Expand Up @@ -133,18 +133,12 @@ static int
iobase_is_closed(PyObject *self)
{
PyObject *res;
int ret;
/* 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;
ret = _PyObject_LookupAttrId(self, &PyId___IOBase_closed, &res);
Py_XDECREF(res);
return ret;
}

/* Flush and close methods */
Expand Down Expand Up @@ -190,20 +184,16 @@ iobase_check_closed(PyObject *self)
int closed;
/* This gets the derived attribute, which is *not* __IOBase_closed
in most cases! */
res = _PyObject_GetAttrWithoutError(self, _PyIO_str_closed);
if (res == NULL) {
if (PyErr_Occurred()) {
closed = _PyObject_LookupAttr(self, _PyIO_str_closed, &res);
if (closed > 0) {
closed = PyObject_IsTrue(res);
Py_DECREF(res);
if (closed > 0) {
PyErr_SetString(PyExc_ValueError, "I/O operation on closed file.");
return -1;
}
return 0;
}
closed = PyObject_IsTrue(res);
Py_DECREF(res);
if (closed <= 0) {
return closed;
}
PyErr_SetString(PyExc_ValueError, "I/O operation on closed file.");
return -1;
return closed;
}

PyObject *
Expand Down Expand Up @@ -273,8 +263,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_GetAttrWithoutError(self, _PyIO_str_closed);
if (res == NULL) {
if (_PyObject_LookupAttr(self, _PyIO_str_closed, &res) <= 0) {
PyErr_Clear();
closed = -1;
}
Expand Down Expand Up @@ -538,8 +527,7 @@ _io__IOBase_readline_impl(PyObject *self, Py_ssize_t limit)
PyObject *peek, *buffer, *result;
Py_ssize_t old_size = -1;

peek = _PyObject_GetAttrWithoutError(self, _PyIO_str_peek);
if (peek == NULL && PyErr_Occurred()) {
if (_PyObject_LookupAttr(self, _PyIO_str_peek, &peek) < 0) {
return NULL;
}

Expand Down
48 changes: 18 additions & 30 deletions Modules/_io/textio.c
Original file line number Diff line number Diff line change
Expand Up @@ -924,14 +924,10 @@ _textiowrapper_set_encoder(textio *self, PyObject *codec_info,
return -1;

/* Get the normalized named of the codec */
res = _PyObject_GetAttrId(codec_info, &PyId_name);
if (res == NULL) {
if (PyErr_ExceptionMatches(PyExc_AttributeError))
PyErr_Clear();
else
return -1;
if (_PyObject_LookupAttrId(codec_info, &PyId_name, &res) < 0) {
return -1;
}
else if (PyUnicode_Check(res)) {
if (res != NULL && PyUnicode_Check(res)) {
const encodefuncentry *e = encodefuncs;
while (e->name != NULL) {
if (_PyUnicode_EqualToASCIIString(res, e->name)) {
Expand Down Expand Up @@ -1177,19 +1173,17 @@ _io_TextIOWrapper___init___impl(textio *self, PyObject *buffer,

if (Py_TYPE(buffer) == &PyBufferedReader_Type ||
Py_TYPE(buffer) == &PyBufferedWriter_Type ||
Py_TYPE(buffer) == &PyBufferedRandom_Type) {
raw = _PyObject_GetAttrId(buffer, &PyId_raw);
Py_TYPE(buffer) == &PyBufferedRandom_Type)
{
if (_PyObject_LookupAttrId(buffer, &PyId_raw, &raw) < 0)
goto error;
/* Cache the raw FileIO object to speed up 'closed' checks */
if (raw == NULL) {
if (PyErr_ExceptionMatches(PyExc_AttributeError))
PyErr_Clear();
if (raw != NULL) {
if (Py_TYPE(raw) == &PyFileIO_Type)
self->raw = raw;
else
goto error;
Py_DECREF(raw);
}
else if (Py_TYPE(raw) == &PyFileIO_Type)
self->raw = raw;
else
Py_DECREF(raw);
}

res = _PyObject_CallMethodId(buffer, &PyId_seekable, NULL);
Expand All @@ -1201,17 +1195,12 @@ _io_TextIOWrapper___init___impl(textio *self, PyObject *buffer,
goto error;
self->seekable = self->telling = r;

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 {
r = _PyObject_LookupAttr(buffer, _PyIO_str_read1, &res);
if (r < 0) {
goto error;
}
Py_XDECREF(res);
self->has_read1 = r;

self->encoding_start_of_stream = 0;
if (_textiowrapper_fix_encoder_state(self) < 0) {
Expand Down Expand Up @@ -3020,10 +3009,9 @@ textiowrapper_newlines_get(textio *self, void *context)
{
PyObject *res;
CHECK_ATTACHED(self);
if (self->decoder == NULL)
Py_RETURN_NONE;
res = _PyObject_GetAttrWithoutError(self->decoder, _PyIO_str_newlines);
if (res == NULL && !PyErr_Occurred()) {
if (self->decoder == NULL ||
_PyObject_LookupAttr(self->decoder, _PyIO_str_newlines, &res) == 0)
{
Py_RETURN_NONE;
}
return res;
Expand Down
Loading

0 comments on commit f320be7

Please sign in to comment.