[Libguestfs] [libnbd] Slight API inconsistency

[Martin Kletzander]

On Thu, Jul 11, 2019 at 09:35:02AM -0500, Eric Blake wrote:
>On 7/11/19 9:23 AM, Martin Kletzander wrote:
>> The callback (e.g. for `nbd_block_status`) now has a support for returning
>> errors thanks to the last parameter (`int *error`), so it was switched to
>> returning void.
>
>No, the callback still returns int.  Where are you seeing it return
>void, because that's wrong.  The documentation states that *error is
>ignored except when the return is -1.
>
>>  But that was not switched everywhere and some code
>> expects it
>> to return `int`.  Yet another inconsistency is in the debug callback,
>> which is
>> supposed to return `void`, I guess, but due to the way the generator is
>> implemented it's defined to return `int` instead.
>
>ALL callbacks (should) return int. Even when the return value is
>ignored. And I don't think that's worth changing.
>
>>
>> So my question is, should all callbacks just return nothing and if there
>> is a
>> need for some information to get back they will just use a pointer to
>> some data
>> (like with the `int *error`)?  Or do we need different return types for
>> callbacks and should `Callback` and `CallbackPersist` be defined as:
>> `string * arg list * ret` ?
>>
>
>If we ever need a callback that returns void or something other than
>int, we can worry about that later. But for now, ALL callbacks should be
>returning int, and the documentation should then cover what that return
>value influences (whether it is ignored, as in the debug callback, or
>whether it controls the use of *error if -1).
>

Oh, good to know, in that case it should be easy to fix.  `void (*` is used in the man pages even for the block_status callback, I think it might be all.  Same for the set_debug_callback.

One more question then.  For callbacks that get saved (i.e. `CallbackPersist`),
should the caller take care of figuring out when it will not get called again
any more?  I am asking regarding the memory deallocation of the opaque
pointer.

Maybe it is guaranteed that:

 a) for `aio_*` functions the callback will be called only once and

 b) all the other functions (currently only `set_debug_callback`) need to be
    either deregistered or the data can be deregistered only after `nbd_close()`
    was called?

I can't really see that handled in the bindings and I wanted to know that in order to finish (or actually just move a little bit) with the Rust bindings.

Thanks,
Martin
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/libguestfs/attachments/20190711/61767513/attachment.sig>