diff mbox series

[libgpiod] bindings: python: replace PyModule_AddObjectRef() with PyModule_AddObjectRef()

Message ID 20231009190729.38675-1-brgl@bgdev.pl
State New
Headers show
Series [libgpiod] bindings: python: replace PyModule_AddObjectRef() with PyModule_AddObjectRef() | expand

Commit Message

Bartosz Golaszewski Oct. 9, 2023, 7:07 p.m. UTC 
From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>

PyModule_AddObjectRef() was added in cpython v3.10 while libgpiod claims
to depend on python v3.9. Replace it with an older variant that steals the
reference to the added object on success.

Reported-by: Phil Howard <phil@gadgetoid.com>
Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
---
 bindings/python/gpiod/ext/module.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

Comments

Phil Howard Oct. 9, 2023, 8:59 p.m. UTC | #1 
On Mon, 9 Oct 2023 at 20:07, Bartosz Golaszewski <brgl@bgdev.pl> wrote:
>
> From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
>
> PyModule_AddObjectRef() was added in cpython v3.10 while libgpiod claims
> to depend on python v3.9. Replace it with an older variant that steals the
> reference to the added object on success.

Ah, fixing this makes much more sense than bumping the dependent
version, thank you, I will update my patch.

>
> Reported-by: Phil Howard <phil@gadgetoid.com>
> Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
> ---
>  bindings/python/gpiod/ext/module.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/bindings/python/gpiod/ext/module.c b/bindings/python/gpiod/ext/module.c
> index 25c252a..b456190 100644
> --- a/bindings/python/gpiod/ext/module.c
> +++ b/bindings/python/gpiod/ext/module.c
> @@ -178,9 +178,9 @@ PyMODINIT_FUNC PyInit__ext(void)
>                 return NULL;
>         }
>
> -       ret = PyModule_AddObjectRef(module, "__all__", all);
> -       Py_DECREF(all);
> +       ret = PyModule_AddObject(module, "__all__", all);
>         if (ret) {
> +               Py_DECREF(all);
>                 Py_DECREF(module);
>                 return NULL;
>         }
> --
> 2.39.2
>
Bartosz Golaszewski Oct. 10, 2023, 6:31 a.m. UTC | #2 
On Mon, Oct 9, 2023 at 11:00 PM Phil Howard <phil@gadgetoid.com> wrote:
>
> On Mon, 9 Oct 2023 at 20:07, Bartosz Golaszewski <brgl@bgdev.pl> wrote:
> >
> > From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
> >
> > PyModule_AddObjectRef() was added in cpython v3.10 while libgpiod claims
> > to depend on python v3.9. Replace it with an older variant that steals the
> > reference to the added object on success.
>
> Ah, fixing this makes much more sense than bumping the dependent
> version, thank you, I will update my patch.
>

And of course the commit message was supposed to read: "bindings:
python: replace PyModule_AddObjectRef() with PyModule_AddObject()".

I'll fix it when applying.

Bart

> >
> > Reported-by: Phil Howard <phil@gadgetoid.com>
> > Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
> > ---
> >  bindings/python/gpiod/ext/module.c | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/bindings/python/gpiod/ext/module.c b/bindings/python/gpiod/ext/module.c
> > index 25c252a..b456190 100644
> > --- a/bindings/python/gpiod/ext/module.c
> > +++ b/bindings/python/gpiod/ext/module.c
> > @@ -178,9 +178,9 @@ PyMODINIT_FUNC PyInit__ext(void)
> >                 return NULL;
> >         }
> >
> > -       ret = PyModule_AddObjectRef(module, "__all__", all);
> > -       Py_DECREF(all);
> > +       ret = PyModule_AddObject(module, "__all__", all);
> >         if (ret) {
> > +               Py_DECREF(all);
> >                 Py_DECREF(module);
> >                 return NULL;
> >         }
> > --
> > 2.39.2
> >
Phil Howard Oct. 10, 2023, 9:50 a.m. UTC | #3 
On Tue, 10 Oct 2023 at 07:31, Bartosz Golaszewski <brgl@bgdev.pl> wrote:
>
> On Mon, Oct 9, 2023 at 11:00 PM Phil Howard <phil@gadgetoid.com> wrote:
> >
> > On Mon, 9 Oct 2023 at 20:07, Bartosz Golaszewski <brgl@bgdev.pl> wrote:
> > >
> > > From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
> > >
> > > PyModule_AddObjectRef() was added in cpython v3.10 while libgpiod claims
> > > to depend on python v3.9. Replace it with an older variant that steals the

I've done some digging and it looks like "PyModule_AddType" [1] helper
function is what's
anchoring the library to Python 3.9 (in the stable ABI as of 3.10)
rather than 3.8.

Is that correct? If so I'll update my patch to reflect the use of
PyModule_AddType.

The raw Python part of the library is compatible back to 3.7 (EOL),
3.8 EOL is this time next year so going < 3.9 is probably not worth the effort.

1. https://github.com/python/cpython/blob/43a6e4fa4934fcc0cbd83f7f3dc1b23a5f79f24b/Python/modsupport.c#L686-L703

