Skip to content
Merged
Show file tree
Hide file tree
Changes from 10 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
57 changes: 24 additions & 33 deletions Doc/c-api/module.rst
Original file line number Diff line number Diff line change
Expand Up @@ -486,12 +486,29 @@ state:
.. versionadded:: 3.10


.. c:function:: int PyModule_Add(PyObject *module, const char *name, PyObject *value)

Similar to :c:func:`PyModule_AddObjectRef`, but "steals" a reference
to *value*.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

PyModule_AddObjectRef() must not be called with NULL. Can you please elaborate the behavior when value is NULL? Something like:

If value is NULL, just return -1.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

PyModule_AddObjectRef() can be called with NULL if an exception is set. Actually, some of my fixes which use PyModule_AddObjectRef() rely on this.

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


.. c:function:: int PyModule_AddObject(PyObject *module, const char *name, PyObject *value)

Similar to :c:func:`PyModule_AddObjectRef`, but steals a reference to
*value* on success (if it returns ``0``).

The new :c:func:`PyModule_AddObjectRef` function is recommended, since it is
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.

Expand All @@ -501,41 +518,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``.
Expand Down
1 change: 1 addition & 0 deletions Doc/data/stable_abi.dat

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

5 changes: 5 additions & 0 deletions Doc/whatsnew/3.13.rst
Original file line number Diff line number Diff line change
Expand Up @@ -767,6 +767,11 @@ 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
: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
----------------------

Expand Down
12 changes: 9 additions & 3 deletions Include/modsupport.h
Original file line number Diff line number Diff line change
Expand Up @@ -23,12 +23,18 @@ PyAPI_FUNC(PyObject *) Py_BuildValue(const char *, ...);
PyAPI_FUNC(PyObject *) Py_VaBuildValue(const char *, va_list);

// Add an attribute with name 'name' and value 'obj' to the module 'mod.
// On success, return 0 on success.
// On success, return 0.
// 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).
#if !defined(Py_LIMITED_API) || Py_LIMITED_API+0 >= 0x030d0000
// Similar to PyModule_AddObjectRef() but steal a reference to 'value'.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I suggest to add "and value can be NULL".

PyAPI_FUNC(int) PyModule_Add(PyObject *mod, const char *name, PyObject *value);
#endif /* Py_LIMITED_API */

// Similar to PyModule_AddObjectRef() and PyModule_Add() but steal
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's not similar to PyModule_AddObjectRef(): value can be NULL.

Suggested change
// Similar to PyModule_AddObjectRef() and PyModule_Add() but steal
// Similar to PyModule_Add() but steal

// a reference to 'value' on success and only on success.
// Errorprone. Should not be used in new code.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IMO it's important important to put it in the documentation. This kind of definition fits perfectly with the Soft Deprecated status. Maybe also soft deprecate this API, similar to getopt and optparse soft deprecation. Example with getopt:

.. deprecated:: 3.13
   The :mod:`getopt` module is :term:`soft deprecated` and will not be
   developed further; development will continue with the :mod:`argparse`
   module.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you suggest to convert the note block into deprecated?

PyAPI_FUNC(int) PyModule_AddObject(PyObject *mod, const char *, PyObject *value);

PyAPI_FUNC(int) PyModule_AddIntConstant(PyObject *, const char *, long);
Expand Down
1 change: 1 addition & 0 deletions Lib/test/test_stable_abi_ctypes.py

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Add :func:`PyModule_Add` function: similar to :c:func:`PyModule_AddObjectRef` and :c:func:`PyModule_AddObject`, but always steals a reference to the value.
2 changes: 2 additions & 0 deletions Misc/stable_abi.toml
Original file line number Diff line number Diff line change
Expand Up @@ -2444,3 +2444,5 @@
added = '3.13'
[function.PyMapping_GetOptionalItemString]
added = '3.13'
[function.PyModule_Add]
added = '3.13'
3 changes: 1 addition & 2 deletions Modules/_curses_panel.c
Original file line number Diff line number Diff line change
Expand Up @@ -662,8 +662,7 @@ _curses_panel_exec(PyObject *mod)
state->PyCursesError = PyErr_NewException(
"_curses_panel.error", NULL, NULL);

