public inbox for linux-gpio@vger.kernel.org
 help / color / mirror / Atom feed
* [libgpiod][PATCH v2 0/8] bindings: python: modernize C extensions
@ 2026-04-23 22:21 Vincent Fazio
  2026-04-23 22:21 ` [libgpiod][PATCH v2 1/8] bindings: python: avoid PyObject_CallMethod during chip finalize Vincent Fazio
                   ` (8 more replies)
  0 siblings, 9 replies; 13+ messages in thread
From: Vincent Fazio @ 2026-04-23 22:21 UTC (permalink / raw)
  To: linux-gpio; +Cc: brgl, Vincent Fazio

This series performs some minor cleanup of the C extension modules and
migrates the module defintions to multi-phase intitialization (PEP 489).

Patches 1 & 2 avoid calling back into python to perform C level object
cleanup.

Patch 3 introduces no real functional change but simplifies the code to
get the same result.

Patch 4 applies suggestions from from a utility maintained by a CPython
core developer to help modernize macro/function usage.

Patch 5 conditionally compiles support for using a standard CPython
function over a backported version bundled within the C extension.

Patches 6-8 migrate the C extensions to use multi-phase initialization
as described in PEP 489 [0]. While not strictly necessary for enabling
free-threaded builds, it makes adding support more straighforward and
sets the stage for both PEP 793 [1] which soft-deprecates PyInit_* and
PEP 803 [2] which could simplify wheel builds.

Patches have been tested against the full matrix of supported versions:
  https://github.com/vfazio/libgpiod/actions/runs/24860932220

[0]: https://peps.python.org/pep-0489/
[1]: https://peps.python.org/pep-0793/
[2]: https://peps.python.org/pep-0803/

Signed-off-by: Vincent Fazio <vfazio@gmail.com>
---
Changes in v2:
- Dropped "bindings: python: use Py_RETURN_NONE in chip_get_line_name" (Bartosz)
- Adjusted patch 4 to simplify Py_None strong reference acquisition
- Updated link to passing pipeline
- Link to v1: https://lore.kernel.org/all/20260422012041.39933-1-vfazio@gmail.com/
---

Vincent Fazio (8):
  bindings: python: avoid PyObject_CallMethod during chip finalize
  bindings: python: avoid PyObject_CallMethod during request finalize
  bindings: python: simplify disallowing _ext.Request from being created
  bindings: python: use suggestions from upgrade_pythoncapi.py
  bindings: python: use PyImport_ImportModuleAttrString when available
  bindings: python: migrate the gpiod._ext module to multi-phase init
  bindings: python: tests: migrate the system module to multi-phase init
  bindings: python: tests: migrate the gpiosim module to multi-phase
    init

 bindings/python/gpiod/ext/chip.c     |  29 ++---
 bindings/python/gpiod/ext/common.c   |   5 +-
 bindings/python/gpiod/ext/internal.h |  10 +-
 bindings/python/gpiod/ext/module.c   |  68 ++++++------
 bindings/python/gpiod/ext/request.c  |  26 ++---
 bindings/python/tests/gpiosim/ext.c  | 153 +++++++++++++++++----------
 bindings/python/tests/system/ext.c   |   7 +-
 7 files changed, 171 insertions(+), 127 deletions(-)

-- 
2.43.0


^ permalink raw reply	[flat|nested] 13+ messages in thread

* [libgpiod][PATCH v2 1/8] bindings: python: avoid PyObject_CallMethod during chip finalize
  2026-04-23 22:21 [libgpiod][PATCH v2 0/8] bindings: python: modernize C extensions Vincent Fazio
@ 2026-04-23 22:21 ` Vincent Fazio
  2026-04-23 22:21 ` [libgpiod][PATCH v2 2/8] bindings: python: avoid PyObject_CallMethod during request finalize Vincent Fazio
                   ` (7 subsequent siblings)
  8 siblings, 0 replies; 13+ messages in thread
From: Vincent Fazio @ 2026-04-23 22:21 UTC (permalink / raw)
  To: linux-gpio; +Cc: brgl, Vincent Fazio

The PyObject_CallMethod function jumps through hoops to find the method
to call and to build a stack (if necessary) to call a given method.

In the case of finalizing the chip object, that overhead can be avoided
by simply sharing a function with `chip_close` to close the underlying
gpiod_chip object.

Signed-off-by: Vincent Fazio <vfazio@gmail.com>
---
 bindings/python/gpiod/ext/chip.c | 18 ++++++++++++------
 1 file changed, 12 insertions(+), 6 deletions(-)

diff --git a/bindings/python/gpiod/ext/chip.c b/bindings/python/gpiod/ext/chip.c
index 53c4b80..be20869 100644
--- a/bindings/python/gpiod/ext/chip.c
+++ b/bindings/python/gpiod/ext/chip.c
@@ -32,10 +32,19 @@ chip_init(chip_object *self, PyObject *args, PyObject *Py_UNUSED(ignored))
 	return 0;
 }
 
+static void internal_chip_close(chip_object *self)
+{
+	if (self->chip) {
+		Py_BEGIN_ALLOW_THREADS;
+		gpiod_chip_close(self->chip);
+		Py_END_ALLOW_THREADS;
+		self->chip = NULL;
+	}
+}
+
 static void chip_finalize(chip_object *self)
 {
-	if (self->chip)
-		PyObject_CallMethod((PyObject *)self, "close", "");
+	internal_chip_close(self);
 }
 
 static PyObject *chip_path(chip_object *self, void *Py_UNUSED(ignored))
@@ -62,10 +71,7 @@ static PyGetSetDef chip_getset[] = {
 
 static PyObject *chip_close(chip_object *self, PyObject *Py_UNUSED(ignored))
 {
-	Py_BEGIN_ALLOW_THREADS;
-	gpiod_chip_close(self->chip);
-	Py_END_ALLOW_THREADS;
-	self->chip = NULL;
+	internal_chip_close(self);
 
 	Py_RETURN_NONE;
 }
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [libgpiod][PATCH v2 2/8] bindings: python: avoid PyObject_CallMethod during request finalize
  2026-04-23 22:21 [libgpiod][PATCH v2 0/8] bindings: python: modernize C extensions Vincent Fazio
  2026-04-23 22:21 ` [libgpiod][PATCH v2 1/8] bindings: python: avoid PyObject_CallMethod during chip finalize Vincent Fazio
