Commit 04d759b1 authored by Serhiy Storchaka's avatar Serhiy Storchaka

Issue #19687: Fixed memory leak on failed Element slice assignment.

Added new tests for Element slice assignments.
parent 828123ce
...@@ -2350,6 +2350,7 @@ class ElementSlicingTest(unittest.TestCase): ...@@ -2350,6 +2350,7 @@ class ElementSlicingTest(unittest.TestCase):
self.assertEqual(e[-2].tag, 'a8') self.assertEqual(e[-2].tag, 'a8')
self.assertRaises(IndexError, lambda: e[12]) self.assertRaises(IndexError, lambda: e[12])
self.assertRaises(IndexError, lambda: e[-12])
def test_getslice_range(self): def test_getslice_range(self):
e = self._make_elem_with_children(6) e = self._make_elem_with_children(6)
...@@ -2368,12 +2369,17 @@ class ElementSlicingTest(unittest.TestCase): ...@@ -2368,12 +2369,17 @@ class ElementSlicingTest(unittest.TestCase):
self.assertEqual(self._elem_tags(e[::3]), ['a0', 'a3', 'a6', 'a9']) self.assertEqual(self._elem_tags(e[::3]), ['a0', 'a3', 'a6', 'a9'])
self.assertEqual(self._elem_tags(e[::8]), ['a0', 'a8']) self.assertEqual(self._elem_tags(e[::8]), ['a0', 'a8'])
self.assertEqual(self._elem_tags(e[1::8]), ['a1', 'a9']) self.assertEqual(self._elem_tags(e[1::8]), ['a1', 'a9'])
self.assertEqual(self._elem_tags(e[3::sys.maxsize]), ['a3'])
self.assertEqual(self._elem_tags(e[3::sys.maxsize<<64]), ['a3'])
def test_getslice_negative_steps(self): def test_getslice_negative_steps(self):
e = self._make_elem_with_children(4) e = self._make_elem_with_children(4)
self.assertEqual(self._elem_tags(e[::-1]), ['a3', 'a2', 'a1', 'a0']) self.assertEqual(self._elem_tags(e[::-1]), ['a3', 'a2', 'a1', 'a0'])
self.assertEqual(self._elem_tags(e[::-2]), ['a3', 'a1']) self.assertEqual(self._elem_tags(e[::-2]), ['a3', 'a1'])
self.assertEqual(self._elem_tags(e[3::-sys.maxsize]), ['a3'])
self.assertEqual(self._elem_tags(e[3::-sys.maxsize-1]), ['a3'])
self.assertEqual(self._elem_tags(e[3::-sys.maxsize<<64]), ['a3'])
def test_delslice(self): def test_delslice(self):
e = self._make_elem_with_children(4) e = self._make_elem_with_children(4)
...@@ -2400,6 +2406,75 @@ class ElementSlicingTest(unittest.TestCase): ...@@ -2400,6 +2406,75 @@ class ElementSlicingTest(unittest.TestCase):
del e[::2] del e[::2]
self.assertEqual(self._subelem_tags(e), ['a1']) self.assertEqual(self._subelem_tags(e), ['a1'])
def test_setslice_single_index(self):
e = self._make_elem_with_children(4)
e[1] = ET.Element('b')
self.assertEqual(self._subelem_tags(e), ['a0', 'b', 'a2', 'a3'])
e[-2] = ET.Element('c')
self.assertEqual(self._subelem_tags(e), ['a0', 'b', 'c', 'a3'])
with self.assertRaises(IndexError):
e[5] = ET.Element('d')
with self.assertRaises(IndexError):
e[-5] = ET.Element('d')
self.assertEqual(self._subelem_tags(e), ['a0', 'b', 'c', 'a3'])
def test_setslice_range(self):
e = self._make_elem_with_children(4)
e[1:3] = [ET.Element('b%s' % i) for i in range(2)]
self.assertEqual(self._subelem_tags(e), ['a0', 'b0', 'b1', 'a3'])
e = self._make_elem_with_children(4)
e[1:3] = [ET.Element('b')]
self.assertEqual(self._subelem_tags(e), ['a0', 'b', 'a3'])
e = self._make_elem_with_children(4)
e[1:3] = [ET.Element('b%s' % i) for i in range(3)]
self.assertEqual(self._subelem_tags(e), ['a0', 'b0', 'b1', 'b2', 'a3'])
def test_setslice_steps(self):
e = self._make_elem_with_children(6)
e[1:5:2] = [ET.Element('b%s' % i) for i in range(2)]
self.assertEqual(self._subelem_tags(e), ['a0', 'b0', 'a2', 'b1', 'a4', 'a5'])
e = self._make_elem_with_children(6)
with self.assertRaises(ValueError):
e[1:5:2] = [ET.Element('b')]
with self.assertRaises(ValueError):
e[1:5:2] = [ET.Element('b%s' % i) for i in range(3)]
with self.assertRaises(ValueError):
e[1:5:2] = []
self.assertEqual(self._subelem_tags(e), ['a0', 'a1', 'a2', 'a3', 'a4', 'a5'])
e = self._make_elem_with_children(4)
e[1::sys.maxsize] = [ET.Element('b')]
self.assertEqual(self._subelem_tags(e), ['a0', 'b', 'a2', 'a3'])
e[1::sys.maxsize<<64] = [ET.Element('c')]
self.assertEqual(self._subelem_tags(e), ['a0', 'c', 'a2', 'a3'])
def test_setslice_negative_steps(self):
e = self._make_elem_with_children(4)
e[2:0:-1] = [ET.Element('b%s' % i) for i in range(2)]
self.assertEqual(self._subelem_tags(e), ['a0', 'b1', 'b0', 'a3'])
e = self._make_elem_with_children(4)
with self.assertRaises(ValueError):
e[2:0:-1] = [ET.Element('b')]
with self.assertRaises(ValueError):
e[2:0:-1] = [ET.Element('b%s' % i) for i in range(3)]
with self.assertRaises(ValueError):
e[2:0:-1] = []
self.assertEqual(self._subelem_tags(e), ['a0', 'a1', 'a2', 'a3'])
e = self._make_elem_with_children(4)
e[1::-sys.maxsize] = [ET.Element('b')]
self.assertEqual(self._subelem_tags(e), ['a0', 'b', 'a2', 'a3'])
e[1::-sys.maxsize-1] = [ET.Element('c')]
self.assertEqual(self._subelem_tags(e), ['a0', 'c', 'a2', 'a3'])
e[1::-sys.maxsize<<64] = [ET.Element('d')]
self.assertEqual(self._subelem_tags(e), ['a0', 'd', 'a2', 'a3'])
class IOTest(unittest.TestCase): class IOTest(unittest.TestCase):
def tearDown(self): def tearDown(self):
......
...@@ -1605,7 +1605,7 @@ element_ass_subscr(PyObject* self_, PyObject* item, PyObject* value) ...@@ -1605,7 +1605,7 @@ element_ass_subscr(PyObject* self_, PyObject* item, PyObject* value)
Py_ssize_t start, stop, step, slicelen, newlen, cur, i; Py_ssize_t start, stop, step, slicelen, newlen, cur, i;
PyObject* recycle = NULL; PyObject* recycle = NULL;
PyObject* seq = NULL; PyObject* seq;
if (!self->extra) { if (!self->extra) {
if (create_extra(self, NULL) < 0) if (create_extra(self, NULL) < 0)
...@@ -1684,7 +1684,7 @@ element_ass_subscr(PyObject* self_, PyObject* item, PyObject* value) ...@@ -1684,7 +1684,7 @@ element_ass_subscr(PyObject* self_, PyObject* item, PyObject* value)
Py_XDECREF(recycle); Py_XDECREF(recycle);
return 0; return 0;
} }
else {
/* A new slice is actually being assigned */ /* A new slice is actually being assigned */
seq = PySequence_Fast(value, ""); seq = PySequence_Fast(value, "");
if (!seq) { if (!seq) {
...@@ -1695,10 +1695,10 @@ element_ass_subscr(PyObject* self_, PyObject* item, PyObject* value) ...@@ -1695,10 +1695,10 @@ element_ass_subscr(PyObject* self_, PyObject* item, PyObject* value)
return -1; return -1;
} }
newlen = PySequence_Size(seq); newlen = PySequence_Size(seq);
}
if (step != 1 && newlen != slicelen) if (step != 1 && newlen != slicelen)
{ {
Py_DECREF(seq);
PyErr_Format(PyExc_ValueError, PyErr_Format(PyExc_ValueError,
"attempt to assign sequence of size %zd " "attempt to assign sequence of size %zd "
"to extended slice of size %zd", "to extended slice of size %zd",
...@@ -1710,9 +1710,7 @@ element_ass_subscr(PyObject* self_, PyObject* item, PyObject* value) ...@@ -1710,9 +1710,7 @@ element_ass_subscr(PyObject* self_, PyObject* item, PyObject* value)
/* Resize before creating the recycle bin, to prevent refleaks. */ /* Resize before creating the recycle bin, to prevent refleaks. */
if (newlen > slicelen) { if (newlen > slicelen) {
if (element_resize(self, newlen - slicelen) < 0) { if (element_resize(self, newlen - slicelen) < 0) {
if (seq) {
Py_DECREF(seq); Py_DECREF(seq);
}
return -1; return -1;
} }
} }
...@@ -1723,9 +1721,7 @@ element_ass_subscr(PyObject* self_, PyObject* item, PyObject* value) ...@@ -1723,9 +1721,7 @@ element_ass_subscr(PyObject* self_, PyObject* item, PyObject* value)
we're done modifying the element */ we're done modifying the element */
recycle = PyList_New(slicelen); recycle = PyList_New(slicelen);
if (!recycle) { if (!recycle) {
if (seq) {
Py_DECREF(seq); Py_DECREF(seq);
}
return -1; return -1;
} }
for (cur = start, i = 0; i < slicelen; for (cur = start, i = 0; i < slicelen;
...@@ -1753,9 +1749,7 @@ element_ass_subscr(PyObject* self_, PyObject* item, PyObject* value) ...@@ -1753,9 +1749,7 @@ element_ass_subscr(PyObject* self_, PyObject* item, PyObject* value)
self->extra->length += newlen - slicelen; self->extra->length += newlen - slicelen;
if (seq) {
Py_DECREF(seq); Py_DECREF(seq);
}
/* discard the recycle bin, and everything in it */ /* discard the recycle bin, and everything in it */
Py_XDECREF(recycle); Py_XDECREF(recycle);
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment