Re: [Libguestfs] [PATCH libnbd v2 3/3] api: Add nbd_clear_debug_callback.

On 8/13/19 10:12 AM, Richard W.M. Jones wrote: > Commit 0904dd113dfa36485623b3c1756dc1aeab3dddeb removed the > theoretical possibility of clearing the debug callback from the handle > by setting it to NULL (although that was dubious from an API point of > view). > > This commit adds an explicit way to do this. Another advantage of > this is we can internally call this API from other places when we want > to clear/free the debug callback. > --- > generator/generator | 11 +++++++++++ > lib/debug.c | 17 ++++++++++++++--- > lib/handle.c | 4 +--- > 3 files changed, 26 insertions(+), 6 deletions(-) > > diff --git a/generator/generator b/generator/generator > index 6ccfc5f..d76eeea 100755 > --- a/generator/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; 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 previously associated > +with the handle (with C<nbd_set_debug_callback>). If not s/not/no/

> +callback was associated this does nothing."; > }; > > "set_handle_name", { > diff --git a/lib/debug.c b/lib/debug.c > index ad4d9cb..c1decb2 100644 > --- a/lib/debug.c > +++ b/lib/debug.c > @@ -38,13 +38,24 @@ nbd_unlocked_get_debug (struct nbd_handle *h) > return h->debug; > } > > +int > +nbd_unlocked_clear_debug_callback (struct nbd_handle *h) > +{ > + if (h->debug_callback) > + /* ignore return value */ > + h->debug_callback (LIBNBD_CALLBACK_FREE, h->debug_data, NULL, NULL); > + h->debug_callback = NULL; > + h->debug_data = NULL; > + return 0; > +} Is it worth returning -1 if no callback was associated, and/or forwarding the return value of callback(FREE, ptr) as the return value of this function? (That's more complicated; I'm also fine with always returning 0 and ignoring the callback(FREE) return).