[Python-checkins] python/dist/src/Objects listobject.c,2.166,2.167

Thu Dec 4 06:25:48 EST 2003

Update of /cvsroot/python/python/dist/src/Objects
In directory sc8-pr-cvs1:/tmp/cvs-serv21077/Objects
Modified Files:
	listobject.c 
Log Message:
Fixes and tests for various "holding pointers when arbitrary Python code
can run" bugs as discussed in
[ 848856 ] couple of new list.sort bugs
Index: listobject.c
===================================================================
RCS file: /cvsroot/python/python/dist/src/Objects/listobject.c,v
retrieving revision 2.166
retrieving revision 2.167
diff -C2 -d -r2.166 -r2.167
*** listobject.c	28 Nov 2003 21:43:02 -0000	2.166
--- listobject.c	4 Dec 2003 11:25:46 -0000	2.167
***************
*** 1849,1853 ****
 	int reverse = 0;
 	PyObject *keyfunc = NULL;
! 	int i, len = 0;
 	PyObject *key, *value, *kvpair;
 	static char *kwlist[] = {"cmp", "key", "reverse", 0};
--- 1849,1853 ----
 	int reverse = 0;
 	PyObject *keyfunc = NULL;
! 	int i;
 	PyObject *key, *value, *kvpair;
 	static char *kwlist[] = {"cmp", "key", "reverse", 0};
***************
*** 1871,1887 ****
 		Py_XINCREF(compare);
 
 	if (keyfunc != NULL) {
! 		len = PyList_GET_SIZE(self);
! 		for (i=0 ; i < len ; i++) {
! 			value = PyList_GET_ITEM(self, i);
 			key = PyObject_CallFunctionObjArgs(keyfunc, value, 
 							 NULL);
 			if (key == NULL) {
 				for (i=i-1 ; i>=0 ; i--) {
! 					kvpair = PyList_GET_ITEM(self, i);
 					value = sortwrapper_getvalue(kvpair);
! 					PyList_SET_ITEM(self, i, value);
 					Py_DECREF(kvpair);
 				}
 				goto dsu_fail;
 			}
--- 1871,1908 ----
 		Py_XINCREF(compare);
 
+ 	/* The list is temporarily made empty, so that mutations performed
+ 	 * by comparison functions can't affect the slice of memory we're
+ 	 * sorting (allowing mutations during sorting is a core-dump
+ 	 * factory, since ob_item may change).
+ 	 */
+ 	saved_ob_size = self->ob_size;
+ 	saved_ob_item = self->ob_item;
+ 	self->ob_size = 0;
+ 	self->ob_item = empty_ob_item = PyMem_NEW(PyObject *, 0);
+ 
 	if (keyfunc != NULL) {
! 		for (i=0 ; i < saved_ob_size ; i++) {
! 			value = saved_ob_item[i];
 			key = PyObject_CallFunctionObjArgs(keyfunc, value, 
 							 NULL);
 			if (key == NULL) {
 				for (i=i-1 ; i>=0 ; i--) {
! 					kvpair = saved_ob_item[i];
 					value = sortwrapper_getvalue(kvpair);
! 					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;
 			}
***************
*** 1889,1893 ****
 			if (kvpair == NULL)
 				goto dsu_fail;
! 			PyList_SET_ITEM(self, i, kvpair);
 		}
 	}
--- 1910,1914 ----
 			if (kvpair == NULL)
 				goto dsu_fail;
! 			saved_ob_item[i] = kvpair;
 		}
 	}
***************
*** 1895,1913 ****
 	/* Reverse sort stability achieved by initially reversing the list,
 	applying a stable forward sort, then reversing the final result. */
! 	if (reverse && self->ob_size > 1)
! 		reverse_slice(self->ob_item, self->ob_item + self->ob_size);
 
 	merge_init(&ms, compare);
 
- 	/* The list is temporarily made empty, so that mutations performed
- 	 * by comparison functions can't affect the slice of memory we're
- 	 * sorting (allowing mutations during sorting is a core-dump
- 	 * factory, since ob_item may change).
- 	 */
- 	saved_ob_size = self->ob_size;
- 	saved_ob_item = self->ob_item;
- 	self->ob_size = 0;
- 	self->ob_item = empty_ob_item = PyMem_NEW(PyObject *, 0);
- 
 	nremaining = saved_ob_size;
 	if (nremaining < 2)
--- 1916,1924 ----
 	/* Reverse sort stability achieved by initially reversing the list,
 	applying a stable forward sort, then reversing the final result. */
! 	if (reverse && saved_ob_size > 1)
! 		reverse_slice(saved_ob_item, saved_ob_item + saved_ob_size);
 
 	merge_init(&ms, compare);
 
 	nremaining = saved_ob_size;
 	if (nremaining < 2)
***************
*** 1960,1963 ****
--- 1971,1983 ----
 	result = Py_None;
 fail:
+ 	if (keyfunc != NULL) {
+ 		for (i=0 ; i < saved_ob_size ; i++) {
+ 			kvpair = saved_ob_item[i];
+ 			value = sortwrapper_getvalue(kvpair);
+ 			saved_ob_item[i] = value;
+ 			Py_DECREF(kvpair);
+ 		}
+ 	}
+ 
 	if (self->ob_item != empty_ob_item || self->ob_size) {
 		/* The user mucked with the list during the sort. */
***************
*** 1969,1992 ****
 		}
 	}
- 	if (self->ob_item == empty_ob_item)
- 		PyMem_FREE(empty_ob_item);
- 	self->ob_size = saved_ob_size;
- 	self->ob_item = saved_ob_item;
- 	merge_freemem(&ms);
 
! 	if (keyfunc != NULL) {
! 		len = PyList_GET_SIZE(self);
! 		for (i=0 ; i < len ; i++) {
! 			kvpair = PyList_GET_ITEM(self, i);
! 			value = sortwrapper_getvalue(kvpair);
! 			PyList_SET_ITEM(self, i, value);
! 			Py_DECREF(kvpair);
! 		}
! 	}
 
! 	if (reverse && self->ob_size > 1)
! 		reverse_slice(self->ob_item, self->ob_item + self->ob_size);
 
 dsu_fail:
 	Py_XDECREF(compare);
 	Py_XINCREF(result);
--- 1989,2003 ----
 		}
 	}
 
! 	if (reverse && saved_ob_size > 1)
! 		reverse_slice(saved_ob_item, saved_ob_item + saved_ob_size);
 
! 	merge_freemem(&ms);
 
 dsu_fail:
+ 	if (self->ob_item == empty_ob_item)
+ 		PyMem_FREE(empty_ob_item);
+ 	self->ob_size = saved_ob_size;
+ 	self->ob_item = saved_ob_item;
 	Py_XDECREF(compare);
 	Py_XINCREF(result);