diff --git a/Doc/c-api/arg.rst b/Doc/c-api/arg.rst index dcf7547c2e0..e968c8f4f80 100644 --- a/Doc/c-api/arg.rst +++ b/Doc/c-api/arg.rst @@ -250,6 +250,14 @@ variable(s) whose address should be passed. the conversion has failed. When the conversion fails, the *converter* function should raise an exception and leave the content of *address* unmodified. + If the *converter* returns Py_CLEANUP_SUPPORTED, it may get called a second time + if the argument parsing eventually fails, giving the converter a chance to release + any memory that it had already allocated. In this second call, the *object* parameter + will be NULL; *address* will have the same value as in the original call. + + .. versionchanged:: 3.1 + Py_CLEANUP_SUPPORTED was added. + ``S`` (string) [PyStringObject \*] Like ``O`` but requires that the Python object is a string object. Raises :exc:`TypeError` if the object is not a string object. The C variable may also diff --git a/Doc/c-api/unicode.rst b/Doc/c-api/unicode.rst index d6cedac610a..e348ee7c872 100644 --- a/Doc/c-api/unicode.rst +++ b/Doc/c-api/unicode.rst @@ -379,11 +379,13 @@ Many of the following APIs take two arguments encoding and errors. These parameters encoding and errors have the same semantics as the ones of the builtin unicode() Unicode object constructor. -Setting encoding to *NULL* causes the default encoding to be used which is -ASCII. The file system calls should use :cdata:`Py_FileSystemDefaultEncoding` -as the encoding for file names. This variable should be treated as read-only: On -some systems, it will be a pointer to a static string, on others, it will change -at run-time (such as when the application invokes setlocale). +Setting encoding to *NULL* causes the default encoding to be used +which is ASCII. The file system calls should use +:cfunc:`PyUnicode_FSConverter` for encoding file names. This uses the +variable :cdata:`Py_FileSystemDefaultEncoding` internally. This +variable should be treated as read-only: On some systems, it will be a +pointer to a static string, on others, it will change at run-time +(such as when the application invokes setlocale). Error handling is set by errors which may also be set to *NULL* meaning to use the default handling defined for the codec. Default error handling for all @@ -782,6 +784,19 @@ the user settings on the machine running the codec. object. Error handling is "strict". Return *NULL* if an exception was raised by the codec. +For decoding file names and other environment strings, :cdata:`Py_FileSystemEncoding` +should be used as the encoding, and ``"surrogateescape"`` should be used as the error +handler. For encoding file names during argument parsing, the ``O&`` converter should +be used, passsing PyUnicode_FSConverter as the conversion function: + +.. cfunction:: int PyUnicode_FSConverter(PyObject* obj, void* result) + + Convert *obj* into *result*, using the file system encoding, and the ``surrogateescape`` + error handler. *result* must be a ``PyObject*``, yielding a bytes or bytearray object + which must be released if it is no longer used. + + .. versionadded:: 3.1 + .. % --- Methods & Slots ---------------------------------------------------- diff --git a/Include/modsupport.h b/Include/modsupport.h index 41f922579ed..5ed34033b98 100644 --- a/Include/modsupport.h +++ b/Include/modsupport.h @@ -43,6 +43,8 @@ PyAPI_FUNC(int) PyModule_AddStringConstant(PyObject *, const char *, const char #define PyModule_AddIntMacro(m, c) PyModule_AddIntConstant(m, #c, c) #define PyModule_AddStringMacro(m, c) PyModule_AddStringConstant(m, #c, c) +#define Py_CLEANUP_SUPPORTED 0x20000 + #define PYTHON_API_VERSION 1013 #define PYTHON_API_STRING "1013" /* The API version is maintained (independently from the Python version) diff --git a/Lib/test/test_capi.py b/Lib/test/test_capi.py index bf8b8a85793..d3d22263208 100644 --- a/Lib/test/test_capi.py +++ b/Lib/test/test_capi.py @@ -115,6 +115,10 @@ class TestPendingCalls(unittest.TestCase): self.pendingcalls_submit(l, n) self.pendingcalls_wait(l, n) +# Bug #6012 +class Test6012(unittest.TestCase): + def test(self): + self.assertEqual(_testcapi.argparsing("Hello", "World"), 1) def test_main(): support.run_unittest(CAPITest) @@ -159,7 +163,7 @@ def test_main(): t.start() t.join() - support.run_unittest(TestPendingCalls) + support.run_unittest(TestPendingCalls, Test6012) if __name__ == "__main__": diff --git a/Misc/NEWS b/Misc/NEWS index 95c4c4d379d..1aafdec93ee 100644 --- a/Misc/NEWS +++ b/Misc/NEWS @@ -12,6 +12,8 @@ What's New in Python 3.1 release candidate 1? Core and Builtins ----------------- +- Issue #6012: Add cleanup support to O& argument parsing. + - Issue #6089: Fixed str.format with certain invalid field specifiers that would raise SystemError. diff --git a/Modules/_testcapimodule.c b/Modules/_testcapimodule.c index b1a7093ae1c..5532e07c17c 100644 --- a/Modules/_testcapimodule.c +++ b/Modules/_testcapimodule.c @@ -1415,6 +1415,36 @@ raise_memoryerror(PyObject *self) return NULL; } +/* Issue 6012 */ +static PyObject *str1, *str2; +static int +failing_converter(PyObject *obj, void *arg) +{ + /* Clone str1, then let the conversion fail. */ + assert(str1); + str2 = str1; + Py_INCREF(str2); + return 0; +} +static PyObject* +argparsing(PyObject *o, PyObject *args) +{ + PyObject *res; + str1 = str2 = NULL; + if (!PyArg_ParseTuple(args, "O&O&", + PyUnicode_FSConverter, &str1, + failing_converter, &str2)) { + if (!str2) + /* argument converter not called? */ + return NULL; + /* Should be 1 */ + res = PyLong_FromLong(Py_REFCNT(str2)); + Py_DECREF(str2); + PyErr_Clear(); + return res; + } + Py_RETURN_NONE; +} static PyMethodDef TestMethods[] = { {"raise_exception", raise_exception, METH_VARARGS}, @@ -1433,7 +1463,6 @@ static PyMethodDef TestMethods[] = { PyDoc_STR("This is a pretty normal docstring.")}, {"test_string_to_double", (PyCFunction)test_string_to_double, METH_NOARGS}, {"test_capsule", (PyCFunction)test_capsule, METH_NOARGS}, - {"getargs_tuple", getargs_tuple, METH_VARARGS}, {"getargs_keywords", (PyCFunction)getargs_keywords, METH_VARARGS|METH_KEYWORDS}, @@ -1468,6 +1497,7 @@ static PyMethodDef TestMethods[] = { #endif {"traceback_print", traceback_print, METH_VARARGS}, {"exception_print", exception_print, METH_VARARGS}, + {"argparsing", argparsing, METH_VARARGS}, {NULL, NULL} /* sentinel */ }; diff --git a/Modules/posixmodule.c b/Modules/posixmodule.c index 7f6568c82ce..d25034f14d7 100644 --- a/Modules/posixmodule.c +++ b/Modules/posixmodule.c @@ -804,8 +804,6 @@ posix_2str(PyObject *args, if (!PyArg_ParseTuple(args, format, PyUnicode_FSConverter, &opath1, PyUnicode_FSConverter, &opath2)) { - Py_XDECREF(opath1); - Py_XDECREF(opath2); return NULL; } path1 = bytes2str(opath1, 1); diff --git a/Objects/unicodeobject.c b/Objects/unicodeobject.c index 3bd1efd9392..47e0933174a 100644 --- a/Objects/unicodeobject.c +++ b/Objects/unicodeobject.c @@ -1539,6 +1539,10 @@ PyUnicode_FSConverter(PyObject* arg, void* addr) PyObject *output = NULL; Py_ssize_t size; void *data; + if (arg == NULL) { + Py_DECREF(*(PyObject**)addr); + return 1; + } if (PyBytes_Check(arg) || PyByteArray_Check(arg)) { output = arg; Py_INCREF(output); @@ -1573,7 +1577,7 @@ PyUnicode_FSConverter(PyObject* arg, void* addr) return 0; } *(PyObject**)addr = output; - return 1; + return Py_CLEANUP_SUPPORTED; } diff --git a/Python/getargs.c b/Python/getargs.c index 15f6dd21810..a5dc3602b2b 100644 --- a/Python/getargs.c +++ b/Python/getargs.c @@ -141,6 +141,7 @@ _PyArg_VaParse_SizeT(PyObject *args, char *format, va_list va) #define GETARGS_CAPSULE_NAME_CLEANUP_PTR "getargs.cleanup_ptr" #define GETARGS_CAPSULE_NAME_CLEANUP_BUFFER "getargs.cleanup_buffer" +#define GETARGS_CAPSULE_NAME_CLEANUP_CONVERT "getargs.cleanup_convert" static void cleanup_ptr(PyObject *self) @@ -194,6 +195,46 @@ addcleanup(void *ptr, PyObject **freelist, PyCapsule_Destructor destr) 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) { @@ -1253,10 +1294,15 @@ convertsimple(PyObject *arg, const char **p_format, va_list *p_va, int flags, typedef int (*converter)(PyObject *, void *); converter convert = va_arg(*p_va, converter); void *addr = va_arg(*p_va, void *); + int res; format++; - if (! (*convert)(arg, addr)) + if (! (res = (*convert)(arg, addr))) return converterr("(unspecified)", arg, msgbuf, bufsize); + if (res == Py_CLEANUP_SUPPORTED && + addcleanup_convert(addr, freelist, convert) == -1) + return converterr("(cleanup problem)", + arg, msgbuf, bufsize); } else { p = va_arg(*p_va, PyObject **);