[Python-checkins] cpython: Issue #14325: Stop using python lists, capsules, and the garbage collector to

Fri Mar 16 13:51:59 CET 2012

http://hg.python.org/cpython/rev/9fc456ac20cf
changeset: 75733:9fc456ac20cf
user: Jean-Paul Calderone <exarkun at twistedmatrix.com>
date: Fri Mar 16 08:51:42 2012 -0400
summary:
 Issue #14325: Stop using python lists, capsules, and the garbage collector to deal with PyArg_Parse* cleanup.
files:
 Python/getargs.c | 225 ++++++++++++++--------------------
 1 files changed, 95 insertions(+), 130 deletions(-)

diff --git a/Python/getargs.c b/Python/getargs.c
--- a/Python/getargs.c
+++ b/Python/getargs.c
@@ -33,16 +33,33 @@
 #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 {
+ void *item;
+ destr_t destructor;
+} freelistentry_t;
+
+typedef struct {
+ 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**);
 
@@ -127,111 +144,54 @@
 #define GETARGS_CAPSULE_NAME_CLEANUP_BUFFER "getargs.cleanup_buffer"
 #define GETARGS_CAPSULE_NAME_CLEANUP_CONVERT "getargs.cleanup_convert"
 
-static void
-cleanup_ptr(PyObject *self)
+static int
+cleanup_ptr(PyObject *self, void *ptr)
 {
- void *ptr = PyCapsule_GetPointer(self, GETARGS_CAPSULE_NAME_CLEANUP_PTR);
 if (ptr) {
 PyMem_FREE(ptr);
 }
-}
-
-static void
-cleanup_buffer(PyObject *self)
-{
- Py_buffer *ptr = (Py_buffer *)PyCapsule_GetPointer(self, GETARGS_CAPSULE_NAME_CLEANUP_BUFFER);
- if (ptr) {
- PyBuffer_Release(ptr);
- }
-}
-
-static int
-addcleanup(void *ptr, PyObject **freelist, int is_buffer)
-{
- PyObject *cobj;
- const char *name;
- PyCapsule_Destructor destr;
-
- if (is_buffer) {
- destr = cleanup_buffer;
- name = GETARGS_CAPSULE_NAME_CLEANUP_BUFFER;
- } else {
- destr = cleanup_ptr;
- name = GETARGS_CAPSULE_NAME_CLEANUP_PTR;
- }
-
- if (!*freelist) {
- *freelist = PyList_New(0);
- if (!*freelist) {
- destr(ptr);
- return -1;
- }
- }
-
- cobj = PyCapsule_New(ptr, name, destr);
- if (!cobj) {
- destr(ptr);
- return -1;
- }
- if (PyList_Append(*freelist, cobj)) {
- Py_DECREF(cobj);
- return -1;
- }
- Py_DECREF(cobj);
- return 0;
-}
-
-static void
-cleanup_convert(PyObject *self)
-{
- typedef int (*destr_t)(PyObject *, void *);
- destr_t destr = (destr_t)PyCapsule_GetContext(self);
- void *ptr = PyCapsule_GetPointer(self,
- GETARGS_CAPSULE_NAME_CLEANUP_CONVERT);
- if (ptr && destr)
- destr(NULL, ptr);
-}
-
-static int
-addcleanup_convert(void *ptr, PyObject **freelist, int (*destr)(PyObject*,void*))
-{
- PyObject *cobj;
- if (!*freelist) {
- *freelist = PyList_New(0);
- if (!*freelist) {
- destr(NULL, ptr);
- return -1;
- }
- }
- cobj = PyCapsule_New(ptr, GETARGS_CAPSULE_NAME_CLEANUP_CONVERT,
- cleanup_convert);
- if (!cobj) {
- destr(NULL, ptr);
- return -1;
- }
- if (PyCapsule_SetContext(cobj, destr) == -1) {
- /* This really should not happen. */
- Py_FatalError("capsule refused setting of context.");
- }
- if (PyList_Append(*freelist, cobj)) {
- Py_DECREF(cobj); /* This will also call destr. */
- return -1;
- }
- Py_DECREF(cobj);
 return 0;
 }
 
 static int
-cleanreturn(int retval, PyObject *freelist)
+cleanup_buffer(PyObject *self, void *ptr)
 {
- 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++)
- PyCapsule_SetDestructor(PyList_GET_ITEM(freelist, i), NULL);
+ Py_buffer *buf = (Py_buffer *)ptr;
+ if (buf) {
+ PyBuffer_Release(buf);
 }
