bindings: python: import gpiod attributes in external module

Previously, the external module relied on gpiod attributes being present
within `globals()` to construct return values back to the caller.

This assumption required callers of the external module to have imported
the attributes to populate `globals()` for the interface to work.

Having this implicit contract is opaque and prone to causing issues if
imports within gpiod modules ever get reworked.

Now, the external module explicitly imports attributes from gpiod in
order to return values back to the caller.

There should be no concern about circular imports as the external module
should be completely imported by the time callers call into it.

Since Py_gpiod_GetGlobalType is no longer used, it has been replaced
with Py_gpiod_GetModuleAttrString which returns a new PyObject*
reference for the named module and attribute that must be decremented
when no longer in use.

Closes: https://github.com/brgl/libgpiod/issues/101
Signed-off-by: Vincent Fazio <vfazio@xes-inc.com>
Link: https://lore.kernel.org/r/20241008175139.1198980-1-vfazio@xes-inc.com
Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>

diff --git a/bindings/python/gpiod/ext/chip.c b/bindings/python/gpiod/ext/chip.c
index e8eaad8a3e4a8046ba6689e7c40f1b1ec515a930..1e97d7d2c72e17c3f4860c86dcac49d32259c706 100644 (file)
--- a/bindings/python/gpiod/ext/chip.c
+++ b/bindings/python/gpiod/ext/chip.c
@@ -75,31 +75,34 @@ static PyObject *chip_get_info(chip_object *self, PyObject *Py_UNUSED(ignored))
        struct gpiod_chip_info *info;
        PyObject *type, *ret;
 
-       type = Py_gpiod_GetGlobalType("ChipInfo");
+       type = Py_gpiod_GetModuleAttrString("gpiod.chip_info", "ChipInfo");
        if (!type)
                return NULL;
 
        info = gpiod_chip_get_info(self->chip);
-       if (!info)
+       if (!info) {
+               Py_DECREF(type);
                return PyErr_SetFromErrno(PyExc_OSError);
+       }
 
-        ret = PyObject_CallFunction(type, "ssI",
-                                    gpiod_chip_info_get_name(info),
-                                    gpiod_chip_info_get_label(info),
-                                    gpiod_chip_info_get_num_lines(info));
-        gpiod_chip_info_free(info);
-        return ret;
+       ret = PyObject_CallFunction(type, "ssI",
+                                   gpiod_chip_info_get_name(info),
+                                   gpiod_chip_info_get_label(info),
+                                   gpiod_chip_info_get_num_lines(info));
+       gpiod_chip_info_free(info);
+       Py_DECREF(type);
+       return ret;
 }
 
 static PyObject *make_line_info(struct gpiod_line_info *info)
 {
-       PyObject *type;
+       PyObject *type, *ret;
 
-       type = Py_gpiod_GetGlobalType("LineInfo");
+       type = Py_gpiod_GetModuleAttrString("gpiod.line_info", "LineInfo");
        if (!type)
                return NULL;
 
-       return PyObject_CallFunction(type, "IsOsiOiiiiOk",
+       ret = PyObject_CallFunction(type, "IsOsiOiiiiOk",
                                gpiod_line_info_get_offset(info),
                                gpiod_line_info_get_name(info),
                                gpiod_line_info_is_used(info) ?
@@ -115,6 +118,8 @@ static PyObject *make_line_info(struct gpiod_line_info *info)
                                gpiod_line_info_is_debounced(info) ?
                                                        Py_True : Py_False,
                                gpiod_line_info_get_debounce_period_us(info));
+       Py_DECREF(type);
+       return ret;
 }
 
 static PyObject *chip_get_line_info(chip_object *self, PyObject *args)
@@ -168,10 +173,6 @@ chip_read_info_event(chip_object *self, PyObject *Py_UNUSED(ignored))
        struct gpiod_info_event *event;
        struct gpiod_line_info *info;
 
-       type = Py_gpiod_GetGlobalType("InfoEvent");
-       if (!type)
-               return NULL;
-
        Py_BEGIN_ALLOW_THREADS;
        event = gpiod_chip_read_info_event(self->chip);
        Py_END_ALLOW_THREADS;
@@ -186,11 +187,19 @@ chip_read_info_event(chip_object *self, PyObject *Py_UNUSED(ignored))
                return NULL;
        }
 
+       type = Py_gpiod_GetModuleAttrString("gpiod.info_event", "InfoEvent");
+       if (!type) {
+               Py_DECREF(info_obj);
+               gpiod_info_event_free(event);
+               return NULL;
+       }
+
        event_obj = PyObject_CallFunction(type, "iKO",
                                gpiod_info_event_get_event_type(event),
                                gpiod_info_event_get_timestamp_ns(event),
                                info_obj);
        Py_DECREF(info_obj);
+       Py_DECREF(type);
        gpiod_info_event_free(event);
        return event_obj;
 }

