11:08 a.m.
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