- Py_XDECREF(freelist);
+ return 0;
+}
+
+static int
+addcleanup(void *ptr, freelist_t *freelist, destr_t destructor)
+{
+ 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);
 return retval;
 }
 
@@ -250,7 +210,7 @@
 const char *formatsave = format;
 Py_ssize_t i, len;
 char *msg;
- PyObject *freelist = NULL;
+ freelist_t freelist = {0, NULL};
 int compat = flags & FLAG_COMPAT;
 
 assert(compat || (args != (PyObject*)NULL));
@@ -306,6 +266,8 @@
 
 format = formatsave;
 
+ freelist.entries = PyMem_New(freelistentry_t, max);
+
 if (compat) {
 if (max == 0) {
 if (args == NULL)
@@ -314,7 +276,7 @@
 "%.200s%s takes no arguments",
 fname==NULL ? "function" : fname,
 fname==NULL ? "" : "()");
- return 0;
+ return cleanreturn(0, &freelist);
 }
 else if (min == 1 && max == 1) {
 if (args == NULL) {
@@ -322,26 +284,26 @@
 "%.200s%s takes at least one argument",
 fname==NULL ? "function" : fname,
 fname==NULL ? "" : "()");
- return 0;
+ return cleanreturn(0, &freelist);
 }
 msg = convertitem(args, &format, p_va, flags, levels,
 msgbuf, sizeof(msgbuf), &freelist);
 if (msg == NULL)
- return cleanreturn(1, freelist);
+ return cleanreturn(1, &freelist);
 seterror(levels[0], msg, levels+1, fname, message);
- return cleanreturn(0, freelist);
+ return cleanreturn(0, &freelist);
 }
 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);
@@ -359,7 +321,7 @@
 Py_SAFE_DOWNCAST(len, Py_ssize_t, long));
 else
 PyErr_SetString(PyExc_TypeError, message);
- return 0;
+ return cleanreturn(0, &freelist);
 }
 
 for (i = 0; i < len; i++) {
@@ -370,7 +332,7 @@
 sizeof(msgbuf), &freelist);
 if (msg) {
 seterror(i+1, msg, levels, fname, msg);
- return cleanreturn(0, freelist);
+ return cleanreturn(0, &freelist);
 }
 }
 
@@ -379,10 +341,10 @@
 *format != '|' && *format != ':' && *format != ';') {
 PyErr_Format(PyExc_SystemError,
 "bad format string: %.200s", formatsave);
- return cleanreturn(0, freelist);
+ return cleanreturn(0, &freelist);
 }
 
- return cleanreturn(1, freelist);
+ return cleanreturn(1, &freelist);
 }
 
 
@@ -446,7 +408,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;
@@ -521,7 +483,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;
@@ -586,7 +548,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;\
@@ -863,7 +825,7 @@
 if (getbuffer(arg, (Py_buffer*)p, &buf) < 0)
 return converterr(buf, arg, msgbuf, bufsize);
 format++;
