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.

diff --git a/Include/listobject.h b/Include/listobject.h
index 62a85180808ba20ab5abe14b91f264d8ffbea9d6..ffce029fac64df74ac2601c712597687ed6f3b5b 100644 (file)
--- 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 4eb588b30bb65690d36c4f844a5e050c4f9ff768..e9f37b3493b2a68b14924665113d82a8f9f8bba5 100644 (file)
--- 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;