[Libguestfs] [PATCH libnbd 2/5] python: Allow Python callbacks to auto-retire by returning an integer.

[Richard W.M. Jones]

On Sat, Aug 10, 2019 at 04:57:01PM -0500, Eric Blake wrote:
> 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?

Ah ha!  Yes I did indeed observe this problem but couldn't understand
what was happening - this explains it.

Thanks,

Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
virt-df lists disk usage of guests without needing to install any
software inside the virtual machine.  Supports Linux and Windows.
http://people.redhat.com/~rjones/virt-df/