[Libguestfs] [PATCH libnbd v2 1/3] generator: Implement OClosure.

[Eric Blake]

On 8/13/19 10:12 AM, Richard W.M. Jones wrote:
> An optional Closure parameter, but otherwise works the same way as
> Closure.
> ---
>  generator/generator | 57 ++++++++++++++++++++++++++++++++++++---------
>  1 file changed, 46 insertions(+), 11 deletions(-)
> 

> @@ -4394,6 +4399,16 @@ let print_python_binding name { args; optargs; ret; may_set_error } =
>    ) args;
>    List.iter (
>      function
> +    | OClosure { cbname } ->
> +       pr "  if (%s_user_data != Py_None) {\n" cbname;
> +       pr "    /* Increment refcount since pointer may be saved by libnbd. */\n";
> +       pr "    Py_INCREF (%s_user_data);\n" cbname;
> +       pr "    if (!PyCallable_Check (%s_user_data)) {\n" cbname;
> +       pr "      PyErr_SetString (PyExc_TypeError,\n";
> +       pr "                       \"callback parameter %s is not callable\");\n" cbname;
> +       pr "      return NULL;\n";

Leaks %s_user_data, because we fail to do a matching Py_DECREF on
failure paths. It would be easier to defer the Py_INCREF to after we
verified it is Callable.  However, on looking further, I see this is
also a pre-existing bug affecting Closure.  For that matter, should we
be using 'py_ret = NULL; goto out;' similar to StringList, to avoid any
other leaks on any other failure paths?  (The generated python code
still probably has a number of poor handling of failures, which needs a
proper audit...)

> +       pr "    }\n";
> +       pr "  }\n"
>      | OFlags (n, _) -> pr "  %s_u32 = %s;\n" n n
>    ) optargs;
>  
> @@ -4423,6 +4438,9 @@ let print_python_binding name { args; optargs; ret; may_set_error } =
>    ) args;
>    List.iter (
>      function
> +    | OClosure { cbname } ->
> +       pr ", %s_user_data ? %s_wrapper : NULL" cbname cbname;

Still looks wrong. %s_user_data is non-NULL; the check here should be
%s_user_data != Py_None.


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

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 488 bytes
Desc: OpenPGP digital signature
URL: <http://listman.redhat.com/archives/libguestfs/attachments/20190813/9b8c9a09/attachment.sig>