diff --git a/bindings/python/gpiod/ext/common.c b/bindings/python/gpiod/ext/common.c
index 07fca8c28c97685bd69cab32ca902923303448ee..62201b669248fce6a278a523f58590ee7cf4920a 100644 (file)
--- a/bindings/python/gpiod/ext/common.c
+++ b/bindings/python/gpiod/ext/common.c
@@ -64,15 +64,19 @@ PyObject *Py_gpiod_SetErrFromErrno(void)
        return PyErr_SetFromErrno(exc);
 }
 
-PyObject *Py_gpiod_GetGlobalType(const char *type_name)
+PyObject *Py_gpiod_GetModuleAttrString(const char *modname,
+                                      const char *attrname)
 {
-       PyObject *globals;
+       PyObject *module, *attribute;
 
-       globals = PyEval_GetGlobals();
-       if (!globals)
+       module = PyImport_ImportModule(modname);
+       if (!module)
                return NULL;
 
-       return PyDict_GetItemString(globals, type_name);
+       attribute = PyObject_GetAttrString(module, attrname);
+       Py_DECREF(module);
+
+       return attribute;
 }
 
 unsigned int Py_gpiod_PyLongAsUnsignedInt(PyObject *pylong)

diff --git a/bindings/python/gpiod/ext/internal.h b/bindings/python/gpiod/ext/internal.h
index 7d223c014f7c99bb37941dd5ade1a87076495d50..15aedfb046c25d8cd295a790263385d3d41224f3 100644 (file)
--- a/bindings/python/gpiod/ext/internal.h
+++ b/bindings/python/gpiod/ext/internal.h
@@ -8,7 +8,8 @@
 #include <Python.h>
 
 PyObject *Py_gpiod_SetErrFromErrno(void);
-PyObject *Py_gpiod_GetGlobalType(const char *type_name);
+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,

diff --git a/bindings/python/gpiod/ext/request.c b/bindings/python/gpiod/ext/request.c
index 5db69fe8a21161d21743c49355c04f624481d2df..e1a2a42e475f67452b06d8eacd19f004c4fa6a5e 100644 (file)
--- a/bindings/python/gpiod/ext/request.c
+++ b/bindings/python/gpiod/ext/request.c
@@ -149,10 +149,6 @@ static PyObject *request_get_values(request_object *self, PyObject *args)
        if (num_offsets < 0)
                return NULL;
 
-       type = Py_gpiod_GetGlobalType("Value");
-       if (!type)
-               return NULL;
-
        iter = PyObject_GetIter(offsets);
        if (!iter)
                return NULL;
@@ -183,18 +179,26 @@ static PyObject *request_get_values(request_object *self, PyObject *args)
        if (ret)
                return Py_gpiod_SetErrFromErrno();
 
+       type = Py_gpiod_GetModuleAttrString("gpiod.line", "Value");
+       if (!type)
+               return NULL;
+
        for (pos = 0; pos < num_offsets; pos++) {
                val = PyObject_CallFunction(type, "i", self->values[pos]);
-               if (!val)
+               if (!val) {
+                       Py_DECREF(type);
                        return NULL;
+               }
 
                ret = PyList_SetItem(values, pos, val);
                if (ret) {
                        Py_DECREF(val);
+                       Py_DECREF(type);
                        return NULL;
                }
        }
 
+       Py_DECREF(type);
        Py_RETURN_NONE;
 }
 
@@ -279,10 +283,6 @@ static PyObject *request_read_edge_events(request_object *self, PyObject *args)
                max_events = 64;
        }
 
-       type = Py_gpiod_GetGlobalType("EdgeEvent");
-       if (!type)
-               return NULL;
-
        Py_BEGIN_ALLOW_THREADS;
        ret = gpiod_line_request_read_edge_events(self->request,
                                                 self->buffer, max_events);
@@ -296,10 +296,17 @@ static PyObject *request_read_edge_events(request_object *self, PyObject *args)
        if (!events)
                return NULL;
 
+       type = Py_gpiod_GetModuleAttrString("gpiod.edge_event", "EdgeEvent");
+       if (!type) {
+               Py_DECREF(events);
+               return NULL;
+       }
+
        for (i = 0; i < num_events; i++) {
                event = gpiod_edge_event_buffer_get_event(self->buffer, i);
                if (!event) {
                        Py_DECREF(events);
+                       Py_DECREF(type);
                        return NULL;
                }
 
@@ -311,6 +318,7 @@ static PyObject *request_read_edge_events(request_object *self, PyObject *args)
                                gpiod_edge_event_get_line_seqno(event));
                if (!event_obj) {
                        Py_DECREF(events);
+                       Py_DECREF(type);
                        return NULL;
                }
 
@@ -318,10 +326,12 @@ static PyObject *request_read_edge_events(request_object *self, PyObject *args)
                if (ret) {
                        Py_DECREF(event_obj);
                        Py_DECREF(events);
+                       Py_DECREF(type);
                        return NULL;
                }
        }
 
+       Py_DECREF(type);
        return events;
 }