From: Guido van Rossum <guido@python.org>
Date: Mon, 16 Apr 2001 00:02:32 +0000 (+0000)
Subject: Tim pointed out a remaining vulnerability in popitem(): the
X-Git-Tag: v2.1c2~7
X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=e04eaec5b60a41b114dd3b7d78c3a389f8534af8;p=thirdparty%2FPython%2Fcpython.git

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?
---

diff --git a/Objects/dictobject.c b/Objects/dictobject.c
index 8574cb9c7237..cc10db04fe44 100644
--- a/Objects/dictobject.c
+++ b/Objects/dictobject.c
@@ -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.