[Libguestfs] [PATCH libnbd 5/6] generator: Implement OClosure.

[Eric Blake]

On 8/13/19 8:32 AM, Richard W.M. Jones wrote:
> On Tue, Aug 13, 2019 at 06:34:11AM -0500, Eric Blake wrote:
>> On 8/13/19 5:06 AM, Richard W.M. Jones wrote:
>>> An optional Closure parameter, but otherwise works the same way as
>>> Closure.
>>
>>> @@ -3778,6 +3777,7 @@ let generate_lib_api_c () =
>>>      ) args;
>>>      List.iter (
>>>        function
>>> +      | OClosure { cbname } -> pr ", %s_callback ? \"<fun>\" : \"NULL\"" cbname
>>
>> Well, it also permits a NULL fn pointer.
> 
> This is right isn't it?

Yes. The commit message says it works the same way, but the fact that we
now explicitly permit NULL _is_ an intended side effect (so at most it's
worth a tweak to the commit message).

> 
>>> @@ -4383,6 +4387,16 @@ let print_python_binding name { args; optargs; ret; may_set_error } =
>>>    ) args;
>>>    List.iter (
>>>      function
>>> +    | OClosure { cbname } ->
>>> +       pr "  if (%s_user_data) {\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;
>>
>> I don't think PyNone is callable; this probably needs to gain a special
>> case for when the user omitted the optional argument and we thus...
> 
> Yeah I'm not sure about this, plus we don't have any test coverage of
> it.  But it doesn't work ...
> 
> $ nbdkit null --run './run nbdsh --connect nbd://localhost -c "buf = nbd.Buffer(512)" -c "h.aio_pread_callback (buf, 0)"'
> Traceback (most recent call last):
>   File "/usr/lib64/python3.7/runpy.py", line 193, in _run_module_as_main
>     "__main__", mod_spec)
>   File "/usr/lib64/python3.7/runpy.py", line 85, in _run_code
>     exec(code, run_globals)
>   File "/home/rjones/d/libnbd/python/nbd.py", line 1417, in <module>
>     nbdsh.shell()
>   File "/home/rjones/d/libnbd/python/nbdsh.py", line 62, in shell
>     exec (c)
>   File "<string>", line 1, in <module>
>   File "/home/rjones/d/libnbd/python/nbd.py", line 923, in aio_pread_callback
>     return libnbdmod.aio_pread_callback (self._o, buf._o, offset, completion, flags)
> TypeError: callback parameter completion is not callable
> 
> It seems like the "if (%_user_data) {" test at the top is not
> sufficient to tell me if the value is None and I've got to do
> something else instead.
> 

Correct - as I see it, %s_user_data will ALWAYS be non-null (because
otherwise the PyArg_ParseTupe() would have failed).  I think we either
just check for PyCallable_Check and silently ignore all other arguments
whatever their type (whether the default PyNone or otherwise), or we
explicitly check if %s_user_data is PyNone or satisfies PyCallable_Check
and throw an exception if not.  The latter is probably nicer (I'm not
sure off-hand how to exactly check for PyNone, but it should be easy
enough to research), but the former is less code.

-- 
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/ca188cb4/attachment.sig>