[pypy-commit] pypy safe-getargs-freelist: Use a C array to manage freelist items, instead of a Python list; this gives predictable cleanup behavior.

exarkun noreply at buildbot.pypy.org
Thu Mar 15 20:33:16 CET 2012


Author: Jean-Paul Calderone <exarkun at twistedmatrix.com>
Branch: safe-getargs-freelist
Changeset: r53703:8725c0f6ed35
Date: 2012-03-15 15:32 -0400
http://bitbucket.org/pypy/pypy/changeset/8725c0f6ed35/

Log:	Use a C array to manage freelist items, instead of a Python list;
	this gives predictable cleanup behavior.

diff --git a/pypy/module/cpyext/src/getargs.c b/pypy/module/cpyext/src/getargs.c
--- a/pypy/module/cpyext/src/getargs.c
+++ b/pypy/module/cpyext/src/getargs.c
@@ -23,16 +23,35 @@
 #define FLAG_COMPAT 1
 #define FLAG_SIZE_T 2
 
+typedef int (*destr_t)(PyObject *, void *);
+
+
+/* Keep track of "objects" that have been allocated or initialized and
+   which will need to be deallocated or cleaned up somehow if overall
+   parsing fails.
+*/
+typedef struct _Py_CleanupFreelistEntry
+{
+  void *item;
+  destr_t destructor;
+} freelistentry_t;
+
+typedef struct _Py_CleanupFreelist
+{
+  int first_available;
+  freelistentry_t *entries;
+} freelist_t;
+
 
 /* Forward */
 static int vgetargs1(PyObject *, const char *, va_list *, int);
 static void seterror(int, const char *, int *, const char *, const char *);
 static char *convertitem(PyObject *, const char **, va_list *, int, int *, 
-                         char *, size_t, PyObject **);
+                         char *, size_t, freelist_t *);
 static char *converttuple(PyObject *, const char **, va_list *, int,
-			  int *, char *, size_t, int, PyObject **);
+			  int *, char *, size_t, int, freelist_t *);
 static char *convertsimple(PyObject *, const char **, va_list *, int, char *,
-			   size_t, PyObject **);
+			   size_t, freelist_t *);
 static Py_ssize_t convertbuffer(PyObject *, void **p, char **);
 static int getbuffer(PyObject *, Py_buffer *, char**);
 
@@ -129,57 +148,58 @@
 
 /* Handle cleanup of allocated memory in case of exception */
 
-static void
-cleanup_ptr(void *ptr)
+static int
+cleanup_ptr(PyObject *self, void *ptr)
 {
-	PyMem_FREE(ptr);
-}
-
-static void
-cleanup_buffer(void *ptr)
-{
-	PyBuffer_Release((Py_buffer *) ptr);
+    if (ptr) {
+        PyMem_FREE(ptr);
+    }
+    return 0;
 }
 
 static int
-addcleanup(void *ptr, PyObject **freelist, void (*destr)(void *))
+cleanup_buffer(PyObject *self, void *ptr)
 {
-	PyObject *cobj;
-	if (!*freelist) {
-		*freelist = PyList_New(0);
-		if (!*freelist) {
-			destr(ptr);
-			return -1;
-		}
-	}
-	cobj = PyCObject_FromVoidPtr(ptr, destr);
-	if (!cobj) {
-		destr(ptr);
-		return -1;
-	}
-	if (PyList_Append(*freelist, cobj)) {
-		Py_DECREF(cobj);
-		return -1;
-	}
-        Py_DECREF(cobj);
-	return 0;
+    Py_buffer *buf = (Py_buffer *)ptr;
+    if (buf) {
+        PyBuffer_Release(buf);
+    }
+    return 0;
 }
 
 static int
-cleanreturn(int retval, PyObject *freelist)
+addcleanup(void *ptr, freelist_t *freelist, destr_t destructor)
 {
-	if (freelist && retval != 0) {
-		/* We were successful, reset the destructors so that they
-		   don't get called. */
-		Py_ssize_t len = PyList_GET_SIZE(freelist), i;
-		for (i = 0; i < len; i++)
-			((PyCObject *) PyList_GET_ITEM(freelist, i))
-				->destructor = NULL;
-	}
-	Py_XDECREF(freelist);
-	return retval;
+    int index;
+
+    index = freelist->first_available;
+    freelist->first_available += 1;
+
+    freelist->entries[index].item = ptr;
+    freelist->entries[index].destructor = destructor;
+
+    return 0;
 }
 
+static int
+cleanreturn(int retval, freelist_t *freelist)
+{
+    int index;
+
+    if (retval == 0)
+    {
+      /* A failure occurred, therefore execute all of the cleanup
+	 functions.
+      */
+      for (index = 0; index < freelist->first_available; ++index)
+      {
+          freelist->entries[index].destructor(NULL, freelist->entries[index].item);
+      }
+    }
+    PyMem_Free(freelist->entries);
+    PyMem_Free(freelist);
+    return retval;
+}
 
 static int
 vgetargs1(PyObject *args, const char *format, va_list *p_va, int flags)
@@ -195,7 +215,7 @@
 	const char *formatsave = format;
 	Py_ssize_t i, len;
 	char *msg;
-	PyObject *freelist = NULL;
+	freelist_t *freelist;
 	int compat = flags & FLAG_COMPAT;
 
 	assert(compat || (args != (PyObject*)NULL));
