From 26e6d1c4ba11d9241a8dd0102e1e45bada5d27c6 Mon Sep 17 00:00:00 2001 From: Serhiy Storchaka Date: Sat, 21 Nov 2020 14:13:50 +0200 Subject: [PATCH 1/9] bpo-42327: Add PyModule_Add(). Also fix possible leaks in initialization of modules: _csv, _curses_panel, _elementtree, _io, _pickle, _stat, _testinternalcapi, _threadmodule, _zoneinfo, _codecs_*, cmath, math, ossaudiodev, time, xxlimited, xxmodule, xxsubtype. --- Doc/c-api/module.rst | 65 +++++++++---------- Doc/whatsnew/3.10.rst | 4 ++ Include/modsupport.h | 11 +++- .../2020-11-11-22-36-29.bpo-42327.ODSZBM.rst | 1 + Modules/_csv.c | 26 ++++---- Modules/_curses_panel.c | 5 +- Modules/_elementtree.c | 36 +++++----- Modules/_io/_iomodule.c | 12 ++-- Modules/_pickle.c | 39 ++++++----- Modules/_stat.c | 12 ++-- Modules/_testinternalcapi.c | 4 +- Modules/_threadmodule.c | 38 ++++++----- Modules/_zoneinfo.c | 4 +- Modules/cjkcodecs/cjkcodecs.h | 7 +- Modules/cmathmodule.c | 18 ++--- Modules/mathmodule.c | 10 +-- Modules/ossaudiodev.c | 21 +++--- Modules/timemodule.c | 7 +- Modules/xxlimited.c | 20 +++--- Modules/xxmodule.c | 18 +++-- Modules/xxsubtype.c | 6 +- PC/python3dll.c | 1 + Python/modsupport.c | 64 +++++++++--------- 23 files changed, 224 insertions(+), 205 deletions(-) create mode 100644 Misc/NEWS.d/next/C API/2020-11-11-22-36-29.bpo-42327.ODSZBM.rst diff --git a/Doc/c-api/module.rst b/Doc/c-api/module.rst index 41a705d9e99156..75ec1115ceaffe 100644 --- a/Doc/c-api/module.rst +++ b/Doc/c-api/module.rst @@ -479,12 +479,35 @@ state: .. versionadded:: 3.10 +.. c:function:: int PyModule_Add(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 ``-1`` if *value* is ``NULL``. It must be called with an exception + raised in this case. + + This function "steals" a reference to *value*. It can be called with + a result of function that returns a new reference without bothering to + check its result or even saving it to a variable. + + Example usage:: + + if (PyModule_Add(module, "spam", PyBytes_FromString(value)) < 0) { + goto error; + } + + .. versionadded:: 3.10 + + .. c:function:: int PyModule_AddObject(PyObject *module, const char *name, PyObject *value) - Similar to :c:func:`PyModule_AddObjectRef`, but steals a reference to + Similar to :c:func:`PyModule_Add`, but only steals a reference to *value* on success (if it returns ``0``). - The new :c:func:`PyModule_AddObjectRef` function is recommended, since it is + The new :c:func:`PyModule_Add` function is recommended, since it is easy to introduce reference leaks by misusing the :c:func:`PyModule_AddObject` function. @@ -494,41 +517,15 @@ state: 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. + :c:func:`Py_XDECREF` *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; - } + PyObject *obj = PyBytes_FromString(value); + if (PyModule_AddObject(module, "spam", obj) < 0) { + Py_XDECREF(obj); + goto error; + } Note that ``Py_XDECREF()`` should be used instead of ``Py_DECREF()`` in this case, since *obj* can be ``NULL``. diff --git a/Doc/whatsnew/3.10.rst b/Doc/whatsnew/3.10.rst index 16cb7efe2984ea..cf5934f1c871d5 100644 --- a/Doc/whatsnew/3.10.rst +++ b/Doc/whatsnew/3.10.rst @@ -508,6 +508,10 @@ New Features success. (Contributed by Victor Stinner in :issue:`1635741`.) +* Added :c:func:`PyModule_Add` function: similar to + :c:func:`PyModule_AddObject` but always steals a reference to the value. + (Contributed by Serhiy Storchaka in :issue:`42327`.) + * Added :c:func:`Py_NewRef` and :c:func:`Py_XNewRef` functions to increment the reference count of an object and return the object. (Contributed by Victor Stinner in :issue:`42262`.) diff --git a/Include/modsupport.h b/Include/modsupport.h index f009d586bf6202..02d214c3affee7 100644 --- a/Include/modsupport.h +++ b/Include/modsupport.h @@ -136,13 +136,20 @@ PyAPI_FUNC(PyObject * const *) _PyArg_UnpackKeywords( void _PyArg_Fini(void); #endif /* Py_LIMITED_API */ +#if !defined(Py_LIMITED_API) || Py_LIMITED_API+0 >= 0x030A0000 +// Add an attribute with name 'name' and value 'value' to the module 'mod'. +// Steal a reference to 'value'. +// On success, return 0. +// On error, raise an exception and return -1. +PyAPI_FUNC(int) PyModule_Add(PyObject *mod, const char *name, PyObject *value); +#endif /* Py_LIMITED_API */ + // 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). +// Similar to PyModule_Add() but steal a reference to 'value' only on success. PyAPI_FUNC(int) PyModule_AddObject(PyObject *mod, const char *, PyObject *value); PyAPI_FUNC(int) PyModule_AddIntConstant(PyObject *, const char *, long); diff --git a/Misc/NEWS.d/next/C API/2020-11-11-22-36-29.bpo-42327.ODSZBM.rst b/Misc/NEWS.d/next/C API/2020-11-11-22-36-29.bpo-42327.ODSZBM.rst new file mode 100644 index 00000000000000..46ed0065128713 --- /dev/null +++ b/Misc/NEWS.d/next/C API/2020-11-11-22-36-29.bpo-42327.ODSZBM.rst @@ -0,0 +1 @@ +Added :func:`PyModule_Add`. diff --git a/Modules/_csv.c b/Modules/_csv.c index 594f6c14727262..089c0f5b41cead 100644 --- a/Modules/_csv.c +++ b/Modules/_csv.c @@ -1642,7 +1642,7 @@ PyInit__csv(void) /* Add version to the module. */ if (PyModule_AddStringConstant(module, "__version__", MODULE_VERSION) == -1) - return NULL; + goto error; /* Set the field limit */ get_csv_state(module)->field_limit = 128 * 1024; @@ -1650,28 +1650,30 @@ PyInit__csv(void) /* Add _dialects dictionary */ get_csv_state(module)->dialects = PyDict_New(); - if (get_csv_state(module)->dialects == NULL) - return NULL; - Py_INCREF(get_csv_state(module)->dialects); - if (PyModule_AddObject(module, "_dialects", get_csv_state(module)->dialects)) - return NULL; + Py_XINCREF(get_csv_state(module)->dialects); + if (PyModule_Add(module, "_dialects", get_csv_state(module)->dialects)) + goto error; /* Add quote styles into dictionary */ for (style = quote_styles; style->name; style++) { if (PyModule_AddIntConstant(module, style->name, style->style) == -1) - return NULL; + goto error; } if (PyModule_AddType(module, &Dialect_Type)) { - return NULL; + goto error; } /* Add the CSV exception object to the module. */ get_csv_state(module)->error_obj = PyErr_NewException("_csv.Error", NULL, NULL); - if (get_csv_state(module)->error_obj == NULL) - return NULL; - Py_INCREF(get_csv_state(module)->error_obj); - PyModule_AddObject(module, "Error", get_csv_state(module)->error_obj); + Py_XINCREF(get_csv_state(module)->error_obj); + if (PyModule_Add(module, "Error", get_csv_state(module)->error_obj) < 0) { + goto error; + } return module; + +error: + Py_DECREF(module); + return NULL; } diff --git a/Modules/_curses_panel.c b/Modules/_curses_panel.c index 1a8f0b636821ff..6fa24b3c127018 100644 --- a/Modules/_curses_panel.c +++ b/Modules/_curses_panel.c @@ -668,9 +668,8 @@ _curses_panel_exec(PyObject *mod) state->PyCursesError = PyErr_NewException( "_curses_panel.error", NULL, NULL); - Py_INCREF(state->PyCursesError); - if (PyModule_AddObject(mod, "error", state->PyCursesError) < 0) { - Py_DECREF(state->PyCursesError); + Py_XINCREF(state->PyCursesError); + if (PyModule_Add(mod, "error", state->PyCursesError) < 0) { return -1; } diff --git a/Modules/_elementtree.c b/Modules/_elementtree.c index 85fdfa7e5ed42c..fe94ade77cdaff 100644 --- a/Modules/_elementtree.c +++ b/Modules/_elementtree.c @@ -4410,42 +4410,40 @@ PyInit__elementtree(void) st = get_elementtree_state(m); if (!(temp = PyImport_ImportModule("copy"))) - return NULL; + goto error; st->deepcopy_obj = PyObject_GetAttrString(temp, "deepcopy"); Py_XDECREF(temp); if (st->deepcopy_obj == NULL) { - return NULL; + goto error; } assert(!PyErr_Occurred()); if (!(st->elementpath_obj = PyImport_ImportModule("xml.etree.ElementPath"))) - return NULL; + goto error; /* link against pyexpat */ expat_capi = PyCapsule_Import(PyExpat_CAPSULE_NAME, 0); - if (expat_capi) { - /* check that it's usable */ - if (strcmp(expat_capi->magic, PyExpat_CAPI_MAGIC) != 0 || + if (!expat_capi) { + goto error; + } + /* check that it's usable */ + if (strcmp(expat_capi->magic, PyExpat_CAPI_MAGIC) != 0 || (size_t)expat_capi->size < sizeof(struct PyExpat_CAPI) || expat_capi->MAJOR_VERSION != XML_MAJOR_VERSION || expat_capi->MINOR_VERSION != XML_MINOR_VERSION || expat_capi->MICRO_VERSION != XML_MICRO_VERSION) { - PyErr_SetString(PyExc_ImportError, - "pyexpat version is incompatible"); - return NULL; - } - } else { - return NULL; + PyErr_SetString(PyExc_ImportError, + "pyexpat version is incompatible"); + goto error; } st->parseerror_obj = PyErr_NewException( "xml.etree.ElementTree.ParseError", PyExc_SyntaxError, NULL ); - Py_INCREF(st->parseerror_obj); - if (PyModule_AddObject(m, "ParseError", st->parseerror_obj) < 0) { - Py_DECREF(st->parseerror_obj); - return NULL; + Py_XINCREF(st->parseerror_obj); + if (PyModule_Add(m, "ParseError", st->parseerror_obj) < 0) { + goto error; } PyTypeObject *types[] = { @@ -4456,9 +4454,13 @@ PyInit__elementtree(void) for (size_t i = 0; i < Py_ARRAY_LENGTH(types); i++) { if (PyModule_AddType(m, types[i]) < 0) { - return NULL; + goto error; } } return m; + +error: + Py_DECREF(m); + return NULL; } diff --git a/Modules/_io/_iomodule.c b/Modules/_io/_iomodule.c index 9147648b243bed..d316f0a1506c17 100644 --- a/Modules/_io/_iomodule.c +++ b/Modules/_io/_iomodule.c @@ -668,17 +668,15 @@ PyInit__io(void) state->unsupported_operation = PyObject_CallFunction( (PyObject *)&PyType_Type, "s(OO){}", "UnsupportedOperation", PyExc_OSError, PyExc_ValueError); - if (state->unsupported_operation == NULL) - goto fail; - Py_INCREF(state->unsupported_operation); - if (PyModule_AddObject(m, "UnsupportedOperation", - state->unsupported_operation) < 0) + Py_XINCREF(state->unsupported_operation); + if (PyModule_Add(m, "UnsupportedOperation", + state->unsupported_operation) < 0) goto fail; /* BlockingIOError, for compatibility */ Py_INCREF(PyExc_BlockingIOError); - if (PyModule_AddObject(m, "BlockingIOError", - (PyObject *) PyExc_BlockingIOError) < 0) + if (PyModule_Add(m, "BlockingIOError", + (PyObject *) PyExc_BlockingIOError) < 0) goto fail; /* Concrete base types of the IO ABCs. diff --git a/Modules/_pickle.c b/Modules/_pickle.c index ed8afefe4c74c8..174b7a18200aae 100644 --- a/Modules/_pickle.c +++ b/Modules/_pickle.c @@ -7973,42 +7973,41 @@ PyInit__pickle(void) /* Add types */ if (PyModule_AddType(m, &Pickler_Type) < 0) { - return NULL; + goto error; } if (PyModule_AddType(m, &Unpickler_Type) < 0) { - return NULL; + goto error; } if (PyModule_AddType(m, &PyPickleBuffer_Type) < 0) { - return NULL; + goto error; } st = _Pickle_GetState(m); /* Initialize the exceptions. */ st->PickleError = PyErr_NewException("_pickle.PickleError", NULL, NULL); - if (st->PickleError == NULL) - return NULL; + Py_XINCREF(st->PickleError); + if (PyModule_Add(m, "PickleError", st->PickleError) < 0) + goto error; + st->PicklingError = \ PyErr_NewException("_pickle.PicklingError", st->PickleError, NULL); - if (st->PicklingError == NULL) - return NULL; + Py_XINCREF(st->PicklingError); + if (PyModule_Add(m, "PicklingError", st->PicklingError) < 0) + goto error; + st->UnpicklingError = \ PyErr_NewException("_pickle.UnpicklingError", st->PickleError, NULL); - if (st->UnpicklingError == NULL) - return NULL; - - Py_INCREF(st->PickleError); - if (PyModule_AddObject(m, "PickleError", st->PickleError) < 0) - return NULL; - Py_INCREF(st->PicklingError); - if (PyModule_AddObject(m, "PicklingError", st->PicklingError) < 0) - return NULL; - Py_INCREF(st->UnpicklingError); - if (PyModule_AddObject(m, "UnpicklingError", st->UnpicklingError) < 0) - return NULL; + Py_XINCREF(st->UnpicklingError); + if (PyModule_Add(m, "UnpicklingError", st->UnpicklingError) < 0) + goto error; if (_Pickle_InitState(st) < 0) - return NULL; + goto error; return m; + +error: + Py_DECREF(m); + return NULL; } diff --git a/Modules/_stat.c b/Modules/_stat.c index 546e6a5f94ca15..7f383d8845c40b 100644 --- a/Modules/_stat.c +++ b/Modules/_stat.c @@ -592,16 +592,16 @@ stat_exec(PyObject *module) ADD_INT_MACRO(module, FILE_ATTRIBUTE_TEMPORARY); ADD_INT_MACRO(module, FILE_ATTRIBUTE_VIRTUAL); - if (PyModule_AddObject(module, "IO_REPARSE_TAG_SYMLINK", - PyLong_FromUnsignedLong(IO_REPARSE_TAG_SYMLINK)) < 0) { + if (PyModule_Add(module, "IO_REPARSE_TAG_SYMLINK", + PyLong_FromUnsignedLong(IO_REPARSE_TAG_SYMLINK)) < 0) { return -1; } - if (PyModule_AddObject(module, "IO_REPARSE_TAG_MOUNT_POINT", - PyLong_FromUnsignedLong(IO_REPARSE_TAG_MOUNT_POINT)) < 0) { + if (PyModule_Add(module, "IO_REPARSE_TAG_MOUNT_POINT", + PyLong_FromUnsignedLong(IO_REPARSE_TAG_MOUNT_POINT)) < 0) { return -1; } - if (PyModule_AddObject(module, "IO_REPARSE_TAG_APPEXECLINK", - PyLong_FromUnsignedLong(IO_REPARSE_TAG_APPEXECLINK)) < 0) { + if (PyModule_Add(module, "IO_REPARSE_TAG_APPEXECLINK", + PyLong_FromUnsignedLong(IO_REPARSE_TAG_APPEXECLINK)) < 0) { return -1; } #endif diff --git a/Modules/_testinternalcapi.c b/Modules/_testinternalcapi.c index df4725ea0a1c82..e1bf42c4f9732b 100644 --- a/Modules/_testinternalcapi.c +++ b/Modules/_testinternalcapi.c @@ -301,8 +301,8 @@ PyInit__testinternalcapi(void) return NULL; } - if (PyModule_AddObject(module, "SIZEOF_PYGC_HEAD", - PyLong_FromSsize_t(sizeof(PyGC_Head))) < 0) { + if (PyModule_Add(module, "SIZEOF_PYGC_HEAD", + PyLong_FromSsize_t(sizeof(PyGC_Head))) < 0) { goto error; } diff --git a/Modules/_threadmodule.c b/Modules/_threadmodule.c index 56ed8a2e2d3f14..6d6dee2ccf980f 100644 --- a/Modules/_threadmodule.c +++ b/Modules/_threadmodule.c @@ -1562,7 +1562,7 @@ static struct PyModuleDef threadmodule = { PyMODINIT_FUNC PyInit__thread(void) { - PyObject *m, *d, *v; + PyObject *m, *v; double time_max; double timeout_max; PyInterpreterState *interp = _PyInterpreterState_GET(); @@ -1595,41 +1595,47 @@ PyInit__thread(void) timeout_max = floor(timeout_max); v = PyFloat_FromDouble(timeout_max); - if (!v) - return NULL; - if (PyModule_AddObject(m, "TIMEOUT_MAX", v) < 0) - return NULL; + if (PyModule_Add(m, "TIMEOUT_MAX", v) < 0) + goto error; /* Add a symbolic constant */ - d = PyModule_GetDict(m); ThreadError = PyExc_RuntimeError; Py_INCREF(ThreadError); - PyDict_SetItemString(d, "error", ThreadError); + Py_INCREF(ThreadError); + if (PyModule_Add(m, "error", ThreadError) < 0) { + goto error; + } Locktype.tp_doc = lock_doc; Py_INCREF(&Locktype); - PyDict_SetItemString(d, "LockType", (PyObject *)&Locktype); + if (PyModule_Add(m, "LockType", (PyObject *)&Locktype) < 0) { + goto error; + } Py_INCREF(&RLocktype); - if (PyModule_AddObject(m, "RLock", (PyObject *)&RLocktype) < 0) - return NULL; + if (PyModule_Add(m, "RLock", (PyObject *)&RLocktype) < 0) + goto error; Py_INCREF(&localtype); - if (PyModule_AddObject(m, "_local", (PyObject *)&localtype) < 0) - return NULL; + if (PyModule_Add(m, "_local", (PyObject *)&localtype) < 0) + goto error; Py_INCREF(&ExceptHookArgsType); - if (PyModule_AddObject(m, "_ExceptHookArgs", - (PyObject *)&ExceptHookArgsType) < 0) - return NULL; + if (PyModule_Add(m, "_ExceptHookArgs", + (PyObject *)&ExceptHookArgsType) < 0) + goto error; interp->num_threads = 0; str_dict = PyUnicode_InternFromString("__dict__"); if (str_dict == NULL) - return NULL; + goto error; /* Initialize the C thread library */ PyThread_init_thread(); return m; + +error: + Py_DECREF(m); + return NULL; } diff --git a/Modules/_zoneinfo.c b/Modules/_zoneinfo.c index 7888cf86de0a5c..a9f91e964f2073 100644 --- a/Modules/_zoneinfo.c +++ b/Modules/_zoneinfo.c @@ -2625,7 +2625,9 @@ zoneinfomodule_exec(PyObject *m) } Py_INCREF(&PyZoneInfo_ZoneInfoType); - PyModule_AddObject(m, "ZoneInfo", (PyObject *)&PyZoneInfo_ZoneInfoType); + if (PyModule_Add(m, "ZoneInfo", (PyObject *)&PyZoneInfo_ZoneInfoType) < 0) { + goto error; + } /* Populate imports */ PyObject *_tzpath_module = PyImport_ImportModule("zoneinfo._tzpath"); diff --git a/Modules/cjkcodecs/cjkcodecs.h b/Modules/cjkcodecs/cjkcodecs.h index e41755b197ffca..fe27cd3f626696 100644 --- a/Modules/cjkcodecs/cjkcodecs.h +++ b/Modules/cjkcodecs/cjkcodecs.h @@ -309,12 +309,11 @@ register_maps(PyObject *module) for (h = mapping_list; h->charset[0] != '\0'; h++) { char mhname[256] = "__map_"; - int r; strcpy(mhname + sizeof("__map_") - 1, h->charset); - r = PyModule_AddObject(module, mhname, - PyCapsule_New((void *)h, PyMultibyteCodec_CAPSULE_NAME, NULL)); - if (r == -1) + if (PyModule_Add(module, mhname, + PyCapsule_New((void *)h, PyMultibyteCodec_CAPSULE_NAME, NULL)) < 0) { return -1; + } } return 0; } diff --git a/Modules/cmathmodule.c b/Modules/cmathmodule.c index 0f22049a170848..81a76a7ff4940d 100644 --- a/Modules/cmathmodule.c +++ b/Modules/cmathmodule.c @@ -1257,30 +1257,30 @@ static PyMethodDef cmath_methods[] = { static int cmath_exec(PyObject *mod) { - if (PyModule_AddObject(mod, "pi", PyFloat_FromDouble(Py_MATH_PI)) < 0) { + if (PyModule_Add(mod, "pi", PyFloat_FromDouble(Py_MATH_PI)) < 0) { return -1; } - if (PyModule_AddObject(mod, "e", PyFloat_FromDouble(Py_MATH_E)) < 0) { + if (PyModule_Add(mod, "e", PyFloat_FromDouble(Py_MATH_E)) < 0) { return -1; } // 2pi - if (PyModule_AddObject(mod, "tau", PyFloat_FromDouble(Py_MATH_TAU)) < 0) { + if (PyModule_Add(mod, "tau", PyFloat_FromDouble(Py_MATH_TAU)) < 0) { return -1; } - if (PyModule_AddObject(mod, "inf", PyFloat_FromDouble(m_inf())) < 0) { + if (PyModule_Add(mod, "inf", PyFloat_FromDouble(m_inf())) < 0) { return -1; } - if (PyModule_AddObject(mod, "infj", - PyComplex_FromCComplex(c_infj())) < 0) { + if (PyModule_Add(mod, "infj", + PyComplex_FromCComplex(c_infj())) < 0) { return -1; } #if !defined(PY_NO_SHORT_FLOAT_REPR) || defined(Py_NAN) - if (PyModule_AddObject(mod, "nan", PyFloat_FromDouble(m_nan())) < 0) { + if (PyModule_Add(mod, "nan", PyFloat_FromDouble(m_nan())) < 0) { return -1; } - if (PyModule_AddObject(mod, "nanj", - PyComplex_FromCComplex(c_nanj())) < 0) { + if (PyModule_Add(mod, "nanj", + PyComplex_FromCComplex(c_nanj())) < 0) { return -1; } #endif diff --git a/Modules/mathmodule.c b/Modules/mathmodule.c index 86b64fb4226907..77a8843f5797df 100644 --- a/Modules/mathmodule.c +++ b/Modules/mathmodule.c @@ -3511,21 +3511,21 @@ math_ulp_impl(PyObject *module, double x) static int math_exec(PyObject *module) { - if (PyModule_AddObject(module, "pi", PyFloat_FromDouble(Py_MATH_PI)) < 0) { + if (PyModule_Add(module, "pi", PyFloat_FromDouble(Py_MATH_PI)) < 0) { return -1; } - if (PyModule_AddObject(module, "e", PyFloat_FromDouble(Py_MATH_E)) < 0) { + if (PyModule_Add(module, "e", PyFloat_FromDouble(Py_MATH_E)) < 0) { return -1; } // 2pi - if (PyModule_AddObject(module, "tau", PyFloat_FromDouble(Py_MATH_TAU)) < 0) { + if (PyModule_Add(module, "tau", PyFloat_FromDouble(Py_MATH_TAU)) < 0) { return -1; } - if (PyModule_AddObject(module, "inf", PyFloat_FromDouble(m_inf())) < 0) { + if (PyModule_Add(module, "inf", PyFloat_FromDouble(m_inf())) < 0) { return -1; } #if !defined(PY_NO_SHORT_FLOAT_REPR) || defined(Py_NAN) - if (PyModule_AddObject(module, "nan", PyFloat_FromDouble(m_nan())) < 0) { + if (PyModule_Add(module, "nan", PyFloat_FromDouble(m_nan())) < 0) { return -1; } #endif diff --git a/Modules/ossaudiodev.c b/Modules/ossaudiodev.c index 2a1ac10814a698..e6ab001fc41e18 100644 --- a/Modules/ossaudiodev.c +++ b/Modules/ossaudiodev.c @@ -1043,7 +1043,7 @@ static PyMethodDef ossaudiodev_methods[] = { #define _EXPORT_INT(mod, name) \ - if (PyModule_AddIntConstant(mod, #name, (long) (name)) == -1) return NULL; + if (PyModule_AddIntConstant(mod, #name, (long) (name)) == -1) {Py_DECREF(mod); return NULL;} static char *control_labels[] = SOUND_DEVICE_LABELS; @@ -1122,18 +1122,23 @@ PyInit_ossaudiodev(void) OSSAudioError = PyErr_NewException("ossaudiodev.OSSAudioError", NULL, NULL); - if (OSSAudioError) { - /* Each call to PyModule_AddObject decrefs it; compensate: */ - Py_INCREF(OSSAudioError); - Py_INCREF(OSSAudioError); - PyModule_AddObject(m, "error", OSSAudioError); - PyModule_AddObject(m, "OSSAudioError", OSSAudioError); + Py_XINCREF(OSSAudioError); + if (PyModule_Add(m, "error", OSSAudioError) < 0) { + Py_DECREF(m); + return NULL; + } + Py_XINCREF(OSSAudioError); + if (PyModule_Add(m, "OSSAudioError", OSSAudioError) < 0) { + Py_DECREF(m); + return NULL; } /* Build 'control_labels' and 'control_names' lists and add them to the module. */ - if (build_namelists(m) == -1) /* XXX what to do here? */ + if (build_namelists(m) == -1) { /* XXX what to do here? */ + Py_DECREF(m); return NULL; + } /* Expose the audio format numbers -- essential! */ _EXPORT_INT(m, AFMT_QUERY); diff --git a/Modules/timemodule.c b/Modules/timemodule.c index 4caacc3b64d7c8..4af53798af56a0 100644 --- a/Modules/timemodule.c +++ b/Modules/timemodule.c @@ -1747,11 +1747,9 @@ init_timezone(PyObject *m) return -1; } #endif // MS_WINDOWS - PyObject *tzname_obj = Py_BuildValue("(NN)", otz0, otz1); - if (tzname_obj == NULL) { + if (PyModule_Add(m, "tzname", Py_BuildValue("(NN)", otz0, otz1)) < 0) { return -1; } - PyModule_AddObject(m, "tzname", tzname_obj); #else // !HAVE_DECL_TZNAME static const time_t YEAR = (365 * 24 + 6) * 3600; time_t t; @@ -1794,10 +1792,9 @@ init_timezone(PyObject *m) PyModule_AddIntConstant(m, "daylight", janzone != julyzone); tzname_obj = Py_BuildValue("(zz)", janname, julyname); } - if (tzname_obj == NULL) { + if (PyModule_Add(m, "tzname", tzname_obj) < 0) { return -1; } - PyModule_AddObject(m, "tzname", tzname_obj); #endif // !HAVE_DECL_TZNAME if (PyErr_Occurred()) { diff --git a/Modules/xxlimited.c b/Modules/xxlimited.c index 5b05a9454a05da..dc8a9df49fa0b8 100644 --- a/Modules/xxlimited.c +++ b/Modules/xxlimited.c @@ -252,29 +252,29 @@ xx_modexec(PyObject *m) /* Add some symbolic constants to the module */ if (ErrorObject == NULL) { ErrorObject = PyErr_NewException("xxlimited.error", NULL, NULL); - if (ErrorObject == NULL) - goto fail; } - Py_INCREF(ErrorObject); - PyModule_AddObject(m, "error", ErrorObject); + Py_XINCREF(ErrorObject); + if (PyModule_Add(m, "error", ErrorObject) < 0) { + goto fail; + } /* Add Xxo */ o = PyType_FromSpec(&Xxo_Type_spec); - if (o == NULL) + if (PyModule_Add(m, "Xxo", o) < 0) { goto fail; - PyModule_AddObject(m, "Xxo", o); + } /* Add Str */ o = PyType_FromSpec(&Str_Type_spec); - if (o == NULL) + if (PyModule_Add(m, "Str", o) < 0) { goto fail; - PyModule_AddObject(m, "Str", o); + } /* Add Null */ o = PyType_FromSpec(&Null_Type_spec); - if (o == NULL) + if (PyModule_Add(m, "Null", o) < 0) { goto fail; - PyModule_AddObject(m, "Null", o); + } return 0; fail: Py_XDECREF(m); diff --git a/Modules/xxmodule.c b/Modules/xxmodule.c index 17b049c4b9a375..32b88e4f951889 100644 --- a/Modules/xxmodule.c +++ b/Modules/xxmodule.c @@ -364,21 +364,27 @@ xx_exec(PyObject *m) /* Add some symbolic constants to the module */ if (ErrorObject == NULL) { ErrorObject = PyErr_NewException("xx.error", NULL, NULL); - if (ErrorObject == NULL) - goto fail; } - Py_INCREF(ErrorObject); - PyModule_AddObject(m, "error", ErrorObject); + Py_XINCREF(ErrorObject); + if (PyModule_Add(m, "error", ErrorObject) < 0) { + goto fail; + } /* Add Str */ if (PyType_Ready(&Str_Type) < 0) goto fail; - PyModule_AddObject(m, "Str", (PyObject *)&Str_Type); + Py_INCREF(&Str_Type); + if (PyModule_Add(m, "Str", (PyObject *)&Str_Type) < 0) { + goto fail; + } /* Add Null */ if (PyType_Ready(&Null_Type) < 0) goto fail; - PyModule_AddObject(m, "Null", (PyObject *)&Null_Type); + Py_INCREF(&Null_Type); + if (PyModule_Add(m, "Null", (PyObject *)&Null_Type) < 0) { + goto fail; + } return 0; fail: Py_XDECREF(m); diff --git a/Modules/xxsubtype.c b/Modules/xxsubtype.c index 7200337724e080..6d2ee5d9e144a5 100644 --- a/Modules/xxsubtype.c +++ b/Modules/xxsubtype.c @@ -278,13 +278,11 @@ xxsubtype_exec(PyObject* m) return -1; Py_INCREF(&spamlist_type); - if (PyModule_AddObject(m, "spamlist", - (PyObject *) &spamlist_type) < 0) + if (PyModule_Add(m, "spamlist", (PyObject *) &spamlist_type) < 0) return -1; Py_INCREF(&spamdict_type); - if (PyModule_AddObject(m, "spamdict", - (PyObject *) &spamdict_type) < 0) + if (PyModule_Add(m, "spamdict", (PyObject *) &spamdict_type) < 0) return -1; return 0; } diff --git a/PC/python3dll.c b/PC/python3dll.c index 27cc315de2dd19..47d0df1fb7a1fd 100644 --- a/PC/python3dll.c +++ b/PC/python3dll.c @@ -340,6 +340,7 @@ EXPORT_FUNC(PyMem_Realloc) EXPORT_FUNC(PyMemoryView_FromMemory) EXPORT_FUNC(PyMemoryView_FromObject) EXPORT_FUNC(PyMemoryView_GetContiguous) +EXPORT_FUNC(PyModule_Add) EXPORT_FUNC(PyModule_AddFunctions) EXPORT_FUNC(PyModule_AddIntConstant) EXPORT_FUNC(PyModule_AddObject) diff --git a/Python/modsupport.c b/Python/modsupport.c index 8655daa1fc5e0e..cb0aa152268c85 100644 --- a/Python/modsupport.c +++ b/Python/modsupport.c @@ -633,45 +633,52 @@ va_build_stack(PyObject **small_stack, Py_ssize_t small_stack_len, } -int -PyModule_AddObjectRef(PyObject *mod, const char *name, PyObject *value) +static int +add_object(PyObject *m, const char *name, PyObject *o) { - if (!PyModule_Check(mod)) { + PyObject *dict; + if (!PyModule_Check(m)) { PyErr_SetString(PyExc_TypeError, - "PyModule_AddObjectRef() first argument " - "must be a module"); + "PyModule_Add() needs module as first arg"); return -1; } - if (!value) { - if (!PyErr_Occurred()) { + if (!o) { + if (!PyErr_Occurred()) PyErr_SetString(PyExc_SystemError, - "PyModule_AddObjectRef() must be called " - "with an exception raised if value is NULL"); - } + "PyModule_Add() needs non-NULL value or exception set"); return -1; } - PyObject *dict = PyModule_GetDict(mod); + dict = PyModule_GetDict(m); if (dict == NULL) { /* Internal error -- modules must have a dict! */ PyErr_Format(PyExc_SystemError, "module '%s' has no __dict__", - PyModule_GetName(mod)); + PyModule_GetName(m)); return -1; } + return PyDict_SetItemString(dict, name, o); +} - if (PyDict_SetItemString(dict, name, value)) { - return -1; - } - return 0; +int +PyModule_AddObjectRef(PyObject *mod, const char *name, PyObject *value) +{ + return add_object(mod, name, value); } +int +PyModule_Add(PyObject *m, const char *name, PyObject *o) +{ + int res = add_object(m, name, o); + Py_XDECREF(o); + return res; +} int -PyModule_AddObject(PyObject *mod, const char *name, PyObject *value) +PyModule_AddObject(PyObject *m, const char *name, PyObject *o) { - int res = PyModule_AddObjectRef(mod, name, value); + int res = add_object(m, name, o); if (res == 0) { - Py_DECREF(value); + Py_DECREF(o); } return res; } @@ -679,25 +686,13 @@ PyModule_AddObject(PyObject *mod, const char *name, PyObject *value) int PyModule_AddIntConstant(PyObject *m, const char *name, long value) { - PyObject *obj = PyLong_FromLong(value); - if (!obj) { - return -1; - } - int res = PyModule_AddObjectRef(m, name, obj); - Py_DECREF(obj); - return res; + return PyModule_Add(m, name, PyLong_FromLong(value)); } int PyModule_AddStringConstant(PyObject *m, const char *name, const char *value) { - PyObject *obj = PyUnicode_FromString(value); - if (!obj) { - return -1; - } - int res = PyModule_AddObjectRef(m, name, obj); - Py_DECREF(obj); - return res; + return PyModule_Add(m, name, PyUnicode_FromString(value)); } int @@ -710,5 +705,6 @@ PyModule_AddType(PyObject *module, PyTypeObject *type) const char *name = _PyType_Name(type); assert(name != NULL); - return PyModule_AddObjectRef(module, name, (PyObject *)type); + Py_INCREF(type); + return PyModule_Add(module, name, (PyObject *)type); } From 5adebfef1327b914dbc1aef96cdcbf0b4594c10e Mon Sep 17 00:00:00 2001 From: Serhiy Storchaka Date: Mon, 10 Jul 2023 12:15:44 +0300 Subject: [PATCH 2/9] Raname PyModule_Add to PyModule_AddNew. --- Doc/c-api/module.rst | 8 ++++---- Doc/whatsnew/3.13.rst | 2 +- Include/modsupport.h | 4 ++-- .../C API/2020-11-11-22-36-29.bpo-42327.ODSZBM.rst | 2 +- Modules/_curses_panel.c | 2 +- Modules/_stat.c | 6 +++--- Modules/_testinternalcapi.c | 4 ++-- Modules/_threadmodule.c | 2 +- Modules/cjkcodecs/cjkcodecs.h | 2 +- Modules/cmathmodule.c | 14 +++++++------- Modules/mathmodule.c | 10 +++++----- Modules/timemodule.c | 4 ++-- Modules/xxsubtype.c | 4 ++-- PC/python3dll.c | 2 +- Python/modsupport.c | 12 ++++++------ 15 files changed, 39 insertions(+), 39 deletions(-) diff --git a/Doc/c-api/module.rst b/Doc/c-api/module.rst index e21007e20b830e..3cfdf549ae7b70 100644 --- a/Doc/c-api/module.rst +++ b/Doc/c-api/module.rst @@ -486,7 +486,7 @@ state: .. versionadded:: 3.10 -.. c:function:: int PyModule_Add(PyObject *module, const char *name, PyObject *value) +.. c:function:: int PyModule_AddNew(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. @@ -502,7 +502,7 @@ state: Example usage:: - if (PyModule_Add(module, "spam", PyBytes_FromString(value)) < 0) { + if (PyModule_AddNew(module, "spam", PyBytes_FromString(value)) < 0) { goto error; } @@ -511,10 +511,10 @@ state: .. c:function:: int PyModule_AddObject(PyObject *module, const char *name, PyObject *value) - Similar to :c:func:`PyModule_Add`, but only steals a reference to + Similar to :c:func:`PyModule_AddNew`, but only steals a reference to *value* on success (if it returns ``0``). - The new :c:func:`PyModule_Add` function is recommended, since it is + The new :c:func:`PyModule_AddNew` function is recommended, since it is easy to introduce reference leaks by misusing the :c:func:`PyModule_AddObject` function. diff --git a/Doc/whatsnew/3.13.rst b/Doc/whatsnew/3.13.rst index a8aaffedc18479..9260aef9b0e897 100644 --- a/Doc/whatsnew/3.13.rst +++ b/Doc/whatsnew/3.13.rst @@ -739,7 +739,7 @@ New Features If the assertion fails, make sure that the size is set before. (Contributed by Victor Stinner in :gh:`106168`.) -* Add :c:func:`PyModule_Add` function: similar to +* Add :c:func:`PyModule_AddNew` function: similar to :c:func:`PyModule_AddObject` but always steals a reference to the value. (Contributed by Serhiy Storchaka in :gh:`86493`.) diff --git a/Include/modsupport.h b/Include/modsupport.h index d9a93a94e12363..b8eb1a7224ca2b 100644 --- a/Include/modsupport.h +++ b/Include/modsupport.h @@ -27,7 +27,7 @@ PyAPI_FUNC(PyObject *) Py_VaBuildValue(const char *, va_list); // Steal a reference to 'value'. // On success, return 0. // On error, raise an exception and return -1. -PyAPI_FUNC(int) PyModule_Add(PyObject *mod, const char *name, PyObject *value); +PyAPI_FUNC(int) PyModule_AddNew(PyObject *mod, const char *name, PyObject *value); #endif /* Py_LIMITED_API */ // Add an attribute with name 'name' and value 'obj' to the module 'mod. @@ -35,7 +35,7 @@ PyAPI_FUNC(int) PyModule_Add(PyObject *mod, const char *name, PyObject *value); // On error, raise an exception and return -1. PyAPI_FUNC(int) PyModule_AddObjectRef(PyObject *mod, const char *name, PyObject *value); -// Similar to PyModule_Add() but steal a reference to 'value' only on success. +// Similar to PyModule_AddNew() but steal a reference to 'value' only on success. PyAPI_FUNC(int) PyModule_AddObject(PyObject *mod, const char *, PyObject *value); PyAPI_FUNC(int) PyModule_AddIntConstant(PyObject *, const char *, long); diff --git a/Misc/NEWS.d/next/C API/2020-11-11-22-36-29.bpo-42327.ODSZBM.rst b/Misc/NEWS.d/next/C API/2020-11-11-22-36-29.bpo-42327.ODSZBM.rst index 46ed0065128713..57f21a512132b7 100644 --- a/Misc/NEWS.d/next/C API/2020-11-11-22-36-29.bpo-42327.ODSZBM.rst +++ b/Misc/NEWS.d/next/C API/2020-11-11-22-36-29.bpo-42327.ODSZBM.rst @@ -1 +1 @@ -Added :func:`PyModule_Add`. +Added :func:`PyModule_AddNew`. diff --git a/Modules/_curses_panel.c b/Modules/_curses_panel.c index 3f24adf7d7fbef..2f89f0168d24c8 100644 --- a/Modules/_curses_panel.c +++ b/Modules/_curses_panel.c @@ -662,7 +662,7 @@ _curses_panel_exec(PyObject *mod) state->PyCursesError = PyErr_NewException( "_curses_panel.error", NULL, NULL); - if (PyModule_Add(mod, "error", Py_XNewRef(state->PyCursesError)) < 0) { + if (PyModule_AddNew(mod, "error", Py_XNewRef(state->PyCursesError)) < 0) { return -1; } diff --git a/Modules/_stat.c b/Modules/_stat.c index c3a309af7acd12..3d7c5f4008aa96 100644 --- a/Modules/_stat.c +++ b/Modules/_stat.c @@ -591,15 +591,15 @@ stat_exec(PyObject *module) ADD_INT_MACRO(module, FILE_ATTRIBUTE_TEMPORARY); ADD_INT_MACRO(module, FILE_ATTRIBUTE_VIRTUAL); - if (PyModule_Add(module, "IO_REPARSE_TAG_SYMLINK", + if (PyModule_AddNew(module, "IO_REPARSE_TAG_SYMLINK", PyLong_FromUnsignedLong(IO_REPARSE_TAG_SYMLINK)) < 0) { return -1; } - if (PyModule_Add(module, "IO_REPARSE_TAG_MOUNT_POINT", + if (PyModule_AddNew(module, "IO_REPARSE_TAG_MOUNT_POINT", PyLong_FromUnsignedLong(IO_REPARSE_TAG_MOUNT_POINT)) < 0) { return -1; } - if (PyModule_Add(module, "IO_REPARSE_TAG_APPEXECLINK", + if (PyModule_AddNew(module, "IO_REPARSE_TAG_APPEXECLINK", PyLong_FromUnsignedLong(IO_REPARSE_TAG_APPEXECLINK)) < 0) { return -1; } diff --git a/Modules/_testinternalcapi.c b/Modules/_testinternalcapi.c index 5cf0f65aa6bdab..9ec429425117b0 100644 --- a/Modules/_testinternalcapi.c +++ b/Modules/_testinternalcapi.c @@ -1379,12 +1379,12 @@ static PyMethodDef module_functions[] = { static int module_exec(PyObject *module) { - if (PyModule_Add(module, "SIZEOF_PYGC_HEAD", + if (PyModule_AddNew(module, "SIZEOF_PYGC_HEAD", PyLong_FromSsize_t(sizeof(PyGC_Head))) < 0) { return 1; } - if (PyModule_Add(module, "SIZEOF_TIME_T", + if (PyModule_AddNew(module, "SIZEOF_TIME_T", PyLong_FromSsize_t(sizeof(time_t))) < 0) { return 1; } diff --git a/Modules/_threadmodule.c b/Modules/_threadmodule.c index b24a81453306dc..67705953a1a510 100644 --- a/Modules/_threadmodule.c +++ b/Modules/_threadmodule.c @@ -1671,7 +1671,7 @@ thread_module_exec(PyObject *module) // Round towards minus infinity timeout_max = floor(timeout_max); - if (PyModule_Add(module, "TIMEOUT_MAX", + if (PyModule_AddNew(module, "TIMEOUT_MAX", PyFloat_FromDouble(timeout_max)) < 0) { return -1; } diff --git a/Modules/cjkcodecs/cjkcodecs.h b/Modules/cjkcodecs/cjkcodecs.h index 766f82983025e4..4c389d3cb9bcf1 100644 --- a/Modules/cjkcodecs/cjkcodecs.h +++ b/Modules/cjkcodecs/cjkcodecs.h @@ -398,7 +398,7 @@ register_maps(PyObject *module) strcpy(mhname + sizeof("__map_") - 1, h->charset); PyObject *capsule = PyCapsule_New((void *)h, MAP_CAPSULE, NULL); - if (PyModule_Add(module, mhname, capsule) < 0) { + if (PyModule_AddNew(module, mhname, capsule) < 0) { return -1; } } diff --git a/Modules/cmathmodule.c b/Modules/cmathmodule.c index 57bc55632be485..4769815355410a 100644 --- a/Modules/cmathmodule.c +++ b/Modules/cmathmodule.c @@ -1217,29 +1217,29 @@ static PyMethodDef cmath_methods[] = { static int cmath_exec(PyObject *mod) { - if (PyModule_Add(mod, "pi", PyFloat_FromDouble(Py_MATH_PI)) < 0) { + if (PyModule_AddNew(mod, "pi", PyFloat_FromDouble(Py_MATH_PI)) < 0) { return -1; } - if (PyModule_Add(mod, "e", PyFloat_FromDouble(Py_MATH_E)) < 0) { + if (PyModule_AddNew(mod, "e", PyFloat_FromDouble(Py_MATH_E)) < 0) { return -1; } // 2pi - if (PyModule_Add(mod, "tau", PyFloat_FromDouble(Py_MATH_TAU)) < 0) { + if (PyModule_AddNew(mod, "tau", PyFloat_FromDouble(Py_MATH_TAU)) < 0) { return -1; } - if (PyModule_Add(mod, "inf", PyFloat_FromDouble(Py_INFINITY)) < 0) { + if (PyModule_AddNew(mod, "inf", PyFloat_FromDouble(Py_INFINITY)) < 0) { return -1; } Py_complex infj = {0.0, Py_INFINITY}; - if (PyModule_Add(mod, "infj", PyComplex_FromCComplex(infj)) < 0) { + if (PyModule_AddNew(mod, "infj", PyComplex_FromCComplex(infj)) < 0) { return -1; } - if (PyModule_Add(mod, "nan", PyFloat_FromDouble(fabs(Py_NAN))) < 0) { + if (PyModule_AddNew(mod, "nan", PyFloat_FromDouble(fabs(Py_NAN))) < 0) { return -1; } Py_complex nanj = {0.0, fabs(Py_NAN)}; - if (PyModule_Add(mod, "nanj", PyComplex_FromCComplex(nanj)) < 0) { + if (PyModule_AddNew(mod, "nanj", PyComplex_FromCComplex(nanj)) < 0) { return -1; } diff --git a/Modules/mathmodule.c b/Modules/mathmodule.c index 0d238c086ac78f..48de3b102d397d 100644 --- a/Modules/mathmodule.c +++ b/Modules/mathmodule.c @@ -4037,20 +4037,20 @@ math_exec(PyObject *module) if (state->str___trunc__ == NULL) { return -1; } - if (PyModule_Add(module, "pi", PyFloat_FromDouble(Py_MATH_PI)) < 0) { + if (PyModule_AddNew(module, "pi", PyFloat_FromDouble(Py_MATH_PI)) < 0) { return -1; } - if (PyModule_Add(module, "e", PyFloat_FromDouble(Py_MATH_E)) < 0) { + if (PyModule_AddNew(module, "e", PyFloat_FromDouble(Py_MATH_E)) < 0) { return -1; } // 2pi - if (PyModule_Add(module, "tau", PyFloat_FromDouble(Py_MATH_TAU)) < 0) { + if (PyModule_AddNew(module, "tau", PyFloat_FromDouble(Py_MATH_TAU)) < 0) { return -1; } - if (PyModule_Add(module, "inf", PyFloat_FromDouble(Py_INFINITY)) < 0) { + if (PyModule_AddNew(module, "inf", PyFloat_FromDouble(Py_INFINITY)) < 0) { return -1; } - if (PyModule_Add(module, "nan", PyFloat_FromDouble(fabs(Py_NAN))) < 0) { + if (PyModule_AddNew(module, "nan", PyFloat_FromDouble(fabs(Py_NAN))) < 0) { return -1; } return 0; diff --git a/Modules/timemodule.c b/Modules/timemodule.c index 912710219bd014..186fe232193eb4 100644 --- a/Modules/timemodule.c +++ b/Modules/timemodule.c @@ -1790,7 +1790,7 @@ init_timezone(PyObject *m) return -1; } #endif // MS_WINDOWS - if (PyModule_Add(m, "tzname", Py_BuildValue("(NN)", otz0, otz1)) < 0) { + if (PyModule_AddNew(m, "tzname", Py_BuildValue("(NN)", otz0, otz1)) < 0) { return -1; } #else // !HAVE_DECL_TZNAME @@ -1835,7 +1835,7 @@ init_timezone(PyObject *m) PyModule_AddIntConstant(m, "daylight", janzone != julyzone); tzname_obj = Py_BuildValue("(zz)", janname, julyname); } - if (PyModule_Add(m, "tzname", tzname_obj) < 0) { + if (PyModule_AddNew(m, "tzname", tzname_obj) < 0) { return -1; } #endif // !HAVE_DECL_TZNAME diff --git a/Modules/xxsubtype.c b/Modules/xxsubtype.c index 82209e3a369fe7..174a92dec1fc15 100644 --- a/Modules/xxsubtype.c +++ b/Modules/xxsubtype.c @@ -274,10 +274,10 @@ xxsubtype_exec(PyObject* m) if (PyType_Ready(&spamdict_type) < 0) return -1; - if (PyModule_Add(m, "spamlist", Py_NewRef(&spamlist_type)) < 0) + if (PyModule_AddNew(m, "spamlist", Py_NewRef(&spamlist_type)) < 0) return -1; - if (PyModule_Add(m, "spamdict", Py_NewRef(&spamdict_type)) < 0) + if (PyModule_AddNew(m, "spamdict", Py_NewRef(&spamdict_type)) < 0) return -1; return 0; } diff --git a/PC/python3dll.c b/PC/python3dll.c index ba87da3b6fb51e..dc9a2b5eb128f8 100755 --- a/PC/python3dll.c +++ b/PC/python3dll.c @@ -372,9 +372,9 @@ EXPORT_FUNC(PyMemoryView_FromBuffer) EXPORT_FUNC(PyMemoryView_FromMemory) EXPORT_FUNC(PyMemoryView_FromObject) EXPORT_FUNC(PyMemoryView_GetContiguous) -EXPORT_FUNC(PyModule_Add) EXPORT_FUNC(PyModule_AddFunctions) EXPORT_FUNC(PyModule_AddIntConstant) +EXPORT_FUNC(PyModule_AddNew) EXPORT_FUNC(PyModule_AddObject) EXPORT_FUNC(PyModule_AddObjectRef) EXPORT_FUNC(PyModule_AddStringConstant) diff --git a/Python/modsupport.c b/Python/modsupport.c index c5c3973d0b41e5..3f545f6195991a 100644 --- a/Python/modsupport.c +++ b/Python/modsupport.c @@ -587,13 +587,13 @@ add_object(PyObject *m, const char *name, PyObject *o) PyObject *dict; if (!PyModule_Check(m)) { PyErr_SetString(PyExc_TypeError, - "PyModule_Add() needs module as first arg"); + "PyModule_AddNew() needs module as first arg"); return -1; } if (!o) { if (!PyErr_Occurred()) PyErr_SetString(PyExc_SystemError, - "PyModule_Add() needs non-NULL value or exception set"); + "PyModule_AddNew() needs non-NULL value or exception set"); return -1; } @@ -614,7 +614,7 @@ PyModule_AddObjectRef(PyObject *mod, const char *name, PyObject *value) } int -PyModule_Add(PyObject *m, const char *name, PyObject *o) +PyModule_AddNew(PyObject *m, const char *name, PyObject *o) { int res = add_object(m, name, o); Py_XDECREF(o); @@ -634,13 +634,13 @@ PyModule_AddObject(PyObject *m, const char *name, PyObject *o) int PyModule_AddIntConstant(PyObject *m, const char *name, long value) { - return PyModule_Add(m, name, PyLong_FromLong(value)); + return PyModule_AddNew(m, name, PyLong_FromLong(value)); } int PyModule_AddStringConstant(PyObject *m, const char *name, const char *value) { - return PyModule_Add(m, name, PyUnicode_FromString(value)); + return PyModule_AddNew(m, name, PyUnicode_FromString(value)); } int @@ -654,5 +654,5 @@ PyModule_AddType(PyObject *module, PyTypeObject *type) assert(name != NULL); Py_INCREF(type); - return PyModule_Add(module, name, (PyObject *)type); + return PyModule_AddNew(module, name, (PyObject *)type); } From 37ff11b7dd6cb46c95ea86694dfe5b541fdebf00 Mon Sep 17 00:00:00 2001 From: Serhiy Storchaka Date: Mon, 10 Jul 2023 12:54:40 +0300 Subject: [PATCH 3/9] Cleanup and update docs. --- Doc/c-api/module.rst | 20 +++------ Doc/whatsnew/3.13.rst | 3 +- Include/modsupport.h | 17 ++++--- .../2020-11-11-22-36-29.bpo-42327.ODSZBM.rst | 2 +- Modules/_curses_panel.c | 2 +- Modules/_stat.c | 12 ++--- Modules/_testinternalcapi.c | 4 +- Modules/_threadmodule.c | 2 +- Modules/cjkcodecs/cjkcodecs.h | 6 ++- Modules/xxsubtype.c | 4 +- Python/modsupport.c | 45 +++++++++---------- 11 files changed, 55 insertions(+), 62 deletions(-) diff --git a/Doc/c-api/module.rst b/Doc/c-api/module.rst index 3cfdf549ae7b70..dcb7bb98a01fbb 100644 --- a/Doc/c-api/module.rst +++ b/Doc/c-api/module.rst @@ -488,17 +488,10 @@ state: .. c:function:: int PyModule_AddNew(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 ``-1`` if *value* is ``NULL``. It must be called with an exception - raised in this case. - - This function "steals" a reference to *value*. It can be called with - a result of function that returns a new reference without bothering to - check its result or even saving it to a variable. + Similar to :c:func:`PyModule_AddObjectRef`, but "steals" a reference + to *value*. + It can be called with a result of function that returns a new reference + without bothering to check its result or even saving it to a variable. Example usage:: @@ -511,10 +504,11 @@ state: .. c:function:: int PyModule_AddObject(PyObject *module, const char *name, PyObject *value) - Similar to :c:func:`PyModule_AddNew`, but only steals a reference to + Similar to :c:func:`PyModule_AddObjectRef`, but steals a reference to *value* on success (if it returns ``0``). - The new :c:func:`PyModule_AddNew` function is recommended, since it is + The new :c:func:`PyModule_AddNew` or :c:func:`PyModule_AddObjectRef` + functions are recommended, since it is easy to introduce reference leaks by misusing the :c:func:`PyModule_AddObject` function. diff --git a/Doc/whatsnew/3.13.rst b/Doc/whatsnew/3.13.rst index 9260aef9b0e897..bf97da25ec024e 100644 --- a/Doc/whatsnew/3.13.rst +++ b/Doc/whatsnew/3.13.rst @@ -740,7 +740,8 @@ New Features (Contributed by Victor Stinner in :gh:`106168`.) * Add :c:func:`PyModule_AddNew` function: similar to - :c:func:`PyModule_AddObject` but always steals a reference to the value. + :c:func:`PyModule_AddObjectRef` and :c:func:`PyModule_AddObject` but + always steals a reference to the value. (Contributed by Serhiy Storchaka in :gh:`86493`.) Porting to Python 3.13 diff --git a/Include/modsupport.h b/Include/modsupport.h index b8eb1a7224ca2b..6f983ebbeae486 100644 --- a/Include/modsupport.h +++ b/Include/modsupport.h @@ -22,20 +22,19 @@ PyAPI_FUNC(int) PyArg_UnpackTuple(PyObject *, const char *, Py_ssize_t, Py_ssize PyAPI_FUNC(PyObject *) Py_BuildValue(const char *, ...); PyAPI_FUNC(PyObject *) Py_VaBuildValue(const char *, va_list); -#if !defined(Py_LIMITED_API) || Py_LIMITED_API+0 >= 0x030A0000 -// Add an attribute with name 'name' and value 'value' to the module 'mod'. -// Steal a reference to 'value'. +// Add an attribute with name 'name' and value 'obj' to the module 'mod. // On success, return 0. // On error, raise an exception and return -1. +PyAPI_FUNC(int) PyModule_AddObjectRef(PyObject *mod, const char *name, PyObject *value); + +#if !defined(Py_LIMITED_API) || Py_LIMITED_API+0 >= 0x030d0000 +// Similar to PyModule_AddObjectRef() but steal a reference to 'value'. PyAPI_FUNC(int) PyModule_AddNew(PyObject *mod, const char *name, PyObject *value); #endif /* Py_LIMITED_API */ -// 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_AddNew() but steal a reference to 'value' only on success. +// Similar to PyModule_AddObjectRef() and PyModule_AddNew() but steal +// a reference to 'value' on success and only on success. +// Errorprone. Should not be used in new code. PyAPI_FUNC(int) PyModule_AddObject(PyObject *mod, const char *, PyObject *value); PyAPI_FUNC(int) PyModule_AddIntConstant(PyObject *, const char *, long); diff --git a/Misc/NEWS.d/next/C API/2020-11-11-22-36-29.bpo-42327.ODSZBM.rst b/Misc/NEWS.d/next/C API/2020-11-11-22-36-29.bpo-42327.ODSZBM.rst index 57f21a512132b7..0a4e64292aef9f 100644 --- a/Misc/NEWS.d/next/C API/2020-11-11-22-36-29.bpo-42327.ODSZBM.rst +++ b/Misc/NEWS.d/next/C API/2020-11-11-22-36-29.bpo-42327.ODSZBM.rst @@ -1 +1 @@ -Added :func:`PyModule_AddNew`. +Add :func:`PyModule_AddNew` function. diff --git a/Modules/_curses_panel.c b/Modules/_curses_panel.c index 2f89f0168d24c8..292b57c083d0c8 100644 --- a/Modules/_curses_panel.c +++ b/Modules/_curses_panel.c @@ -662,7 +662,7 @@ _curses_panel_exec(PyObject *mod) state->PyCursesError = PyErr_NewException( "_curses_panel.error", NULL, NULL); - if (PyModule_AddNew(mod, "error", Py_XNewRef(state->PyCursesError)) < 0) { + if (PyModule_AddObjectRef(mod, "error", state->PyCursesError) < 0) { return -1; } diff --git a/Modules/_stat.c b/Modules/_stat.c index 3d7c5f4008aa96..1e69a62d414102 100644 --- a/Modules/_stat.c +++ b/Modules/_stat.c @@ -592,16 +592,16 @@ stat_exec(PyObject *module) ADD_INT_MACRO(module, FILE_ATTRIBUTE_VIRTUAL); if (PyModule_AddNew(module, "IO_REPARSE_TAG_SYMLINK", - PyLong_FromUnsignedLong(IO_REPARSE_TAG_SYMLINK)) < 0) { - return -1; + PyLong_FromUnsignedLong(IO_REPARSE_TAG_SYMLINK)) < 0) { + return -1; } if (PyModule_AddNew(module, "IO_REPARSE_TAG_MOUNT_POINT", - PyLong_FromUnsignedLong(IO_REPARSE_TAG_MOUNT_POINT)) < 0) { - return -1; + PyLong_FromUnsignedLong(IO_REPARSE_TAG_MOUNT_POINT)) < 0) { + return -1; } if (PyModule_AddNew(module, "IO_REPARSE_TAG_APPEXECLINK", - PyLong_FromUnsignedLong(IO_REPARSE_TAG_APPEXECLINK)) < 0) { - return -1; + PyLong_FromUnsignedLong(IO_REPARSE_TAG_APPEXECLINK)) < 0) { + return -1; } #endif diff --git a/Modules/_testinternalcapi.c b/Modules/_testinternalcapi.c index 9ec429425117b0..87e218d9536ea5 100644 --- a/Modules/_testinternalcapi.c +++ b/Modules/_testinternalcapi.c @@ -1380,12 +1380,12 @@ static int module_exec(PyObject *module) { if (PyModule_AddNew(module, "SIZEOF_PYGC_HEAD", - PyLong_FromSsize_t(sizeof(PyGC_Head))) < 0) { + PyLong_FromSsize_t(sizeof(PyGC_Head))) < 0) { return 1; } if (PyModule_AddNew(module, "SIZEOF_TIME_T", - PyLong_FromSsize_t(sizeof(time_t))) < 0) { + PyLong_FromSsize_t(sizeof(time_t))) < 0) { return 1; } diff --git a/Modules/_threadmodule.c b/Modules/_threadmodule.c index 67705953a1a510..2900def8ee0345 100644 --- a/Modules/_threadmodule.c +++ b/Modules/_threadmodule.c @@ -1672,7 +1672,7 @@ thread_module_exec(PyObject *module) timeout_max = floor(timeout_max); if (PyModule_AddNew(module, "TIMEOUT_MAX", - PyFloat_FromDouble(timeout_max)) < 0) { + PyFloat_FromDouble(timeout_max)) < 0) { return -1; } diff --git a/Modules/cjkcodecs/cjkcodecs.h b/Modules/cjkcodecs/cjkcodecs.h index 4c389d3cb9bcf1..ee588785e7403f 100644 --- a/Modules/cjkcodecs/cjkcodecs.h +++ b/Modules/cjkcodecs/cjkcodecs.h @@ -398,7 +398,11 @@ register_maps(PyObject *module) strcpy(mhname + sizeof("__map_") - 1, h->charset); PyObject *capsule = PyCapsule_New((void *)h, MAP_CAPSULE, NULL); - if (PyModule_AddNew(module, mhname, capsule) < 0) { + if (capsule == NULL) { + return -1; + } + if (PyModule_AddObject(module, mhname, capsule) < 0) { + Py_DECREF(capsule); return -1; } } diff --git a/Modules/xxsubtype.c b/Modules/xxsubtype.c index 174a92dec1fc15..9e4a3d66ef41bd 100644 --- a/Modules/xxsubtype.c +++ b/Modules/xxsubtype.c @@ -274,10 +274,10 @@ xxsubtype_exec(PyObject* m) if (PyType_Ready(&spamdict_type) < 0) return -1; - if (PyModule_AddNew(m, "spamlist", Py_NewRef(&spamlist_type)) < 0) + if (PyModule_AddObjectRef(m, "spamlist", (PyObject *)&spamlist_type) < 0) return -1; - if (PyModule_AddNew(m, "spamdict", Py_NewRef(&spamdict_type)) < 0) + if (PyModule_AddObjectRef(m, "spamdict", (PyObject *)&spamdict_type) < 0) return -1; return 0; } diff --git a/Python/modsupport.c b/Python/modsupport.c index 3f545f6195991a..10c427f9d89b63 100644 --- a/Python/modsupport.c +++ b/Python/modsupport.c @@ -581,52 +581,48 @@ _Py_VaBuildStack(PyObject **small_stack, Py_ssize_t small_stack_len, } -static int -add_object(PyObject *m, const char *name, PyObject *o) +int +PyModule_AddObjectRef(PyObject *mod, const char *name, PyObject *value) { - PyObject *dict; - if (!PyModule_Check(m)) { + if (!PyModule_Check(mod)) { PyErr_SetString(PyExc_TypeError, - "PyModule_AddNew() needs module as first arg"); + "PyModule_AddObjectRef() first argument " + "must be a module"); return -1; } - if (!o) { - if (!PyErr_Occurred()) + if (!value) { + if (!PyErr_Occurred()) { PyErr_SetString(PyExc_SystemError, - "PyModule_AddNew() needs non-NULL value or exception set"); + "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; } - return PyDict_SetItemString(dict, name, o); -} - -int -PyModule_AddObjectRef(PyObject *mod, const char *name, PyObject *value) -{ - return add_object(mod, name, value); + return PyDict_SetItemString(dict, name, value); } int -PyModule_AddNew(PyObject *m, const char *name, PyObject *o) +PyModule_AddNew(PyObject *mod, const char *name, PyObject *value) { - int res = add_object(m, name, o); - Py_XDECREF(o); + int res = PyModule_AddObjectRef(mod, name, value); + Py_XDECREF(value); return res; } int -PyModule_AddObject(PyObject *m, const char *name, PyObject *o) +PyModule_AddObject(PyObject *mod, const char *name, PyObject *value) { - int res = add_object(m, name, o); + int res = PyModule_AddObjectRef(mod, name, value); if (res == 0) { - Py_DECREF(o); + Py_DECREF(value); } return res; } @@ -653,6 +649,5 @@ PyModule_AddType(PyObject *module, PyTypeObject *type) const char *name = _PyType_Name(type); assert(name != NULL); - Py_INCREF(type); - return PyModule_AddNew(module, name, (PyObject *)type); + return PyModule_AddObjectRef(module, name, (PyObject *)type); } From 952015c333ba53c605717b12a832d632e2a5adb4 Mon Sep 17 00:00:00 2001 From: Serhiy Storchaka Date: Mon, 10 Jul 2023 13:35:09 +0300 Subject: [PATCH 4/9] Regenerate the limited APthe I --- Doc/data/stable_abi.dat | 1 + Lib/test/test_stable_abi_ctypes.py | 1 + Misc/stable_abi.toml | 2 ++ 3 files changed, 4 insertions(+) diff --git a/Doc/data/stable_abi.dat b/Doc/data/stable_abi.dat index 7fb002cd80369b..9ff0bd96090934 100644 --- a/Doc/data/stable_abi.dat +++ b/Doc/data/stable_abi.dat @@ -399,6 +399,7 @@ function,PyModuleDef_Init,3.5,, var,PyModuleDef_Type,3.5,, function,PyModule_AddFunctions,3.7,, function,PyModule_AddIntConstant,3.2,, +function,PyModule_AddNew,3.13,, function,PyModule_AddObject,3.2,, function,PyModule_AddObjectRef,3.10,, function,PyModule_AddStringConstant,3.2,, diff --git a/Lib/test/test_stable_abi_ctypes.py b/Lib/test/test_stable_abi_ctypes.py index 038c978e7bbd02..9840921f0b0c78 100644 --- a/Lib/test/test_stable_abi_ctypes.py +++ b/Lib/test/test_stable_abi_ctypes.py @@ -425,6 +425,7 @@ def test_windows_feature_macros(self): "PyModuleDef_Type", "PyModule_AddFunctions", "PyModule_AddIntConstant", + "PyModule_AddNew", "PyModule_AddObject", "PyModule_AddObjectRef", "PyModule_AddStringConstant", diff --git a/Misc/stable_abi.toml b/Misc/stable_abi.toml index bc7259f11816f3..c0ec803fee495a 100644 --- a/Misc/stable_abi.toml +++ b/Misc/stable_abi.toml @@ -2432,3 +2432,5 @@ added = '3.13' [function.PyWeakref_GetRef] added = '3.13' +[function.PyModule_AddNew] + added = '3.13' From 2da12b01a915c05df48b1066695b4ff718618a8a Mon Sep 17 00:00:00 2001 From: Serhiy Storchaka Date: Mon, 10 Jul 2023 15:16:46 +0300 Subject: [PATCH 5/9] Fix more PyModule_AddObject usages. --- Modules/_decimal/_decimal.c | 24 ++++++++++----------- Modules/_ssl.c | 8 +++---- Modules/posixmodule.c | 42 ++++++++++++++----------------------- 3 files changed, 32 insertions(+), 42 deletions(-) diff --git a/Modules/_decimal/_decimal.c b/Modules/_decimal/_decimal.c index 7e1809cfb98b29..f3dfdfa056ff8c 100644 --- a/Modules/_decimal/_decimal.c +++ b/Modules/_decimal/_decimal.c @@ -5950,7 +5950,7 @@ PyInit__decimal(void) Py_DECREF(base); /* add to module */ - CHECK_INT(PyModule_AddObject(m, cm->name, Py_NewRef(cm->ex))); + CHECK_INT(PyModule_AddObjectRef(m, cm->name, cm->ex)); /* add to signal tuple */ PyTuple_SET_ITEM(SignalTuple, i, Py_NewRef(cm->ex)); @@ -5979,39 +5979,39 @@ PyInit__decimal(void) ASSIGN_PTR(cm->ex, PyErr_NewException(cm->fqname, base, NULL)); Py_DECREF(base); - CHECK_INT(PyModule_AddObject(m, cm->name, Py_NewRef(cm->ex))); + CHECK_INT(PyModule_AddObjectRef(m, cm->name, cm->ex)); } /* Init default context template first */ ASSIGN_PTR(state->default_context_template, PyObject_CallObject((PyObject *)state->PyDecContext_Type, NULL)); - CHECK_INT(PyModule_AddObject(m, "DefaultContext", - Py_NewRef(state->default_context_template))); + CHECK_INT(PyModule_AddObjectRef(m, "DefaultContext", + state->default_context_template)); #ifndef WITH_DECIMAL_CONTEXTVAR ASSIGN_PTR(state->tls_context_key, PyUnicode_FromString("___DECIMAL_CTX__")); - CHECK_INT(PyModule_AddObject(m, "HAVE_CONTEXTVAR", Py_NewRef(Py_False))); + CHECK_INT(PyModule_AddObjectRef(m, "HAVE_CONTEXTVAR", Py_False)); #else ASSIGN_PTR(state->current_context_var, PyContextVar_New("decimal_context", NULL)); - CHECK_INT(PyModule_AddObject(m, "HAVE_CONTEXTVAR", Py_NewRef(Py_True))); + CHECK_INT(PyModule_AddObjectRef(m, "HAVE_CONTEXTVAR", Py_True)); #endif - CHECK_INT(PyModule_AddObject(m, "HAVE_THREADS", Py_NewRef(Py_True))); + CHECK_INT(PyModule_AddObjectRef(m, "HAVE_THREADS", Py_True)); /* Init basic context template */ ASSIGN_PTR(state->basic_context_template, PyObject_CallObject((PyObject *)state->PyDecContext_Type, NULL)); init_basic_context(state->basic_context_template); - CHECK_INT(PyModule_AddObject(m, "BasicContext", - Py_NewRef(state->basic_context_template))); + CHECK_INT(PyModule_AddObjectRef(m, "BasicContext", + state->basic_context_template)); /* Init extended context template */ ASSIGN_PTR(state->extended_context_template, PyObject_CallObject((PyObject *)state->PyDecContext_Type, NULL)); init_extended_context(state->extended_context_template); - CHECK_INT(PyModule_AddObject(m, "ExtendedContext", - Py_NewRef(state->extended_context_template))); + CHECK_INT(PyModule_AddObjectRef(m, "ExtendedContext", + state->extended_context_template)); /* Init mpd_ssize_t constants */ @@ -6030,7 +6030,7 @@ PyInit__decimal(void) /* Init string constants */ for (i = 0; i < _PY_DEC_ROUND_GUARD; i++) { ASSIGN_PTR(round_map[i], PyUnicode_InternFromString(mpd_round_string[i])); - CHECK_INT(PyModule_AddObject(m, mpd_round_string[i], Py_NewRef(round_map[i]))); + CHECK_INT(PyModule_AddObjectRef(m, mpd_round_string[i], round_map[i])); } /* Add specification version number */ diff --git a/Modules/_ssl.c b/Modules/_ssl.c index df1496925f678e..f4ad1f4f046715 100644 --- a/Modules/_ssl.c +++ b/Modules/_ssl.c @@ -6085,22 +6085,22 @@ sslmodule_init_versioninfo(PyObject *m) */ libver = OpenSSL_version_num(); r = PyLong_FromUnsignedLong(libver); - if (r == NULL || PyModule_AddObject(m, "OPENSSL_VERSION_NUMBER", r)) + if (PyModule_AddNew(m, "OPENSSL_VERSION_NUMBER", r) < 0) return -1; parse_openssl_version(libver, &major, &minor, &fix, &patch, &status); r = Py_BuildValue("IIIII", major, minor, fix, patch, status); - if (r == NULL || PyModule_AddObject(m, "OPENSSL_VERSION_INFO", r)) + if (PyModule_AddNew(m, "OPENSSL_VERSION_INFO", r) < 0) return -1; r = PyUnicode_FromString(OpenSSL_version(OPENSSL_VERSION)); - if (r == NULL || PyModule_AddObject(m, "OPENSSL_VERSION", r)) + if (PyModule_AddNew(m, "OPENSSL_VERSION", r) < 0) return -1; libver = OPENSSL_VERSION_NUMBER; parse_openssl_version(libver, &major, &minor, &fix, &patch, &status); r = Py_BuildValue("IIIII", major, minor, fix, patch, status); - if (r == NULL || PyModule_AddObject(m, "_OPENSSL_API_VERSION", r)) + if (PyModule_AddNew(m, "_OPENSSL_API_VERSION", r) < 0) return -1; return 0; diff --git a/Modules/posixmodule.c b/Modules/posixmodule.c index aef802c232c6ce..473a6e1d605391 100644 --- a/Modules/posixmodule.c +++ b/Modules/posixmodule.c @@ -13466,7 +13466,7 @@ setup_confname_table(struct constdef *table, size_t tablesize, } Py_DECREF(o); } - return PyModule_AddObject(module, tablename, d); + return PyModule_AddNew(module, tablename, d); } /* Return -1 on failure, 0 on success. */ @@ -16781,11 +16781,9 @@ posixmodule_exec(PyObject *m) #endif /* Initialize environ dictionary */ - PyObject *v = convertenviron(); - Py_XINCREF(v); - if (v == NULL || PyModule_AddObject(m, "environ", v) != 0) + if (PyModule_AddNew(m, "environ", convertenviron()) != 0) { return -1; - Py_DECREF(v); + } if (all_ins(m)) return -1; @@ -16793,15 +16791,16 @@ posixmodule_exec(PyObject *m) if (setup_confname_tables(m)) return -1; - PyModule_AddObject(m, "error", Py_NewRef(PyExc_OSError)); + if (PyModule_AddObjectRef(m, "error", PyExc_OSError) < 0) { + return -1; + } #if defined(HAVE_WAITID) && !defined(__APPLE__) waitid_result_desc.name = MODNAME ".waitid_result"; PyObject *WaitidResultType = (PyObject *)PyStructSequence_NewType(&waitid_result_desc); - if (WaitidResultType == NULL) { + if (PyModule_AddObjectRef(m, "waitid_result", WaitidResultType) < 0) { return -1; } - PyModule_AddObject(m, "waitid_result", Py_NewRef(WaitidResultType)); state->WaitidResultType = WaitidResultType; #endif @@ -16810,39 +16809,36 @@ posixmodule_exec(PyObject *m) stat_result_desc.fields[8].name = PyStructSequence_UnnamedField; stat_result_desc.fields[9].name = PyStructSequence_UnnamedField; PyObject *StatResultType = (PyObject *)PyStructSequence_NewType(&stat_result_desc); - if (StatResultType == NULL) { + if (PyModule_AddObjectRef(m, "stat_result", StatResultType) < 0) { return -1; } - PyModule_AddObject(m, "stat_result", Py_NewRef(StatResultType)); state->StatResultType = StatResultType; state->statresult_new_orig = ((PyTypeObject *)StatResultType)->tp_new; ((PyTypeObject *)StatResultType)->tp_new = statresult_new; statvfs_result_desc.name = "os.statvfs_result"; /* see issue #19209 */ PyObject *StatVFSResultType = (PyObject *)PyStructSequence_NewType(&statvfs_result_desc); - if (StatVFSResultType == NULL) { + if (PyModule_AddObjectRef(m, "statvfs_result", StatVFSResultType) < 0) { return -1; } - PyModule_AddObject(m, "statvfs_result", Py_NewRef(StatVFSResultType)); state->StatVFSResultType = StatVFSResultType; #if defined(HAVE_SCHED_SETPARAM) || defined(HAVE_SCHED_SETSCHEDULER) || defined(POSIX_SPAWN_SETSCHEDULER) || defined(POSIX_SPAWN_SETSCHEDPARAM) sched_param_desc.name = MODNAME ".sched_param"; PyObject *SchedParamType = (PyObject *)PyStructSequence_NewType(&sched_param_desc); - if (SchedParamType == NULL) { + if (PyModule_AddObjectRef(m, "sched_param", SchedParamType) < 0) { return -1; } - PyModule_AddObject(m, "sched_param", Py_NewRef(SchedParamType)); + PyModule_AddObjectRef(m, "sched_param", SchedParamType); state->SchedParamType = SchedParamType; ((PyTypeObject *)SchedParamType)->tp_new = os_sched_param; #endif /* initialize TerminalSize_info */ PyObject *TerminalSizeType = (PyObject *)PyStructSequence_NewType(&TerminalSize_desc); - if (TerminalSizeType == NULL) { + if (PyModule_AddObjectRef(m, "terminal_size", TerminalSizeType) < 0) { return -1; } - PyModule_AddObject(m, "terminal_size", Py_NewRef(TerminalSizeType)); state->TerminalSizeType = TerminalSizeType; /* initialize scandir types */ @@ -16853,26 +16849,22 @@ posixmodule_exec(PyObject *m) state->ScandirIteratorType = ScandirIteratorType; PyObject *DirEntryType = PyType_FromModuleAndSpec(m, &DirEntryType_spec, NULL); - if (DirEntryType == NULL) { + if (PyModule_AddObjectRef(m, "DirEntry", DirEntryType) < 0) { return -1; } - PyModule_AddObject(m, "DirEntry", Py_NewRef(DirEntryType)); state->DirEntryType = DirEntryType; times_result_desc.name = MODNAME ".times_result"; PyObject *TimesResultType = (PyObject *)PyStructSequence_NewType(×_result_desc); - if (TimesResultType == NULL) { + if (PyModule_AddObjectRef(m, "times_result", TimesResultType) < 0) { return -1; } - PyModule_AddObject(m, "times_result", Py_NewRef(TimesResultType)); state->TimesResultType = TimesResultType; PyTypeObject *UnameResultType = PyStructSequence_NewType(&uname_result_desc); - if (UnameResultType == NULL) { + if (PyModule_AddObjectRef(m, "uname_result", (PyObject *)UnameResultType) < 0) { return -1; } - ; - PyModule_AddObject(m, "uname_result", Py_NewRef(UnameResultType)); state->UnameResultType = (PyObject *)UnameResultType; if ((state->billion = PyLong_FromLong(1000000000)) == NULL) @@ -16915,9 +16907,7 @@ posixmodule_exec(PyObject *m) Py_DECREF(unicode); } - PyModule_AddObject(m, "_have_functions", list); - - return 0; + return PyModule_AddNew(m, "_have_functions", list); } From 9dc8caccf8365d0b55f5a261534f0c20b72b1352 Mon Sep 17 00:00:00 2001 From: Serhiy Storchaka Date: Thu, 13 Jul 2023 17:26:15 +0300 Subject: [PATCH 6/9] Address the last unresolved review comment. --- Misc/NEWS.d/next/C API/2020-11-11-22-36-29.bpo-42327.ODSZBM.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Misc/NEWS.d/next/C API/2020-11-11-22-36-29.bpo-42327.ODSZBM.rst b/Misc/NEWS.d/next/C API/2020-11-11-22-36-29.bpo-42327.ODSZBM.rst index 0a4e64292aef9f..d1895f1ab6f081 100644 --- a/Misc/NEWS.d/next/C API/2020-11-11-22-36-29.bpo-42327.ODSZBM.rst +++ b/Misc/NEWS.d/next/C API/2020-11-11-22-36-29.bpo-42327.ODSZBM.rst @@ -1 +1 @@ -Add :func:`PyModule_AddNew` function. +Add :func:`PyModule_AddNew` function: similar to :c:func:`PyModule_AddObjectRef` and :c:func:`PyModule_AddObject`, but always steals a reference to the value. From 5be0f6a6767701239034999da1d220add9d194c9 Mon Sep 17 00:00:00 2001 From: Serhiy Storchaka Date: Mon, 17 Jul 2023 20:49:19 +0300 Subject: [PATCH 7/9] Rename PyModule_AddNew() back to PyModule_Add(). --- Doc/c-api/module.rst | 6 +++--- Doc/data/stable_abi.dat | 2 +- Doc/whatsnew/3.13.rst | 2 +- Include/modsupport.h | 4 ++-- Lib/test/test_stable_abi_ctypes.py | 2 +- .../C API/2020-11-11-22-36-29.bpo-42327.ODSZBM.rst | 2 +- Misc/stable_abi.toml | 2 +- Modules/_ssl.c | 8 ++++---- Modules/_stat.c | 6 +++--- Modules/_testinternalcapi.c | 4 ++-- Modules/_threadmodule.c | 2 +- Modules/cmathmodule.c | 14 +++++++------- Modules/mathmodule.c | 10 +++++----- Modules/posixmodule.c | 6 +++--- Modules/timemodule.c | 4 ++-- PC/python3dll.c | 2 +- Python/modsupport.c | 6 +++--- 17 files changed, 41 insertions(+), 41 deletions(-) diff --git a/Doc/c-api/module.rst b/Doc/c-api/module.rst index dcb7bb98a01fbb..90303411390778 100644 --- a/Doc/c-api/module.rst +++ b/Doc/c-api/module.rst @@ -486,7 +486,7 @@ state: .. versionadded:: 3.10 -.. c:function:: int PyModule_AddNew(PyObject *module, const char *name, PyObject *value) +.. c:function:: int PyModule_Add(PyObject *module, const char *name, PyObject *value) Similar to :c:func:`PyModule_AddObjectRef`, but "steals" a reference to *value*. @@ -495,7 +495,7 @@ state: Example usage:: - if (PyModule_AddNew(module, "spam", PyBytes_FromString(value)) < 0) { + if (PyModule_Add(module, "spam", PyBytes_FromString(value)) < 0) { goto error; } @@ -507,7 +507,7 @@ state: Similar to :c:func:`PyModule_AddObjectRef`, but steals a reference to *value* on success (if it returns ``0``). - The new :c:func:`PyModule_AddNew` or :c:func:`PyModule_AddObjectRef` + The new :c:func:`PyModule_Add` or :c:func:`PyModule_AddObjectRef` functions are recommended, since it is easy to introduce reference leaks by misusing the :c:func:`PyModule_AddObject` function. diff --git a/Doc/data/stable_abi.dat b/Doc/data/stable_abi.dat index 8e81a56cd07242..aa1edf54637058 100644 --- a/Doc/data/stable_abi.dat +++ b/Doc/data/stable_abi.dat @@ -399,9 +399,9 @@ type,PyModuleDef,3.2,,full-abi type,PyModuleDef_Base,3.2,,full-abi function,PyModuleDef_Init,3.5,, var,PyModuleDef_Type,3.5,, +function,PyModule_Add,3.13,, function,PyModule_AddFunctions,3.7,, function,PyModule_AddIntConstant,3.2,, -function,PyModule_AddNew,3.13,, function,PyModule_AddObject,3.2,, function,PyModule_AddObjectRef,3.10,, function,PyModule_AddStringConstant,3.2,, diff --git a/Doc/whatsnew/3.13.rst b/Doc/whatsnew/3.13.rst index 94185e16eff449..04dd16ad7e148b 100644 --- a/Doc/whatsnew/3.13.rst +++ b/Doc/whatsnew/3.13.rst @@ -767,7 +767,7 @@ New Features If the assertion fails, make sure that the size is set before. (Contributed by Victor Stinner in :gh:`106168`.) -* Add :c:func:`PyModule_AddNew` function: similar to +* Add :c:func:`PyModule_Add` function: similar to :c:func:`PyModule_AddObjectRef` and :c:func:`PyModule_AddObject` but always steals a reference to the value. (Contributed by Serhiy Storchaka in :gh:`86493`.) diff --git a/Include/modsupport.h b/Include/modsupport.h index 6f983ebbeae486..51061c5bc8090a 100644 --- a/Include/modsupport.h +++ b/Include/modsupport.h @@ -29,10 +29,10 @@ PyAPI_FUNC(int) PyModule_AddObjectRef(PyObject *mod, const char *name, PyObject #if !defined(Py_LIMITED_API) || Py_LIMITED_API+0 >= 0x030d0000 // Similar to PyModule_AddObjectRef() but steal a reference to 'value'. -PyAPI_FUNC(int) PyModule_AddNew(PyObject *mod, const char *name, PyObject *value); +PyAPI_FUNC(int) PyModule_Add(PyObject *mod, const char *name, PyObject *value); #endif /* Py_LIMITED_API */ -// Similar to PyModule_AddObjectRef() and PyModule_AddNew() but steal +// Similar to PyModule_AddObjectRef() and PyModule_Add() but steal // a reference to 'value' on success and only on success. // Errorprone. Should not be used in new code. PyAPI_FUNC(int) PyModule_AddObject(PyObject *mod, const char *, PyObject *value); diff --git a/Lib/test/test_stable_abi_ctypes.py b/Lib/test/test_stable_abi_ctypes.py index f3db0069a979f6..4e74bb374c93bf 100644 --- a/Lib/test/test_stable_abi_ctypes.py +++ b/Lib/test/test_stable_abi_ctypes.py @@ -425,9 +425,9 @@ def test_windows_feature_macros(self): "PyMethodDescr_Type", "PyModuleDef_Init", "PyModuleDef_Type", + "PyModule_Add", "PyModule_AddFunctions", "PyModule_AddIntConstant", - "PyModule_AddNew", "PyModule_AddObject", "PyModule_AddObjectRef", "PyModule_AddStringConstant", diff --git a/Misc/NEWS.d/next/C API/2020-11-11-22-36-29.bpo-42327.ODSZBM.rst b/Misc/NEWS.d/next/C API/2020-11-11-22-36-29.bpo-42327.ODSZBM.rst index d1895f1ab6f081..3d935aceb57a79 100644 --- a/Misc/NEWS.d/next/C API/2020-11-11-22-36-29.bpo-42327.ODSZBM.rst +++ b/Misc/NEWS.d/next/C API/2020-11-11-22-36-29.bpo-42327.ODSZBM.rst @@ -1 +1 @@ -Add :func:`PyModule_AddNew` function: similar to :c:func:`PyModule_AddObjectRef` and :c:func:`PyModule_AddObject`, but always steals a reference to the value. +Add :func:`PyModule_Add` function: similar to :c:func:`PyModule_AddObjectRef` and :c:func:`PyModule_AddObject`, but always steals a reference to the value. diff --git a/Misc/stable_abi.toml b/Misc/stable_abi.toml index 669574ec53699a..dd2c9910b83ccb 100644 --- a/Misc/stable_abi.toml +++ b/Misc/stable_abi.toml @@ -2444,5 +2444,5 @@ added = '3.13' [function.PyMapping_GetOptionalItemString] added = '3.13' -[function.PyModule_AddNew] +[function.PyModule_Add] added = '3.13' diff --git a/Modules/_ssl.c b/Modules/_ssl.c index f4ad1f4f046715..b6f6a0a12bb2db 100644 --- a/Modules/_ssl.c +++ b/Modules/_ssl.c @@ -6085,22 +6085,22 @@ sslmodule_init_versioninfo(PyObject *m) */ libver = OpenSSL_version_num(); r = PyLong_FromUnsignedLong(libver); - if (PyModule_AddNew(m, "OPENSSL_VERSION_NUMBER", r) < 0) + if (PyModule_Add(m, "OPENSSL_VERSION_NUMBER", r) < 0) return -1; parse_openssl_version(libver, &major, &minor, &fix, &patch, &status); r = Py_BuildValue("IIIII", major, minor, fix, patch, status); - if (PyModule_AddNew(m, "OPENSSL_VERSION_INFO", r) < 0) + if (PyModule_Add(m, "OPENSSL_VERSION_INFO", r) < 0) return -1; r = PyUnicode_FromString(OpenSSL_version(OPENSSL_VERSION)); - if (PyModule_AddNew(m, "OPENSSL_VERSION", r) < 0) + if (PyModule_Add(m, "OPENSSL_VERSION", r) < 0) return -1; libver = OPENSSL_VERSION_NUMBER; parse_openssl_version(libver, &major, &minor, &fix, &patch, &status); r = Py_BuildValue("IIIII", major, minor, fix, patch, status); - if (PyModule_AddNew(m, "_OPENSSL_API_VERSION", r) < 0) + if (PyModule_Add(m, "_OPENSSL_API_VERSION", r) < 0) return -1; return 0; diff --git a/Modules/_stat.c b/Modules/_stat.c index 1e69a62d414102..6cea26175dee5e 100644 --- a/Modules/_stat.c +++ b/Modules/_stat.c @@ -591,15 +591,15 @@ stat_exec(PyObject *module) ADD_INT_MACRO(module, FILE_ATTRIBUTE_TEMPORARY); ADD_INT_MACRO(module, FILE_ATTRIBUTE_VIRTUAL); - if (PyModule_AddNew(module, "IO_REPARSE_TAG_SYMLINK", + if (PyModule_Add(module, "IO_REPARSE_TAG_SYMLINK", PyLong_FromUnsignedLong(IO_REPARSE_TAG_SYMLINK)) < 0) { return -1; } - if (PyModule_AddNew(module, "IO_REPARSE_TAG_MOUNT_POINT", + if (PyModule_Add(module, "IO_REPARSE_TAG_MOUNT_POINT", PyLong_FromUnsignedLong(IO_REPARSE_TAG_MOUNT_POINT)) < 0) { return -1; } - if (PyModule_AddNew(module, "IO_REPARSE_TAG_APPEXECLINK", + if (PyModule_Add(module, "IO_REPARSE_TAG_APPEXECLINK", PyLong_FromUnsignedLong(IO_REPARSE_TAG_APPEXECLINK)) < 0) { return -1; } diff --git a/Modules/_testinternalcapi.c b/Modules/_testinternalcapi.c index 215f4fb2b1a973..a1b5f31fd0bc88 100644 --- a/Modules/_testinternalcapi.c +++ b/Modules/_testinternalcapi.c @@ -1493,12 +1493,12 @@ static PyMethodDef module_functions[] = { static int module_exec(PyObject *module) { - if (PyModule_AddNew(module, "SIZEOF_PYGC_HEAD", + if (PyModule_Add(module, "SIZEOF_PYGC_HEAD", PyLong_FromSsize_t(sizeof(PyGC_Head))) < 0) { return 1; } - if (PyModule_AddNew(module, "SIZEOF_TIME_T", + if (PyModule_Add(module, "SIZEOF_TIME_T", PyLong_FromSsize_t(sizeof(time_t))) < 0) { return 1; } diff --git a/Modules/_threadmodule.c b/Modules/_threadmodule.c index e30bd15d6ce845..d8a797f34dbc4b 100644 --- a/Modules/_threadmodule.c +++ b/Modules/_threadmodule.c @@ -1671,7 +1671,7 @@ thread_module_exec(PyObject *module) // Round towards minus infinity timeout_max = floor(timeout_max); - if (PyModule_AddNew(module, "TIMEOUT_MAX", + if (PyModule_Add(module, "TIMEOUT_MAX", PyFloat_FromDouble(timeout_max)) < 0) { return -1; } diff --git a/Modules/cmathmodule.c b/Modules/cmathmodule.c index 4769815355410a..57bc55632be485 100644 --- a/Modules/cmathmodule.c +++ b/Modules/cmathmodule.c @@ -1217,29 +1217,29 @@ static PyMethodDef cmath_methods[] = { static int cmath_exec(PyObject *mod) { - if (PyModule_AddNew(mod, "pi", PyFloat_FromDouble(Py_MATH_PI)) < 0) { + if (PyModule_Add(mod, "pi", PyFloat_FromDouble(Py_MATH_PI)) < 0) { return -1; } - if (PyModule_AddNew(mod, "e", PyFloat_FromDouble(Py_MATH_E)) < 0) { + if (PyModule_Add(mod, "e", PyFloat_FromDouble(Py_MATH_E)) < 0) { return -1; } // 2pi - if (PyModule_AddNew(mod, "tau", PyFloat_FromDouble(Py_MATH_TAU)) < 0) { + if (PyModule_Add(mod, "tau", PyFloat_FromDouble(Py_MATH_TAU)) < 0) { return -1; } - if (PyModule_AddNew(mod, "inf", PyFloat_FromDouble(Py_INFINITY)) < 0) { + if (PyModule_Add(mod, "inf", PyFloat_FromDouble(Py_INFINITY)) < 0) { return -1; } Py_complex infj = {0.0, Py_INFINITY}; - if (PyModule_AddNew(mod, "infj", PyComplex_FromCComplex(infj)) < 0) { + if (PyModule_Add(mod, "infj", PyComplex_FromCComplex(infj)) < 0) { return -1; } - if (PyModule_AddNew(mod, "nan", PyFloat_FromDouble(fabs(Py_NAN))) < 0) { + if (PyModule_Add(mod, "nan", PyFloat_FromDouble(fabs(Py_NAN))) < 0) { return -1; } Py_complex nanj = {0.0, fabs(Py_NAN)}; - if (PyModule_AddNew(mod, "nanj", PyComplex_FromCComplex(nanj)) < 0) { + if (PyModule_Add(mod, "nanj", PyComplex_FromCComplex(nanj)) < 0) { return -1; } diff --git a/Modules/mathmodule.c b/Modules/mathmodule.c index 48de3b102d397d..0d238c086ac78f 100644 --- a/Modules/mathmodule.c +++ b/Modules/mathmodule.c @@ -4037,20 +4037,20 @@ math_exec(PyObject *module) if (state->str___trunc__ == NULL) { return -1; } - if (PyModule_AddNew(module, "pi", PyFloat_FromDouble(Py_MATH_PI)) < 0) { + if (PyModule_Add(module, "pi", PyFloat_FromDouble(Py_MATH_PI)) < 0) { return -1; } - if (PyModule_AddNew(module, "e", PyFloat_FromDouble(Py_MATH_E)) < 0) { + if (PyModule_Add(module, "e", PyFloat_FromDouble(Py_MATH_E)) < 0) { return -1; } // 2pi - if (PyModule_AddNew(module, "tau", PyFloat_FromDouble(Py_MATH_TAU)) < 0) { + if (PyModule_Add(module, "tau", PyFloat_FromDouble(Py_MATH_TAU)) < 0) { return -1; } - if (PyModule_AddNew(module, "inf", PyFloat_FromDouble(Py_INFINITY)) < 0) { + if (PyModule_Add(module, "inf", PyFloat_FromDouble(Py_INFINITY)) < 0) { return -1; } - if (PyModule_AddNew(module, "nan", PyFloat_FromDouble(fabs(Py_NAN))) < 0) { + if (PyModule_Add(module, "nan", PyFloat_FromDouble(fabs(Py_NAN))) < 0) { return -1; } return 0; diff --git a/Modules/posixmodule.c b/Modules/posixmodule.c index 473a6e1d605391..71d41fe632b42d 100644 --- a/Modules/posixmodule.c +++ b/Modules/posixmodule.c @@ -13466,7 +13466,7 @@ setup_confname_table(struct constdef *table, size_t tablesize, } Py_DECREF(o); } - return PyModule_AddNew(module, tablename, d); + return PyModule_Add(module, tablename, d); } /* Return -1 on failure, 0 on success. */ @@ -16781,7 +16781,7 @@ posixmodule_exec(PyObject *m) #endif /* Initialize environ dictionary */ - if (PyModule_AddNew(m, "environ", convertenviron()) != 0) { + if (PyModule_Add(m, "environ", convertenviron()) != 0) { return -1; } @@ -16907,7 +16907,7 @@ posixmodule_exec(PyObject *m) Py_DECREF(unicode); } - return PyModule_AddNew(m, "_have_functions", list); + return PyModule_Add(m, "_have_functions", list); } diff --git a/Modules/timemodule.c b/Modules/timemodule.c index 186fe232193eb4..912710219bd014 100644 --- a/Modules/timemodule.c +++ b/Modules/timemodule.c @@ -1790,7 +1790,7 @@ init_timezone(PyObject *m) return -1; } #endif // MS_WINDOWS - if (PyModule_AddNew(m, "tzname", Py_BuildValue("(NN)", otz0, otz1)) < 0) { + if (PyModule_Add(m, "tzname", Py_BuildValue("(NN)", otz0, otz1)) < 0) { return -1; } #else // !HAVE_DECL_TZNAME @@ -1835,7 +1835,7 @@ init_timezone(PyObject *m) PyModule_AddIntConstant(m, "daylight", janzone != julyzone); tzname_obj = Py_BuildValue("(zz)", janname, julyname); } - if (PyModule_AddNew(m, "tzname", tzname_obj) < 0) { + if (PyModule_Add(m, "tzname", tzname_obj) < 0) { return -1; } #endif // !HAVE_DECL_TZNAME diff --git a/PC/python3dll.c b/PC/python3dll.c index a31a4694d1d3cf..0b54c5a707231c 100755 --- a/PC/python3dll.c +++ b/PC/python3dll.c @@ -374,9 +374,9 @@ EXPORT_FUNC(PyMemoryView_FromBuffer) EXPORT_FUNC(PyMemoryView_FromMemory) EXPORT_FUNC(PyMemoryView_FromObject) EXPORT_FUNC(PyMemoryView_GetContiguous) +EXPORT_FUNC(PyModule_Add) EXPORT_FUNC(PyModule_AddFunctions) EXPORT_FUNC(PyModule_AddIntConstant) -EXPORT_FUNC(PyModule_AddNew) EXPORT_FUNC(PyModule_AddObject) EXPORT_FUNC(PyModule_AddObjectRef) EXPORT_FUNC(PyModule_AddStringConstant) diff --git a/Python/modsupport.c b/Python/modsupport.c index 10c427f9d89b63..18b3322ae81d11 100644 --- a/Python/modsupport.c +++ b/Python/modsupport.c @@ -610,7 +610,7 @@ PyModule_AddObjectRef(PyObject *mod, const char *name, PyObject *value) } int -PyModule_AddNew(PyObject *mod, const char *name, PyObject *value) +PyModule_Add(PyObject *mod, const char *name, PyObject *value) { int res = PyModule_AddObjectRef(mod, name, value); Py_XDECREF(value); @@ -630,13 +630,13 @@ PyModule_AddObject(PyObject *mod, const char *name, PyObject *value) int PyModule_AddIntConstant(PyObject *m, const char *name, long value) { - return PyModule_AddNew(m, name, PyLong_FromLong(value)); + return PyModule_Add(m, name, PyLong_FromLong(value)); } int PyModule_AddStringConstant(PyObject *m, const char *name, const char *value) { - return PyModule_AddNew(m, name, PyUnicode_FromString(value)); + return PyModule_Add(m, name, PyUnicode_FromString(value)); } int From f33c54a342bf30179668181078abffc6087e18ef Mon Sep 17 00:00:00 2001 From: Serhiy Storchaka Date: Mon, 17 Jul 2023 22:32:30 +0300 Subject: [PATCH 8/9] Remove fixes of module initializations. --- Modules/_curses_panel.c | 3 ++- Modules/_decimal/_decimal.c | 24 ++++++++++----------- Modules/_ssl.c | 8 +++---- Modules/_stat.c | 18 ++++++++-------- Modules/_testinternalcapi.c | 8 +++---- Modules/_threadmodule.c | 4 ++-- Modules/cmathmodule.c | 15 ++++++------- Modules/mathmodule.c | 10 ++++----- Modules/posixmodule.c | 42 +++++++++++++++++++++++-------------- Modules/timemodule.c | 7 +++++-- Modules/xxsubtype.c | 6 ++++-- 11 files changed, 81 insertions(+), 64 deletions(-) diff --git a/Modules/_curses_panel.c b/Modules/_curses_panel.c index 292b57c083d0c8..a3124ff80551e0 100644 --- a/Modules/_curses_panel.c +++ b/Modules/_curses_panel.c @@ -662,7 +662,8 @@ _curses_panel_exec(PyObject *mod) state->PyCursesError = PyErr_NewException( "_curses_panel.error", NULL, NULL); - if (PyModule_AddObjectRef(mod, "error", state->PyCursesError) < 0) { + if (PyModule_AddObject(mod, "error", Py_NewRef(state->PyCursesError)) < 0) { + Py_DECREF(state->PyCursesError); return -1; } diff --git a/Modules/_decimal/_decimal.c b/Modules/_decimal/_decimal.c index f9dc6e875fa5fc..e3dc304066b45b 100644 --- a/Modules/_decimal/_decimal.c +++ b/Modules/_decimal/_decimal.c @@ -5957,7 +5957,7 @@ PyInit__decimal(void) Py_DECREF(base); /* add to module */ - CHECK_INT(PyModule_AddObjectRef(m, cm->name, cm->ex)); + CHECK_INT(PyModule_AddObject(m, cm->name, Py_NewRef(cm->ex))); /* add to signal tuple */ PyTuple_SET_ITEM(state->SignalTuple, i, Py_NewRef(cm->ex)); @@ -5986,39 +5986,39 @@ PyInit__decimal(void) ASSIGN_PTR(cm->ex, PyErr_NewException(cm->fqname, base, NULL)); Py_DECREF(base); - CHECK_INT(PyModule_AddObjectRef(m, cm->name, cm->ex)); + CHECK_INT(PyModule_AddObject(m, cm->name, Py_NewRef(cm->ex))); } /* Init default context template first */ ASSIGN_PTR(state->default_context_template, PyObject_CallObject((PyObject *)state->PyDecContext_Type, NULL)); - CHECK_INT(PyModule_AddObjectRef(m, "DefaultContext", - state->default_context_template)); + CHECK_INT(PyModule_AddObject(m, "DefaultContext", + Py_NewRef(state->default_context_template))); #ifndef WITH_DECIMAL_CONTEXTVAR ASSIGN_PTR(state->tls_context_key, PyUnicode_FromString("___DECIMAL_CTX__")); - CHECK_INT(PyModule_AddObjectRef(m, "HAVE_CONTEXTVAR", Py_False)); + CHECK_INT(PyModule_AddObject(m, "HAVE_CONTEXTVAR", Py_NewRef(Py_False))); #else ASSIGN_PTR(state->current_context_var, PyContextVar_New("decimal_context", NULL)); - CHECK_INT(PyModule_AddObjectRef(m, "HAVE_CONTEXTVAR", Py_True)); + CHECK_INT(PyModule_AddObject(m, "HAVE_CONTEXTVAR", Py_NewRef(Py_True))); #endif - CHECK_INT(PyModule_AddObjectRef(m, "HAVE_THREADS", Py_True)); + CHECK_INT(PyModule_AddObject(m, "HAVE_THREADS", Py_NewRef(Py_True))); /* Init basic context template */ ASSIGN_PTR(state->basic_context_template, PyObject_CallObject((PyObject *)state->PyDecContext_Type, NULL)); init_basic_context(state->basic_context_template); - CHECK_INT(PyModule_AddObjectRef(m, "BasicContext", - state->basic_context_template)); + CHECK_INT(PyModule_AddObject(m, "BasicContext", + Py_NewRef(state->basic_context_template))); /* Init extended context template */ ASSIGN_PTR(state->extended_context_template, PyObject_CallObject((PyObject *)state->PyDecContext_Type, NULL)); init_extended_context(state->extended_context_template); - CHECK_INT(PyModule_AddObjectRef(m, "ExtendedContext", - state->extended_context_template)); + CHECK_INT(PyModule_AddObject(m, "ExtendedContext", + Py_NewRef(state->extended_context_template))); /* Init mpd_ssize_t constants */ @@ -6037,7 +6037,7 @@ PyInit__decimal(void) /* Init string constants */ for (i = 0; i < _PY_DEC_ROUND_GUARD; i++) { ASSIGN_PTR(state->round_map[i], PyUnicode_InternFromString(mpd_round_string[i])); - CHECK_INT(PyModule_AddObjectRef(m, mpd_round_string[i], state->round_map[i])); + CHECK_INT(PyModule_AddObject(m, mpd_round_string[i], Py_NewRef(state->round_map[i]))); } /* Add specification version number */ diff --git a/Modules/_ssl.c b/Modules/_ssl.c index b6f6a0a12bb2db..df1496925f678e 100644 --- a/Modules/_ssl.c +++ b/Modules/_ssl.c @@ -6085,22 +6085,22 @@ sslmodule_init_versioninfo(PyObject *m) */ libver = OpenSSL_version_num(); r = PyLong_FromUnsignedLong(libver); - if (PyModule_Add(m, "OPENSSL_VERSION_NUMBER", r) < 0) + if (r == NULL || PyModule_AddObject(m, "OPENSSL_VERSION_NUMBER", r)) return -1; parse_openssl_version(libver, &major, &minor, &fix, &patch, &status); r = Py_BuildValue("IIIII", major, minor, fix, patch, status); - if (PyModule_Add(m, "OPENSSL_VERSION_INFO", r) < 0) + if (r == NULL || PyModule_AddObject(m, "OPENSSL_VERSION_INFO", r)) return -1; r = PyUnicode_FromString(OpenSSL_version(OPENSSL_VERSION)); - if (PyModule_Add(m, "OPENSSL_VERSION", r) < 0) + if (r == NULL || PyModule_AddObject(m, "OPENSSL_VERSION", r)) return -1; libver = OPENSSL_VERSION_NUMBER; parse_openssl_version(libver, &major, &minor, &fix, &patch, &status); r = Py_BuildValue("IIIII", major, minor, fix, patch, status); - if (PyModule_Add(m, "_OPENSSL_API_VERSION", r) < 0) + if (r == NULL || PyModule_AddObject(m, "_OPENSSL_API_VERSION", r)) return -1; return 0; diff --git a/Modules/_stat.c b/Modules/_stat.c index 6cea26175dee5e..9747d848cbacb8 100644 --- a/Modules/_stat.c +++ b/Modules/_stat.c @@ -591,17 +591,17 @@ stat_exec(PyObject *module) ADD_INT_MACRO(module, FILE_ATTRIBUTE_TEMPORARY); ADD_INT_MACRO(module, FILE_ATTRIBUTE_VIRTUAL); - if (PyModule_Add(module, "IO_REPARSE_TAG_SYMLINK", - PyLong_FromUnsignedLong(IO_REPARSE_TAG_SYMLINK)) < 0) { - return -1; + if (PyModule_AddObject(module, "IO_REPARSE_TAG_SYMLINK", + PyLong_FromUnsignedLong(IO_REPARSE_TAG_SYMLINK)) < 0) { + return -1; } - if (PyModule_Add(module, "IO_REPARSE_TAG_MOUNT_POINT", - PyLong_FromUnsignedLong(IO_REPARSE_TAG_MOUNT_POINT)) < 0) { - return -1; + if (PyModule_AddObject(module, "IO_REPARSE_TAG_MOUNT_POINT", + PyLong_FromUnsignedLong(IO_REPARSE_TAG_MOUNT_POINT)) < 0) { + return -1; } - if (PyModule_Add(module, "IO_REPARSE_TAG_APPEXECLINK", - PyLong_FromUnsignedLong(IO_REPARSE_TAG_APPEXECLINK)) < 0) { - return -1; + if (PyModule_AddObject(module, "IO_REPARSE_TAG_APPEXECLINK", + PyLong_FromUnsignedLong(IO_REPARSE_TAG_APPEXECLINK)) < 0) { + return -1; } #endif diff --git a/Modules/_testinternalcapi.c b/Modules/_testinternalcapi.c index a1b5f31fd0bc88..7745dd5abc22f0 100644 --- a/Modules/_testinternalcapi.c +++ b/Modules/_testinternalcapi.c @@ -1493,13 +1493,13 @@ static PyMethodDef module_functions[] = { static int module_exec(PyObject *module) { - if (PyModule_Add(module, "SIZEOF_PYGC_HEAD", - PyLong_FromSsize_t(sizeof(PyGC_Head))) < 0) { + if (PyModule_AddObject(module, "SIZEOF_PYGC_HEAD", + PyLong_FromSsize_t(sizeof(PyGC_Head))) < 0) { return 1; } - if (PyModule_Add(module, "SIZEOF_TIME_T", - PyLong_FromSsize_t(sizeof(time_t))) < 0) { + if (PyModule_AddObject(module, "SIZEOF_TIME_T", + PyLong_FromSsize_t(sizeof(time_t))) < 0) { return 1; } diff --git a/Modules/_threadmodule.c b/Modules/_threadmodule.c index d8a797f34dbc4b..82393309b67039 100644 --- a/Modules/_threadmodule.c +++ b/Modules/_threadmodule.c @@ -1671,8 +1671,8 @@ thread_module_exec(PyObject *module) // Round towards minus infinity timeout_max = floor(timeout_max); - if (PyModule_Add(module, "TIMEOUT_MAX", - PyFloat_FromDouble(timeout_max)) < 0) { + if (PyModule_AddObject(module, "TIMEOUT_MAX", + PyFloat_FromDouble(timeout_max)) < 0) { return -1; } diff --git a/Modules/cmathmodule.c b/Modules/cmathmodule.c index 57bc55632be485..db8b321e72e8ce 100644 --- a/Modules/cmathmodule.c +++ b/Modules/cmathmodule.c @@ -1217,29 +1217,30 @@ static PyMethodDef cmath_methods[] = { static int cmath_exec(PyObject *mod) { - if (PyModule_Add(mod, "pi", PyFloat_FromDouble(Py_MATH_PI)) < 0) { + if (PyModule_AddObject(mod, "pi", PyFloat_FromDouble(Py_MATH_PI)) < 0) { return -1; } - if (PyModule_Add(mod, "e", PyFloat_FromDouble(Py_MATH_E)) < 0) { + if (PyModule_AddObject(mod, "e", PyFloat_FromDouble(Py_MATH_E)) < 0) { return -1; } // 2pi - if (PyModule_Add(mod, "tau", PyFloat_FromDouble(Py_MATH_TAU)) < 0) { + if (PyModule_AddObject(mod, "tau", PyFloat_FromDouble(Py_MATH_TAU)) < 0) { return -1; } - if (PyModule_Add(mod, "inf", PyFloat_FromDouble(Py_INFINITY)) < 0) { + if (PyModule_AddObject(mod, "inf", PyFloat_FromDouble(Py_INFINITY)) < 0) { return -1; } Py_complex infj = {0.0, Py_INFINITY}; - if (PyModule_Add(mod, "infj", PyComplex_FromCComplex(infj)) < 0) { + if (PyModule_AddObject(mod, "infj", + PyComplex_FromCComplex(infj)) < 0) { return -1; } - if (PyModule_Add(mod, "nan", PyFloat_FromDouble(fabs(Py_NAN))) < 0) { + if (PyModule_AddObject(mod, "nan", PyFloat_FromDouble(fabs(Py_NAN))) < 0) { return -1; } Py_complex nanj = {0.0, fabs(Py_NAN)}; - if (PyModule_Add(mod, "nanj", PyComplex_FromCComplex(nanj)) < 0) { + if (PyModule_AddObject(mod, "nanj", PyComplex_FromCComplex(nanj)) < 0) { return -1; } diff --git a/Modules/mathmodule.c b/Modules/mathmodule.c index 0d238c086ac78f..f5679fe3a6f362 100644 --- a/Modules/mathmodule.c +++ b/Modules/mathmodule.c @@ -4037,20 +4037,20 @@ math_exec(PyObject *module) if (state->str___trunc__ == NULL) { return -1; } - if (PyModule_Add(module, "pi", PyFloat_FromDouble(Py_MATH_PI)) < 0) { + if (PyModule_AddObject(module, "pi", PyFloat_FromDouble(Py_MATH_PI)) < 0) { return -1; } - if (PyModule_Add(module, "e", PyFloat_FromDouble(Py_MATH_E)) < 0) { + if (PyModule_AddObject(module, "e", PyFloat_FromDouble(Py_MATH_E)) < 0) { return -1; } // 2pi - if (PyModule_Add(module, "tau", PyFloat_FromDouble(Py_MATH_TAU)) < 0) { + if (PyModule_AddObject(module, "tau", PyFloat_FromDouble(Py_MATH_TAU)) < 0) { return -1; } - if (PyModule_Add(module, "inf", PyFloat_FromDouble(Py_INFINITY)) < 0) { + if (PyModule_AddObject(module, "inf", PyFloat_FromDouble(Py_INFINITY)) < 0) { return -1; } - if (PyModule_Add(module, "nan", PyFloat_FromDouble(fabs(Py_NAN))) < 0) { + if (PyModule_AddObject(module, "nan", PyFloat_FromDouble(fabs(Py_NAN))) < 0) { return -1; } return 0; diff --git a/Modules/posixmodule.c b/Modules/posixmodule.c index 71d41fe632b42d..aef802c232c6ce 100644 --- a/Modules/posixmodule.c +++ b/Modules/posixmodule.c @@ -13466,7 +13466,7 @@ setup_confname_table(struct constdef *table, size_t tablesize, } Py_DECREF(o); } - return PyModule_Add(module, tablename, d); + return PyModule_AddObject(module, tablename, d); } /* Return -1 on failure, 0 on success. */ @@ -16781,9 +16781,11 @@ posixmodule_exec(PyObject *m) #endif /* Initialize environ dictionary */ - if (PyModule_Add(m, "environ", convertenviron()) != 0) { + PyObject *v = convertenviron(); + Py_XINCREF(v); + if (v == NULL || PyModule_AddObject(m, "environ", v) != 0) return -1; - } + Py_DECREF(v); if (all_ins(m)) return -1; @@ -16791,16 +16793,15 @@ posixmodule_exec(PyObject *m) if (setup_confname_tables(m)) return -1; - if (PyModule_AddObjectRef(m, "error", PyExc_OSError) < 0) { - return -1; - } + PyModule_AddObject(m, "error", Py_NewRef(PyExc_OSError)); #if defined(HAVE_WAITID) && !defined(__APPLE__) waitid_result_desc.name = MODNAME ".waitid_result"; PyObject *WaitidResultType = (PyObject *)PyStructSequence_NewType(&waitid_result_desc); - if (PyModule_AddObjectRef(m, "waitid_result", WaitidResultType) < 0) { + if (WaitidResultType == NULL) { return -1; } + PyModule_AddObject(m, "waitid_result", Py_NewRef(WaitidResultType)); state->WaitidResultType = WaitidResultType; #endif @@ -16809,36 +16810,39 @@ posixmodule_exec(PyObject *m) stat_result_desc.fields[8].name = PyStructSequence_UnnamedField; stat_result_desc.fields[9].name = PyStructSequence_UnnamedField; PyObject *StatResultType = (PyObject *)PyStructSequence_NewType(&stat_result_desc); - if (PyModule_AddObjectRef(m, "stat_result", StatResultType) < 0) { + if (StatResultType == NULL) { return -1; } + PyModule_AddObject(m, "stat_result", Py_NewRef(StatResultType)); state->StatResultType = StatResultType; state->statresult_new_orig = ((PyTypeObject *)StatResultType)->tp_new; ((PyTypeObject *)StatResultType)->tp_new = statresult_new; statvfs_result_desc.name = "os.statvfs_result"; /* see issue #19209 */ PyObject *StatVFSResultType = (PyObject *)PyStructSequence_NewType(&statvfs_result_desc); - if (PyModule_AddObjectRef(m, "statvfs_result", StatVFSResultType) < 0) { + if (StatVFSResultType == NULL) { return -1; } + PyModule_AddObject(m, "statvfs_result", Py_NewRef(StatVFSResultType)); state->StatVFSResultType = StatVFSResultType; #if defined(HAVE_SCHED_SETPARAM) || defined(HAVE_SCHED_SETSCHEDULER) || defined(POSIX_SPAWN_SETSCHEDULER) || defined(POSIX_SPAWN_SETSCHEDPARAM) sched_param_desc.name = MODNAME ".sched_param"; PyObject *SchedParamType = (PyObject *)PyStructSequence_NewType(&sched_param_desc); - if (PyModule_AddObjectRef(m, "sched_param", SchedParamType) < 0) { + if (SchedParamType == NULL) { return -1; } - PyModule_AddObjectRef(m, "sched_param", SchedParamType); + PyModule_AddObject(m, "sched_param", Py_NewRef(SchedParamType)); state->SchedParamType = SchedParamType; ((PyTypeObject *)SchedParamType)->tp_new = os_sched_param; #endif /* initialize TerminalSize_info */ PyObject *TerminalSizeType = (PyObject *)PyStructSequence_NewType(&TerminalSize_desc); - if (PyModule_AddObjectRef(m, "terminal_size", TerminalSizeType) < 0) { + if (TerminalSizeType == NULL) { return -1; } + PyModule_AddObject(m, "terminal_size", Py_NewRef(TerminalSizeType)); state->TerminalSizeType = TerminalSizeType; /* initialize scandir types */ @@ -16849,22 +16853,26 @@ posixmodule_exec(PyObject *m) state->ScandirIteratorType = ScandirIteratorType; PyObject *DirEntryType = PyType_FromModuleAndSpec(m, &DirEntryType_spec, NULL); - if (PyModule_AddObjectRef(m, "DirEntry", DirEntryType) < 0) { + if (DirEntryType == NULL) { return -1; } + PyModule_AddObject(m, "DirEntry", Py_NewRef(DirEntryType)); state->DirEntryType = DirEntryType; times_result_desc.name = MODNAME ".times_result"; PyObject *TimesResultType = (PyObject *)PyStructSequence_NewType(×_result_desc); - if (PyModule_AddObjectRef(m, "times_result", TimesResultType) < 0) { + if (TimesResultType == NULL) { return -1; } + PyModule_AddObject(m, "times_result", Py_NewRef(TimesResultType)); state->TimesResultType = TimesResultType; PyTypeObject *UnameResultType = PyStructSequence_NewType(&uname_result_desc); - if (PyModule_AddObjectRef(m, "uname_result", (PyObject *)UnameResultType) < 0) { + if (UnameResultType == NULL) { return -1; } + ; + PyModule_AddObject(m, "uname_result", Py_NewRef(UnameResultType)); state->UnameResultType = (PyObject *)UnameResultType; if ((state->billion = PyLong_FromLong(1000000000)) == NULL) @@ -16907,7 +16915,9 @@ posixmodule_exec(PyObject *m) Py_DECREF(unicode); } - return PyModule_Add(m, "_have_functions", list); + PyModule_AddObject(m, "_have_functions", list); + + return 0; } diff --git a/Modules/timemodule.c b/Modules/timemodule.c index 912710219bd014..3607855dbd8f27 100644 --- a/Modules/timemodule.c +++ b/Modules/timemodule.c @@ -1790,9 +1790,11 @@ init_timezone(PyObject *m) return -1; } #endif // MS_WINDOWS - if (PyModule_Add(m, "tzname", Py_BuildValue("(NN)", otz0, otz1)) < 0) { + PyObject *tzname_obj = Py_BuildValue("(NN)", otz0, otz1); + if (tzname_obj == NULL) { return -1; } + PyModule_AddObject(m, "tzname", tzname_obj); #else // !HAVE_DECL_TZNAME static const time_t YEAR = (365 * 24 + 6) * 3600; time_t t; @@ -1835,9 +1837,10 @@ init_timezone(PyObject *m) PyModule_AddIntConstant(m, "daylight", janzone != julyzone); tzname_obj = Py_BuildValue("(zz)", janname, julyname); } - if (PyModule_Add(m, "tzname", tzname_obj) < 0) { + if (tzname_obj == NULL) { return -1; } + PyModule_AddObject(m, "tzname", tzname_obj); #endif // !HAVE_DECL_TZNAME if (PyErr_Occurred()) { diff --git a/Modules/xxsubtype.c b/Modules/xxsubtype.c index 9e4a3d66ef41bd..744ba7bf5d28b6 100644 --- a/Modules/xxsubtype.c +++ b/Modules/xxsubtype.c @@ -274,10 +274,12 @@ xxsubtype_exec(PyObject* m) if (PyType_Ready(&spamdict_type) < 0) return -1; - if (PyModule_AddObjectRef(m, "spamlist", (PyObject *)&spamlist_type) < 0) + if (PyModule_AddObject(m, "spamlist", + Py_NewRef(&spamlist_type)) < 0) return -1; - if (PyModule_AddObjectRef(m, "spamdict", (PyObject *)&spamdict_type) < 0) + if (PyModule_AddObject(m, "spamdict", + Py_NewRef(&spamdict_type)) < 0) return -1; return 0; } From fdc7ece225a99d5e739b420c21cbf23ac2badb73 Mon Sep 17 00:00:00 2001 From: Serhiy Storchaka Date: Mon, 17 Jul 2023 22:59:43 +0300 Subject: [PATCH 9/9] Update docs. --- Doc/c-api/module.rst | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/Doc/c-api/module.rst b/Doc/c-api/module.rst index 90303411390778..d35b302fce6aa6 100644 --- a/Doc/c-api/module.rst +++ b/Doc/c-api/module.rst @@ -524,12 +524,18 @@ state: PyObject *obj = PyBytes_FromString(value); if (PyModule_AddObject(module, "spam", obj) < 0) { + // If 'obj' is not NULL and PyModule_AddObject() failed, + // 'obj' strong reference must be deleted with Py_XDECREF(). + // If 'obj' is NULL, Py_XDECREF() does nothing. Py_XDECREF(obj); goto error; } + // PyModule_AddObject() stole a reference to obj: + // Py_XDECREF(obj) is not needed here. - Note that ``Py_XDECREF()`` should be used instead of ``Py_DECREF()`` in - this case, since *obj* can be ``NULL``. + .. deprecated:: 3.13 + + :c:func:`PyModule_AddObject` is :term:`soft deprecated`. .. c:function:: int PyModule_AddIntConstant(PyObject *module, const char *name, long value)