search for: debug_fn_free

...it a/tests/closure-lifetimes.c b/tests/closure-lifetimes.c index d8ea3f7..c1815b0 100644 --- a/tests/closure-lifetimes.c +++ b/tests/closure-lifetimes.c @@ -38,50 +38,59 @@ static char *nbdkit_delay[] = "delay-read=10", NULL }; -static unsigned debug_fn_valid; -static unsigned debug_fn_free; -static unsigned read_cb_valid; -static unsigned read_cb_free; -static unsigned completion_cb_valid; -static unsigned completion_cb_free; +static unsigned debug_fn_called; +static unsigned debug_fn_freed; +static unsigned read_cb_called; +static unsigned read_cb_freed; +static unsigned completion_...

...it a/tests/closure-lifetimes.c b/tests/closure-lifetimes.c index e21a0e9..41c7f65 100644 --- a/tests/closure-lifetimes.c +++ b/tests/closure-lifetimes.c @@ -38,50 +38,58 @@ static char *nbdkit_delay[] = "delay-read=10", NULL }; -static unsigned debug_fn_valid; -static unsigned debug_fn_free; -static unsigned read_cb_valid; -static unsigned read_cb_free; -static unsigned completion_cb_valid; -static unsigned completion_cb_free; +static unsigned debug_fn_called; +static unsigned debug_fn_freed; +static unsigned read_cb_called; +static unsigned read_cb_freed; +static unsigned completion_...

...n of freeing a closure easier to type, because we aren't repeating the pattern. > +++ b/tests/closure-lifetimes.c > @@ -38,50 +38,58 @@ static char *nbdkit_delay[] = > "delay-read=10", > NULL }; > > -static unsigned debug_fn_valid; > -static unsigned debug_fn_free; > -static unsigned read_cb_valid; > -static unsigned read_cb_free; > -static unsigned completion_cb_valid; > -static unsigned completion_cb_free; > +static unsigned debug_fn_called; > +static unsigned debug_fn_freed; > +static unsigned read_cb_called; > +static unsigned rea...

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

..., a future edit to lib/sockets.c to add in a debug statement there will cause a use-after-free at a distance. > +++ b/tests/closure-lifetimes.c > +static int > +debug_fn (unsigned valid_flag, void *opaque, > + const char *context, const char *msg) > +{ Is it worth assert(!debug_fn_free), to prove that we never have use-after-free? > + if (valid_flag & LIBNBD_CALLBACK_VALID) > + debug_fn_valid++; > + if (valid_flag & LIBNBD_CALLBACK_FREE) > + debug_fn_free++; > + return 0; > +} > + > +static int > +read_cb (unsigned valid_flag, void *op...

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.

I think I've addressed everything that was raised in review. Some of the highlights: - Callbacks should be freed reliably along all exit paths. - There's a simple test of closure lifetimes. - I've tried to use VALID|FREE in all the places where I'm confident that it's safe and correct to do. There may be more places. Note this is an optimization and shouldn't

...gt; +#include <assert.h> + +#include <libnbd.h> + +static char *nbdkit[] = { "nbdkit", "-s", "--exit-with-parent", "-v", + "null", "size=512", NULL }; + +static unsigned debug_fn_valid; +static unsigned debug_fn_free; +static unsigned read_cb_valid; +static unsigned read_cb_free; +static unsigned completion_cb_valid; +static unsigned completion_cb_free; + +static int +debug_fn (unsigned valid_flag, void *opaque, + const char *context, const char *msg) +{ + if (valid_flag & LIBNBD_CALLBACK_VALID) +...

...losure-lifetimes.c +++ b/tests/closure-lifetimes.c @@ -101,10 +101,10 @@ main (int argc, char *argv[]) nbd = nbd_create (); if (nbd == NULL) NBD_ERROR; - nbd_set_debug_callback (nbd, debug_fn, NULL); + nbd_set_debug_callback (nbd, (nbd_debug_callback) { .callback = debug_fn }); assert (debug_fn_free == 0); - nbd_set_debug_callback (nbd, debug_fn, NULL); + nbd_set_debug_callback (nbd, (nbd_debug_callback) { .callback = debug_fn}); assert (debug_fn_free == 1); debug_fn_free = 0; @@ -117,8 +117,9 @@ main (int argc, char *argv[]) if (nbd_connect_command (nbd, nbdkit) == -1) NBD_ERRO...

...ary 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 @@ -42,6 +42,8 @@ static unsigned debug_fn_called; static unsigned debug_fn_freed; static unsigned read_cb_called; static unsigned read_cb_freed; +static unsigned block_status_cb_called; +static unsigned block_status_cb_freed; static unsigned completion_cb_called; static unsigned completion_cb_freed; @@ -74,6 +76,21 @@ read_cb_free (void *opaque) read_cb_freed++; } +s...

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