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

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?