Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions Include/cpython/dictobject.h
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ PyAPI_FUNC(PyObject *) _PyDict_GetItem_KnownHash(PyObject *mp, PyObject *key,
Py_hash_t hash);
PyAPI_FUNC(PyObject *) _PyDict_GetItemIdWithError(PyObject *dp,
struct _Py_Identifier *key);
PyAPI_FUNC(PyObject *) _PyDict_GetItemStringWithError(PyObject *, const char *);
PyAPI_FUNC(PyObject *) PyDict_SetDefault(
PyObject *mp, PyObject *key, PyObject *defaultobj);
PyAPI_FUNC(int) _PyDict_SetItem_KnownHash(PyObject *mp, PyObject *key,
Expand Down
10 changes: 7 additions & 3 deletions Modules/_csv.c
Original file line number Diff line number Diff line change
Expand Up @@ -132,7 +132,7 @@ get_dialect_from_registry(PyObject * name_obj)
{
PyObject *dialect_obj;

dialect_obj = PyDict_GetItem(_csvstate_global->dialects, name_obj);
dialect_obj = PyDict_GetItemWithError(_csvstate_global->dialects, name_obj);
if (dialect_obj == NULL) {
if (!PyErr_Occurred())
PyErr_Format(_csvstate_global->error_obj, "unknown dialect");
Expand Down Expand Up @@ -1434,8 +1434,12 @@ csv_register_dialect(PyObject *module, PyObject *args, PyObject *kwargs)
static PyObject *
csv_unregister_dialect(PyObject *module, PyObject *name_obj)
{
if (PyDict_DelItem(_csvstate_global->dialects, name_obj) < 0)
return PyErr_Format(_csvstate_global->error_obj, "unknown dialect");
if (PyDict_DelItem(_csvstate_global->dialects, name_obj) < 0) {
if (PyErr_ExceptionMatches(PyExc_KeyError)) {
PyErr_Format(_csvstate_global->error_obj, "unknown dialect");
}
return NULL;
}
Py_RETURN_NONE;
}

Expand Down
20 changes: 13 additions & 7 deletions Modules/_elementtree.c
Original file line number Diff line number Diff line change
Expand Up @@ -339,7 +339,7 @@ get_attrib_from_keywords(PyObject *kwds)
if (attrib_str == NULL) {
return NULL;
}
PyObject *attrib = PyDict_GetItem(kwds, attrib_str);
PyObject *attrib = PyDict_GetItemWithError(kwds, attrib_str);

if (attrib) {
/* If attrib was found in kwds, copy its value and remove it from
Expand All @@ -356,7 +356,8 @@ get_attrib_from_keywords(PyObject *kwds)
Py_DECREF(attrib);
attrib = NULL;
}
} else {
}
else if (!PyErr_Occurred()) {
attrib = PyDict_New();
}

Expand Down Expand Up @@ -1393,9 +1394,13 @@ _elementtree_Element_get_impl(ElementObject *self, PyObject *key,
if (!self->extra || self->extra->attrib == Py_None)
value = default_value;
else {
value = PyDict_GetItem(self->extra->attrib, key);
if (!value)
value = PyDict_GetItemWithError(self->extra->attrib, key);
if (!value) {
if (PyErr_Occurred()) {
return NULL;
}
value = default_value;
}
}

Py_INCREF(value);
Expand Down Expand Up @@ -2843,11 +2848,12 @@ makeuniversal(XMLParserObject* self, const char* string)
if (!key)
return NULL;

value = PyDict_GetItem(self->names, key);
value = PyDict_GetItemWithError(self->names, key);

if (value) {
Py_INCREF(value);
} else {
}
else if (!PyErr_Occurred()) {
/* new name. convert to universal name, and decode as
necessary */

Expand Down Expand Up @@ -2969,7 +2975,7 @@ expat_default_handler(XMLParserObject* self, const XML_Char* data_in,
if (!key)
return;

value = PyDict_GetItem(self->entity, key);
value = PyDict_GetItemWithError(self->entity, key);

if (value) {
if (TreeBuilder_CheckExact(self->target))
Expand Down
5 changes: 4 additions & 1 deletion Modules/_json.c
Original file line number Diff line number Diff line change
Expand Up @@ -746,12 +746,15 @@ _parse_object_unicode(PyScannerObject *s, PyObject *pystr, Py_ssize_t idx, Py_ss
key = scanstring_unicode(pystr, idx + 1, s->strict, &next_idx);
if (key == NULL)
goto bail;
memokey = PyDict_GetItem(s->memo, key);
memokey = PyDict_GetItemWithError(s->memo, key);
if (memokey != NULL) {
Py_INCREF(memokey);
Py_DECREF(key);
key = memokey;
}
else if (PyErr_Occurred()) {
goto bail;
}
else {
if (PyDict_SetItem(s->memo, key, key) < 0)
goto bail;
Expand Down
47 changes: 19 additions & 28 deletions Modules/_sre.c
Original file line number Diff line number Diff line change
Expand Up @@ -1899,12 +1899,7 @@ match_getslice_by_index(MatchObject* self, Py_ssize_t index, PyObject* def)
void* ptr;
Py_ssize_t i, j;

if (index < 0 || index >= self->groups) {
/* raise IndexError if we were given a bad group number */
PyErr_SetString(
PyExc_IndexError,
"no such group"
);
if (index < 0) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Was this change intentional? At first glance it looks like the code you've removed actually matters.

Is this function only ever called with a pre-validated index (e.g. the one returned from match_getindex())? If so, it would be helpful to have a comment here indicating that validation of the index must be done by the caller. And if that's the case then why have this check (and short-circuit) here?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The removed code was moved into match_getindex() because it was repeated after every call of match_getindex(). This function is only called with valid index or the result of match_getindex(). This check was here just to make the caller place simpler. Will move it to the caller place and add an assert instead.

return NULL;
}

Expand Down Expand Up @@ -1940,16 +1935,24 @@ match_getindex(MatchObject* self, PyObject* index)
return 0;

if (PyIndex_Check(index)) {
return PyNumber_AsSsize_t(index, NULL);
i = PyNumber_AsSsize_t(index, NULL);
}
else {
i = -1;

i = -1;

if (self->pattern->groupindex) {
index = PyDict_GetItem(self->pattern->groupindex, index);
if (index && PyLong_Check(index)) {
i = PyLong_AsSsize_t(index);
if (self->pattern->groupindex) {
index = PyDict_GetItemWithError(self->pattern->groupindex, index);
if (index && PyLong_Check(index)) {
i = PyLong_AsSsize_t(index);
}
}
}
if (i < 0 || i >= self->groups) {
/* raise IndexError if we were given a bad group number */
if (!PyErr_Occurred()) {
PyErr_SetString(PyExc_IndexError, "no such group");
}
return -1;
}

return i;
Expand Down Expand Up @@ -2114,11 +2117,7 @@ _sre_SRE_Match_start_impl(MatchObject *self, PyObject *group)
{
Py_ssize_t index = match_getindex(self, group);

if (index < 0 || index >= self->groups) {
PyErr_SetString(
PyExc_IndexError,
"no such group"
);
if (index < 0) {
return -1;
}

Expand All @@ -2141,11 +2140,7 @@ _sre_SRE_Match_end_impl(MatchObject *self, PyObject *group)
{
Py_ssize_t index = match_getindex(self, group);

if (index < 0 || index >= self->groups) {
PyErr_SetString(
PyExc_IndexError,
"no such group"
);
if (index < 0) {
return -1;
}

Expand Down Expand Up @@ -2195,11 +2190,7 @@ _sre_SRE_Match_span_impl(MatchObject *self, PyObject *group)
{
Py_ssize_t index = match_getindex(self, group);

if (index < 0 || index >= self->groups) {
PyErr_SetString(
PyExc_IndexError,
"no such group"
);
if (index < 0) {
return NULL;
}

Expand Down
5 changes: 4 additions & 1 deletion Modules/_struct.c
Original file line number Diff line number Diff line change
Expand Up @@ -2088,12 +2088,15 @@ cache_struct_converter(PyObject *fmt, PyObject **ptr)
return 0;
}

s_object = PyDict_GetItem(cache, fmt);
s_object = PyDict_GetItemWithError(cache, fmt);
if (s_object != NULL) {
Py_INCREF(s_object);
*ptr = s_object;
return Py_CLEANUP_SUPPORTED;
}
else if (PyErr_Occurred()) {
return 0;
}

s_object = PyObject_CallFunctionObjArgs((PyObject *)(&PyStructType), fmt, NULL);
if (s_object != NULL) {
Expand Down
7 changes: 5 additions & 2 deletions Modules/_testmultiphase.c
Original file line number Diff line number Diff line change
Expand Up @@ -53,11 +53,14 @@ static PyObject *
Example_getattro(ExampleObject *self, PyObject *name)
{
if (self->x_attr != NULL) {
PyObject *v = PyDict_GetItem(self->x_attr, name);
PyObject *v = PyDict_GetItemWithError(self->x_attr, name);
if (v != NULL) {
Py_INCREF(v);
return v;
}
else if (PyErr_Occurred()) {
return NULL;
}
}
return PyObject_GenericGetAttr((PyObject *)self, name);
}
Expand All @@ -72,7 +75,7 @@ Example_setattr(ExampleObject *self, const char *name, PyObject *v)
}
if (v == NULL) {
int rv = PyDict_DelItemString(self->x_attr, name);
if (rv < 0)
if (rv < 0 && PyErr_ExceptionMatches(PyExc_KeyError))
PyErr_SetString(PyExc_AttributeError,
"delete non-existing Example attribute");
return rv;
Expand Down
26 changes: 16 additions & 10 deletions Modules/_threadmodule.c
Original file line number Diff line number Diff line change
Expand Up @@ -814,8 +814,11 @@ _ldict(localobject *self)
return NULL;
}

dummy = PyDict_GetItem(tdict, self->key);
dummy = PyDict_GetItemWithError(tdict, self->key);
if (dummy == NULL) {
if (PyErr_Occurred()) {
return NULL;
}
ldict = _local_create_dummy(self);
if (ldict == NULL)
return NULL;
Expand Down Expand Up @@ -931,14 +934,17 @@ local_getattro(localobject *self, PyObject *name)
(PyObject *)self, name, ldict, 0);

/* Optimization: just look in dict ourselves */
value = PyDict_GetItem(ldict, name);
if (value == NULL)
/* Fall back on generic to get __class__ and __dict__ */
return _PyObject_GenericGetAttrWithDict(
(PyObject *)self, name, ldict, 0);

Py_INCREF(value);
return value;
value = PyDict_GetItemWithError(ldict, name);
if (value != NULL) {
Py_INCREF(value);
return value;
}
else if (PyErr_Occurred()) {
return NULL;
}
/* Fall back on generic to get __class__ and __dict__ */
return _PyObject_GenericGetAttrWithDict(
(PyObject *)self, name, ldict, 0);
}

/* Called when a dummy is destroyed. */
Expand All @@ -958,7 +964,7 @@ _localdummy_destroyed(PyObject *localweakref, PyObject *dummyweakref)
self = (localobject *) obj;
if (self->dummies != NULL) {
PyObject *ldict;
ldict = PyDict_GetItem(self->dummies, dummyweakref);
ldict = PyDict_GetItemWithError(self->dummies, dummyweakref);
if (ldict != NULL) {
PyDict_DelItem(self->dummies, dummyweakref);
}
Expand Down
14 changes: 10 additions & 4 deletions Modules/itertoolsmodule.c
Original file line number Diff line number Diff line change
Expand Up @@ -4418,6 +4418,7 @@ static PyTypeObject ziplongest_type;
static PyObject *
zip_longest_new(PyTypeObject *type, PyObject *args, PyObject *kwds)
{
_Py_IDENTIFIER(fillvalue);
ziplongestobject *lz;
Py_ssize_t i;
PyObject *ittuple; /* tuple of iterators */
Expand All @@ -4426,10 +4427,15 @@ zip_longest_new(PyTypeObject *type, PyObject *args, PyObject *kwds)
Py_ssize_t tuplesize;

if (kwds != NULL && PyDict_CheckExact(kwds) && PyDict_GET_SIZE(kwds) > 0) {
fillvalue = PyDict_GetItemString(kwds, "fillvalue");
if (fillvalue == NULL || PyDict_GET_SIZE(kwds) > 1) {
PyErr_SetString(PyExc_TypeError,
"zip_longest() got an unexpected keyword argument");
fillvalue = NULL;
if (PyDict_GET_SIZE(kwds) == 1) {
fillvalue = _PyDict_GetItemIdWithError(kwds, &PyId_fillvalue);
}
if (fillvalue == NULL) {
if (!PyErr_Occurred()) {
PyErr_SetString(PyExc_TypeError,
"zip_longest() got an unexpected keyword argument");
}
return NULL;
}
}
Expand Down
6 changes: 5 additions & 1 deletion Modules/main.c
Original file line number Diff line number Diff line change
Expand Up @@ -501,10 +501,14 @@ pymain_free(_PyMain *pymain)
static int
pymain_sys_path_add_path0(PyInterpreterState *interp, PyObject *path0)
{
_Py_IDENTIFIER(path);
PyObject *sys_path;
PyObject *sysdict = interp->sysdict;
if (sysdict != NULL) {
sys_path = PyDict_GetItemString(sysdict, "path");
sys_path = _PyDict_GetItemIdWithError(sysdict, &PyId_path);
if (sys_path == NULL && PyErr_Occurred()) {
goto error;
}
}
else {
sys_path = NULL;
Expand Down
3 changes: 3 additions & 0 deletions Modules/posixmodule.c
Original file line number Diff line number Diff line change
Expand Up @@ -9721,6 +9721,9 @@ os_unsetenv_impl(PyObject *module, PyObject *name)
*/
if (PyDict_DelItem(posix_putenv_garbage, name)) {
/* really not much we can do; just leak */
if (!PyErr_ExceptionMatches(PyExc_KeyError)) {
return NULL;
}
PyErr_Clear();
}
Py_RETURN_NONE;
Expand Down
28 changes: 18 additions & 10 deletions Modules/pyexpat.c
Original file line number Diff line number Diff line change
Expand Up @@ -226,10 +226,13 @@ string_intern(xmlparseobject *self, const char* str)
return result;
if (!self->intern)
return result;
value = PyDict_GetItem(self->intern, result);
value = PyDict_GetItemWithError(self->intern, result);
if (!value) {
if (PyDict_SetItem(self->intern, result, result) == 0)
if (!PyErr_Occurred() &&
PyDict_SetItem(self->intern, result, result) == 0)
{
return result;
}
else {
Py_DECREF(result);
return NULL;
Expand Down Expand Up @@ -1604,13 +1607,18 @@ static int init_handler_descrs(void)
hi->getset.set = (setter)xmlparse_handler_setter;
hi->getset.closure = &handler_info[i];

PyObject *descr;
if (PyDict_GetItemString(Xmlparsetype.tp_dict, hi->name))
continue;
descr = PyDescr_NewGetSet(&Xmlparsetype, &hi->getset);

PyObject *descr = PyDescr_NewGetSet(&Xmlparsetype, &hi->getset);
if (descr == NULL)
return -1;

if (PyDict_GetItemWithError(Xmlparsetype.tp_dict, PyDescr_NAME(descr))) {
Py_DECREF(descr);
continue;
}
else if (PyErr_Occurred()) {
Py_DECREF(descr);
return -1;
}
if (PyDict_SetItem(Xmlparsetype.tp_dict, PyDescr_NAME(descr), descr) < 0) {
Py_DECREF(descr);
return -1;
Expand Down Expand Up @@ -1682,8 +1690,8 @@ MODULE_INITFUNC(void)
Py_DECREF(m);
return NULL;
}
errors_module = PyDict_GetItem(d, errmod_name);
if (errors_module == NULL) {
errors_module = PyDict_GetItemWithError(d, errmod_name);
if (errors_module == NULL && !PyErr_Occurred()) {
errors_module = PyModule_New(MODULE_NAME ".errors");
if (errors_module != NULL) {
_PyImport_SetModule(errmod_name, errors_module);
Expand All @@ -1692,7 +1700,7 @@ MODULE_INITFUNC(void)
}
}
Py_DECREF(errmod_name);
model_module = PyDict_GetItem(d, modelmod_name);
model_module = PyDict_GetItemWithError(d, modelmod_name);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Doesn't the error need to be returned or cleared?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is returned below, at line 1713.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The model_module gets reset on line 1705, so the error from the first attempt may remain uncleared or swallowed

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Line 1705 is executed only no error was raised at this line.

Error handling in this function (as well as in many other module initialization functions) is pretty poor. Results of PyModule_AddObject() and derived functions are not checked, and references are leaked in case of error. But this is different issue(s).

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Then shouldn't line 1704 have && !PyErr_Occurred() like line 1694 does?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Already added.

if (model_module == NULL) {
model_module = PyModule_New(MODULE_NAME ".model");
if (model_module != NULL) {
Expand Down
Loading