[Libguestfs] [PATCH libnbd] generator: Swap parameters of nbd_add_close_callback.

[Eric Blake]

On 7/16/19 11:56 AM, Richard W.M. Jones wrote:

>>>> The second way is consistent with how other callbacks work throughout
>>>> the API (ie. having the user_data passed first).
>>>> ---
>>>>  generator/generator | 10 +++++-----
>>>>  lib/handle.c        |  2 +-
>>>>  2 files changed, 6 insertions(+), 6 deletions(-)
>>>
>>> ACK.
>>>
>>
>> A bit of bike-shedding:
>>
>> In libc, we have qsort_r() which takes the function pointer before the
>> opaque data.
>>
>> I'm trying to find other common frameworks that have common Closure
>> conventions, to see if we should instead swap our nbd_aio_FOO functions
>> to take user_data after the function pointers, instead of this switch to
>> the nbd_add_close_callback parameter order.

pthread_create()/pthread_cleanup_push() in libc, and glib's
g_thread_new(), also use opaque after function pointer.

More history on qsort_r (much of which I did):
http://austingroupbugs.net/view.php?id=900

Originally, BSD and glibc picked opposite orderings, but based on my
arguments about the glibc behavior being slightly easier to use was
enough to convince the FreeBSD authors to propose a patch to switch
their qsort_r to match glibc.

As an odd example, libvirt has:

int virConnectDomainEventRegisterAny(virConnectPtr conn,
                                     virDomainPtr dom, /* Optional, to
filter */
                                     int eventID,

virConnectDomainEventGenericCallback cb,
                                     void *opaque,
                                     virFreeCallback freecb);

where there are two callback functions with a shared opaque, but the
opaque comes between the two arguments.  But if you overlook the point
of the freecb, then it falls in line with the idea of opaque after the
function pointer, rather than before.

> 
> I don't really mind except to say we should do it consistently one way
> or the other, and we should decide which way to do it fairly soon :-)

Indeed, and since we've already broken API, now is the time to make the
switch (whichever one we do switch) so that we can cut another
pre-stable release.  I'm now leaning 60-40 towards dropping this patch
and instead teaching the generator to gather the opaque argument last
rather than first.

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