- if (addcleanup(p, freelist, 1)) {
+ if (addcleanup(p, freelist, cleanup_buffer)) {
 return converterr(
 "(cleanup problem)",
 arg, msgbuf, bufsize);
@@ -908,7 +870,7 @@
 if (getbuffer(arg, p, &buf) < 0)
 return converterr(buf, arg, msgbuf, bufsize);
 }
- if (addcleanup(p, freelist, 1)) {
+ if (addcleanup(p, freelist, cleanup_buffer)) {
 return converterr(
 "(cleanup problem)",
 arg, msgbuf, bufsize);
@@ -1120,7 +1082,7 @@
 PyErr_NoMemory();
 RETURN_ERR_OCCURRED;
 }
- if (addcleanup(*buffer, freelist, 0)) {
+ if (addcleanup(*buffer, freelist, cleanup_ptr)) {
 Py_DECREF(s);
 return converterr(
 "(cleanup problem)",
@@ -1162,7 +1124,7 @@
 PyErr_NoMemory();
 RETURN_ERR_OCCURRED;
 }
- if (addcleanup(*buffer, freelist, 0)) {
+ if (addcleanup(*buffer, freelist, cleanup_ptr)) {
 Py_DECREF(s);
 return converterr("(cleanup problem)",
 arg, msgbuf, bufsize);
@@ -1223,7 +1185,7 @@
 return converterr("(unspecified)",
 arg, msgbuf, bufsize);
 if (res == Py_CLEANUP_SUPPORTED &&
- addcleanup_convert(addr, freelist, convert) == -1)
+ addcleanup(addr, freelist, convert) == -1)
 return converterr("(cleanup problem)",
 arg, msgbuf, bufsize);
 }
@@ -1254,7 +1216,7 @@
 PyBuffer_Release((Py_buffer*)p);
 return converterr("contiguous buffer", arg, msgbuf, bufsize);
 }
- if (addcleanup(p, freelist, 1)) {
+ if (addcleanup(p, freelist, cleanup_buffer)) {
 return converterr(
 "(cleanup problem)",
 arg, msgbuf, bufsize);
@@ -1442,7 +1404,8 @@
 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 = {0, NULL};
 
 assert(args != NULL && PyTuple_Check(args));
 assert(keywords == NULL || PyDict_Check(keywords));
@@ -1466,6 +1429,8 @@
 for (len=0; kwlist[len]; len++)
 continue;
 
+ freelist.entries = PyMem_New(freelistentry_t, len);
+
 nargs = PyTuple_GET_SIZE(args);
 nkeywords = (keywords == NULL) ? 0 : PyDict_Size(keywords);
 if (nargs + nkeywords > len) {
@@ -1490,7 +1455,7 @@
 PyErr_Format(PyExc_RuntimeError,
 "More keyword list entries (%d) than "
 "format specifiers (%d)", len, i);
- return cleanreturn(0, freelist);
+ return cleanreturn(0, &freelist);
 }
 current_arg = NULL;
 if (nkeywords) {
@@ -1504,11 +1469,11 @@
 "Argument given by name ('%s') "
 "and position (%d)",
 keyword, i+1);
- return cleanreturn(0, freelist);
+ return cleanreturn(0, &freelist);
 }
 }
 else if (nkeywords && PyErr_Occurred())
- return cleanreturn(0, freelist);
+ return cleanreturn(0, &freelist);
 else if (i < nargs)
 current_arg = PyTuple_GET_ITEM(args, i);
 
@@ -1517,7 +1482,7 @@
 levels, msgbuf, sizeof(msgbuf), &freelist);
 if (msg) {
 seterror(i+1, msg, levels, fname, custom_msg);
- return cleanreturn(0, freelist);
+ return cleanreturn(0, &freelist);
 }
 continue;
 }
@@ -1526,14 +1491,14 @@
 PyErr_Format(PyExc_TypeError, "Required argument "
 "'%s' (pos %d) not found",
 keyword, i+1);
- return cleanreturn(0, freelist);
+ return cleanreturn(0, &freelist);
 }
 /* current code reports success when all required args
 * fulfilled and no keyword args left, with no further
 * validation. XXX Maybe skip this in debug build ?
 */
 if (!nkeywords)
- return cleanreturn(1, freelist);
+ return cleanreturn(1, &freelist);
 
 /* We are into optional args, skip thru to any remaining
 * keyword args */
@@ -1541,7 +1506,7 @@
 if (msg) {
 PyErr_Format(PyExc_RuntimeError, "%s: '%s'", msg,
 format);
- return cleanreturn(0, freelist);
+ return cleanreturn(0, &freelist);
 }
 }
 
@@ -1549,7 +1514,7 @@
 PyErr_Format(PyExc_RuntimeError,
 "more argument specifiers than keyword list entries "
 "(remaining format:'%s')", format);
- return cleanreturn(0, freelist);
+ return cleanreturn(0, &freelist);
 }
 
 /* make sure there are no extraneous keyword arguments */
@@ -1562,7 +1527,7 @@
 if (!PyUnicode_Check(key)) {
 PyErr_SetString(PyExc_TypeError,
 "keywords must be strings");
- return cleanreturn(0, freelist);
+ return cleanreturn(0, &freelist);
 }
 /* check that _PyUnicode_AsString() result is not NULL */
 ks = _PyUnicode_AsString(key);
@@ -1579,12 +1544,12 @@
 "'%U' is an invalid keyword "
 "argument for this function",
 key);
- return cleanreturn(0, freelist);
+ return cleanreturn(0, &freelist);
 }
 }
 }
 
- return cleanreturn(1, freelist);
+ return cleanreturn(1, &freelist);
 }
 
 
-- 
Repository URL: http://hg.python.org/cpython
