[Libguestfs] [PATCH libnbd 1/4] api: Combine callback and user_data into a single struct.

[Richard W.M. Jones]

On Tue, Aug 13, 2019 at 10:06:06PM -0500, Eric Blake wrote:
> On 8/13/19 5:36 PM, Richard W.M. Jones wrote:
> > The definition of functions that take a callback is changed so that
> > the callback and user_data are combined into a single structure, eg:
> > 
> >   int64_t nbd_aio_pread (struct nbd_handle *h,
> >             void *buf, size_t count, uint64_t offset,
> > -           int (*completion_callback) (/*..*/), void *user_data,
> > +           nbd_completion_callback completion_callback,
> >             uint32_t flags);
> > 
> > Several nbd_*_callback structures are defined.  The one corresponding
> > to the example above is:
> > 
> >   typedef struct {
> >     void *user_data;
> >     int (*callback) (unsigned valid_flag, void *user_data, int *error);
> >   } nbd_completion_callback;
> > 
> > The nbd_aio_pread function can now be called using:
> > 
> >   nbd_aio_pread (nbd, buf, sizeof buf, offset,
> >                  (nbd_completion_callback) { .callback = my_fn,
> >                                              .user_data = my_data },
> 
> Is it worth arranging the C struct to match the typical argument
> ordering of user_data last?  It doesn't make any real difference (the
> struct size doesn't change, and the compiler handles out-of-order member
> initialization just fine), but doing it could allow the use of:
> 
> nbd_completion_callback cb = { my_fn, my_data };
> nbd_aio_pread (nbd, buf, sizeof buf, offset, cb, 0);
> 
> where the omission of .member= designators may result in less typing,
> but only if the member order matches.

Sure.  What would be the arrangement when all 3 members are
present?  I guess { callback, user_data, free }?

> > +++ b/docs/libnbd.pod
> > @@ -598,14 +598,25 @@ will use your login name):
> >  
> >  =head1 CALLBACKS
> >  
> > -Some libnbd calls take function pointers (eg.
> > -C<nbd_set_debug_callback>, C<nbd_aio_pread>).  Libnbd can call these
> > -functions while processing.
> > -
> > -Callbacks have an opaque C<void *user_data> pointer.  This is passed
> > -as the second parameter to the callback.  The opaque pointer is only
> > -used from the C API, since in other languages you can use closures to
> > -achieve the same outcome.
> > +Some libnbd calls take callbacks (eg.  C<nbd_set_debug_callback>,
> 
> 2 spaces looks odd (emacs thinks eg. ended a sentence)

OK fixed in my copy.

Interesting fact about emacs I-search: It doesn't let you search for
double spaces for some reason.

> > +++ b/examples/batched-read-write.c
> > @@ -53,12 +53,13 @@ try_deadlock (void *arg)
> >  
> >    /* Issue commands. */
> >    cookies[0] = nbd_aio_pread (nbd, in, packetsize, 0,
> > -                              NULL, NULL, 0);
> > +                              NBD_NULL_CALLBACK(completion), 0);
> 
> A bit more verbose, but the macro cuts it down from something even
> longer to type.  I can live with this conversion.

I was trying to write a generic macro (ie. just ‘NBD_NULL_CALLBACK’)
for this, but I don't believe it's possible.

> > +++ b/examples/glib-main-loop.c
> > @@ -384,7 +384,8 @@ read_data (gpointer user_data)
> >  
> >    if (nbd_aio_pread (gssrc->nbd, buffers[i].data,
> >                       BUFFER_SIZE, buffers[i].offset,
> > -                     finished_read, &buffers[i], 0) == -1) {
> > +                     (nbd_completion_callback) { .callback = finished_read, .user_data = &buffers[i] },
> > +                     0) == -1) {
> >      fprintf (stderr, "%s\n", nbd_get_error ());
> >      exit (EXIT_FAILURE);
> >    }
> > @@ -428,7 +429,8 @@ write_data (gpointer user_data)
> >    buffer->state = BUFFER_WRITING;
> >    if (nbd_aio_pwrite (gsdest->nbd, buffer->data,
> >                        BUFFER_SIZE, buffer->offset,
> > -                      finished_write, buffer, 0) == -1) {
> > +                      (nbd_completion_callback) { .callback = finished_write, .user_data = buffer },
> 
> Worth splitting the long lines?

It's tricky to format these.  Even after splitting the line they still
go over 7x chars.

Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
libguestfs lets you edit virtual machines.  Supports shell scripting,
bindings from many languages.  http://libguestfs.org