Skip to content

Commit e04eaec

Browse files
committed
Tim pointed out a remaining vulnerability in popitem(): the
PyTuple_New() could *conceivably* clear the dict, so move the test for an empty dict after the tuple allocation. It means that we waste time allocating and deallocating a 2-tuple when the dict is empty, but who cares. It also means that when the dict is empty *and* there's no memory to allocate a 2-tuple, we raise MemoryError, not KeyError -- but that may actually a good idea: if there's no room for a lousy 2-tuple, what are the chances that there's room for a KeyError instance?
1 parent a4dd011 commit e04eaec

1 file changed

Lines changed: 6 additions & 5 deletions

File tree

Objects/dictobject.c

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1155,11 +1155,6 @@ dict_popitem(dictobject *mp, PyObject *args)
11551155

11561156
if (!PyArg_NoArgs(args))
11571157
return NULL;
1158-
if (mp->ma_used == 0) {
1159-
PyErr_SetString(PyExc_KeyError,
1160-
"popitem(): dictionary is empty");
1161-
return NULL;
1162-
}
11631158
/* Allocate the result tuple first. Believe it or not,
11641159
* this allocation could trigger a garbage collection which
11651160
* could resize the dict, which would invalidate the pointer
@@ -1169,6 +1164,12 @@ dict_popitem(dictobject *mp, PyObject *args)
11691164
res = PyTuple_New(2);
11701165
if (res == NULL)
11711166
return NULL;
1167+
if (mp->ma_used == 0) {
1168+
Py_DECREF(res);
1169+
PyErr_SetString(PyExc_KeyError,
1170+
"popitem(): dictionary is empty");
1171+
return NULL;
1172+
}
11721173
/* Set ep to "the first" dict entry with a value. We abuse the hash
11731174
* field of slot 0 to hold a search finger:
11741175
* If slot 0 has a value, use slot 0.

0 commit comments

Comments
 (0)