@ 2026-04-23 22:21 ` Vincent Fazio
  2026-04-23 22:21 ` [libgpiod][PATCH v2 3/8] bindings: python: simplify disallowing _ext.Request from being created Vincent Fazio
                   ` (6 subsequent siblings)
  8 siblings, 0 replies; 13+ messages in thread
From: Vincent Fazio @ 2026-04-23 22:21 UTC (permalink / raw)
  To: linux-gpio; +Cc: brgl, Vincent Fazio

The PyObject_CallMethod function jumps through hoops to find the method
to call and to build a stack (if necessary) to call a given method.

In the case of finalizing the request object, that overhead can be
avoided by simply sharing a function with `request_release` to close the
underlying gpiod_line_request object.

Signed-off-by: Vincent Fazio <vfazio@gmail.com>
---
 bindings/python/gpiod/ext/request.c | 18 ++++++++++++------
 1 file changed, 12 insertions(+), 6 deletions(-)

diff --git a/bindings/python/gpiod/ext/request.c b/bindings/python/gpiod/ext/request.c
index 9acf828..1e1f65a 100644
--- a/bindings/python/gpiod/ext/request.c
+++ b/bindings/python/gpiod/ext/request.c
@@ -22,10 +22,19 @@ static int request_init(PyObject *Py_UNUSED(ignored0),
 	return -1;
 }
 
+static void internal_request_release(request_object *self)
+{
+	if (self->request){
+		Py_BEGIN_ALLOW_THREADS;
+		gpiod_line_request_release(self->request);
+		Py_END_ALLOW_THREADS;
+		self->request = NULL;
+	}
+}
+
 static void request_finalize(request_object *self)
 {
-	if (self->request)
-		PyObject_CallMethod((PyObject *)self, "release", "");
+	internal_request_release(self);
 
 	if (self->offsets)
 		PyMem_Free(self->offsets);
@@ -121,10 +130,7 @@ static PyGetSetDef request_getset[] = {
 static PyObject *
 request_release(request_object *self, PyObject *Py_UNUSED(ignored))
 {
-	Py_BEGIN_ALLOW_THREADS;
-	gpiod_line_request_release(self->request);
-	Py_END_ALLOW_THREADS;
-	self->request = NULL;
+	internal_request_release(self);
 
 	Py_RETURN_NONE;
 }
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [libgpiod][PATCH v2 3/8] bindings: python: simplify disallowing _ext.Request from being created
  2026-04-23 22:21 [libgpiod][PATCH v2 0/8] bindings: python: modernize C extensions Vincent Fazio
  2026-04-23 22:21 ` [libgpiod][PATCH v2 1/8] bindings: python: avoid PyObject_CallMethod during chip finalize Vincent Fazio
  2026-04-23 22:21 ` [libgpiod][PATCH v2 2/8] bindings: python: avoid PyObject_CallMethod during request finalize Vincent Fazio
@ 2026-04-23 22:21 ` Vincent Fazio
  2026-04-23 22:21 ` [libgpiod][PATCH v2 4/8] bindings: python: use suggestions from upgrade_pythoncapi.py Vincent Fazio
                   ` (5 subsequent siblings)
  8 siblings, 0 replies; 13+ messages in thread
From: Vincent Fazio @ 2026-04-23 22:21 UTC (permalink / raw)
  To: linux-gpio; +Cc: brgl, Vincent Fazio

Previously, tp_new and tp_init functions were defined for Request with
the tp_init function raising a NotImplementedError to prevent the object
from being initialized (__init__, the stage after __new__).

Similar results can be achieved by removing the tp_new and tp_init
functions which raise a slightly different error:

  TypeError: cannot create 'gpiod._ext.Request' instances

As this is an internal only detail, there should be impact to users.

Signed-off-by: Vincent Fazio <vfazio@gmail.com>
---
 bindings/python/gpiod/ext/request.c | 12 ------------
 1 file changed, 12 deletions(-)

diff --git a/bindings/python/gpiod/ext/request.c b/bindings/python/gpiod/ext/request.c
index 1e1f65a..9940382 100644
--- a/bindings/python/gpiod/ext/request.c
+++ b/bindings/python/gpiod/ext/request.c
@@ -12,16 +12,6 @@ typedef struct {
 	struct gpiod_edge_event_buffer *buffer;
 } request_object;
 
