#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.

diff --git a/Include/cobject.h b/Include/cobject.h
index 499dfadddfee163cd08dcf9e3143c20ff0a6d380..4e24d7dcd62ba8e6e8a83aeb46bd1bf692b8743c 100644 (file)
--- 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 5372f0b9a926f13c29c176a520b056d7d84e1472..5c9367b39c5841a7241a6ec0694e458d84bae762 100644 (file)
--- 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 a1ee68622addc115d2c24cb9ad884dd98048ed7a..c437491c27c26bc57aa60557b43abf0180bc8a84 100644 (file)
--- 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 13c3f4b9a6ef4dfe9241e15a4a0dd11ba92fd1eb..9b1207f04eea2ffc39094ffd72169da94e7d494a 100644 (file)
--- 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;