Libguestfs - Jul 2019 - Re: [PATCH libnbd] generator: Swap parameters of nbd_add_close_callback.

The API changes from:

  int nbd_add_close_callback (struct nbd_handle *h,
                              nbd_close_callback cb,
                              void *user_data);

to:

  int nbd_add_close_callback (struct nbd_handle *h,
                              void *user_data,
                              nbd_close_callback cb);

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(-)

diff --git a/generator/generator b/generator/generator
index 951349d..ed05ea9 100755
--- a/generator/generator
+++ b/generator/generator
@@ -3305,8 +3305,8 @@ let generate_include_libnbd_h ()    pr "#define
LIBNBD_HAVE_NBD_GET_ERRNO 1\n";
   pr "\n";
   pr "extern int nbd_add_close_callback (struct nbd_handle *h,\n";
-  pr "                                   nbd_close_callback cb,\n";
-  pr "                                   void *user_data);\n";
+  pr "                                   void *user_data,\n";
+  pr "                                   nbd_close_callback cb);\n";
   pr "#define LIBNBD_HAVE_NBD_ADD_CLOSE_CALLBACK 1\n";
   pr "\n";
   List.iter (
@@ -3593,7 +3593,7 @@ from other programming languages.
 
  typedef void (*nbd_close_callback) (void *user_data);
  int nbd_add_close_callback (struct nbd_handle *nbd,
-                             nbd_close_callback cb, void *user_data);
+                             void *user_data, nbd_close_callback cb);
 
 ";
 
@@ -4195,7 +4195,7 @@ let print_python_binding name { args; ret }      | Closure
(false, _) -> ()
     | Closure (true, _) ->
        pr "  /* This ensures the callback data is freed eventually.
*/\n";
-       pr "  nbd_add_close_callback (h, free_%s_user_data,
user_data);\n" name
+       pr "  nbd_add_close_callback (h, user_data,
free_%s_user_data);\n" name
     | Flags _ -> ()
     | Int _ -> ()
     | Int64 _ -> ()
@@ -4862,7 +4862,7 @@ let print_ocaml_binding (name, { args; ret })    (match
has_closures with
    | None | Some false -> ()
    | Some true ->
-      pr "  nbd_add_close_callback (h, free_%s_user_data,
user_data);\n" name
+      pr "  nbd_add_close_callback (h, user_data,
free_%s_user_data);\n" name
   );
 
   let errcode diff --git a/lib/handle.c b/lib/handle.c
index 5003227..00d0ac2 100644
--- a/lib/handle.c
+++ b/lib/handle.c
@@ -203,7 +203,7 @@ nbd_unlocked_add_meta_context (struct nbd_handle *h, const
char *name)
  */
 int
 nbd_add_close_callback (struct nbd_handle *h,
-                        nbd_close_callback cb, void *user_data)
+                        void *user_data, nbd_close_callback cb)
 {
   int ret;
   struct close_callback *cc;
-- 
2.22.0

On 7/16/19 10:04 AM, Richard W.M. Jones wrote:
> The API changes from:
> 
>   int nbd_add_close_callback (struct nbd_handle *h,
>                               nbd_close_callback cb,
>                               void *user_data);
> 
> to:
> 
>   int nbd_add_close_callback (struct nbd_handle *h,
>                               void *user_data,
>                               nbd_close_callback cb);
> 
> 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.

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3226
Virtualization:  qemu.org | libvirt.org

On 7/16/19 10:39 AM, Eric Blake wrote:
> On 7/16/19 10:04 AM, Richard W.M. Jones wrote:
>> The API changes from:
>>
>>   int nbd_add_close_callback (struct nbd_handle *h,
>>                               nbd_close_callback cb,
>>                               void *user_data);
>>
>> to:
>>
>>   int nbd_add_close_callback (struct nbd_handle *h,
>>                               void *user_data,
>>                               nbd_close_callback cb);
>>
>> 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.


-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3226
Virtualization:  qemu.org | libvirt.org