search for: read_cb_free

...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_cb_called; +static unsigned completion_cb_freed; static int...

...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_cb_called; +static unsigned completion_cb_freed; static int...

...(h->pid == -1) { set_error (ESRCH, "no subprocess exists"); diff --git a/tests/closure-lifetimes.c b/tests/closure-lifetimes.c index 5a7cd9c..70788b6 100644 --- a/tests/closure-lifetimes.c +++ b/tests/closure-lifetimes.c @@ -143,7 +143,7 @@ main (int argc, char *argv[]) assert (read_cb_freed == 1); assert (completion_cb_freed == 1); - nbd_kill_command (nbd, 0); + nbd_kill_subprocess (nbd, 0); nbd_close (nbd); /* Test command callbacks are freed if the handle is closed without @@ -162,7 +162,7 @@ main (int argc, char *argv[])...

This is my proposal for fixing the documentation to match practice (namely, that completion.callback is not invoked in the cases where the aio call itself reports errors); we could instead try to go the other direction and tweak the generator to guarantee that both completion.callback and completion.free are reached no matter what, but that felt more invasive to me. Eric Blake (2): api: Tighten

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.

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

...right (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++; } +static int +block_status_cb (void *opaque, const char *meta, uint6...

...ed valid_flag, void *opaque, > + const void *subbuf, size_t count, > + uint64_t offset, unsigned status, int *error) > +{ Same here. > + if (valid_flag & LIBNBD_CALLBACK_VALID) > + read_cb_valid++; > + if (valid_flag & LIBNBD_CALLBACK_FREE) > + read_cb_free++; > + return 0; > +} > + > +static int > +completion_cb (unsigned valid_flag, void *opaque, > + int64_t cookie, int *error) > +{ and again > + if (valid_flag & LIBNBD_CALLBACK_VALID) > + completion_cb_valid++; > + if (valid_flag & LIBNBD_...

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

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

...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 read_cb_freed; > +static unsigned completion_cb_called; > +static unsi...

...tatic 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) + debug_fn_valid++; + if (valid_flag & LIBNBD_CALLBACK_F...

..., "--exit-with-parent", "-v", "--filter=delay", "null", "size=512", - "delay-read=3", + "delay-read=10", NULL }; static unsigned debug_fn_valid; @@ -134,6 +134,7 @@ main (int argc, char *argv[]) assert (read_cb_free == 1); assert (completion_cb_free == 1); + nbd_kill_command (nbd, 0); nbd_close (nbd); /* Test command callbacks are freed if the handle is closed without @@ -149,6 +150,7 @@ main (int argc, char *argv[]) read_cb, NULL,...

...NULL, - completion_cb, NULL, 0); + cookie = nbd_aio_pread_structured (nbd, buf, sizeof buf, 0, + read_cb, NULL, + completion_cb, NULL, 0); if (cookie == -1) NBD_ERROR; assert (read_cb_free == 0); assert (completion_cb_free == 0); @@ -143,9 +143,9 @@ main (int argc, char *argv[]) if (nbd == NULL) NBD_ERROR; if (nbd_connect_command (nbd, nbdkit_delay) == -1) NBD_ERROR; - cookie = nbd_aio_pread_structured_callback (nbd, buf, sizeof buf, 0, -...

This applies on top of the OClosure v2 series posted a few minutes ago. Rich.

...completion_cb, NULL, 0); + (nbd_chunk_callback) { .callback = read_cb }, + (nbd_completion_callback) { .callback = completion_cb }, + 0); if (cookie == -1) NBD_ERROR; assert (read_cb_free == 0); assert (completion_cb_free == 0); @@ -144,8 +145,9 @@ main (int argc, char *argv[]) if (nbd_connect_command (nbd, nbdkit_delay) == -1) NBD_ERROR; cookie = nbd_aio_pread_structured (nbd, buf, sizeof buf, 0, - read_cb, NULL, -...