Re: [Libguestfs] [PATCH 1/2] python: Avoid crash if callback parameters cannot be built
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 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