search for: debug_callback

...rator/generator +++ b/generator/generator @@ -1013,6 +1013,17 @@ a handle then messages are printed on C<stderr>. The callback should not call C<nbd_*> APIs on the same handle since it can be called while holding the handle lock and will cause a deadlock."; +}; + + "clear_debug_callback", { + default_call with + args = []; + ret = RErr; + shortdesc = "clear the debug callback"; + longdesc = "\ +Remove the debug callback if one was previously associated +with the handle (with C<nbd_set_debug_callback>). If not +callback was associated this...

..._free_callback (h, cmd->cb.user_data); + /* Free the persistent buffer if there is one and if there's an * associated free callback. */ diff --git a/lib/debug.c b/lib/debug.c index ad4d9cb..34d4184 100644 --- a/lib/debug.c +++ b/lib/debug.c @@ -42,9 +42,11 @@ int nbd_unlocked_set_debug_callback (struct nbd_handle *h, nbd_debug_callback debug_callback, void *data) { - if (h->debug_callback) + if (h->debug_callback) { /* ignore return value */ h->debug_callback (LIBNBD_CALLBACK_FREE, h->debug_data, NULL, NULL); + nbd_internal_fre...

...cb.completion.free) + cmd->cb.completion.free (cmd->cb.completion.user_data); free (cmd); } diff --git a/lib/debug.c b/lib/debug.c index eec2051..a0e6636 100644 --- a/lib/debug.c +++ b/lib/debug.c @@ -41,7 +41,9 @@ nbd_unlocked_get_debug (struct nbd_handle *h) int nbd_unlocked_clear_debug_callback (struct nbd_handle *h) { - FREE_CALLBACK (h->debug_callback); + if (h->debug_callback.free) + h->debug_callback.free (h->debug_callback.user_data); + memset (&h->debug_callback, 0, sizeof h->debug_callback); return 0; } -- 2.20.1

v1 was here: https://www.redhat.com/archives/libguestfs/2019-August/msg00168.html I pushed uncontroversial patches 1-4 v2: - The implementation of OClosure (new patch 1) in Python is fixed. - Patch 2 (old patch 5) is unchanged. - I added a new API for removing debug callbacks. I think this approach has some advantages over using OClosure. - I didn't yet do any work on changing the

...erator > @@ -1013,6 +1013,17 @@ a handle then messages are printed on C<stderr>. > > The callback should not call C<nbd_*> APIs on the same handle since it can > be called while holding the handle lock and will cause a deadlock."; > +}; > + > + "clear_debug_callback", { > + default_call with > + args = []; > + ret = RErr; Is RErr the right type, or can we make this a 'never fails' function? > + shortdesc = "clear the debug callback"; > + longdesc = "\ > +Remove the debug callback if one was previou...

...+ FREE_CALLBACK (cmd->cb.fn.chunk); + FREE_CALLBACK (cmd->cb.completion); free (cmd); } diff --git a/lib/debug.c b/lib/debug.c index 1dd6240..e1ec675 100644 --- a/lib/debug.c +++ b/lib/debug.c @@ -41,11 +41,7 @@ nbd_unlocked_get_debug (struct nbd_handle *h) int nbd_unlocked_clear_debug_callback (struct nbd_handle *h) { - if (h->debug_callback.callback) - if (h->debug_callback.free) - /* ignore return value */ - h->debug_callback.free (h->debug_callback.user_data); - h->debug_callback.callback = NULL; + FREE_CALLBACK (h->debug_callback); return 0; }...

For example nbd_set_debug takes a callback function. Previously this was defined explicitly inside the function parameters. This commit defines a new public typedef: typedef int (*nbd_debug_callback) (unsigned valid_flag, void *user_data, const char *context, const char *msg); and then uses the typedef like this: extern int nbd_set_debug_callback (struct nbd_handle *h, nbd_debug_callback debug_callback,...

...e (cmd->cb.completion.user_data); > > free (cmd); > } > diff --git a/lib/debug.c b/lib/debug.c > index eec2051..a0e6636 100644 > --- a/lib/debug.c > +++ b/lib/debug.c > @@ -41,7 +41,9 @@ nbd_unlocked_get_debug (struct nbd_handle *h) > int > nbd_unlocked_clear_debug_callback (struct nbd_handle *h) > { > - FREE_CALLBACK (h->debug_callback); > + if (h->debug_callback.free) > + h->debug_callback.free (h->debug_callback.user_data); > + memset (&h->debug_callback, 0, sizeof h->debug_callback); > return 0; > } > >...

...changed, 226 insertions(+), 182 deletions(-) diff --git a/docs/libnbd.pod b/docs/libnbd.pod index b38def0..9177825 100644 --- a/docs/libnbd.pod +++ 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 t...

