On 2/17/23 18:39, Richard W.M. Jones wrote: > Version 1 was here: > > https://listman.redhat.com/archives/libguestfs/2023-February/030732.html > > Following Eric's suggestion here: > > https://listman.redhat.com/archives/libguestfs/2023-February/030746.html > > let's decrement the reference of py_array right after adding it to the > args. (This works even if args fails to be built).
I agree; (not having seen your new patch) I'd come to the same conclusion after reading Eric's comment. We need to drop the initial reference on py_array unconditionally -- if we succeed constructing the tuple, it takes ownership, so we should drop our original (temporary) reference; and if the tuple construction fails, there's nobody to foist py_array upon, so we need to drop it just the same. > > However the other part of Eric's suggestion is wrong as it ends up > calling Py_DECREF (args) when args == NULL along the error path. This > lead me to look more closely at this patch: > > https://listman.redhat.com/archives/libguestfs/2019-January/020346.html > > which I believe is wrong (at least, the part that fiddles with the > reference to args). I cannot reproduce the original problem, nor can > I find any justification by looking at the documentation of > PyObject_CallObject. > > So we start by reverting that commit. Yup, when I first looked at your v1 patch, git-blame had led me to commit 85235aec8377 ("python: fix call of Python handlers of events", 2019-01-22), and I found the Py_INCREF() call dubious. Laszlo _______________________________________________ Libguestfs mailing list Libguestfs@redhat.com https://listman.redhat.com/mailman/listinfo/libguestfs
