On Mon, Feb 20, 2023 at 09:45:33AM +0100, Laszlo Ersek wrote: > On 2/17/23 17:52, Eric Blake wrote: > > On Thu, Feb 16, 2023 at 03:09:02PM +0100, Laszlo Ersek wrote: > > >> - Py_BuildValue with the "O" format specifier transfers the new list's > >> *sole* reference (= ownership) to the just-built higher-level object "args" > > > > Reference transfer is done with "N", not "O". That would be an > > alternative to decreasing the refcount of py_array on success, but not > > eliminate the need to decrease the refcount on Py_BuildValue failure. > > > >> > >> - when "args" is killed (decref'd), it takes care of "py_array". > >> > >> Consequently, if Py_BuildValue fails, "py_array" continues owning the > >> new list -- and I believe that, if we take the new error branch, we leak > >> the object pointed-to by "py_array". Is that the case? > > > > Not quite. "O" is different than "N". > > I agree with you *now*, looking up the "O" specification at > <https://docs.python.org/3/c-api/arg.html#building-values>. > > However, when I was writing my email, I looked up Py_BuildValue at that > time as well, just elsewhere. I don't know where. Really. And then that > documentation said that the reference count would *not* be increased. I > distinctly remember that, because it surprised me -- I actually recalled > an *even earlier* experience reading the documentation, which had again > stated that "O" would increase the reference count. > > So right now, I have three (inconsistent) memories: > > - original (old) memory: "O" increments the refcount > - recent memory: "O" does not increment the refcount > - your reminder: "O" does increment the refcount
I looked at the source, and 'O' definitely increments the refcount whereas 'N' does not. See: https://github.com/python/cpython/blob/b1b375e2670a58fc37cb4c2629ed73b045159918/Python/modsupport.c#L463 and: https://github.com/python/cpython/blob/b1b375e2670a58fc37cb4c2629ed73b045159918/Python/modsupport.c#L475 Rich. > I guess I must have misread something (I can't find the document now!). > Sorry about that; I agree we need to drop the original py_array > reference unconditionally. > > Laszlo -- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones Read my programming and virtualization blog: http://rwmj.wordpress.com nbdkit - Flexible, fast NBD server with plugins https://gitlab.com/nbdkit/nbdkit _______________________________________________ Libguestfs mailing list Libguestfs@redhat.com https://listman.redhat.com/mailman/listinfo/libguestfs
