From 8021875bbcf7385e651def51bc597472a569042c Mon Sep 17 00:00:00 2001 From: Victor Stinner Date: Wed, 4 Nov 2020 13:59:15 +0100 Subject: [PATCH] bpo-1635741: Add PyModule_AddObjectRef() function (GH-23122) Added PyModule_AddObjectRef() function: similar to PyModule_AddObjectRef() but don't steal a reference to the value on success. --- Doc/c-api/module.rst | 102 +++++++++++++++--- Doc/whatsnew/3.10.rst | 5 + Include/modsupport.h | 10 +- ...2020-11-03-11-52-27.bpo-1635741.aDYJKB.rst | 3 + Python/modsupport.c | 70 ++++++------ 5 files changed, 145 insertions(+), 45 deletions(-) create mode 100644 Misc/NEWS.d/next/C API/2020-11-03-11-52-27.bpo-1635741.aDYJKB.rst diff --git a/Doc/c-api/module.rst b/Doc/c-api/module.rst index 6e9474bfa40..41a705d9e99 100644 --- a/Doc/c-api/module.rst +++ b/Doc/c-api/module.rst @@ -264,7 +264,7 @@ of the following two module creation functions: instead; only use this if you are sure you need it. Before it is returned from in the initialization function, the resulting module -object is typically populated using functions like :c:func:`PyModule_AddObject`. +object is typically populated using functions like :c:func:`PyModule_AddObjectRef`. .. _multi-phase-initialization: @@ -437,26 +437,102 @@ a function called from a module execution slot (if using multi-phase initialization), can use the following functions to help initialize the module state: +.. c:function:: int PyModule_AddObjectRef(PyObject *module, const char *name, PyObject *value) + + Add an object to *module* as *name*. This is a convenience function which + can be used from the module's initialization function. + + On success, return ``0``. On error, raise an exception and return ``-1``. + + Return ``NULL`` if *value* is ``NULL``. It must be called with an exception + raised in this case. + + Example usage:: + + static int + add_spam(PyObject *module, int value) + { + PyObject *obj = PyLong_FromLong(value); + if (obj == NULL) { + return -1; + } + int res = PyModule_AddObjectRef(module, "spam", obj); + Py_DECREF(obj); + return res; + } + + The example can also be written without checking explicitly if *obj* is + ``NULL``:: + + static int + add_spam(PyObject *module, int value) + { + PyObject *obj = PyLong_FromLong(value); + int res = PyModule_AddObjectRef(module, "spam", obj); + Py_XDECREF(obj); + return res; + } + + Note that ``Py_XDECREF()`` should be used instead of ``Py_DECREF()`` in + this case, since *obj* can be ``NULL``. + + .. versionadded:: 3.10 + + .. c:function:: int PyModule_AddObject(PyObject *module, const char *name, PyObject *value) - Add an object to *module* as *name*. This is a convenience function which can - be used from the module's initialization function. This steals a reference to - *value* on success. Return ``-1`` on error, ``0`` on success. + Similar to :c:func:`PyModule_AddObjectRef`, but steals a reference to + *value* on success (if it returns ``0``). + + The new :c:func:`PyModule_AddObjectRef` function is recommended, since it is + easy to introduce reference leaks by misusing the + :c:func:`PyModule_AddObject` function. .. note:: - Unlike other functions that steal references, ``PyModule_AddObject()`` only - decrements the reference count of *value* **on success**. + Unlike other functions that steal references, ``PyModule_AddObject()`` + only decrements the reference count of *value* **on success**. This means that its return value must be checked, and calling code must - :c:func:`Py_DECREF` *value* manually on error. Example usage:: + :c:func:`Py_DECREF` *value* manually on error. + + Example usage:: + + static int + add_spam(PyObject *module, int value) + { + PyObject *obj = PyLong_FromLong(value); + if (obj == NULL) { + return -1; + } + if (PyModule_AddObject(module, "spam", obj) < 0) { + Py_DECREF(obj); + return -1; + } + // PyModule_AddObject() stole a reference to obj: + // Py_DECREF(obj) is not needed here + return 0; + } + + The example can also be written without checking explicitly if *obj* is + ``NULL``:: + + static int + add_spam(PyObject *module, int value) + { + PyObject *obj = PyLong_FromLong(value); + if (PyModule_AddObject(module, "spam", obj) < 0) { + Py_XDECREF(obj); + return -1; + } + // PyModule_AddObject() stole a reference to obj: + // Py_DECREF(obj) is not needed here + return 0; + } + + Note that ``Py_XDECREF()`` should be used instead of ``Py_DECREF()`` in + this case, since *obj* can be ``NULL``. - Py_INCREF(spam); - if (PyModule_AddObject(module, "spam", spam) < 0) { - Py_DECREF(module); - Py_DECREF(spam); - return NULL; - } .. c:function:: int PyModule_AddIntConstant(PyObject *module, const char *name, long value) diff --git a/Doc/whatsnew/3.10.rst b/Doc/whatsnew/3.10.rst index 89fc3007782..9d9284897be 100644 --- a/Doc/whatsnew/3.10.rst +++ b/Doc/whatsnew/3.10.rst @@ -374,6 +374,11 @@ New Features * Added :c:func:`PyUnicode_AsUTF8AndSize` to the limited C API. (Contributed by Alex Gaynor in :issue:`41784`.) +* Added :c:func:`PyModule_AddObjectRef` function: similar to + :c:func:`PyModule_AddObjectRef` but don't steal a reference to the value on + success. + (Contributed by Victor Stinner in :issue:`1635741`.) + Porting to Python 3.10 ---------------------- diff --git a/Include/modsupport.h b/Include/modsupport.h index 4c4aab65bac..f009d586bf6 100644 --- a/Include/modsupport.h +++ b/Include/modsupport.h @@ -136,7 +136,15 @@ PyAPI_FUNC(PyObject * const *) _PyArg_UnpackKeywords( void _PyArg_Fini(void); #endif /* Py_LIMITED_API */ -PyAPI_FUNC(int) PyModule_AddObject(PyObject *, const char *, PyObject *); +// Add an attribute with name 'name' and value 'obj' to the module 'mod. +// On success, return 0 on success. +// On error, raise an exception and return -1. +PyAPI_FUNC(int) PyModule_AddObjectRef(PyObject *mod, const char *name, PyObject *value); + +// Similar to PyModule_AddObjectRef() but steal a reference to 'obj' +// (Py_DECREF(obj)) on success (if it returns 0). +PyAPI_FUNC(int) PyModule_AddObject(PyObject *mod, const char *, PyObject *value); + PyAPI_FUNC(int) PyModule_AddIntConstant(PyObject *, const char *, long); PyAPI_FUNC(int) PyModule_AddStringConstant(PyObject *, const char *, const char *); #if !defined(Py_LIMITED_API) || Py_LIMITED_API+0 >= 0x03090000 diff --git a/Misc/NEWS.d/next/C API/2020-11-03-11-52-27.bpo-1635741.aDYJKB.rst b/Misc/NEWS.d/next/C API/2020-11-03-11-52-27.bpo-1635741.aDYJKB.rst new file mode 100644 index 00000000000..2ab1afb922f --- /dev/null +++ b/Misc/NEWS.d/next/C API/2020-11-03-11-52-27.bpo-1635741.aDYJKB.rst @@ -0,0 +1,3 @@ +Added :c:func:`PyModule_AddObjectRef` function: similar to +:c:func:`PyModule_AddObjectRef` but don't steal a reference to the value on +success. Patch by Victor Stinner. diff --git a/Python/modsupport.c b/Python/modsupport.c index 2dabcf38340..8655daa1fc5 100644 --- a/Python/modsupport.c +++ b/Python/modsupport.c @@ -634,56 +634,70 @@ va_build_stack(PyObject **small_stack, Py_ssize_t small_stack_len, int -PyModule_AddObject(PyObject *m, const char *name, PyObject *o) +PyModule_AddObjectRef(PyObject *mod, const char *name, PyObject *value) { - PyObject *dict; - if (!PyModule_Check(m)) { + if (!PyModule_Check(mod)) { PyErr_SetString(PyExc_TypeError, - "PyModule_AddObject() needs module as first arg"); + "PyModule_AddObjectRef() first argument " + "must be a module"); return -1; } - if (!o) { - if (!PyErr_Occurred()) - PyErr_SetString(PyExc_TypeError, - "PyModule_AddObject() needs non-NULL value"); + if (!value) { + if (!PyErr_Occurred()) { + PyErr_SetString(PyExc_SystemError, + "PyModule_AddObjectRef() must be called " + "with an exception raised if value is NULL"); + } return -1; } - dict = PyModule_GetDict(m); + PyObject *dict = PyModule_GetDict(mod); if (dict == NULL) { /* Internal error -- modules must have a dict! */ PyErr_Format(PyExc_SystemError, "module '%s' has no __dict__", - PyModule_GetName(m)); + PyModule_GetName(mod)); return -1; } - if (PyDict_SetItemString(dict, name, o)) + + if (PyDict_SetItemString(dict, name, value)) { return -1; - Py_DECREF(o); + } return 0; } + +int +PyModule_AddObject(PyObject *mod, const char *name, PyObject *value) +{ + int res = PyModule_AddObjectRef(mod, name, value); + if (res == 0) { + Py_DECREF(value); + } + return res; +} + int PyModule_AddIntConstant(PyObject *m, const char *name, long value) { - PyObject *o = PyLong_FromLong(value); - if (!o) + PyObject *obj = PyLong_FromLong(value); + if (!obj) { return -1; - if (PyModule_AddObject(m, name, o) == 0) - return 0; - Py_DECREF(o); - return -1; + } + int res = PyModule_AddObjectRef(m, name, obj); + Py_DECREF(obj); + return res; } int PyModule_AddStringConstant(PyObject *m, const char *name, const char *value) { - PyObject *o = PyUnicode_FromString(value); - if (!o) + PyObject *obj = PyUnicode_FromString(value); + if (!obj) { return -1; - if (PyModule_AddObject(m, name, o) == 0) - return 0; - Py_DECREF(o); - return -1; + } + int res = PyModule_AddObjectRef(m, name, obj); + Py_DECREF(obj); + return res; } int @@ -696,11 +710,5 @@ PyModule_AddType(PyObject *module, PyTypeObject *type) const char *name = _PyType_Name(type); assert(name != NULL); - Py_INCREF(type); - if (PyModule_AddObject(module, name, (PyObject *)type) < 0) { - Py_DECREF(type); - return -1; - } - - return 0; + return PyModule_AddObjectRef(module, name, (PyObject *)type); }