The freeing feature can now be done by associating a free callback with a closure's user_data, so having the valid_flag is no longer necessary and it can be completely removed. This mostly reverts commit 2d9b98e96772e282d51dafac07f16387dadc8afa. --- TODO | 2 - docs/libnbd.pod | 64 ++-------------- examples/glib-main-loop.c | 14 +--- examples/strict-structured-reads.c | 63 +++++++--------- generator/generator | 84 +++++++++------------ generator/states-reply-simple.c | 3 +- generator/states-reply-structured.c | 45 ++++-------- generator/states-reply.c | 3 +- generator/states.c | 3 +- interop/dirty-bitmap.c | 5 +- interop/structured-read.c | 5 +- lib/aio.c | 11 --- lib/debug.c | 7 +- lib/handle.c | 4 +- tests/closure-lifetimes.c | 109 +++++++++++++++++----------- tests/meta-base-allocation.c | 7 +- tests/oldstyle.c | 5 +- tests/server-death.c | 5 +- 18 files changed, 165 insertions(+), 274 deletions(-) diff --git a/TODO b/TODO index 8e067c0..03fda1f 100644 --- a/TODO +++ b/TODO @@ -26,8 +26,6 @@ Improve function trace output so that: Suggested API improvements: general: - synchronous APIs that have a timeout or can be cancelled - - separate free_callback corresponding to every Closure - (instead of using valid_flag) connecting: - nbd_connect_tcp: allow control over whether IPv4 or IPv6 is desired diff --git a/docs/libnbd.pod b/docs/libnbd.pod index 51b1a03..0068b84 100644 --- a/docs/libnbd.pod +++ b/docs/libnbd.pod @@ -608,59 +608,6 @@ 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. -=head2 Callback lifetimes - -All callbacks have an C<unsigned valid_flag> parameter which is used -to help with the lifetime of the callback. C<valid_flag> contains the -I<logical or> of: - -=over 4 - -=item C<LIBNBD_CALLBACK_VALID> - -The callback parameters are valid 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); - } - if (valid_flag & LIBNBD_CALLBACK_FREE) { - /* If you need to free something, for example: */ - free (user_data); - } - return 0; - } - -If you don't care about the freeing feature then it is also correct to -write this: - - int my_debug_fn (unsigned valid_flag, void *user_data, - const char *context, const char *msg) - { - if (!(valid_flag & LIBNBD_CALLBACK_VALID)) return 0; - - // Rest of callback as normal. - } - -The valid flag is only present in the C API. It is not needed when -using garbage-collected programming languages. - =head2 Callbacks and locking The callbacks are invoked at a point where the libnbd lock is held; as @@ -673,10 +620,10 @@ All of the low-level commands have a completion callback variant that registers a callback function used right before the command is marked complete. -When C<valid_flag> includes C<LIBNBD_CALLBACK_VALID>, and the -completion callback returns C<1>, the command is automatically retired -(there is no need to call C<nbd_aio_command_completed>); for any other -return value, the command still needs to be retired. +When the completion callback returns C<1>, the command is +automatically retired (there is no need to call +C<nbd_aio_command_completed>); for any other return value, the command +still needs to be retired. =head2 Callbacks with C<int *error> parameter @@ -688,8 +635,7 @@ all of the completion callbacks, include a parameter C<error> containing the value of any error detected so far; if the callback function fails, it should assign back into C<error> and return C<-1> to change the resulting error of the overall command. Assignments -into C<error> are ignored for any other return value or when -C<valid_flag> did not contain C<LIBNBD_CALLBACK_VALID>; similarly, +into C<error> are ignored for any other return value; similarly, assigning C<0> into C<error> does not have an effect. =head1 SEE ALSO diff --git a/examples/glib-main-loop.c b/examples/glib-main-loop.c index 05a59e3..216eb5e 100644 --- a/examples/glib-main-loop.c +++ b/examples/glib-main-loop.c @@ -268,9 +268,9 @@ static GMainLoop *loop; static void connected (struct NBDSource *source); static gboolean read_data (gpointer user_data); -static int finished_read (unsigned valid_flag, void *vp, int *error); +static int finished_read (void *vp, int *error); static gboolean write_data (gpointer user_data); -static int finished_write (unsigned valid_flag, void *vp, int *error); +static int finished_write (void *vp, int *error); int main (int argc, char *argv[]) @@ -394,13 +394,10 @@ read_data (gpointer user_data) /* This callback is called from libnbd when any read command finishes. */ static int -finished_read (unsigned valid_flag, void *vp, int *error) +finished_read (void *vp, int *error) { struct buffer *buffer = vp; - if (!(valid_flag & LIBNBD_CALLBACK_VALID)) - return 0; - if (gssrc == NULL) return 0; @@ -442,13 +439,10 @@ write_data (gpointer user_data) /* This callback is called from libnbd when any write command finishes. */ static int -finished_write (unsigned valid_flag, void *vp, int *error) +finished_write (void *vp, int *error) { struct buffer *buffer = vp; - if (!(valid_flag & LIBNBD_CALLBACK_VALID)) - return 0; - if (gsdest == NULL) return 0; diff --git a/examples/strict-structured-reads.c b/examples/strict-structured-reads.c index b3880b7..701b216 100644 --- a/examples/strict-structured-reads.c +++ b/examples/strict-structured-reads.c @@ -51,16 +51,13 @@ static int64_t total_bytes; static int total_success; static int -read_chunk (unsigned valid_flag, void *opaque, +read_chunk (void *opaque, const void *bufv, size_t count, uint64_t offset, unsigned status, int *error) { struct data *data = opaque; struct range *r, **prev; - if (!(valid_flag & LIBNBD_CALLBACK_VALID)) - return 0; - /* libnbd guarantees this: */ assert (offset >= data->offset); assert (offset + count <= data->offset + data->count); @@ -131,46 +128,41 @@ read_chunk (unsigned valid_flag, void *opaque, } static int -read_verify (unsigned valid_flag, void *opaque, int *error) +read_verify (void *opaque, int *error) { int ret = 0; - if (valid_flag & LIBNBD_CALLBACK_VALID) { - struct data *data = opaque; + struct data *data = opaque; - ret = -1; - total_reads++; - total_chunks += data->chunks; - if (*error) - goto cleanup; - assert (data->chunks > 0); - if (data->flags & LIBNBD_CMD_FLAG_DF) { - total_df_reads++; - if (data->chunks > 1) { - fprintf (stderr, "buggy server: too many chunks for DF flag\n"); - *error = EPROTO; - goto cleanup; - } - } - if (data->remaining && !*error) { - fprintf (stderr, "buggy server: not enough chunks on success\n"); + ret = -1; + total_reads++; + total_chunks += data->chunks; + if (*error) + goto cleanup; + assert (data->chunks > 0); + if (data->flags & LIBNBD_CMD_FLAG_DF) { + total_df_reads++; + if (data->chunks > 1) { + fprintf (stderr, "buggy server: too many chunks for DF flag\n"); *error = EPROTO; goto cleanup; } - total_bytes += data->count; - total_success++; - ret = 0; - - cleanup: - while (data->remaining) { - struct range *r = data->remaining; - data->remaining = r->next; - free (r); - } } + if (data->remaining && !*error) { + fprintf (stderr, "buggy server: not enough chunks on success\n"); + *error = EPROTO; + goto cleanup; + } + total_bytes += data->count; + total_success++; + ret = 0; - if (valid_flag & LIBNBD_CALLBACK_FREE) - free (opaque); + cleanup: + while (data->remaining) { + struct range *r = data->remaining; + data->remaining = r->next; + free (r); + } return ret; } @@ -235,6 +227,7 @@ main (int argc, char *argv[]) *r = (struct range) { .first = offset, .last = offset + maxsize, }; *d = (struct data) { .offset = offset, .count = maxsize, .flags = flags, .remaining = r, }; + /* XXX *d and *r are both leaked. */ if (nbd_aio_pread_structured_callback (nbd, buf, sizeof buf, offset, read_chunk, d, read_verify, d, flags) == -1) { diff --git a/generator/generator b/generator/generator index a0322ee..0187c02 100755 --- a/generator/generator +++ b/generator/generator @@ -3190,7 +3190,7 @@ module C : sig val generate_docs_libnbd_api_pod : unit -> unit val print_arg_list : ?handle:bool -> ?types:bool -> arg list -> optarg list -> unit - val print_cbarg_list : ?valid_flag:bool -> ?types:bool -> cbarg list -> unit + val print_cbarg_list : ?types:bool -> cbarg list -> unit val errcode_of_ret : ret -> string option val type_of_ret : ret -> string end = struct @@ -3430,13 +3430,8 @@ let print_extern name args optargs ret = print_call name args optargs ret; pr ";\n" -let print_cbarg_list ?(valid_flag = true) ?(types = true) cbargs = +let print_cbarg_list ?(types = true) cbargs = pr "("; - if valid_flag then ( - if types then pr "unsigned "; - pr "valid_flag"; - pr ", "; - ); if types then pr "void *"; pr "user_data"; @@ -3567,9 +3562,6 @@ let generate_include_libnbd_h () = pr "#define %-40s %d\n" n i ) constants; pr "\n"; - pr "#define LIBNBD_CALLBACK_VALID 1\n"; - pr "#define LIBNBD_CALLBACK_FREE 2\n"; - pr "\n"; pr "extern struct nbd_handle *nbd_create (void);\n"; pr "#define LIBNBD_HAVE_NBD_CREATE 1\n"; pr "\n"; @@ -4226,26 +4218,25 @@ let print_python_binding name { args; optargs; ret; may_set_error } = pr "{\n"; pr " int ret = 0;\n"; pr "\n"; - pr " if (valid_flag & LIBNBD_CALLBACK_VALID) {\n"; - pr " PyGILState_STATE py_save = PyGILState_UNLOCKED;\n"; - pr " PyObject *py_args, *py_ret;\n"; + pr " PyGILState_STATE py_save = PyGILState_UNLOCKED;\n"; + pr " PyObject *py_args, *py_ret;\n"; List.iter ( function | CBArrayAndLen (UInt32 n, len) -> - pr " PyObject *py_%s = PyList_New (%s);\n" n len; - pr " for (size_t i = 0; i < %s; ++i)\n" len; - pr " PyList_SET_ITEM (py_%s, i, PyLong_FromUnsignedLong (%s[i]));\n" n n + pr " PyObject *py_%s = PyList_New (%s);\n" n len; + pr " for (size_t i = 0; i < %s; ++i)\n" len; + pr " PyList_SET_ITEM (py_%s, i, PyLong_FromUnsignedLong (%s[i]));\n" n n | CBBytesIn _ | CBInt _ | CBInt64 _ -> () | CBMutable (Int n) -> - pr " PyObject *py_%s_modname = PyUnicode_FromString (\"ctypes\");\n" n; - pr " if (!py_%s_modname) { PyErr_PrintEx (0); return -1; }\n" n; - pr " PyObject *py_%s_mod = PyImport_Import (py_%s_modname);\n" n n; - pr " Py_DECREF (py_%s_modname);\n" n; - pr " if (!py_%s_mod) { PyErr_PrintEx (0); return -1; }\n" n; - pr " PyObject *py_%s = PyObject_CallMethod (py_%s_mod, \"c_int\", \"i\", *%s);\n" n n n; - pr " if (!py_%s) { PyErr_PrintEx (0); return -1; }\n" n; + pr " PyObject *py_%s_modname = PyUnicode_FromString (\"ctypes\");\n" n; + pr " if (!py_%s_modname) { PyErr_PrintEx (0); return -1; }\n" n; + pr " PyObject *py_%s_mod = PyImport_Import (py_%s_modname);\n" n n; + pr " Py_DECREF (py_%s_modname);\n" n; + pr " if (!py_%s_mod) { PyErr_PrintEx (0); return -1; }\n" n; + pr " PyObject *py_%s = PyObject_CallMethod (py_%s_mod, \"c_int\", \"i\", *%s);\n" n n n; + pr " if (!py_%s) { PyErr_PrintEx (0); return -1; }\n" n; | CBString _ | CBUInt _ | CBUInt64 _ -> () @@ -4253,7 +4244,7 @@ let print_python_binding name { args; optargs; ret; may_set_error } = ) cbargs; pr "\n"; - pr " py_args = Py_BuildValue (\"(\""; + pr " py_args = Py_BuildValue (\"(\""; List.iter ( function | CBArrayAndLen (UInt32 n, len) -> pr " \"O\"" @@ -4278,42 +4269,41 @@ let print_python_binding name { args; optargs; ret; may_set_error } = | CBArrayAndLen _ | CBMutable _ -> assert false ) cbargs; pr ");\n"; - pr " Py_INCREF (py_args);\n"; + pr " Py_INCREF (py_args);\n"; pr "\n"; - pr " if (PyEval_ThreadsInitialized ())\n"; - pr " py_save = PyGILState_Ensure ();\n"; + pr " if (PyEval_ThreadsInitialized ())\n"; + pr " py_save = PyGILState_Ensure ();\n"; pr "\n"; - pr " py_ret = PyObject_CallObject ((PyObject *)user_data, py_args);\n"; + pr " py_ret = PyObject_CallObject ((PyObject *)user_data, py_args);\n"; pr "\n"; - pr " if (PyEval_ThreadsInitialized ())\n"; - pr " PyGILState_Release (py_save);\n"; + pr " if (PyEval_ThreadsInitialized ())\n"; + pr " PyGILState_Release (py_save);\n"; pr "\n"; - pr " Py_DECREF (py_args);\n"; + pr " Py_DECREF (py_args);\n"; pr "\n"; - pr " if (py_ret != NULL) {\n"; - pr " Py_DECREF (py_ret); /* return value is discarded */\n"; - pr " }\n"; - pr " else {\n"; - pr " ret = -1;\n"; - pr " PyErr_PrintEx (0); /* print exception */\n"; - pr " };\n"; + pr " if (py_ret != NULL) {\n"; + pr " Py_DECREF (py_ret); /* return value is discarded */\n"; + pr " }\n"; + pr " else {\n"; + pr " ret = -1;\n"; + pr " PyErr_PrintEx (0); /* print exception */\n"; + pr " };\n"; pr "\n"; List.iter ( function | CBArrayAndLen (UInt32 n, _) -> - pr " Py_DECREF (py_%s);\n" n + pr " Py_DECREF (py_%s);\n" n | CBMutable (Int n) -> - pr " PyObject *py_%s_ret = PyObject_GetAttrString (py_%s, \"value\");\n" n n; - pr " *%s = PyLong_AsLong (py_%s_ret);\n" n n; - pr " Py_DECREF (py_%s_ret);\n" n; - pr " Py_DECREF (py_%s);\n" n + pr " PyObject *py_%s_ret = PyObject_GetAttrString (py_%s, \"value\");\n" n n; + pr " *%s = PyLong_AsLong (py_%s_ret);\n" n n; + pr " Py_DECREF (py_%s_ret);\n" n; + pr " Py_DECREF (py_%s);\n" n | CBBytesIn _ | CBInt _ | CBInt64 _ | CBString _ | CBUInt _ | CBUInt64 _ -> () | CBArrayAndLen _ | CBMutable _ -> assert false ) cbargs; - pr " }\n"; pr "\n"; pr " return ret;\n"; pr "}\n"; @@ -5159,7 +5149,7 @@ let print_ocaml_binding (name, { args; optargs; ret }) = pr "/* Wrapper for %s callback of %s. */\n" cbname name; pr "static int\n"; pr "%s_%s_wrapper_locked " name cbname; - C.print_cbarg_list ~valid_flag:false cbargs; + C.print_cbarg_list cbargs; pr "\n"; pr "{\n"; pr " CAMLparam0 ();\n"; @@ -5239,13 +5229,11 @@ let print_ocaml_binding (name, { args; optargs; ret }) = pr "{\n"; pr " int ret = 0;\n"; pr "\n"; - pr " if (valid_flag & LIBNBD_CALLBACK_VALID) {\n"; pr " caml_leave_blocking_section ();\n"; pr " ret = %s_%s_wrapper_locked " name cbname; - C.print_cbarg_list ~valid_flag:false ~types:false cbargs; + C.print_cbarg_list ~types:false cbargs; pr ";\n"; pr " caml_enter_blocking_section ();\n"; - pr " }\n"; pr "\n"; pr " return ret;\n"; pr "}\n"; diff --git a/generator/states-reply-simple.c b/generator/states-reply-simple.c index 9684bc4..11354e5 100644 --- a/generator/states-reply-simple.c +++ b/generator/states-reply-simple.c @@ -64,8 +64,7 @@ int error = 0; assert (cmd->error == 0); - if (cmd->cb.fn.chunk (LIBNBD_CALLBACK_VALID|LIBNBD_CALLBACK_FREE, - cmd->cb.fn_user_data, + if (cmd->cb.fn.chunk (cmd->cb.fn_user_data, cmd->data, cmd->count, cmd->offset, LIBNBD_READ_DATA, &error) == -1) cmd->error = error ? error : EPROTO; diff --git a/generator/states-reply-structured.c b/generator/states-reply-structured.c index b016cd7..846cc35 100644 --- a/generator/states-reply-structured.c +++ b/generator/states-reply-structured.c @@ -18,17 +18,6 @@ /* State machine for parsing structured replies from the server. */ -static unsigned -valid_flags (struct nbd_handle *h) -{ - unsigned valid = LIBNBD_CALLBACK_VALID; - uint16_t flags = be16toh (h->sbuf.sr.structured_reply.flags); - - if (flags & NBD_REPLY_FLAG_DONE) - valid |= LIBNBD_CALLBACK_FREE; - return valid; -} - /*----- End of prologue. -----*/ /* STATE MACHINE */ { @@ -306,18 +295,18 @@ valid_flags (struct nbd_handle *h) } if (cmd->type == NBD_CMD_READ && cmd->cb.fn.chunk) { int scratch = error; - unsigned valid = valid_flags (h); + uint16_t flags = be16toh (h->sbuf.sr.structured_reply.flags); /* Different from successful reads: inform the callback about the * current error rather than any earlier one. If the callback fails * without setting errno, then use the server's error below. */ - if (cmd->cb.fn.chunk (valid, cmd->cb.fn_user_data, + if (cmd->cb.fn.chunk (cmd->cb.fn_user_data, cmd->data + (offset - cmd->offset), 0, offset, LIBNBD_READ_ERROR, &scratch) == -1) if (cmd->error == 0) cmd->error = scratch; - if (valid & LIBNBD_CALLBACK_FREE) { + if (flags & NBD_REPLY_FLAG_DONE) { cmd->cb.fn.chunk = NULL; /* because we've freed it */ nbd_internal_free_callback (h, cmd->cb.fn_user_data); } @@ -402,15 +391,15 @@ valid_flags (struct nbd_handle *h) assert (cmd); /* guaranteed by CHECK */ if (cmd->cb.fn.chunk) { int error = cmd->error; - unsigned valid = valid_flags (h); + uint16_t flags = be16toh (h->sbuf.sr.structured_reply.flags); - if (cmd->cb.fn.chunk (valid, cmd->cb.fn_user_data, + if (cmd->cb.fn.chunk (cmd->cb.fn_user_data, cmd->data + (offset - cmd->offset), length - sizeof offset, offset, LIBNBD_READ_DATA, &error) == -1) if (cmd->error == 0) cmd->error = error ? error : EPROTO; - if (valid & LIBNBD_CALLBACK_FREE) { + if (flags & NBD_REPLY_FLAG_DONE) { cmd->cb.fn.chunk = NULL; /* because we've freed it */ nbd_internal_free_callback (h, cmd->cb.fn_user_data); } @@ -469,15 +458,15 @@ valid_flags (struct nbd_handle *h) memset (cmd->data + offset, 0, length); if (cmd->cb.fn.chunk) { int error = cmd->error; - unsigned valid = valid_flags (h); + uint16_t flags = be16toh (h->sbuf.sr.structured_reply.flags); - if (cmd->cb.fn.chunk (valid, cmd->cb.fn_user_data, + if (cmd->cb.fn.chunk (cmd->cb.fn_user_data, cmd->data + offset, length, cmd->offset + offset, LIBNBD_READ_HOLE, &error) == -1) if (cmd->error == 0) cmd->error = error ? error : EPROTO; - if (valid & LIBNBD_CALLBACK_FREE) { + if (flags & NBD_REPLY_FLAG_DONE) { cmd->cb.fn.chunk = NULL; /* because we've freed it */ nbd_internal_free_callback (h, cmd->cb.fn_user_data); } @@ -526,14 +515,14 @@ valid_flags (struct nbd_handle *h) if (meta_context) { /* Call the caller's extent function. */ int error = cmd->error; - unsigned valid = valid_flags (h); + uint16_t flags = be16toh (h->sbuf.sr.structured_reply.flags); - if (cmd->cb.fn.extent (valid, cmd->cb.fn_user_data, + if (cmd->cb.fn.extent (cmd->cb.fn_user_data, meta_context->name, cmd->offset, &h->bs_entries[1], (length-4) / 4, &error) == -1) if (cmd->error == 0) cmd->error = error ? error : EPROTO; - if (valid & LIBNBD_CALLBACK_FREE) { + if (flags & NBD_REPLY_FLAG_DONE) { cmd->cb.fn.extent = NULL; /* because we've freed it */ nbd_internal_free_callback (h, cmd->cb.fn_user_data); } @@ -553,16 +542,10 @@ valid_flags (struct nbd_handle *h) flags = be16toh (h->sbuf.sr.structured_reply.flags); if (flags & NBD_REPLY_FLAG_DONE) { - if (cmd->type == NBD_CMD_BLOCK_STATUS && cmd->cb.fn.extent) { - cmd->cb.fn.extent (LIBNBD_CALLBACK_FREE, cmd->cb.fn_user_data, - NULL, 0, NULL, 0, NULL); + if (cmd->type == NBD_CMD_BLOCK_STATUS && cmd->cb.fn.extent) nbd_internal_free_callback (h, cmd->cb.fn_user_data); - } - if (cmd->type == NBD_CMD_READ && cmd->cb.fn.chunk) { - cmd->cb.fn.chunk (LIBNBD_CALLBACK_FREE, cmd->cb.fn_user_data, - NULL, 0, 0, 0, NULL); + if (cmd->type == NBD_CMD_READ && cmd->cb.fn.chunk) nbd_internal_free_callback (h, cmd->cb.fn_user_data); - } cmd->cb.fn.chunk = NULL; SET_NEXT_STATE (%^FINISH_COMMAND); } diff --git a/generator/states-reply.c b/generator/states-reply.c index d5cba1a..1e0e92c 100644 --- a/generator/states-reply.c +++ b/generator/states-reply.c @@ -173,8 +173,7 @@ save_reply_state (struct nbd_handle *h) int r; assert (cmd->type != NBD_CMD_DISC); - r = cmd->cb.completion (LIBNBD_CALLBACK_VALID|LIBNBD_CALLBACK_FREE, - cmd->cb.user_data, &error); + r = cmd->cb.completion (cmd->cb.user_data, &error); nbd_internal_free_callback (h, cmd->cb.user_data); cmd->cb.completion = NULL; /* because we've freed it */ switch (r) { diff --git a/generator/states.c b/generator/states.c index 313d2c9..4d3a93c 100644 --- a/generator/states.c +++ b/generator/states.c @@ -126,8 +126,7 @@ void abort_commands (struct nbd_handle *h, int r; assert (cmd->type != NBD_CMD_DISC); - r = cmd->cb.completion (LIBNBD_CALLBACK_VALID|LIBNBD_CALLBACK_FREE, - cmd->cb.user_data, &error); + r = cmd->cb.completion (cmd->cb.user_data, &error); nbd_internal_free_callback (h, cmd->cb.user_data); cmd->cb.completion = NULL; /* because we've freed it */ switch (r) { diff --git a/interop/dirty-bitmap.c b/interop/dirty-bitmap.c index aca0564..fbd6f29 100644 --- a/interop/dirty-bitmap.c +++ b/interop/dirty-bitmap.c @@ -42,14 +42,11 @@ struct data { }; static int -cb (unsigned valid_flag, void *opaque, const char *metacontext, uint64_t offset, +cb (void *opaque, const char *metacontext, uint64_t offset, uint32_t *entries, size_t len, int *error) { struct data *data = opaque; - if (!(valid_flag & LIBNBD_CALLBACK_VALID)) - return 0; - /* libnbd does not actually verify that a server is fully compliant * to the spec; the asserts marked [qemu-nbd] are thus dependent on * the fact that qemu-nbd is compliant. diff --git a/interop/structured-read.c b/interop/structured-read.c index 0b189d1..3e62b05 100644 --- a/interop/structured-read.c +++ b/interop/structured-read.c @@ -48,16 +48,13 @@ struct data { }; static int -read_cb (unsigned valid_flag, void *opaque, +read_cb (void *opaque, const void *bufv, size_t count, uint64_t offset, unsigned status, int *error) { struct data *data = opaque; const char *buf = bufv; - if (!(valid_flag & LIBNBD_CALLBACK_VALID)) - return 0; - /* The NBD spec allows chunks to be reordered; we are relying on the * fact that qemu-nbd does not do so. */ diff --git a/lib/aio.c b/lib/aio.c index 8aa4547..4cc61cb 100644 --- a/lib/aio.c +++ b/lib/aio.c @@ -32,17 +32,6 @@ void nbd_internal_retire_and_free_command (struct nbd_handle *h, struct command *cmd) { - /* Free the callbacks. */ - if (cmd->type == NBD_CMD_BLOCK_STATUS && cmd->cb.fn.extent) - cmd->cb.fn.extent (LIBNBD_CALLBACK_FREE, cmd->cb.fn_user_data, - NULL, 0, NULL, 0, NULL); - if (cmd->type == NBD_CMD_READ && cmd->cb.fn.chunk) - cmd->cb.fn.chunk (LIBNBD_CALLBACK_FREE, cmd->cb.fn_user_data, - NULL, 0, 0, 0, NULL); - if (cmd->cb.completion) - cmd->cb.completion (LIBNBD_CALLBACK_FREE, cmd->cb.user_data, - NULL); - /* Free the closures if there's an associated free callback. */ nbd_internal_free_callback (h, cmd->cb.fn_user_data); nbd_internal_free_callback (h, cmd->cb.user_data); diff --git a/lib/debug.c b/lib/debug.c index 34d4184..bd0c465 100644 --- a/lib/debug.c +++ b/lib/debug.c @@ -42,11 +42,8 @@ int nbd_unlocked_set_debug_callback (struct nbd_handle *h, nbd_debug_callback debug_callback, void *data) { - if (h->debug_callback) { - /* ignore return value */ - h->debug_callback (LIBNBD_CALLBACK_FREE, h->debug_data, NULL, NULL); + if (h->debug_callback) nbd_internal_free_callback (h, h->debug_data); - } h->debug_callback = debug_callback; h->debug_data = data; return 0; @@ -80,7 +77,7 @@ nbd_internal_debug (struct nbd_handle *h, const char *fs, ...) if (h->debug_callback) /* ignore return value */ - h->debug_callback (LIBNBD_CALLBACK_VALID, h->debug_data, context, msg); + h->debug_callback (h->debug_data, context, msg); else fprintf (stderr, "libnbd: debug: %s: %s: %s\n", h->hname, context ? : "unknown", msg); diff --git a/lib/handle.c b/lib/handle.c index 5a47bd8..4c59622 100644 --- a/lib/handle.c +++ b/lib/handle.c @@ -113,10 +113,8 @@ nbd_close (struct nbd_handle *h) return; /* Free user callbacks first. */ - if (h->debug_callback) { - h->debug_callback (LIBNBD_CALLBACK_FREE, h->debug_data, NULL, NULL); + if (h->debug_callback) nbd_internal_free_callback (h, h->debug_data); - } h->debug_callback = NULL; free (h->bs_entries); diff --git 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_cb_called; +static unsigned completion_cb_freed; static int -debug_fn (unsigned valid_flag, void *opaque, +debug_fn (void *opaque, const char *context, const char *msg) { - if (valid_flag & LIBNBD_CALLBACK_VALID) - debug_fn_valid++; - if (valid_flag & LIBNBD_CALLBACK_FREE) - debug_fn_free++; + debug_fn_called++; return 0; } +static void +debug_fn_free (void *ptr, void *opaque) +{ + debug_fn_freed++; +} + static int -read_cb (unsigned valid_flag, void *opaque, +read_cb (void *opaque, const void *subbuf, size_t count, uint64_t offset, unsigned status, int *error) { - assert (read_cb_free == 0); + assert (read_cb_freed == 0); - if (valid_flag & LIBNBD_CALLBACK_VALID) - read_cb_valid++; - if (valid_flag & LIBNBD_CALLBACK_FREE) - read_cb_free++; + read_cb_called++; return 0; } +static void +read_cb_free (void *ptr, void *opaque) +{ + read_cb_freed++; +} + static int -completion_cb (unsigned valid_flag, void *opaque, int *error) +completion_cb (void *opaque, int *error) { - assert (completion_cb_free == 0); + assert (completion_cb_freed == 0); - if (valid_flag & LIBNBD_CALLBACK_VALID) - completion_cb_valid++; - if (valid_flag & LIBNBD_CALLBACK_FREE) - completion_cb_free++; + completion_cb_called++; return 0; } +static void +completion_cb_free (void *ptr, void *opaque) +{ + completion_cb_freed++; +} + #define NBD_ERROR \ do { \ fprintf (stderr, "%s: %s\n", argv[0], nbd_get_error ()); \ @@ -101,35 +110,44 @@ main (int argc, char *argv[]) nbd = nbd_create (); if (nbd == NULL) NBD_ERROR; - nbd_set_debug_callback (nbd, debug_fn, NULL); - assert (debug_fn_free == 0); + if (nbd_add_free_callback (nbd, &debug_fn, debug_fn_free, NULL) == -1) + NBD_ERROR; + nbd_set_debug_callback (nbd, debug_fn, &debug_fn); + assert (debug_fn_freed == 0); - nbd_set_debug_callback (nbd, debug_fn, NULL); - assert (debug_fn_free == 1); + if (nbd_add_free_callback (nbd, &debug_fn, debug_fn_free, NULL) == -1) + NBD_ERROR; + nbd_set_debug_callback (nbd, debug_fn, &debug_fn); + assert (debug_fn_freed == 1); - debug_fn_free = 0; + debug_fn_freed = 0; nbd_close (nbd); - assert (debug_fn_free == 1); + assert (debug_fn_freed == 1); /* Test command callbacks are freed when the command is retired. */ nbd = nbd_create (); if (nbd == NULL) NBD_ERROR; if (nbd_connect_command (nbd, nbdkit) == -1) NBD_ERROR; + if (nbd_add_free_callback (nbd, &read_cb, read_cb_free, NULL) == -1) + NBD_ERROR; + if (nbd_add_free_callback (nbd, &completion_cb, completion_cb_free, NULL) + == -1) + NBD_ERROR; cookie = nbd_aio_pread_structured_callback (nbd, buf, sizeof buf, 0, - read_cb, NULL, - completion_cb, NULL, 0); + read_cb, &read_cb, + completion_cb, &completion_cb, 0); if (cookie == -1) NBD_ERROR; - assert (read_cb_free == 0); - assert (completion_cb_free == 0); + assert (read_cb_freed == 0); + assert (completion_cb_freed == 0); while (!nbd_aio_command_completed (nbd, cookie)) { if (nbd_poll (nbd, -1) == -1) NBD_ERROR; } - assert (read_cb_valid == 1); - assert (completion_cb_valid == 1); - assert (read_cb_free == 1); - assert (completion_cb_free == 1); + assert (read_cb_called == 1); + assert (completion_cb_called == 1); + assert (read_cb_freed == 1); + assert (completion_cb_freed == 1); nbd_kill_command (nbd, 0); nbd_close (nbd); @@ -137,21 +155,26 @@ main (int argc, char *argv[]) /* Test command callbacks are freed if the handle is closed without * running the commands. */ - read_cb_valid = read_cb_free = - completion_cb_valid = completion_cb_free = 0; + read_cb_called = read_cb_freed = + completion_cb_called = completion_cb_freed = 0; nbd = nbd_create (); if (nbd == NULL) NBD_ERROR; if (nbd_connect_command (nbd, nbdkit_delay) == -1) NBD_ERROR; + if (nbd_add_free_callback (nbd, &read_cb, read_cb_free, NULL) == -1) + NBD_ERROR; + if (nbd_add_free_callback (nbd, &completion_cb, completion_cb_free, NULL) + == -1) + NBD_ERROR; cookie = nbd_aio_pread_structured_callback (nbd, buf, sizeof buf, 0, - read_cb, NULL, - completion_cb, NULL, 0); + read_cb, &read_cb, + completion_cb, &completion_cb, 0); if (cookie == -1) NBD_ERROR; nbd_kill_command (nbd, 0); nbd_close (nbd); - assert (read_cb_free == 1); - assert (completion_cb_free == 1); + assert (read_cb_freed == 1); + assert (completion_cb_freed == 1); exit (EXIT_SUCCESS); } diff --git a/tests/meta-base-allocation.c b/tests/meta-base-allocation.c index c36a77f..0219ce9 100644 --- a/tests/meta-base-allocation.c +++ b/tests/meta-base-allocation.c @@ -30,7 +30,7 @@ #include <libnbd.h> -static int check_extent (unsigned valid_flag, void *data, +static int check_extent (void *data, const char *metacontext, uint64_t offset, uint32_t *entries, size_t nr_entries, int *error); @@ -129,7 +129,7 @@ main (int argc, char *argv[]) } static int -check_extent (unsigned valid_flag, void *data, +check_extent (void *data, const char *metacontext, uint64_t offset, uint32_t *entries, size_t nr_entries, int *error) @@ -137,9 +137,6 @@ check_extent (unsigned valid_flag, void *data, size_t i; int id; - if (!(valid_flag & LIBNBD_CALLBACK_VALID)) - return 0; - id = * (int *)data; printf ("extent: id=%d, metacontext=%s, offset=%" PRIu64 ", " diff --git a/tests/oldstyle.c b/tests/oldstyle.c index afbda61..1382d06 100644 --- a/tests/oldstyle.c +++ b/tests/oldstyle.c @@ -37,15 +37,12 @@ static char wbuf[512] = { 1, 2, 3, 4 }, rbuf[512]; static const char *progname; static int -pread_cb (unsigned valid_flag, void *data, +pread_cb (void *data, const void *buf, size_t count, uint64_t offset, unsigned status, int *error) { int *calls; - if (!(valid_flag & LIBNBD_CALLBACK_VALID)) - return 0; - calls = data; ++*calls; diff --git a/tests/server-death.c b/tests/server-death.c index 7854527..256b45a 100644 --- a/tests/server-death.c +++ b/tests/server-death.c @@ -33,11 +33,8 @@ static bool trim_retired; static const char *progname; static int -callback (unsigned valid_flag, void *ignored, int *error) +callback (void *ignored, int *error) { - if (!(valid_flag & LIBNBD_CALLBACK_VALID)) - return 0; - if (*error != ENOTCONN) { fprintf (stderr, "%s: unexpected error in trim callback: %s\n", progname, strerror (*error)); -- 2.22.0