...and this is a normal callback. - -=item C<LIBNBD_CALLBACK_FREE> - -This is the last time the library will call this function. Any data -associated with the callback can be freed. - -=item other bits - -Other bits in C<valid_flag> should be ignored. - -=back - -For example C<nbd_set_debug_callback> sets up a callback which you -could define like this: - - int my_debug_fn (unsigned valid_flag, void *user_data, - const char *context, const char *msg) - { - if (valid_flag & LIBNBD_CALLBACK_VALID) { - printf ("context = %s, msg = %s\n", context, msg); -...

Patches 1 & 2 are rather complex, but the end result is that we pass closures + user_data + free function in single struct parameters as I described previously in this email: https://www.redhat.com/archives/libguestfs/2019-August/msg00210.html Patch 3 adds a convenient FREE_CALLBACK macro which seems a worthwhile simplification if you buy into 1 & 2. Patch 4 adds another macro which is

...on, &error); FREE_CALLBACK (cmd->cb.completion); switch (r) { case -1: diff --git a/lib/debug.c b/lib/debug.c index e1ec675..eec2051 100644 --- a/lib/debug.c +++ b/lib/debug.c @@ -84,7 +84,7 @@ nbd_internal_debug (struct nbd_handle *h, const char *fs, ...) if (h->debug_callback.callback) /* ignore return value */ - h->debug_callback.callback (h->debug_callback.user_data, context, msg); + CALL_CALLBACK (h->debug_callback, context, msg); else fprintf (stderr, "libnbd: debug: %s: %s: %s\n", h->hname, context ? : "un...

...5 +1,5 @@ /* NBD client library in userspace - * Copyright (C) 2013-2019 Red Hat Inc. + * Copyright (C) 2013-2020 Red Hat Inc. * * This library is free software; you can redistribute it and/or * modify it under the terms of the GNU Lesser General Public @@ -48,12 +48,12 @@ nbd_unlocked_clear_debug_callback (struct nbd_handle *h) int nbd_unlocked_set_debug_callback (struct nbd_handle *h, - nbd_debug_callback debug_callback) + nbd_debug_callback *debug_callback) { /* This can't fail at the moment - see implementation above. */...

..._fn>> function is called. -=item C<LIBNBD_CALLBACK_FREE> - -This is the last time the library will call this function. Any data -associated with the callback can be freed. - -=item other bits - -Other bits in C<valid_flag> should be ignored. - -=back - -For example C<nbd_set_debug_callback> sets up a callback which you -could define like this: - - int my_debug_fn (unsigned valid_flag, void *user_data, - const char *context, const char *msg) - { - if (valid_flag & LIBNBD_CALLBACK_VALID) { - printf ("context = %s, msg = %s\n", context, msg); -...

As proposed here: https://www.redhat.com/archives/libguestfs/2019-August/msg00130.html I didn't actually read Eric's replies to that yet because I've been concentrating on writing these patches all day. Anyway here they are and I'll look at what Eric said about the proposal next. Rich.

As promised in my earlier thread on libnbd completion callback question. Eric Blake (2): generator: Refactor handling of closures in unlocked functions generator: Free closures on failure docs/libnbd.pod | 2 +- generator/C.ml | 48 +++++++++++------ generator/C.mli | 1 + lib/debug.c | 7 +-- lib/opt.c | 31 ++++++-----

...t error = cmd->error ? cmd->error : ENOTCONN; int r; diff --git a/lib/debug.c b/lib/debug.c index eec2051..90d2f71 100644 --- a/lib/debug.c +++ b/lib/debug.c @@ -82,7 +82,7 @@ nbd_internal_debug (struct nbd_handle *h, const char *fs, ...) if (r == -1) goto out; - if (h->debug_callback.callback) + if (CALLBACK_IS_NOT_NULL (h->debug_callback)) /* ignore return value */ CALL_CALLBACK (h->debug_callback, context, msg); else diff --git a/lib/internal.h b/lib/internal.h index dc3676a..a18d581 100644 --- a/lib/internal.h +++ b/lib/internal.h @@ -273,6 +273,11 @@ str...

...is_locked then ( pr " if (h->public_state != get_next_state (h))\n"; pr " h->public_state = get_next_state (h);\n"; diff --git a/lib/debug.c b/lib/debug.c index 1b503d9..b598ad3 100644 --- a/lib/debug.c +++ b/lib/debug.c @@ -54,6 +54,7 @@ nbd_unlocked_set_debug_callback (struct nbd_handle *h, nbd_unlocked_clear_debug_callback (h); h->debug_callback = *debug_callback; + SET_CALLBACK_TO_NULL (*debug_callback); return 0; } diff --git a/lib/opt.c b/lib/opt.c index 003ecf8..6ea8326 100644 --- a/lib/opt.c +++ b/lib/opt.c @@ -156,6 +156,7 @@ nbd_unlocked_...

...h index 301b798..d8b0eed 100644 --- a/lib/internal.h +++ b/lib/internal.h @@ -47,6 +47,7 @@ struct meta_context; struct socket; struct command; +struct free_callback; struct nbd_handle { /* Unique name assigned to this handle for debug messages @@ -87,6 +88,10 @@ struct nbd_handle { nbd_debug_callback debug_callback; void *debug_data; + /* Free callbacks kept in pointer order. */ + struct free_callback *free_callbacks; + size_t nr_free_callbacks, alloc_free_callbacks; + /* State machine. * * The actual current state is ‘state’. ‘public_state’ is updated @@ -220,6 +225,12 @@ s...

This is a combination of these two earlier series: https://www.redhat.com/archives/libguestfs/2019-August/msg00235.html https://www.redhat.com/archives/libguestfs/2019-August/msg00240.html plus changes to allow .callback = NULL / .free != NULL, and to reduce the complexity of freeing callbacks. Although it's rather long there's nothing complex here. We might consider squashing some