From aa241e014979612e2c8851bf9b12c16bcc07b161 Mon Sep 17 00:00:00 2001 From: Raymond Hettinger Date: Sun, 26 Sep 2004 19:24:20 +0000 Subject: [PATCH] Checkin Tim's fix to an error discussed on python-dev. Also, add a testcase. Formerly, the list_extend() code used several local variables to remember its state across iterations. Since an iteration could call arbitrary Python code, it was possible for the list state to be changed. The new code uses dynamic structure references instead of C locals. So, they are always up-to-date. After list_resize() is called, its size has been updated but the new cells are filled with NULLs. These needed to be filled before arbitrary iteration code was called; otherwise, that code could attempt to modify a list that was in a semi-invalid state. The solution was to change the ob->size field back to a value reflecting the actual number of valid cells. --- Lib/test/test_builtin.py | 5 +++++ Objects/listobject.c | 30 ++++++++++++++++++++---------- 2 files changed, 25 insertions(+), 10 deletions(-) diff --git a/Lib/test/test_builtin.py b/Lib/test/test_builtin.py index 8e3a925032d..9f909250e86 100644 --- a/Lib/test/test_builtin.py +++ b/Lib/test/test_builtin.py @@ -712,6 +712,11 @@ def test_list(self): # http://sources.redhat.com/ml/newlib/2002/msg00369.html self.assertRaises(MemoryError, list, xrange(sys.maxint // 2)) + # This code used to segfault in Py2.4a3 + x = [] + x.extend(-y for y in x) + self.assertEqual(x, []) + def test_long(self): self.assertEqual(long(314), 314L) self.assertEqual(long(3.14), 3L) diff --git a/Objects/listobject.c b/Objects/listobject.c index 1fb77b9760c..44616e56a03 100644 --- a/Objects/listobject.c +++ b/Objects/listobject.c @@ -769,12 +769,20 @@ listextend(PyListObject *self, PyObject *b) } m = self->ob_size; mn = m + n; - if (list_resize(self, mn) == -1) - goto error; - memset(&(self->ob_item[m]), 0, sizeof(*self->ob_item) * n); + if (mn >= m) { + /* Make room. */ + if (list_resize(self, mn) == -1) + goto error; + /* Make the list sane again. */ + self->ob_size = m; + } + /* Else m + n overflowed; on the chance that n lied, and there really + * is enough room, ignore it. If n was telling the truth, we'll + * eventually run out of memory during the loop. + */ /* Run iterator to exhaustion. */ - for (i = m; ; i++) { + for (;;) { PyObject *item = iternext(it); if (item == NULL) { if (PyErr_Occurred()) { @@ -785,8 +793,11 @@ listextend(PyListObject *self, PyObject *b) } break; } - if (i < mn) - PyList_SET_ITEM(self, i, item); /* steals ref */ + if (self->ob_size < self->allocated) { + /* steals ref */ + PyList_SET_ITEM(self, self->ob_size, item); + ++self->ob_size; + } else { int status = app1(self, item); Py_DECREF(item); /* append creates a new ref */ @@ -796,10 +807,9 @@ listextend(PyListObject *self, PyObject *b) } /* Cut back result list if initial guess was too large. */ - if (i < mn && self != NULL) { - if (list_ass_slice(self, i, mn, (PyObject *)NULL) != 0) - goto error; - } + if (self->ob_size < self->allocated) + list_resize(self, self->ob_size); /* shrinking can't fail */ + Py_DECREF(it); Py_RETURN_NONE;