From 51b4ade30615dccaa7f591456528821f4320d5ff Mon Sep 17 00:00:00 2001 From: Tim Peters Date: Thu, 29 Jul 2004 04:07:15 +0000 Subject: Fix obscure breakage (relative to 2.3) in listsort: the test for list mutation during list.sort() used to rely on that listobject.c always NULL'ed ob_item when ob_size fell to 0. That's no longer true, so the test for list mutation during a sort is no longer reliable. Changed the test to rely instead on that listobject.c now never NULLs-out ob_item after (if ever) ob_item gets a non-NULL value. This new assumption is also documented now, as a required invariant in listobject.h. The new assumption allowed some real simplification to some of the hairier code in listsort(), so is a Good Thing on that count. --- Include/listobject.h | 3 +++ Objects/listobject.c | 38 ++++++++++++-------------------------- 2 files changed, 15 insertions(+), 26 deletions(-) diff --git a/Include/listobject.h b/Include/listobject.h index 62a8518..ffce029 100644 --- a/Include/listobject.h +++ b/Include/listobject.h @@ -30,6 +30,9 @@ typedef struct { * 0 <= ob_size <= allocated * len(list) == ob_size * ob_item == NULL implies ob_size == allocated == 0 + * If ob_item ever becomes non-NULL, it remains non-NULL for the + * life of the list object. The check for mutation in list.sort() + * relies on this odd detail. */ int allocated; } PyListObject; diff --git a/Objects/listobject.c b/Objects/listobject.c index 4eb588b..e9f37b3 100644 --- a/Objects/listobject.c +++ b/Objects/listobject.c @@ -1906,7 +1906,6 @@ listsort(PyListObject *self, PyObject *args, PyObject *kwds) int minrun; int saved_ob_size, saved_allocated; PyObject **saved_ob_item; - PyObject **empty_ob_item; PyObject *compare = NULL; PyObject *result = NULL; /* guilty until proved innocent */ int reverse = 0; @@ -1941,9 +1940,8 @@ listsort(PyListObject *self, PyObject *args, PyObject *kwds) saved_ob_size = self->ob_size; saved_ob_item = self->ob_item; saved_allocated = self->allocated; - self->ob_size = 0; - self->ob_item = empty_ob_item = PyMem_NEW(PyObject *, 0); - self->allocated = 0; + self->ob_size = self->allocated = 0; + self->ob_item = NULL; if (keyfunc != NULL) { for (i=0 ; i < saved_ob_size ; i++) { @@ -1957,18 +1955,6 @@ listsort(PyListObject *self, PyObject *args, PyObject *kwds) saved_ob_item[i] = value; Py_DECREF(kvpair); } - if (self->ob_item != empty_ob_item - || self->ob_size) { - /* If the list changed *as well* we - have two errors. We let the first - one "win", but we shouldn't let - what's in the list currently - leak. */ - (void)list_ass_slice( - self, 0, self->ob_size, - (PyObject *)NULL); - } - goto dsu_fail; } kvpair = build_sortwrapper(key, value); @@ -2044,14 +2030,12 @@ fail: } } - if (self->ob_item != empty_ob_item || self->ob_size) { - /* The user mucked with the list during the sort. */ - (void)list_ass_slice(self, 0, self->ob_size, (PyObject *)NULL); - if (result != NULL) { - PyErr_SetString(PyExc_ValueError, - "list modified during sort"); - result = NULL; - } + if (self->ob_item != NULL && result != NULL) { + /* The user mucked with the list during the sort, + * and we don't already have another error to report. + */ + PyErr_SetString(PyExc_ValueError, "list modified during sort"); + result = NULL; } if (reverse && saved_ob_size > 1) @@ -2060,8 +2044,10 @@ fail: merge_freemem(&ms); dsu_fail: - if (self->ob_item == empty_ob_item) - PyMem_FREE(empty_ob_item); + if (self->ob_item != NULL) { + (void)list_ass_slice(self, 0, self->ob_size, (PyObject *)NULL); + PyMem_FREE(self->ob_item); + } self->ob_size = saved_ob_size; self->ob_item = saved_ob_item; self->allocated = saved_allocated; -- cgit v0.12