On 8/10/19 12:02 PM, Richard W.M. Jones wrote:
> See equivalent change for OCaml in
> commit d881d160e1cd9c9964782300a7652ffb4e506c27.
> ---
>  generator/generator | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 

The idea makes sense, but I'm not sure if the code is correct:

> diff --git a/generator/generator b/generator/generator
> index 0107724..0523f0a 100755
> --- a/generator/generator
> +++ b/generator/generator
> @@ -4135,7 +4135,8 @@ let print_python_binding name { args; optargs; ret; 
> may_set_error } =
>         pr "    Py_DECREF (py_args);\n";
>         pr "\n";
>         pr "    if (py_ret != NULL) {\n";
> -       pr "      Py_DECREF (py_ret); /* return value is discarded */\n";
> +       pr "      ret = PyLong_AsLong (py_ret);\n";
> +       pr "      Py_DECREF (py_ret);\n";

This doesn't detect if the user returned a non-integer type (in which
case ret will be -1) - are we okay blindly returning -1 regardless of
whether the user returned actual -1 vs. if they returned some other
non-integer Python object that has no __int__ conversion?  Or do we need
to use PyErr_Occurred() to distinguish between the two cases?  This is
particularly interesting since we document that the C callback must
return -1 before any update to *err will take effect; do we want Python
to have to return -1 for that effect, or is it okay if python raises an
exception and we safely catch that as the way to translate to a C return
of -1?

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

Attachment: signature.asc
Description: OpenPGP digital signature

_______________________________________________
Libguestfs mailing list
[email protected]
https://www.redhat.com/mailman/listinfo/libguestfs

Reply via email to