Tim pointed out a remaining vulnerability in popitem(): the
authorGuido van Rossum <guido@python.org>
Mon, 16 Apr 2001 00:02:32 +0000 (00:02 +0000)
committerGuido van Rossum <guido@python.org>
Mon, 16 Apr 2001 00:02:32 +0000 (00:02 +0000)
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?

Objects/dictobject.c

index 8574cb9c723798b7bc19d18a7451b7dec8ab0d10..cc10db04fe4485f79eba55e64133820be21899e7 100644 (file)
@@ -1155,11 +1155,6 @@ dict_popitem(dictobject *mp, PyObject *args)
 
        if (!PyArg_NoArgs(args))
                return NULL;
-       if (mp->ma_used == 0) {
-               PyErr_SetString(PyExc_KeyError,
-                               "popitem(): dictionary is empty");
-               return NULL;
-       }
        /* Allocate the result tuple first.  Believe it or not,
         * this allocation could trigger a garbage collection which
         * could resize the dict, which would invalidate the pointer
@@ -1169,6 +1164,12 @@ dict_popitem(dictobject *mp, PyObject *args)
        res = PyTuple_New(2);
        if (res == NULL)
                return NULL;
+       if (mp->ma_used == 0) {
+               Py_DECREF(res);
+               PyErr_SetString(PyExc_KeyError,
+                               "popitem(): dictionary is empty");
+               return NULL;
+       }
        /* Set ep to "the first" dict entry with a value.  We abuse the hash
         * field of slot 0 to hold a search finger:
         * If slot 0 has a value, use slot 0.