> > > reference to the added object on success.
> >
> > Ah, fixing this makes much more sense than bumping the dependent
> > version, thank you, I will update my patch.
> >
>
> And of course the commit message was supposed to read: "bindings:
> python: replace PyModule_AddObjectRef() with PyModule_AddObject()".
>
> I'll fix it when applying.
>
> Bart
>
> > >
> > > Reported-by: Phil Howard <phil@gadgetoid.com>
> > > Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
> > > ---
> > >  bindings/python/gpiod/ext/module.c | 4 ++--
> > >  1 file changed, 2 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/bindings/python/gpiod/ext/module.c b/bindings/python/gpiod/ext/module.c
> > > index 25c252a..b456190 100644
> > > --- a/bindings/python/gpiod/ext/module.c
> > > +++ b/bindings/python/gpiod/ext/module.c
> > > @@ -178,9 +178,9 @@ PyMODINIT_FUNC PyInit__ext(void)
> > >                 return NULL;
> > >         }
> > >
> > > -       ret = PyModule_AddObjectRef(module, "__all__", all);
> > > -       Py_DECREF(all);
> > > +       ret = PyModule_AddObject(module, "__all__", all);
> > >         if (ret) {
> > > +               Py_DECREF(all);
> > >                 Py_DECREF(module);
> > >                 return NULL;
> > >         }
> > > --
> > > 2.39.2
> > >
Bartosz Golaszewski Oct. 10, 2023, 10:01 a.m. UTC | #4 
On Tue, 10 Oct 2023 at 11:50, Phil Howard <phil@gadgetoid.com> wrote:
>
> On Tue, 10 Oct 2023 at 07:31, Bartosz Golaszewski <brgl@bgdev.pl> wrote:
> >
> > On Mon, Oct 9, 2023 at 11:00 PM Phil Howard <phil@gadgetoid.com> wrote:
> > >
> > > On Mon, 9 Oct 2023 at 20:07, Bartosz Golaszewski <brgl@bgdev.pl> wrote:
> > > >
> > > > From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
> > > >
> > > > PyModule_AddObjectRef() was added in cpython v3.10 while libgpiod claims
> > > > to depend on python v3.9. Replace it with an older variant that steals the
>
> I've done some digging and it looks like "PyModule_AddType" [1] helper
> function is what's
> anchoring the library to Python 3.9 (in the stable ABI as of 3.10)
> rather than 3.8.
>
> Is that correct? If so I'll update my patch to reflect the use of
> PyModule_AddType.
>
> The raw Python part of the library is compatible back to 3.7 (EOL),
> 3.8 EOL is this time next year so going < 3.9 is probably not worth the effort.
>

Agreed, and most distros ship >=v3.9 anyway.

Bart

> 1. https://github.com/python/cpython/blob/43a6e4fa4934fcc0cbd83f7f3dc1b23a5f79f24b/Python/modsupport.c#L686-L703
>
> > > > reference to the added object on success.
> > >
> > > Ah, fixing this makes much more sense than bumping the dependent
> > > version, thank you, I will update my patch.
> > >
> >
> > And of course the commit message was supposed to read: "bindings:
> > python: replace PyModule_AddObjectRef() with PyModule_AddObject()".
> >
> > I'll fix it when applying.
> >
> > Bart
> >
> > > >
> > > > Reported-by: Phil Howard <phil@gadgetoid.com>
> > > > Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
> > > > ---
> > > >  bindings/python/gpiod/ext/module.c | 4 ++--
> > > >  1 file changed, 2 insertions(+), 2 deletions(-)
> > > >
> > > > diff --git a/bindings/python/gpiod/ext/module.c b/bindings/python/gpiod/ext/module.c
> > > > index 25c252a..b456190 100644
> > > > --- a/bindings/python/gpiod/ext/module.c
> > > > +++ b/bindings/python/gpiod/ext/module.c
> > > > @@ -178,9 +178,9 @@ PyMODINIT_FUNC PyInit__ext(void)
> > > >                 return NULL;
> > > >         }
> > > >
> > > > -       ret = PyModule_AddObjectRef(module, "__all__", all);
> > > > -       Py_DECREF(all);
> > > > +       ret = PyModule_AddObject(module, "__all__", all);
> > > >         if (ret) {
> > > > +               Py_DECREF(all);
> > > >                 Py_DECREF(module);
> > > >                 return NULL;
> > > >         }
> > > > --
> > > > 2.39.2
> > > >
diff mbox series

Patch

diff --git a/bindings/python/gpiod/ext/module.c b/bindings/python/gpiod/ext/module.c
index 25c252a..b456190 100644
--- a/bindings/python/gpiod/ext/module.c
+++ b/bindings/python/gpiod/ext/module.c
@@ -178,9 +178,9 @@  PyMODINIT_FUNC PyInit__ext(void)
 		return NULL;
 	}
 
-	ret = PyModule_AddObjectRef(module, "__all__", all);
-	Py_DECREF(all);
+	ret = PyModule_AddObject(module, "__all__", all);
 	if (ret) {
+		Py_DECREF(all);
 		Py_DECREF(module);
 		return NULL;
 	}