if (PyModule_AddObject(mod, "error", Py_NewRef(state->PyCursesError)) < 0) {
Py_DECREF(state->PyCursesError);
if (PyModule_AddObjectRef(mod, "error", state->PyCursesError) < 0) {
return -1;
}

Expand Down
24 changes: 12 additions & 12 deletions Modules/_decimal/_decimal.c
Original file line number Diff line number Diff line change
Expand Up @@ -5957,7 +5957,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(state->SignalTuple, i, Py_NewRef(cm->ex));
Expand Down Expand Up @@ -5986,39 +5986,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 */
Expand All @@ -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_AddObject(m, mpd_round_string[i], Py_NewRef(state->round_map[i])));
CHECK_INT(PyModule_AddObjectRef(m, mpd_round_string[i], state->round_map[i]));
}

/* Add specification version number */
Expand Down
8 changes: 4 additions & 4 deletions Modules/_ssl.c
Original file line number Diff line number Diff line change
Expand Up @@ -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_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 (r == NULL || PyModule_AddObject(m, "OPENSSL_VERSION_INFO", r))
if (PyModule_Add(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_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 (r == NULL || PyModule_AddObject(m, "_OPENSSL_API_VERSION", r))
if (PyModule_Add(m, "_OPENSSL_API_VERSION", r) < 0)
return -1;

return 0;
Expand Down
18 changes: 9 additions & 9 deletions Modules/_stat.c
Original file line number Diff line number Diff line change
Expand Up @@ -591,17 +591,17 @@ 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) {
return -1;
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) {
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_APPEXECLINK",
PyLong_FromUnsignedLong(IO_REPARSE_TAG_APPEXECLINK)) < 0) {
return -1;
if (PyModule_Add(module, "IO_REPARSE_TAG_APPEXECLINK",
PyLong_FromUnsignedLong(IO_REPARSE_TAG_APPEXECLINK)) < 0) {
return -1;
}
#endif

Expand Down
8 changes: 4 additions & 4 deletions Modules/_testinternalcapi.c
Original file line number Diff line number Diff line change
Expand Up @@ -1493,13 +1493,13 @@ static PyMethodDef module_functions[] = {
static int
module_exec(PyObject *module)
{
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) {
return 1;
}

if (PyModule_AddObject(module, "SIZEOF_TIME_T",
PyLong_FromSsize_t(sizeof(time_t))) < 0) {
if (PyModule_Add(module, "SIZEOF_TIME_T",
PyLong_FromSsize_t(sizeof(time_t))) < 0) {
return 1;
}

Expand Down
4 changes: 2 additions & 2 deletions Modules/_threadmodule.c
Original file line number Diff line number Diff line change
Expand Up @@ -1671,8 +1671,8 @@ thread_module_exec(PyObject *module)
// Round towards minus infinity
timeout_max = floor(timeout_max);

if (PyModule_AddObject(module, "TIMEOUT_MAX",
PyFloat_FromDouble(timeout_max)) < 0) {
if (PyModule_Add(module, "TIMEOUT_MAX",
PyFloat_FromDouble(timeout_max)) < 0) {
return -1;
}

Expand Down
15 changes: 7 additions & 8 deletions Modules/cmathmodule.c
Original file line number Diff line number Diff line change
Expand Up @@ -1217,30 +1217,29 @@ 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(Py_INFINITY)) < 0) {
if (PyModule_Add(mod, "inf", PyFloat_FromDouble(Py_INFINITY)) < 0) {
return -1;
}

Py_complex infj = {0.0, Py_INFINITY};
if (PyModule_AddObject(mod, "infj",
PyComplex_FromCComplex(infj)) < 0) {
if (PyModule_Add(mod, "infj", PyComplex_FromCComplex(infj)) < 0) {
return -1;
}
if (PyModule_AddObject(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_AddObject(mod, "nanj", PyComplex_FromCComplex(nanj)) < 0) {
if (PyModule_Add(mod, "nanj", PyComplex_FromCComplex(nanj)) < 0) {
return -1;
}

Expand Down
10 changes: 5 additions & 5 deletions Modules/mathmodule.c
Original file line number Diff line number Diff line change
Expand Up @@ -4037,20 +4037,20 @@ math_exec(PyObject *module)
if (state->str___trunc__ == NULL) {
return -1;
}
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(Py_INFINITY)) < 0) {
if (PyModule_Add(module, "inf", PyFloat_FromDouble(Py_INFINITY)) < 0) {
return -1;
}
if (PyModule_AddObject(module, "nan", PyFloat_FromDouble(fabs(Py_NAN))) < 0) {
if (PyModule_Add(module, "nan", PyFloat_FromDouble(fabs(Py_NAN))) < 0) {
return -1;
}
return 0;
Expand Down
Loading