[Libguestfs] [libnbd PATCH] python: Plug some memory leaks on error paths

[Eric Blake]

On 9/8/20 6:11 PM, Eric Blake wrote:
> Inspection of the generated code showed several places where we did
> not release references on all error paths.  In particular, switching
> things to always jump to an out: label, even when the underlying C
> function cannot fail, makes it easier to clean up when the user passes
> wrong types to a function call.
> 
> One of the easiest triggers without this patch was this one-liner:
> $ nbdsh -c 'h.block_status(512, 0, 1)'
> 
> which fails due to '1' not being callable, but leaked malloc'd memory
> in the process.
> ---
>   generator/Python.ml | 55 +++++++++++++++++++++++++++++----------------
>   1 file changed, 36 insertions(+), 19 deletions(-)
> 

> @@ -395,12 +396,13 @@ let print_python_binding name { args; optargs; ret; may_set_error } =
>          pr "  %s_buf = nbd_internal_py_get_aio_buffer (%s);\n" n n
>       | Closure { cbname } ->
>          pr "  /* Increment refcount since pointer may be saved by libnbd. */\n";
> -       pr "  Py_INCREF (%s_user_data->fn);\n" cbname;
>          pr "  if (!PyCallable_Check (%s_user_data->fn)) {\n" cbname;
>          pr "    PyErr_SetString (PyExc_TypeError,\n";
>          pr "                     \"callback parameter %s is not callable\");\n" cbname;
> -       pr "    return NULL;\n";
> -       pr "  }\n"
> +       pr "    %s_user_data->fn = NULL;\n" cbname;
> +       pr "    goto out;\n";
> +       pr "  }\n";
> +       pr "  Py_INCREF (%s_user_data->fn);\n" cbname

Hmm. This fixed the problem if there is one closure, but still has 
issues if there are both a Closure and OClosure in the same function 
(nbd_io_pread_structured).  I'll push a followup patch to further clean it.

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3226
Virtualization:  qemu.org | libvirt.org