#3668: When PyArg_ParseTuple correctly parses a s* format, but raises an

exception afterwards (for a subsequent parameter), the user code will
not call PyBuffer_Release() and memory will leak.

Reviewed by Amaury Forgeot d'Arc.

4 files changed, 60 insertions, 23 deletions

diff --git a/Include/cobject.h b/Include/cobject.h
index 499dfad..4e24d7d 100644
--- a/Include/cobject.h
+++ b/Include/cobject.h
@@ -48,6 +48,15 @@ PyAPI_FUNC(void *) PyCObject_Import(char *module_name, char *cobject_name);
 /* Modify a C object. Fails (==0) if object has a destructor. */
 PyAPI_FUNC(int) PyCObject_SetVoidPtr(PyObject *self, void *cobj);
 
+
+typedef struct {
+    PyObject_HEAD
+    void *cobject;
+    void *desc;
+    void (*destructor)(void *);
+} PyCObject;
+
+
 #ifdef __cplusplus
 }
 #endif
diff --git a/Misc/NEWS b/Misc/NEWS
index 5372f0b..5c9367b 100644
--- a/Misc/NEWS
+++ b/Misc/NEWS
@@ -12,6 +12,10 @@ What's New in Python 3.0 release candidate 1
 Core and Builtins
 -----------------
 
+- Issue #3668: Fix a memory leak with the "s*" argument parser in
+  PyArg_ParseTuple and friends, which occurred when the argument for "s*" 
+  was correctly parsed but parsing of subsequent arguments failed.
+
 - Issue #3611: An exception __context__ could be cleared in a complex pattern
   involving a __del__ method re-raising an exception.
 
diff --git a/Objects/cobject.c b/Objects/cobject.c
index a1ee686..c437491 100644
--- a/Objects/cobject.c
+++ b/Objects/cobject.c
@@ -9,13 +9,6 @@
 typedef void (*destructor1)(void *);
 typedef void (*destructor2)(void *, void*);
 
-typedef struct {
-    PyObject_HEAD
-    void *cobject;
-    void *desc;
-    void (*destructor)(void *);
-} PyCObject;
-
 PyObject *
 PyCObject_FromVoidPtr(void *cobj, void (*destr)(void *))
 {
diff --git a/Python/getargs.c b/Python/getargs.c
index 13c3f4b..9b1207f 100644
--- a/Python/getargs.c
+++ b/Python/getargs.c
@@ -139,24 +139,35 @@ _PyArg_VaParse_SizeT(PyObject *args, char *format, va_list va)
 
 /* Handle cleanup of allocated memory in case of exception */
 
+static void
+cleanup_ptr(void *ptr)
+{
+	PyMem_FREE(ptr);
+}
+
+static void
+cleanup_buffer(void *ptr)
+{
+	PyBuffer_Release((Py_buffer *) ptr);
+}
+
 static int
-addcleanup(void *ptr, PyObject **freelist)
+addcleanup(void *ptr, PyObject **freelist, void (*destr)(void *))
 {
 	PyObject *cobj;
 	if (!*freelist) {
 		*freelist = PyList_New(0);
 		if (!*freelist) {
-			PyMem_FREE(ptr);
+			destr(ptr);
 			return -1;
 		}
 	}
-	cobj = PyCObject_FromVoidPtr(ptr, NULL);
+	cobj = PyCObject_FromVoidPtr(ptr, destr);
 	if (!cobj) {
-		PyMem_FREE(ptr);
+		destr(ptr);
 		return -1;
 	}
 	if (PyList_Append(*freelist, cobj)) {
-                PyMem_FREE(ptr);
 		Py_DECREF(cobj);
 		return -1;
 	}
@@ -167,15 +178,15 @@ addcleanup(void *ptr, PyObject **freelist)
 static int
 cleanreturn(int retval, PyObject *freelist)
 {
-	if (freelist) {
-		if (retval == 0) {
-			Py_ssize_t len = PyList_GET_SIZE(freelist), i;
-			for (i = 0; i < len; i++)
-                                PyMem_FREE(PyCObject_AsVoidPtr(
-                                		PyList_GET_ITEM(freelist, i)));
-		}
-		Py_DECREF(freelist);
-	}
+	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;
 }
 
@@ -807,6 +818,11 @@ convertsimple(PyObject *arg, const char **p_format, va_list *p_va, int flags,
 				if (getbuffer(arg, p, &buf) < 0)
 					return converterr(buf, arg, msgbuf, bufsize);
 			}
+			if (addcleanup(p, freelist, cleanup_buffer)) {
+				return converterr(
+					"(cleanup problem)",
+					arg, msgbuf, bufsize);
+			}
 			format++;
 		} else if (*format == '#') {
 			void **p = (void **)va_arg(*p_va, char **);
@@ -856,6 +872,11 @@ convertsimple(PyObject *arg, const char **p_format, va_list *p_va, int flags,
 			if (getbuffer(arg, (Py_buffer*)p, &buf) < 0)
 				return converterr(buf, arg, msgbuf, bufsize);
 			format++;
+			if (addcleanup(p, freelist, cleanup_buffer)) {
+				return converterr(
+					"(cleanup problem)",
+					arg, msgbuf, bufsize);
+			}
 			break;
 		}
 		count = convertbuffer(arg, p, &buf);
@@ -889,6 +910,11 @@ convertsimple(PyObject *arg, const char **p_format, va_list *p_va, int flags,
 				if (getbuffer(arg, p, &buf) < 0)
 					return converterr(buf, arg, msgbuf, bufsize);
 			}
+			if (addcleanup(p, freelist, cleanup_buffer)) {
+				return converterr(
+					"(cleanup problem)",
+					arg, msgbuf, bufsize);
+			}
 			format++;
 		} else if (*format == '#') { /* any buffer-like object */
 			void **p = (void **)va_arg(*p_va, char **);
@@ -1094,7 +1120,7 @@ convertsimple(PyObject *arg, const char **p_format, va_list *p_va, int flags,
 						"(memory error)",
 						arg, msgbuf, bufsize);
 				}
-				if (addcleanup(*buffer, freelist)) {
+				if (addcleanup(*buffer, freelist, cleanup_ptr)) {
 					Py_DECREF(s);
 					return converterr(
 						"(cleanup problem)",
@@ -1136,7 +1162,7 @@ convertsimple(PyObject *arg, const char **p_format, va_list *p_va, int flags,
 				return converterr("(memory error)",
 						  arg, msgbuf, bufsize);
 			}
-			if (addcleanup(*buffer, freelist)) {
+			if (addcleanup(*buffer, freelist, cleanup_ptr)) {
 				Py_DECREF(s);
 				return converterr("(cleanup problem)",
 						arg, msgbuf, bufsize);
@@ -1249,6 +1275,11 @@ convertsimple(PyObject *arg, const char **p_format, va_list *p_va, int flags,
 				PyErr_Clear();
 				return converterr("read-write buffer", arg, msgbuf, bufsize);
 			}
+			if (addcleanup(p, freelist, cleanup_buffer)) {
+				return converterr(
+					"(cleanup problem)",
+					arg, msgbuf, bufsize);
+			}
 			if (!PyBuffer_IsContiguous((Py_buffer*)p, 'C'))
 				return converterr("contiguous buffer", arg, msgbuf, bufsize);
 			break;