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

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.

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

> +++ 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.

> +++ 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?