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.
This commit is contained in:
parent
014f103705
commit
51b4ade306
|
@ -30,6 +30,9 @@ typedef struct {
|
||||||
* 0 <= ob_size <= allocated
|
* 0 <= ob_size <= allocated
|
||||||
* len(list) == ob_size
|
* len(list) == ob_size
|
||||||
* ob_item == NULL implies ob_size == allocated == 0
|
* 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;
|
int allocated;
|
||||||
} PyListObject;
|
} PyListObject;
|
||||||
|
|
|
@ -1906,7 +1906,6 @@ listsort(PyListObject *self, PyObject *args, PyObject *kwds)
|
||||||
int minrun;
|
int minrun;
|
||||||
int saved_ob_size, saved_allocated;
|
int saved_ob_size, saved_allocated;
|
||||||
PyObject **saved_ob_item;
|
PyObject **saved_ob_item;
|
||||||
PyObject **empty_ob_item;
|
|
||||||
PyObject *compare = NULL;
|
PyObject *compare = NULL;
|
||||||
PyObject *result = NULL; /* guilty until proved innocent */
|
PyObject *result = NULL; /* guilty until proved innocent */
|
||||||
int reverse = 0;
|
int reverse = 0;
|
||||||
|
@ -1941,9 +1940,8 @@ listsort(PyListObject *self, PyObject *args, PyObject *kwds)
|
||||||
saved_ob_size = self->ob_size;
|
saved_ob_size = self->ob_size;
|
||||||
saved_ob_item = self->ob_item;
|
saved_ob_item = self->ob_item;
|
||||||
saved_allocated = self->allocated;
|
saved_allocated = self->allocated;
|
||||||
self->ob_size = 0;
|
self->ob_size = self->allocated = 0;
|
||||||
self->ob_item = empty_ob_item = PyMem_NEW(PyObject *, 0);
|
self->ob_item = NULL;
|
||||||
self->allocated = 0;
|
|
||||||
|
|
||||||
if (keyfunc != NULL) {
|
if (keyfunc != NULL) {
|
||||||
for (i=0 ; i < saved_ob_size ; i++) {
|
for (i=0 ; i < saved_ob_size ; i++) {
|
||||||
|
@ -1957,18 +1955,6 @@ listsort(PyListObject *self, PyObject *args, PyObject *kwds)
|
||||||
saved_ob_item[i] = value;
|
saved_ob_item[i] = value;
|
||||||
Py_DECREF(kvpair);
|
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;
|
goto dsu_fail;
|
||||||
}
|
}
|
||||||
kvpair = build_sortwrapper(key, value);
|
kvpair = build_sortwrapper(key, value);
|
||||||
|
@ -2044,15 +2030,13 @@ fail:
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
if (self->ob_item != empty_ob_item || self->ob_size) {
|
if (self->ob_item != NULL && result != NULL) {
|
||||||
/* The user mucked with the list during the sort. */
|
/* The user mucked with the list during the sort,
|
||||||
(void)list_ass_slice(self, 0, self->ob_size, (PyObject *)NULL);
|
* and we don't already have another error to report.
|
||||||
if (result != NULL) {
|
*/
|
||||||
PyErr_SetString(PyExc_ValueError,
|
PyErr_SetString(PyExc_ValueError, "list modified during sort");
|
||||||
"list modified during sort");
|
|
||||||
result = NULL;
|
result = NULL;
|
||||||
}
|
}
|
||||||
}
|
|
||||||
|
|
||||||
if (reverse && saved_ob_size > 1)
|
if (reverse && saved_ob_size > 1)
|
||||||
reverse_slice(saved_ob_item, saved_ob_item + saved_ob_size);
|
reverse_slice(saved_ob_item, saved_ob_item + saved_ob_size);
|
||||||
|
@ -2060,8 +2044,10 @@ fail:
|
||||||
merge_freemem(&ms);
|
merge_freemem(&ms);
|
||||||
|
|
||||||
dsu_fail:
|
dsu_fail:
|
||||||
if (self->ob_item == empty_ob_item)
|
if (self->ob_item != NULL) {
|
||||||
PyMem_FREE(empty_ob_item);
|
(void)list_ass_slice(self, 0, self->ob_size, (PyObject *)NULL);
|
||||||
|
PyMem_FREE(self->ob_item);
|
||||||
|
}
|
||||||
self->ob_size = saved_ob_size;
|
self->ob_size = saved_ob_size;
|
||||||
self->ob_item = saved_ob_item;
|
self->ob_item = saved_ob_item;
|
||||||
self->allocated = saved_allocated;
|
self->allocated = saved_allocated;
|
||||||
|
|
Loading…
Reference in New Issue