@@ -251,16 +271,20 @@
 	
 	format = formatsave;
 	
+	freelist = PyMem_New(freelist_t, 1);
+	freelist->first_available = 0;
+	freelist->entries = PyMem_New(freelistentry_t, max);
+
 	if (compat) {
 		if (max == 0) {
 			if (args == NULL)
-				return 1;
+			    return cleanreturn(1, freelist);
 			PyOS_snprintf(msgbuf, sizeof(msgbuf),
 				      "%.200s%s takes no arguments",
 				      fname==NULL ? "function" : fname,
 				      fname==NULL ? "" : "()");
 			PyErr_SetString(PyExc_TypeError, msgbuf);
-			return 0;
+			return cleanreturn(0, freelist);
 		}
 		else if (min == 1 && max == 1) {
 			if (args == NULL) {
@@ -269,10 +293,10 @@
 					      fname==NULL ? "function" : fname,
 					      fname==NULL ? "" : "()");
 				PyErr_SetString(PyExc_TypeError, msgbuf);
-				return 0;
+				return cleanreturn(0, freelist);
 			}
 			msg = convertitem(args, &format, p_va, flags, levels, 
-					  msgbuf, sizeof(msgbuf), &freelist);
+					  msgbuf, sizeof(msgbuf), freelist);
 			if (msg == NULL)
 				return cleanreturn(1, freelist);
 			seterror(levels[0], msg, levels+1, fname, message);
@@ -281,14 +305,14 @@
 		else {
 			PyErr_SetString(PyExc_SystemError,
 			    "old style getargs format uses new features");
-			return 0;
+			return cleanreturn(0, freelist);
 		}
 	}
 	
 	if (!PyTuple_Check(args)) {
 		PyErr_SetString(PyExc_SystemError,
 		    "new style getargs format but argument is not a tuple");
-		return 0;
+		return cleanreturn(0, freelist);
 	}
 	
 	len = PyTuple_GET_SIZE(args);
@@ -308,7 +332,7 @@
 			message = msgbuf;
 		}
 		PyErr_SetString(PyExc_TypeError, message);
-		return 0;
+		return cleanreturn(0, freelist);
 	}
 	
 	for (i = 0; i < len; i++) {
@@ -316,7 +340,7 @@
 			format++;
 		msg = convertitem(PyTuple_GET_ITEM(args, i), &format, p_va,
 				  flags, levels, msgbuf, 
-				  sizeof(msgbuf), &freelist);
+				  sizeof(msgbuf), freelist);
 		if (msg) {
 			seterror(i+1, msg, levels, fname, message);
 			return cleanreturn(0, freelist);
@@ -395,7 +419,7 @@
 static char *
 converttuple(PyObject *arg, const char **p_format, va_list *p_va, int flags,
              int *levels, char *msgbuf, size_t bufsize, int toplevel, 
-             PyObject **freelist)
+             freelist_t *freelist)
 {
 	int level = 0;
 	int n = 0;
@@ -472,7 +496,7 @@
 
 static char *
 convertitem(PyObject *arg, const char **p_format, va_list *p_va, int flags,
-            int *levels, char *msgbuf, size_t bufsize, PyObject **freelist)
+            int *levels, char *msgbuf, size_t bufsize, freelist_t *freelist)
 {
 	char *msg;
 	const char *format = *p_format;
@@ -539,7 +563,7 @@
 
 static char *
 convertsimple(PyObject *arg, const char **p_format, va_list *p_va, int flags,
-              char *msgbuf, size_t bufsize, PyObject **freelist)
+              char *msgbuf, size_t bufsize, freelist_t *freelist)
 {
 	/* For # codes */
 #define FETCH_SIZE	int *q=NULL;Py_ssize_t *q2=NULL;\
@@ -1501,7 +1525,9 @@
 	const char *fname, *msg, *custom_msg, *keyword;
 	int min = INT_MAX;
 	int i, len, nargs, nkeywords;
-	PyObject *freelist = NULL, *current_arg;
+	PyObject *current_arg;
+	freelist_t *freelist;
+
 
 	assert(args != NULL && PyTuple_Check(args));
 	assert(keywords == NULL || PyDict_Check(keywords));
@@ -1525,6 +1551,10 @@
 	for (len=0; kwlist[len]; len++)
 		continue;
 
+	freelist = PyMem_New(freelist_t, 1);
+	freelist->first_available = 0;
+	freelist->entries = PyMem_New(freelistentry_t, len);
+
 	nargs = PyTuple_GET_SIZE(args);
 	nkeywords = (keywords == NULL) ? 0 : PyDict_Size(keywords);
 	if (nargs + nkeywords > len) {
@@ -1535,7 +1565,7 @@
 			     len,
 			     (len == 1) ? "" : "s",
 			     nargs + nkeywords);
-		return 0;
+		return cleanreturn(0, freelist);
 	}
 
 	/* convert tuple args and keyword args in same loop, using kwlist to drive process */
@@ -1573,7 +1603,7 @@
 			
 		if (current_arg) {
 			msg = convertitem(current_arg, &format, p_va, flags,
-				levels, msgbuf, sizeof(msgbuf), &freelist);
+				levels, msgbuf, sizeof(msgbuf), freelist);
 			if (msg) {
 				seterror(i+1, msg, levels, fname, custom_msg);
 				return cleanreturn(0, freelist);


More information about the pypy-commit mailing list