-static int request_init(PyObject *Py_UNUSED(ignored0),
-			PyObject *Py_UNUSED(ignored1),
-			PyObject *Py_UNUSED(ignored2))
-{
-	PyErr_SetString(PyExc_NotImplementedError,
-			"_ext.LineRequest cannot be instantiated");
-
-	return -1;
-}
-
 static void internal_request_release(request_object *self)
 {
 	if (self->request){
@@ -390,8 +380,6 @@ PyTypeObject request_type = {
 	.tp_name = "gpiod._ext.Request",
 	.tp_basicsize = sizeof(request_object),
 	.tp_flags = Py_TPFLAGS_DEFAULT,
-	.tp_new = PyType_GenericNew,
-	.tp_init = (initproc)request_init,
 	.tp_finalize = (destructor)request_finalize,
 	.tp_dealloc = (destructor)Py_gpiod_dealloc,
 	.tp_getset = request_getset,
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [libgpiod][PATCH v2 4/8] bindings: python: use suggestions from upgrade_pythoncapi.py
  2026-04-23 22:21 [libgpiod][PATCH v2 0/8] bindings: python: modernize C extensions Vincent Fazio
                   ` (2 preceding siblings ...)
  2026-04-23 22:21 ` [libgpiod][PATCH v2 3/8] bindings: python: simplify disallowing _ext.Request from being created Vincent Fazio
@ 2026-04-23 22:21 ` Vincent Fazio
  2026-04-23 22:21 ` [libgpiod][PATCH v2 5/8] bindings: python: use PyImport_ImportModuleAttrString when available Vincent Fazio
                   ` (4 subsequent siblings)
  8 siblings, 0 replies; 13+ messages in thread
From: Vincent Fazio @ 2026-04-23 22:21 UTC (permalink / raw)
  To: linux-gpio; +Cc: brgl, Vincent Fazio

The upgrade_pythoncapi.py script performs transforms on C extension
sources to use new syntax [0]. It's generally used with a shim header
for compatibility, but the header is not necessary for these changes.

The suggested changes are:

  * Use the `Py_IsNone` function introduced in CPython 3.10 [1] over
    direct comparison to `Py_None`

  * Use `PyObject_Free` instead of `PyObject_Del` since `PyObject_Del`
    has been aliased to `PyObject_Free` for nearly 24 years [2] and has
    been announced as deprecated [3]

  * Use the `Py_NewRef` function introduced in CPython 3.10 [4] to
    increment and return a new reference.

[0]: https://pythoncapi-compat.readthedocs.io/en/latest/index.html
[1]: https://github.com/python/cpython/commit/09bbebea163fe7303264cf4069c51d4d2f22fde4
[2]: https://github.com/python/cpython/commit/3e7b893899bdaa294a72e894694b099a1d370765
[3]: https://docs.python.org/3/c-api/allocation.html#deprecated-aliases
[4]: https://github.com/python/cpython/commit/53a03aafd5812018a3821a2e83063fd3d6cd2576

Signed-off-by: Vincent Fazio <vfazio@gmail.com>
---
 bindings/python/gpiod/ext/chip.c    | 11 +++--------
 bindings/python/gpiod/ext/common.c  |  2 +-
 bindings/python/gpiod/ext/request.c |  2 +-
 bindings/python/tests/gpiosim/ext.c |  2 +-
 4 files changed, 6 insertions(+), 11 deletions(-)

diff --git a/bindings/python/gpiod/ext/chip.c b/bindings/python/gpiod/ext/chip.c
index be20869..4d96dd8 100644
--- a/bindings/python/gpiod/ext/chip.c
+++ b/bindings/python/gpiod/ext/chip.c
@@ -172,12 +172,7 @@ static PyObject *chip_get_line_name(chip_object *self, PyObject *args)
 		return Py_gpiod_SetErrFromErrno();
 
 	name = gpiod_line_info_get_name(info);
-	if (!name) {
-		Py_INCREF(Py_None);
-		line_name = Py_None;
-	} else {
-		line_name = PyUnicode_FromString(name);
-	}
+	line_name = (name) ? PyUnicode_FromString(name) : Py_NewRef(Py_None);
 
 	gpiod_line_info_free(info);
 
@@ -272,7 +267,7 @@ make_request_config(PyObject *consumer_obj, PyObject *event_buffer_size_obj)
 		return NULL;
 	}
 
-	if (consumer_obj != Py_None) {
+	if (!Py_IsNone(consumer_obj)) {
 		consumer = PyUnicode_AsUTF8(consumer_obj);
 		if (!consumer) {
 			gpiod_request_config_free(req_cfg);
@@ -282,7 +277,7 @@ make_request_config(PyObject *consumer_obj, PyObject *event_buffer_size_obj)
 		gpiod_request_config_set_consumer(req_cfg, consumer);
 	}
 
-	if (event_buffer_size_obj != Py_None) {
+	if (!Py_IsNone(event_buffer_size_obj)) {
 		event_buffer_size = PyLong_AsSize_t(event_buffer_size_obj);
 		if (PyErr_Occurred()) {
 			gpiod_request_config_free(req_cfg);
diff --git a/bindings/python/gpiod/ext/common.c b/bindings/python/gpiod/ext/common.c
index 62201b6..7d2dda7 100644
--- a/bindings/python/gpiod/ext/common.c
+++ b/bindings/python/gpiod/ext/common.c
@@ -12,7 +12,7 @@ void Py_gpiod_dealloc(PyObject *self)
 	if (ret < 0)
 		return;
 
-	PyObject_Del(self);
+	PyObject_Free(self);
 }
 
 PyObject *Py_gpiod_SetErrFromErrno(void)
diff --git a/bindings/python/gpiod/ext/request.c b/bindings/python/gpiod/ext/request.c
index 9940382..46e07ae 100644
--- a/bindings/python/gpiod/ext/request.c
+++ b/bindings/python/gpiod/ext/request.c
@@ -286,7 +286,7 @@ static PyObject *request_read_edge_events(request_object *self, PyObject *args)
 	if (!ret)
 		return NULL;
 
-	if (max_events_obj != Py_None) {
+	if (!Py_IsNone(max_events_obj)) {
 		max_events = PyLong_AsSize_t(max_events_obj);
 		if (PyErr_Occurred())
 			return NULL;
diff --git a/bindings/python/tests/gpiosim/ext.c b/bindings/python/tests/gpiosim/ext.c
index cb5611a..1ebd5af 100644
--- a/bindings/python/tests/gpiosim/ext.c
+++ b/bindings/python/tests/gpiosim/ext.c
@@ -116,7 +116,7 @@ static void chip_dealloc(PyObject *self)
 	if (ret < 0)
 		return;
 
-	PyObject_Del(self);
+	PyObject_Free(self);
 }
 
 static PyObject *chip_dev_path(chip_object *self, void *Py_UNUSED(ignored))
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [libgpiod][PATCH v2 5/8] bindings: python: use PyImport_ImportModuleAttrString when available
  2026-04-23 22:21 [libgpiod][PATCH v2 0/8] bindings: python: modernize C extensions Vincent Fazio
                   ` (3 preceding siblings ...)
  2026-04-23 22:21 ` [libgpiod][PATCH v2 4/8] bindings: python: use suggestions from upgrade_pythoncapi.py Vincent Fazio
@ 2026-04-23 22:21 ` Vincent Fazio
  2026-04-23 22:21 ` [libgpiod][PATCH v2 6/8] bindings: python: migrate the gpiod._ext module to multi-phase init Vincent Fazio
                   ` (3 subsequent siblings)
  8 siblings, 0 replies; 13+ messages in thread
From: Vincent Fazio @ 2026-04-23 22:21 UTC (permalink / raw)
  To: linux-gpio; +Cc: brgl, Vincent Fazio

Commit 035ad74fdd introduced Py_gpiod_GetModuleAttrString as a way to
get a module's attribute and was inspired by CPython's internal function
_PyImport_GetModuleAttrString.

CPython 3.14 added PyImport_ImportModuleAttrString to the public API [0]
which allows us to use the standard function when available.

[0]: https://github.com/python/cpython/pull/128912

Signed-off-by: Vincent Fazio <vfazio@gmail.com>
---
 bindings/python/gpiod/ext/common.c   |  3 +++
 bindings/python/gpiod/ext/internal.h | 10 ++++++++--
 2 files changed, 11 insertions(+), 2 deletions(-)

diff --git a/bindings/python/gpiod/ext/common.c b/bindings/python/gpiod/ext/common.c
index 7d2dda7..fc28e96 100644
--- a/bindings/python/gpiod/ext/common.c
+++ b/bindings/python/gpiod/ext/common.c
@@ -64,6 +64,8 @@ PyObject *Py_gpiod_SetErrFromErrno(void)
 	return PyErr_SetFromErrno(exc);
 }
 
+/* TODO: remove when 3.14 is the minimum version */
+#if PY_VERSION_HEX < 0x030E0000
 PyObject *Py_gpiod_GetModuleAttrString(const char *modname,
 				       const char *attrname)
 {
@@ -78,6 +80,7 @@ PyObject *Py_gpiod_GetModuleAttrString(const char *modname,
 
 	return attribute;
 }
+#endif
 
 unsigned int Py_gpiod_PyLongAsUnsignedInt(PyObject *pylong)
 {
diff --git a/bindings/python/gpiod/ext/internal.h b/bindings/python/gpiod/ext/internal.h
index 15aedfb..2996884 100644
--- a/bindings/python/gpiod/ext/internal.h
+++ b/bindings/python/gpiod/ext/internal.h
@@ -8,8 +8,6 @@
 #include <Python.h>
 
 PyObject *Py_gpiod_SetErrFromErrno(void);
-PyObject *Py_gpiod_GetModuleAttrString(const char *modname,
-				       const char *attrname);
 unsigned int Py_gpiod_PyLongAsUnsignedInt(PyObject *pylong);
 void Py_gpiod_dealloc(PyObject *self);
 PyObject *Py_gpiod_MakeRequestObject(struct gpiod_line_request *request,
@@ -17,4 +15,12 @@ PyObject *Py_gpiod_MakeRequestObject(struct gpiod_line_request *request,
 struct gpiod_line_config *Py_gpiod_LineConfigGetData(PyObject *obj);
 struct gpiod_line_settings *Py_gpiod_LineSettingsGetData(PyObject *obj);
 
+#if PY_VERSION_HEX >= 0x030E0000
+/* Alias to standard function available in 3.14 */
+#define Py_gpiod_GetModuleAttrString PyImport_ImportModuleAttrString
+#else
+PyObject *Py_gpiod_GetModuleAttrString(const char *modname,
+				       const char *attrname);
+#endif /* PY_VERSION_HEX >= 0x030E0000 */
+
 #endif /* __LIBGPIOD_PYTHON_MODULE_H__ */
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [libgpiod][PATCH v2 6/8] bindings: python: migrate the gpiod._ext module to multi-phase init
  2026-04-23 22:21 [libgpiod][PATCH v2 0/8] bindings: python: modernize C extensions Vincent Fazio
                   ` (4 preceding siblings ...)
  2026-04-23 22:21 ` [libgpiod][PATCH v2 5/8] bindings: python: use PyImport_ImportModuleAttrString when available Vincent Fazio
@ 2026-04-23 22:21 ` Vincent Fazio
  2026-04-23 22:21 ` [libgpiod][PATCH v2 7/8] bindings: python: tests: migrate the system " Vincent Fazio
                   ` (2 subsequent siblings)
  8 siblings, 0 replies; 13+ messages in thread
From: Vincent Fazio @ 2026-04-23 22:21 UTC (permalink / raw)
  To: linux-gpio; +Cc: brgl, Vincent Fazio

Single-phase initialization has been classified as legacy within CPython
documentation [0] with multi-phase being its successor [1].

As such, switch to the new methodology.

[0]: https://docs.python.org/3/c-api/extension-modules.html#legacy-single-phase-initialization
[1]: https://docs.python.org/3/c-api/extension-modules.html#multi-phase-initialization

Signed-off-by: Vincent Fazio <vfazio@gmail.com>
---
 bindings/python/gpiod/ext/module.c | 68 +++++++++++++++---------------
 1 file changed, 33 insertions(+), 35 deletions(-)

diff --git a/bindings/python/gpiod/ext/module.c b/bindings/python/gpiod/ext/module.c
index 25c252a..71fa3c2 100644
--- a/bindings/python/gpiod/ext/module.c
+++ b/bindings/python/gpiod/ext/module.c
@@ -135,12 +135,6 @@ static PyMethodDef module_methods[] = {
 	{ }
 };
 
-static PyModuleDef module_def = {
-	PyModuleDef_HEAD_INIT,
-	.m_name = "gpiod._ext",
-	.m_methods = module_methods,
-};
-
 extern PyTypeObject chip_type;
 extern PyTypeObject line_config_type;
 extern PyTypeObject line_settings_type;
@@ -154,53 +148,57 @@ static PyTypeObject *types[] = {
 	NULL,
 };
 
-PyMODINIT_FUNC PyInit__ext(void)
+static int module_exec(PyObject* module)
 {
 	const struct module_const *modconst;
-	PyObject *module, *all;
+	PyObject *all;
 	PyTypeObject **type;
 	int ret;
 
-	module = PyModule_Create(&module_def);
-	if (!module)
-		return NULL;
-
 	ret = PyModule_AddStringConstant(module, "api_version",
 					 gpiod_api_version());
-	if (ret) {
-		Py_DECREF(module);
-		return NULL;
-	}
+
+	if (ret < 0)
+		return -1;
 
 	all = PyList_New(0);
-	if (!all) {
-		Py_DECREF(module);
-		return NULL;
-	}
+	if (!all)
+		return -1;
 
 	ret = PyModule_AddObjectRef(module, "__all__", all);
 	Py_DECREF(all);
-	if (ret) {
-		Py_DECREF(module);
-		return NULL;
-	}
+	if (ret)
+		return -1;
 
 	for (type = types; *type; type++) {
 		ret = PyModule_AddType(module, *type);
-		if (ret) {
-			Py_DECREF(module);
-			return NULL;
-		}
+		if (ret < 0)
+			return -1;
 	}
 
 	for (modconst = module_constants; modconst->name; modconst++) {
-		ret = PyModule_AddIntConstant(module,
-					      modconst->name, modconst->val);
-		if (ret) {
-			Py_DECREF(module);
-			return NULL;
-		}
+		ret = PyModule_AddIntConstant(module, modconst->name,
+					      modconst->val);
+		if (ret < 0)
+			return -1;
 	}
 
-	return module;
+	return 0;
+}
+
+static struct PyModuleDef_Slot module_slots[] = {
+	{Py_mod_exec, module_exec},
+	{0, NULL},
+};
+
+static PyModuleDef module_def = {
+	PyModuleDef_HEAD_INIT,
+	.m_name = "gpiod._ext",
+	.m_methods = module_methods,
+	.m_slots = module_slots,
+};
+
+PyMODINIT_FUNC PyInit__ext(void)
+{
+	return PyModuleDef_Init(&module_def);
 }
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [libgpiod][PATCH v2 7/8] bindings: python: tests: migrate the system module to multi-phase init
  2026-04-23 22:21 [libgpiod][PATCH v2 0/8] bindings: python: modernize C extensions Vincent Fazio
                   ` (5 preceding siblings ...)
  2026-04-23 22:21 ` [libgpiod][PATCH v2 6/8] bindings: python: migrate the gpiod._ext module to multi-phase init Vincent Fazio
@ 2026-04-23 22:21 ` Vincent Fazio
  2026-04-24  9:03   ` Bartosz Golaszewski
  2026-04-23 22:21 ` [libgpiod][PATCH v2 8/8] bindings: python: tests: migrate the gpiosim " Vincent Fazio
  2026-04-24  9:14 ` [libgpiod][PATCH v2 0/8] bindings: python: modernize C extensions Bartosz Golaszewski
  8 siblings, 1 reply; 13+ messages in thread
From: Vincent Fazio @ 2026-04-23 22:21 UTC (permalink / raw)
  To: linux-gpio; +Cc: brgl, Vincent Fazio

Single-phase initialization has been classified as legacy within CPython
documentation [0] with multi-phase being its successor [1].

As such, switch to the new methodology.

[0]: https://docs.python.org/3/c-api/extension-modules.html#legacy-single-phase-initialization
[1]: https://docs.python.org/3/c-api/extension-modules.html#multi-phase-initialization

Signed-off-by: Vincent Fazio <vfazio@gmail.com>
---
 bindings/python/tests/system/ext.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/bindings/python/tests/system/ext.c b/bindings/python/tests/system/ext.c
index c982fa6..800648e 100644
--- a/bindings/python/tests/system/ext.c
+++ b/bindings/python/tests/system/ext.c
@@ -67,13 +67,18 @@ static PyMethodDef module_methods[] = {
 	{ }
 };
 
+static struct PyModuleDef_Slot module_slots[] = {
+	{0, NULL},
+};
+
 static PyModuleDef module_def = {
 	PyModuleDef_HEAD_INIT,
 	.m_name = "system._ext",
 	.m_methods = module_methods,
+	.m_slots = module_slots,
 };
 
 PyMODINIT_FUNC PyInit__ext(void)
 {
-	return PyModule_Create(&module_def);
+	return PyModuleDef_Init(&module_def);
 }
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [libgpiod][PATCH v2 8/8] bindings: python: tests: migrate the gpiosim module to multi-phase init
  2026-04-23 22:21 [libgpiod][PATCH v2 0/8] bindings: python: modernize C extensions Vincent Fazio
                   ` (6 preceding siblings ...)
  2026-04-23 22:21 ` [libgpiod][PATCH v2 7/8] bindings: python: tests: migrate the system " Vincent Fazio
@ 2026-04-23 22:21 ` Vincent Fazio
  2026-04-24  9:14 ` [libgpiod][PATCH v2 0/8] bindings: python: modernize C extensions Bartosz Golaszewski
  8 siblings, 0 replies; 13+ messages in thread
From: Vincent Fazio @ 2026-04-23 22:21 UTC (permalink / raw)
  To: linux-gpio; +Cc: brgl, Vincent Fazio

Single-phase initialization has been classified as legacy within CPython
documentation [0] with multi-phase being its successor [1].

As such, switch to the new methodology.

Switching requires converting `gpiosim.Chip` to be a heap-type [2] in
order to access the module's state [3].

[0]: https://docs.python.org/3/c-api/extension-modules.html#legacy-single-phase-initialization
[1]: https://docs.python.org/3/c-api/extension-modules.html#multi-phase-initialization
[2]: https://docs.python.org/3/c-api/typeobj.html#heap-types
[3]: https://docs.python.org/3/howto/isolating-extensions.html#heap-types

Signed-off-by: Vincent Fazio <vfazio@gmail.com>
---
 bindings/python/tests/gpiosim/ext.c | 151 +++++++++++++++++-----------
 1 file changed, 94 insertions(+), 57 deletions(-)

diff --git a/bindings/python/tests/gpiosim/ext.c b/bindings/python/tests/gpiosim/ext.c
index 1ebd5af..9954dd5 100644
--- a/bindings/python/tests/gpiosim/ext.c
+++ b/bindings/python/tests/gpiosim/ext.c
@@ -45,21 +45,6 @@ struct module_state {
 	struct gpiosim_ctx *sim_ctx;
 };
 
-static void free_module_state(void *mod)
-{
-	struct module_state *state = PyModule_GetState((PyObject *)mod);
-
-	if (state->sim_ctx)
-		gpiosim_ctx_unref(state->sim_ctx);
-}
-
-static PyModuleDef module_def = {
-	PyModuleDef_HEAD_INIT,
-	.m_name = "gpiosim._ext",
-	.m_size = sizeof(struct module_state),
-	.m_free = free_module_state,
-};
-
 typedef struct {
 	PyObject_HEAD
 	struct gpiosim_dev *dev;
@@ -71,13 +56,11 @@ static int chip_init(chip_object *self,
 		     PyObject *Py_UNUSED(ignored1))
 {
 	struct module_state *state;
-	PyObject *mod;
 
-	mod = PyState_FindModule(&module_def);
-	if (!mod)
-		return -1;
+	state = PyType_GetModuleState(Py_TYPE(self));
 
-	state = PyModule_GetState(mod);
+	if (!state)
+		return -1;
 
 	self->dev = gpiosim_dev_new(state->sim_ctx);
 	if (!self->dev) {
@@ -111,12 +94,14 @@ static void chip_finalize(chip_object *self)
 static void chip_dealloc(PyObject *self)
 {
 	int ret;
+	PyTypeObject *tp = Py_TYPE(self);
 
 	ret = PyObject_CallFinalizerFromDealloc(self);
 	if (ret < 0)
 		return;
 
 	PyObject_Free(self);
+	Py_DECREF(tp);
 }
 
 static PyObject *chip_dev_path(chip_object *self, void *Py_UNUSED(ignored))
@@ -289,58 +274,110 @@ static PyMethodDef chip_methods[] = {
 	{ }
 };
 
-static PyTypeObject chip_type = {
-	PyVarObject_HEAD_INIT(NULL, 0)
-	.tp_name = "gpiosim.Chip",
-	.tp_basicsize = sizeof(chip_object),
-	.tp_flags = Py_TPFLAGS_DEFAULT,
-	.tp_new = PyType_GenericNew,
-	.tp_init = (initproc)chip_init,
-	.tp_finalize = (destructor)chip_finalize,
-	.tp_dealloc = (destructor)chip_dealloc,
-	.tp_methods = chip_methods,
-	.tp_getset = chip_getset,
+static PyType_Slot chip_type_slots[] = {
+	{Py_tp_new, PyType_GenericNew},
+	{Py_tp_init, (initproc)chip_init},
+	{Py_tp_finalize, (destructor)chip_finalize},
+	{Py_tp_dealloc, (destructor)chip_dealloc},
+	{Py_tp_methods, chip_methods},
+	{Py_tp_getset, chip_getset},
+	{0, 0}
 };
 
-PyMODINIT_FUNC PyInit__ext(void)
+/*
+See xxlimited.c and _bz2module.c for inspiration.
+
+As part of transitioning to multi-phase module initialization, the
+gpiosim.Chip type needs to become heap allocated so that it can access
+module state.
+
+We disallow subclassing by not specifying Py_TPFLAGS_BASETYPE. This
+allows the module to use PyType_GetModuleState() since it may otherwise
+not return the proper module if a subclass is invoking the method.
+
+Note:
+We do not hold PyObject references so no reference cycles should exist. As such,
+we do not set Py_TPFLAGS_HAVE_GC nor define either tp_traverse or tp_clear.
+
+There is still some ongoing debate about this this use case however:
+  https://github.com/python/cpython/issues/116946
+
+Note:
+If we allow subclassing in the future, reconsider use of PyObject_Free vs using
+the function defined in the tp_free slot.
+
+See also:
+  https://github.com/python/cpython/pull/138329#issuecomment-3242079564
+  https://github.com/python/cpython/issues/116946#issuecomment-3242135537
+  https://github.com/python/cpython/pull/139073
+  https://github.com/python/cpython/commit/ec689187957cc80af56b9a63251bbc295bafd781
+*/
+static PyType_Spec chip_type_spec = {
+	.name = "gpiosim.Chip",
+	.basicsize = sizeof(chip_object),
+	.flags = (Py_TPFLAGS_DEFAULT | Py_TPFLAGS_IMMUTABLETYPE),
+	.slots = chip_type_slots,
+};
+
+static int module_exec(PyObject *module)
 {
 	const struct module_const *modconst;
 	struct module_state *state;
-	PyObject *module;
+	PyObject *chip_type_obj;
 	int ret;
 
-	module = PyModule_Create(&module_def);
-	if (!module)
-		return NULL;
-
-	ret = PyState_AddModule(module, &module_def);
-	if (ret) {
-		Py_DECREF(module);
-		return NULL;
-	}
-
 	state = PyModule_GetState(module);
 
 	state->sim_ctx = gpiosim_ctx_new();
 	if (!state->sim_ctx) {
-		Py_DECREF(module);
-		return PyErr_SetFromErrno(PyExc_OSError);
+		PyErr_SetFromErrno(PyExc_OSError);
+		return -1;
 	}
 
-	ret = PyModule_AddType(module, &chip_type);
-	if (ret) {
-		Py_DECREF(module);
-		return NULL;
-	}
+	chip_type_obj = PyType_FromModuleAndSpec(module, &chip_type_spec, NULL);
+
+	if (!chip_type_obj)
+		return -1;
+
+	ret = PyModule_AddType(module, (PyTypeObject*)chip_type_obj);
+	Py_DECREF(chip_type_obj);
+	if (ret < 0)
+		return -1;
 
 	for (modconst = module_constants; modconst->name; modconst++) {
-		ret = PyModule_AddIntConstant(module,
-					      modconst->name, modconst->val);
-		if (ret) {
-			Py_DECREF(module);
-			return NULL;
-		}
+		ret = PyModule_AddIntConstant(module, modconst->name,
+					      modconst->val);
+		if (ret < 0)
+			return -1;
+	}
+
+	return 0;
+}
+
+static void free_module_state(void *mod)
+{
+	struct module_state *state = PyModule_GetState((PyObject *)mod);
+
+	if (state->sim_ctx) {
+		gpiosim_ctx_unref(state->sim_ctx);
+		state->sim_ctx = NULL;
 	}
+}
+
+static struct PyModuleDef_Slot module_slots[] = {
+	{Py_mod_exec, module_exec},
+	{0, NULL},
+};
+
+static PyModuleDef module_def = {
+	PyModuleDef_HEAD_INIT,
+	.m_name = "gpiosim._ext",
+	.m_size = sizeof(struct module_state),
+	.m_free = free_module_state,
+	.m_slots = module_slots,
+};
 
-	return module;
+PyMODINIT_FUNC PyInit__ext(void)
+{
+	return PyModuleDef_Init(&module_def);
 }
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* Re: [libgpiod][PATCH v2 7/8] bindings: python: tests: migrate the system module to multi-phase init
  2026-04-23 22:21 ` [libgpiod][PATCH v2 7/8] bindings: python: tests: migrate the system " Vincent Fazio
@ 2026-04-24  9:03   ` Bartosz Golaszewski
  2026-04-24  9:05     ` Bartosz Golaszewski
  0 siblings, 1 reply; 13+ messages in thread
From: Bartosz Golaszewski @ 2026-04-24  9:03 UTC (permalink / raw)
  To: Vincent Fazio; +Cc: linux-gpio

On Fri, Apr 24, 2026 at 12:21 AM Vincent Fazio <vfazio@gmail.com> wrote:
>
> Single-phase initialization has been classified as legacy within CPython
> documentation [0] with multi-phase being its successor [1].
>
> As such, switch to the new methodology.
>
> [0]: https://docs.python.org/3/c-api/extension-modules.html#legacy-single-phase-initialization
> [1]: https://docs.python.org/3/c-api/extension-modules.html#multi-phase-initialization
>
> Signed-off-by: Vincent Fazio <vfazio@gmail.com>
> ---
>  bindings/python/tests/system/ext.c | 7 ++++++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
>
> diff --git a/bindings/python/tests/system/ext.c b/bindings/python/tests/system/ext.c
> index c982fa6..800648e 100644
> --- a/bindings/python/tests/system/ext.c
> +++ b/bindings/python/tests/system/ext.c
> @@ -67,13 +67,18 @@ static PyMethodDef module_methods[] = {
>         { }
>  };
>
> +static struct PyModuleDef_Slot module_slots[] = {
> +       {0, NULL},
> +};

Just an inconsistency I noticed while I was about to pick it up, why
do we have a NULL sentinel in the gpiod extension but not here?

Bart

> +
>  static PyModuleDef module_def = {
>         PyModuleDef_HEAD_INIT,
>         .m_name = "system._ext",
>         .m_methods = module_methods,
> +       .m_slots = module_slots,
>  };
>
>  PyMODINIT_FUNC PyInit__ext(void)
>  {
> -       return PyModule_Create(&module_def);
> +       return PyModuleDef_Init(&module_def);
>  }
> --
> 2.43.0
>

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [libgpiod][PATCH v2 7/8] bindings: python: tests: migrate the system module to multi-phase init
  2026-04-24  9:03   ` Bartosz Golaszewski
@ 2026-04-24  9:05     ` Bartosz Golaszewski
  2026-04-24  9:07       ` Bartosz Golaszewski
  0 siblings, 1 reply; 13+ messages in thread
From: Bartosz Golaszewski @ 2026-04-24  9:05 UTC (permalink / raw)
  To: Vincent Fazio; +Cc: linux-gpio

On Fri, Apr 24, 2026 at 11:03 AM Bartosz Golaszewski <brgl@kernel.org> wrote:
>
> On Fri, Apr 24, 2026 at 12:21 AM Vincent Fazio <vfazio@gmail.com> wrote:
> >
> > Single-phase initialization has been classified as legacy within CPython
> > documentation [0] with multi-phase being its successor [1].
> >
> > As such, switch to the new methodology.
> >
> > [0]: https://docs.python.org/3/c-api/extension-modules.html#legacy-single-phase-initialization
> > [1]: https://docs.python.org/3/c-api/extension-modules.html#multi-phase-initialization
> >
> > Signed-off-by: Vincent Fazio <vfazio@gmail.com>
> > ---
> >  bindings/python/tests/system/ext.c | 7 ++++++-
> >  1 file changed, 6 insertions(+), 1 deletion(-)
> >
> > diff --git a/bindings/python/tests/system/ext.c b/bindings/python/tests/system/ext.c
> > index c982fa6..800648e 100644
> > --- a/bindings/python/tests/system/ext.c
> > +++ b/bindings/python/tests/system/ext.c
> > @@ -67,13 +67,18 @@ static PyMethodDef module_methods[] = {
> >         { }
> >  };
> >
> > +static struct PyModuleDef_Slot module_slots[] = {
> > +       {0, NULL},
> > +};
>
> Just an inconsistency I noticed while I was about to pick it up, why
> do we have a NULL sentinel in the gpiod extension but not here?
>

Docs are pretty clear on this so let me change that when applying.

https://docs.python.org/3/c-api/module.html#c.PyModuleDef.m_slots

Bart

> Bart
>
> > +
> >  static PyModuleDef module_def = {
> >         PyModuleDef_HEAD_INIT,
> >         .m_name = "system._ext",
> >         .m_methods = module_methods,
> > +       .m_slots = module_slots,
> >  };
> >
> >  PyMODINIT_FUNC PyInit__ext(void)
> >  {
> > -       return PyModule_Create(&module_def);
> > +       return PyModuleDef_Init(&module_def);
> >  }
> > --
> > 2.43.0
> >

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [libgpiod][PATCH v2 7/8] bindings: python: tests: migrate the system module to multi-phase init
  2026-04-24  9:05     ` Bartosz Golaszewski
@ 2026-04-24  9:07       ` Bartosz Golaszewski
  0 siblings, 0 replies; 13+ messages in thread
From: Bartosz Golaszewski @ 2026-04-24  9:07 UTC (permalink / raw)
  To: Vincent Fazio; +Cc: linux-gpio

On Fri, Apr 24, 2026 at 11:05 AM Bartosz Golaszewski <brgl@kernel.org> wrote:
>
> On Fri, Apr 24, 2026 at 11:03 AM Bartosz Golaszewski <brgl@kernel.org> wrote:
> >
> > On Fri, Apr 24, 2026 at 12:21 AM Vincent Fazio <vfazio@gmail.com> wrote:
> > >
> > > Single-phase initialization has been classified as legacy within CPython
> > > documentation [0] with multi-phase being its successor [1].
> > >
> > > As such, switch to the new methodology.
> > >
> > > [0]: https://docs.python.org/3/c-api/extension-modules.html#legacy-single-phase-initialization
> > > [1]: https://docs.python.org/3/c-api/extension-modules.html#multi-phase-initialization
> > >
> > > Signed-off-by: Vincent Fazio <vfazio@gmail.com>
> > > ---
> > >  bindings/python/tests/system/ext.c | 7 ++++++-
> > >  1 file changed, 6 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/bindings/python/tests/system/ext.c b/bindings/python/tests/system/ext.c
> > > index c982fa6..800648e 100644
> > > --- a/bindings/python/tests/system/ext.c
> > > +++ b/bindings/python/tests/system/ext.c
> > > @@ -67,13 +67,18 @@ static PyMethodDef module_methods[] = {
> > >         { }
> > >  };
> > >
> > > +static struct PyModuleDef_Slot module_slots[] = {
> > > +       {0, NULL},
> > > +};
> >
> > Just an inconsistency I noticed while I was about to pick it up, why
> > do we have a NULL sentinel in the gpiod extension but not here?
> >
>
> Docs are pretty clear on this so let me change that when applying.
>
> https://docs.python.org/3/c-api/module.html#c.PyModuleDef.m_slots
>
> Bart

Sorry for the noise and yes, I am sleep deprived. My brain
misinterpreted the 0, NULL for an actual slot entry. Nevermind these
messages.

Bart

>
> > Bart
> >
> > > +
> > >  static PyModuleDef module_def = {
> > >         PyModuleDef_HEAD_INIT,
> > >         .m_name = "system._ext",
> > >         .m_methods = module_methods,
> > > +       .m_slots = module_slots,
> > >  };
> > >
> > >  PyMODINIT_FUNC PyInit__ext(void)
> > >  {
> > > -       return PyModule_Create(&module_def);
> > > +       return PyModuleDef_Init(&module_def);
> > >  }
> > > --
> > > 2.43.0
> > >

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [libgpiod][PATCH v2 0/8] bindings: python: modernize C extensions
  2026-04-23 22:21 [libgpiod][PATCH v2 0/8] bindings: python: modernize C extensions Vincent Fazio
                   ` (7 preceding siblings ...)
  2026-04-23 22:21 ` [libgpiod][PATCH v2 8/8] bindings: python: tests: migrate the gpiosim " Vincent Fazio
@ 2026-04-24  9:14 ` Bartosz Golaszewski
  8 siblings, 0 replies; 13+ messages in thread
From: Bartosz Golaszewski @ 2026-04-24  9:14 UTC (permalink / raw)
  To: linux-gpio, Vincent Fazio; +Cc: Bartosz Golaszewski, brgl


On Thu, 23 Apr 2026 17:21:17 -0500, Vincent Fazio wrote:
> This series performs some minor cleanup of the C extension modules and
> migrates the module defintions to multi-phase intitialization (PEP 489).
> 
> Patches 1 & 2 avoid calling back into python to perform C level object
> cleanup.
> 
> Patch 3 introduces no real functional change but simplifies the code to
> get the same result.
> 
> [...]

Applied, thanks!

[1/8] bindings: python: avoid PyObject_CallMethod during chip finalize
      https://git.kernel.org/brgl/c/53be72baca59ef4488f8c998d9cf6b172ad9aa00
[2/8] bindings: python: avoid PyObject_CallMethod during request finalize
      https://git.kernel.org/brgl/c/e5b6f6ecb9f81097129d6d05b5e6b2a1ba75d8d2
[3/8] bindings: python: simplify disallowing _ext.Request from being created
      https://git.kernel.org/brgl/c/55ce0cf68158bb61ea1ed52d4a827e7759beba6d
[4/8] bindings: python: use suggestions from upgrade_pythoncapi.py
      https://git.kernel.org/brgl/c/63d4fd7ad704073f6e629fdb66c4302cf398f502
[5/8] bindings: python: use PyImport_ImportModuleAttrString when available
      https://git.kernel.org/brgl/c/9106b6c190e9852157ef0e4f0683823862a289ff
[6/8] bindings: python: migrate the gpiod._ext module to multi-phase init
      https://git.kernel.org/brgl/c/89ffee834920bf7daa85b529ca9877b90d4d75e1
[7/8] bindings: python: tests: migrate the system module to multi-phase init
      https://git.kernel.org/brgl/c/bf4211f3e19bf6acea2e3a49dfd46a734dc90a41
[8/8] bindings: python: tests: migrate the gpiosim module to multi-phase init
      https://git.kernel.org/brgl/c/7acbbcf7789719f577d0ddba2298167ea73c57ee

Best regards,
-- 
Bartosz Golaszewski <bartosz.golaszewski@oss.qualcomm.com>

^ permalink raw reply	[flat|nested] 13+ messages in thread

end of thread, other threads:[~2026-04-24  9:15 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-04-23 22:21 [libgpiod][PATCH v2 0/8] bindings: python: modernize C extensions Vincent Fazio
2026-04-23 22:21 ` [libgpiod][PATCH v2 1/8] bindings: python: avoid PyObject_CallMethod during chip finalize Vincent Fazio
2026-04-23 22:21 ` [libgpiod][PATCH v2 2/8] bindings: python: avoid PyObject_CallMethod during request finalize Vincent Fazio
2026-04-23 22:21 ` [libgpiod][PATCH v2 3/8] bindings: python: simplify disallowing _ext.Request from being created Vincent Fazio
2026-04-23 22:21 ` [libgpiod][PATCH v2 4/8] bindings: python: use suggestions from upgrade_pythoncapi.py Vincent Fazio
2026-04-23 22:21 ` [libgpiod][PATCH v2 5/8] bindings: python: use PyImport_ImportModuleAttrString when available Vincent Fazio
2026-04-23 22:21 ` [libgpiod][PATCH v2 6/8] bindings: python: migrate the gpiod._ext module to multi-phase init Vincent Fazio
2026-04-23 22:21 ` [libgpiod][PATCH v2 7/8] bindings: python: tests: migrate the system " Vincent Fazio
2026-04-24  9:03   ` Bartosz Golaszewski
2026-04-24  9:05     ` Bartosz Golaszewski
2026-04-24  9:07       ` Bartosz Golaszewski
2026-04-23 22:21 ` [libgpiod][PATCH v2 8/8] bindings: python: tests: migrate the gpiosim " Vincent Fazio
2026-04-24  9:14 ` [libgpiod][PATCH v2 0/8] bindings: python: modernize C extensions Bartosz Golaszewski

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox