Update of /cvsroot/python/python/dist/src/Objects In directory sc8-pr-cvs1.sourceforge.net:/tmp/cvs-serv28350/Objects Modified Files: listobject.c Log Message: * drop the unreasonable list invariant that ob_item should never come back to NULL during the lifetime of the object. * listobject.c nevertheless did not conform to the other invariants, either; fixed. * listobject.c now uses list_clear() as the obvious internal way to clear a list, instead of abusing list_ass_slice() for that. It makes it easier to enforce the invariant about ob_item == NULL. * listsort() sets allocated to -1 during sort; any mutation will set it to a value >= 0, so it is a safe way to detect mutation. A negative value for allocated does not cause a problem elsewhere currently. test_sort.py has a new test for this fix. * listsort() leak: if items were added to the list during the sort, AND if these items had a __del__ that puts still more stuff into the list, then this more stuff (and the PyObject** array to hold them) were overridden at the end of listsort() and never released. Index: listobject.c =================================================================== RCS file: /cvsroot/python/python/dist/src/Objects/listobject.c,v retrieving revision 2.211 retrieving revision 2.212 diff -C2 -d -r2.211 -r2.212 *** listobject.c 29 Jul 2004 10:56:55 -0000 2.211 --- listobject.c 29 Jul 2004 12:40:23 -0000 2.212 *************** *** 487,490 **** --- 487,513 ---- static int + list_clear(PyListObject *a) + { + int i; + PyObject **item = a->ob_item; + if (item != NULL) { + /* Because XDECREF can recursively invoke operations on + this list, we make it empty first. */ + i = a->ob_size; + a->ob_size = 0; + a->ob_item = NULL; + a->allocated = 0; + while (--i >= 0) { + Py_XDECREF(item[i]); + } + PyMem_FREE(item); + } + /* Never fails; the return value can be ignored. + Note that there is no guarantee that the list is actually empty + at this point, because XDECREF may have populated it again! */ + return 0; + } + + static int list_ass_slice(PyListObject *a, int ilow, int ihigh, PyObject *v) { *************** *** 531,536 **** else if (ihigh > a->ob_size) ihigh = a->ob_size; ! item = a->ob_item; d = n - (ihigh-ilow); if (ihigh > ilow) { p = recycle = PyMem_NEW(PyObject *, (ihigh-ilow)); --- 554,564 ---- else if (ihigh > a->ob_size) ihigh = a->ob_size; ! d = n - (ihigh-ilow); + if (a->ob_size + d == 0) { + Py_XDECREF(v_as_SF); + return list_clear(a); + } + item = a->ob_item; if (ihigh > ilow) { p = recycle = PyMem_NEW(PyObject *, (ihigh-ilow)); *************** *** 577,584 **** PyMem_DEL(recycle); } - if (a->ob_size == 0 && a->ob_item != NULL) { - PyMem_FREE(a->ob_item); - a->ob_item = NULL; - } Py_XDECREF(v_as_SF); return 0; --- 605,608 ---- *************** *** 610,619 **** if (n < 1) { ! items = self->ob_item; ! self->ob_item = NULL; ! self->ob_size = 0; ! for (i = 0; i < size; i++) ! Py_XDECREF(items[i]); ! PyMem_DEL(items); Py_INCREF(self); return (PyObject *)self; --- 634,638 ---- if (n < 1) { ! (void)list_clear(self); Py_INCREF(self); return (PyObject *)self; *************** *** 1909,1912 **** --- 1928,1932 ---- int saved_ob_size, saved_allocated; PyObject **saved_ob_item; + PyObject **final_ob_item; PyObject *compare = NULL; PyObject *result = NULL; /* guilty until proved innocent */ *************** *** 1943,1948 **** saved_ob_item = self->ob_item; saved_allocated = self->allocated; ! self->ob_size = self->allocated = 0; self->ob_item = NULL; if (keyfunc != NULL) { --- 1963,1969 ---- saved_ob_item = self->ob_item; saved_allocated = self->allocated; ! self->ob_size = 0; self->ob_item = NULL; + self->allocated = -1; /* any operation will reset it to >= 0 */ if (keyfunc != NULL) { *************** *** 2033,2037 **** } ! 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. --- 2054,2058 ---- } ! if (self->allocated != -1 && result != NULL) { /* The user mucked with the list during the sort, * and we don't already have another error to report. *************** *** 2047,2057 **** dsu_fail: ! 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; Py_XDECREF(compare); Py_XINCREF(result); --- 2068,2084 ---- dsu_fail: ! final_ob_item = self->ob_item; ! i = self->ob_size; self->ob_size = saved_ob_size; self->ob_item = saved_ob_item; self->allocated = saved_allocated; + if (final_ob_item != NULL) { + /* we cannot use list_clear() for this because it does not + guarantee that the list is really empty when it returns */ + while (--i >= 0) { + Py_XDECREF(final_ob_item[i]); + } + PyMem_FREE(final_ob_item); + } Py_XDECREF(compare); Py_XINCREF(result); *************** *** 2208,2218 **** } - static int - list_clear(PyListObject *lp) - { - (void) PyList_SetSlice((PyObject *)lp, 0, lp->ob_size, 0); - return 0; - } - static PyObject * list_richcompare(PyObject *v, PyObject *w, int op) --- 2235,2238 ---- *************** *** 2296,2303 **** return -1; /* Empty previous contents */ ! self->allocated = self->ob_size; ! if (self->ob_size != 0) { ! if (list_ass_slice(self, 0, self->ob_size, (PyObject *)NULL) != 0) ! return -1; } if (arg != NULL) { --- 2316,2321 ---- return -1; /* Empty previous contents */ ! if (self->ob_item != NULL) { ! (void)list_clear(self); } if (arg != NULL) {