In preparation for closure lifetimes, split up the Closure so it no longer describes a list of closures, but a single callback. This changes the API because functions which take 2 or more closures now pass a separate user_data for each one. --- docs/libnbd.pod | 3 +- examples/strict-structured-reads.c | 2 +- generator/generator | 760 ++++++++++++---------------- generator/states-reply-simple.c | 2 +- generator/states-reply-structured.c | 8 +- lib/internal.h | 3 +- lib/rw.c | 32 +- 7 files changed, 364 insertions(+), 446 deletions(-) diff --git a/docs/libnbd.pod b/docs/libnbd.pod index 5608e63..631bb3b 100644 --- a/docs/libnbd.pod +++ b/docs/libnbd.pod @@ -487,8 +487,7 @@ C<nbd_set_debug_callback>, C<nbd_pread_callback>). Libnbd can call these functions while processing. Callbacks have an opaque C<void *user_data> pointer. This is passed -as the first parameter to the callback. libnbd functions that take -two callback pointers share the same opaque data for both calls. +as the first parameter to the callback. The callbacks are invoked at a point where the libnbd lock is held; as such, it is unsafe for the callback to call any C<nbd_*> APIs on the diff --git a/examples/strict-structured-reads.c b/examples/strict-structured-reads.c index 92eb3e6..a50f662 100644 --- a/examples/strict-structured-reads.c +++ b/examples/strict-structured-reads.c @@ -225,7 +225,7 @@ main (int argc, char *argv[]) *d = (struct data) { .offset = offset, .count = maxsize, .flags = flags, .remaining = r, }; if (nbd_aio_pread_structured_callback (nbd, buf, sizeof buf, offset, - read_chunk, read_verify, d, + read_chunk, d, read_verify, d, flags) == -1) { fprintf (stderr, "%s\n", nbd_get_error ()); exit (EXIT_FAILURE); diff --git a/generator/generator b/generator/generator index fdacd71..3b57713 100755 --- a/generator/generator +++ b/generator/generator @@ -849,10 +849,10 @@ and arg = written by the function *) | BytesPersistIn of string * string (* same as above, but buffer persists *) | BytesPersistOut of string * string -| Closure of bool * closure list (* void *opaque + one or more closures - flag if true means callbacks persist - in the handle, false means they only - exist during the function call *) +| Closure of bool * closure (* function pointer + void *opaque + flag if true means callbacks persist + in the handle, false means they only + exist during the function call *) | Flags of string (* NBD_CMD_FLAG_* flags *) | Int of string (* small int *) | Int64 of string (* 64 bit signed int *) @@ -921,8 +921,8 @@ Return the state of the debug flag on this handle."; "set_debug_callback", { default_call with args = [ Closure (true, - [{ cbname="debug_fn"; - cbargs=[String "context"; String "msg"] }]) ]; + { cbname="debug_fn"; + cbargs=[String "context"; String "msg"] }) ]; ret = RErr; shortdesc = "set the debug callback"; longdesc = "\ @@ -1351,10 +1351,10 @@ protocol extensions)."; default_call with args = [ BytesOut ("buf", "count"); UInt64 "offset"; Closure (false, - [{ cbname="chunk"; - cbargs=[BytesIn ("subbuf", "count"); - UInt64 "offset"; Int "status"; - Mutable (Int "error")] }]); + { cbname="chunk"; + cbargs=[BytesIn ("subbuf", "count"); + UInt64 "offset"; Int "status"; + Mutable (Int "error")] }); Flags "flags" ]; ret = RErr; permitted_states = [ Connected ]; @@ -1542,12 +1542,12 @@ punching a hole."; default_call with args = [ UInt64 "count"; UInt64 "offset"; Closure (false, - [{ cbname="extent"; - cbargs=[String "metacontext"; - UInt64 "offset"; - ArrayAndLen (UInt32 "entries", - "nr_entries"); - Mutable (Int "error")]} ]); + { cbname="extent"; + cbargs=[String "metacontext"; + UInt64 "offset"; + ArrayAndLen (UInt32 "entries", + "nr_entries"); + Mutable (Int "error")]} ); Flags "flags" ]; ret = RErr; permitted_states = [ Connected ]; @@ -1726,8 +1726,8 @@ C<nbd_pread>."; default_call with args = [ BytesPersistOut ("buf", "count"); UInt64 "offset"; Closure (true, - [{ cbname="callback"; - cbargs=[Int64 "cookie"; Mutable (Int "error")] } ]); + { cbname="callback"; + cbargs=[Int64 "cookie"; Mutable (Int "error")] } ); Flags "flags" ]; ret = RInt64; permitted_states = [ Connected ]; @@ -1756,11 +1756,11 @@ cause a deadlock."; default_call with args = [ BytesPersistOut ("buf", "count"); UInt64 "offset"; Closure (true, - [{ cbname="chunk"; - cbargs=[BytesIn ("subbuf", "count"); - UInt64 "offset"; - Int "status"; - Mutable (Int "error");]} ]); + { cbname="chunk"; + cbargs=[BytesIn ("subbuf", "count"); + UInt64 "offset"; + Int "status"; + Mutable (Int "error");]} ); Flags "flags" ]; ret = RInt64; permitted_states = [ Connected ]; @@ -1778,14 +1778,15 @@ documented in C<nbd_pread_structured>."; default_call with args = [ BytesPersistOut ("buf", "count"); UInt64 "offset"; Closure (true, - [{ cbname="chunk"; - cbargs=[BytesIn ("subbuf", "count"); - UInt64 "offset"; - Int "status"; - Mutable (Int "error"); ]}; - { cbname="callback"; - cbargs=[Int64 "cookie"; - Mutable (Int "error"); ]} ]); + { cbname="chunk"; + cbargs=[BytesIn ("subbuf", "count"); + UInt64 "offset"; + Int "status"; + Mutable (Int "error"); ]}); + Closure (true, + { cbname="callback"; + cbargs=[Int64 "cookie"; + Mutable (Int "error"); ]} ); Flags "flags" ]; ret = RInt64; permitted_states = [ Connected ]; @@ -1829,8 +1830,8 @@ C<nbd_pwrite>."; default_call with args = [ BytesPersistIn ("buf", "count"); UInt64 "offset"; Closure (true, - [{ cbname="callback"; - cbargs=[Int64 "cookie"; Mutable (Int "error")]} ]); + { cbname="callback"; + cbargs=[Int64 "cookie"; Mutable (Int "error")]} ); Flags "flags" ]; ret = RInt64; permitted_states = [ Connected ]; @@ -1894,8 +1895,8 @@ Parameters behave as documented in C<nbd_flush>."; "aio_flush_callback", { default_call with args = [ Closure (true, - [{ cbname="callback"; - cbargs=[Int64 "cookie"; Mutable (Int "error")]} ]); + { cbname="callback"; + cbargs=[Int64 "cookie"; Mutable (Int "error")]} ); Flags "flags" ]; ret = RInt64; permitted_states = [ Connected ]; @@ -1937,8 +1938,8 @@ Parameters behave as documented in C<nbd_trim>."; default_call with args = [ UInt64 "count"; UInt64 "offset"; Closure (true, - [{ cbname="callback"; - cbargs=[Int64 "cookie"; Mutable (Int "error")]} ]); + { cbname="callback"; + cbargs=[Int64 "cookie"; Mutable (Int "error")]} ); Flags "flags" ]; ret = RInt64; permitted_states = [ Connected ]; @@ -1980,8 +1981,8 @@ Parameters behave as documented in C<nbd_cache>."; default_call with args = [ UInt64 "count"; UInt64 "offset"; Closure (true, - [{ cbname="callback"; - cbargs=[Int64 "cookie"; Mutable (Int "error")]} ]); + { cbname="callback"; + cbargs=[Int64 "cookie"; Mutable (Int "error")]} ); Flags "flags" ]; ret = RInt64; permitted_states = [ Connected ]; @@ -2023,8 +2024,8 @@ Parameters behave as documented in C<nbd_zero>."; default_call with args = [ UInt64 "count"; UInt64 "offset"; Closure (true, - [{ cbname="callback"; - cbargs=[Int64 "cookie"; Mutable (Int "error")]} ]); + { cbname="callback"; + cbargs=[Int64 "cookie"; Mutable (Int "error")]} ); Flags "flags" ]; ret = RInt64; permitted_states = [ Connected ]; @@ -2052,11 +2053,11 @@ cause a deadlock."; default_call with args = [ UInt64 "count"; UInt64 "offset"; Closure (true, - [{ cbname="extent"; - cbargs=[String "metacontext"; UInt64 "offset"; - ArrayAndLen (UInt32 "entries", - "nr_entries"); - Mutable (Int "error")] } ]); + { cbname="extent"; + cbargs=[String "metacontext"; UInt64 "offset"; + ArrayAndLen (UInt32 "entries", + "nr_entries"); + Mutable (Int "error")] } ); Flags "flags" ]; ret = RInt64; permitted_states = [ Connected ]; @@ -2073,13 +2074,14 @@ Parameters behave as documented in C<nbd_block_status>."; default_call with args = [ UInt64 "count"; UInt64 "offset"; Closure (true, - [{ cbname="extent"; - cbargs=[String "metacontext"; UInt64 "offset"; - ArrayAndLen (UInt32 "entries", - "nr_entries"); - Mutable (Int "error")]}; - { cbname="callback"; - cbargs=[Int64 "cookie"; Mutable (Int "error")]} ]); + { cbname="extent"; + cbargs=[String "metacontext"; UInt64 "offset"; + ArrayAndLen (UInt32 "entries", + "nr_entries"); + Mutable (Int "error")]}); + Closure (true, + { cbname="callback"; + cbargs=[Int64 "cookie"; Mutable (Int "error")]} ); Flags "flags" ]; ret = RInt64; permitted_states = [ Connected ]; @@ -3137,20 +3139,6 @@ let () = name ) handle_calls; - (* Closure also must appear only once, but can be anywhere. *) - List.iter ( - fun (name, { args }) -> - let rec loop = function - | [] -> () - | Closure _ :: xs -> - if List.exists (function Closure _ -> true | _ -> false) xs then - failwithf "%s: Closure must appear only once" name - | x :: xs -> - loop xs - in - loop args - ) handle_calls; - (* !may_set_error is incompatible with permitted_states != [] because * an incorrect state will result in set_error being called by the * generated wrapper. @@ -3186,8 +3174,7 @@ let rec name_of_arg = function | BytesOut (n, len) -> [n; len] | BytesPersistIn (n, len) -> [n; len] | BytesPersistOut (n, len) -> [n; len] -| Closure (_, closures) -> - List.map (fun { cbname } -> cbname) closures @ ["user_data"] +| Closure (_, { cbname }) -> [cbname; sprintf "%s_user_data" cbname ] | Flags n -> [n] | Int n -> [n] | Int64 n -> [n] @@ -3241,19 +3228,16 @@ let rec print_arg_list ?(handle = false) ?(user_data = false) pr "%s, " n; if types then pr "size_t "; pr "%s" len - | Closure (_, cls) -> - List.iter ( - fun { cbname; cbargs } -> - if types then ( - pr "int (*%s) " cbname; - print_arg_list ~user_data:true cbargs; - ) - else - pr "%s" cbname; - pr ", " - ) cls; + | Closure (_, { cbname; cbargs }) -> + if types then ( + pr "int (*%s) " cbname; + print_arg_list ~user_data:true cbargs; + ) + else + pr "%s" cbname; + pr ", "; if types then pr "void *"; - pr "user_data" + pr "%s_user_data" cbname | Flags n -> if types then pr "uint32_t "; pr "%s" n @@ -3811,154 +3795,140 @@ let print_python_binding name { args; ret } = *) List.iter ( function - | Closure (persistent, cls) -> - pr "struct %s_user_data {\n" name; - List.iter ( - fun { cbname } -> - pr " PyObject *%s;\n" cbname - ) cls; - pr "};\n"; - pr "\n"; - + | Closure (persistent, { cbname; cbargs }) -> (* Persistent closures need an explicit function to decrement * the closure refcounts and free the user_data struct. *) if persistent then ( pr "static void\n"; - pr "free_%s_user_data (void *vp)\n" name; + pr "free_%s_%s_user_data (void *vp)\n" name cbname; pr "{\n"; - pr " struct %s_user_data *user_data = vp;\n" name; + pr " PyObject *user_data = vp;\n"; pr "\n"; - List.iter ( - fun { cbname } -> - pr " Py_DECREF (user_data->%s);\n" cbname - ) cls; - pr " free (user_data);\n"; + pr " Py_DECREF (user_data);\n"; pr "}\n"; pr "\n"; ); + pr "/* Wrapper for %s callback of %s. */\n" cbname name; + pr "static int\n"; + pr "%s_%s_wrapper " name cbname; + C.print_arg_list ~user_data:true cbargs; + pr "\n"; + pr "{\n"; + pr " int ret;\n"; + pr " PyGILState_STATE py_save = PyGILState_UNLOCKED;\n"; + pr " PyObject *py_args, *py_ret;\n"; List.iter ( - fun { cbname; cbargs } -> - pr "static int\n"; - pr "%s_%s_wrapper " name cbname; - C.print_arg_list ~user_data:true cbargs; - pr "\n"; - pr "{\n"; - pr " int ret;\n"; - pr " PyGILState_STATE py_save = PyGILState_UNLOCKED;\n"; - pr " PyObject *py_args, *py_ret;\n"; - List.iter ( - function - | ArrayAndLen (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 - | BytesIn _ - | Int _ - | Int64 _ -> () - | Mutable (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; - | String n - | UInt64 n -> () - (* The following not yet implemented for callbacks XXX *) - | ArrayAndLen _ | Bool _ | BytesOut _ - | BytesPersistIn _ | BytesPersistOut _ - | Closure _ - | Flags _ | Mutable _ - | Path _ | SockAddrAndLen _ | StringList _ - | UInt _ | UInt32 _ -> assert false - ) cbargs; - pr "\n"; + function + | ArrayAndLen (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 + | BytesIn _ + | Int _ + | Int64 _ -> () + | Mutable (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; + | String n + | UInt64 n -> () + (* The following not yet implemented for callbacks XXX *) + | ArrayAndLen _ | Bool _ | BytesOut _ + | BytesPersistIn _ | BytesPersistOut _ + | Closure _ + | Flags _ | Mutable _ + | Path _ | SockAddrAndLen _ | StringList _ + | UInt _ | UInt32 _ -> assert false + ) cbargs; + pr "\n"; - pr " py_args = Py_BuildValue (\"(\""; - List.iter ( - function - | ArrayAndLen (UInt32 n, len) -> pr " \"O\"" - | BytesIn (n, len) -> pr " \"y#\"" - | Int n -> pr " \"i\"" - | Int64 n -> pr " \"L\"" - | Mutable (Int n) -> pr " \"O\"" - | String n -> pr " \"s\"" - | UInt64 n -> pr " \"K\"" - (* The following not yet implemented for callbacks XXX *) - | ArrayAndLen _ | Bool _ | BytesOut _ - | BytesPersistIn _ | BytesPersistOut _ - | Closure _ - | Flags _ | Mutable _ - | Path _ | SockAddrAndLen _ | StringList _ - | UInt _ | UInt32 _ -> assert false - ) cbargs; - pr " \")\""; - List.iter ( - function - | ArrayAndLen (UInt32 n, _) -> pr ", py_%s" n - | BytesIn (n, len) -> pr ", %s, (int) %s" n len - | Mutable (Int n) -> pr ", py_%s" n - | Int n | Int64 n - | String n - | UInt64 n -> pr ", %s" n - (* The following not yet implemented for callbacks XXX *) - | ArrayAndLen _ | Bool _ | BytesOut _ - | BytesPersistIn _ | BytesPersistOut _ - | Closure _ - | Flags _ | Mutable _ - | Path _ | SockAddrAndLen _ | StringList _ - | UInt _ | UInt32 _ -> assert false - ) cbargs; - pr ");\n"; - pr " Py_INCREF (py_args);\n"; - pr "\n"; - pr " if (PyEval_ThreadsInitialized ())\n"; - pr " py_save = PyGILState_Ensure ();\n"; - pr "\n"; - pr " py_ret = PyObject_CallObject (((struct %s_user_data *)user_data)->%s, py_args);\n" name cbname; - pr "\n"; - pr " if (PyEval_ThreadsInitialized ())\n"; - pr " PyGILState_Release (py_save);\n"; - pr "\n"; - pr " Py_DECREF (py_args);\n"; - pr "\n"; - pr " if (py_ret != NULL) {\n"; - pr " ret = 0;\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 - | ArrayAndLen (UInt32 n, _) -> - pr " Py_DECREF (py_%s);\n" n - | Mutable (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 - | BytesIn _ - | Int _ | Int64 _ - | String _ - | UInt64 _ -> () - (* The following not yet implemented for callbacks XXX *) - | ArrayAndLen _ | Bool _ | BytesOut _ - | BytesPersistIn _ | BytesPersistOut _ - | Closure _ - | Flags _ | Mutable _ - | Path _ | SockAddrAndLen _ | StringList _ - | UInt _ | UInt32 _ -> assert false - ) cbargs; - pr " return ret;\n"; - pr "}\n"; - pr "\n" - ) cls + pr " py_args = Py_BuildValue (\"(\""; + List.iter ( + function + | ArrayAndLen (UInt32 n, len) -> pr " \"O\"" + | BytesIn (n, len) -> pr " \"y#\"" + | Int n -> pr " \"i\"" + | Int64 n -> pr " \"L\"" + | Mutable (Int n) -> pr " \"O\"" + | String n -> pr " \"s\"" + | UInt64 n -> pr " \"K\"" + (* The following not yet implemented for callbacks XXX *) + | ArrayAndLen _ | Bool _ | BytesOut _ + | BytesPersistIn _ | BytesPersistOut _ + | Closure _ + | Flags _ | Mutable _ + | Path _ | SockAddrAndLen _ | StringList _ + | UInt _ | UInt32 _ -> assert false + ) cbargs; + pr " \")\""; + List.iter ( + function + | ArrayAndLen (UInt32 n, _) -> pr ", py_%s" n + | BytesIn (n, len) -> pr ", %s, (int) %s" n len + | Mutable (Int n) -> pr ", py_%s" n + | Int n | Int64 n + | String n + | UInt64 n -> pr ", %s" n + (* The following not yet implemented for callbacks XXX *) + | ArrayAndLen _ | Bool _ | BytesOut _ + | BytesPersistIn _ | BytesPersistOut _ + | Closure _ + | Flags _ | Mutable _ + | Path _ | SockAddrAndLen _ | StringList _ + | UInt _ | UInt32 _ -> assert false + ) cbargs; + pr ");\n"; + pr " Py_INCREF (py_args);\n"; + pr "\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 "\n"; + pr " if (PyEval_ThreadsInitialized ())\n"; + pr " PyGILState_Release (py_save);\n"; + pr "\n"; + pr " Py_DECREF (py_args);\n"; + pr "\n"; + pr " if (py_ret != NULL) {\n"; + pr " ret = 0;\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 + | ArrayAndLen (UInt32 n, _) -> + pr " Py_DECREF (py_%s);\n" n + | Mutable (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 + | BytesIn _ + | Int _ | Int64 _ + | String _ + | UInt64 _ -> () + (* The following not yet implemented for callbacks XXX *) + | ArrayAndLen _ | Bool _ | BytesOut _ + | BytesPersistIn _ | BytesPersistOut _ + | Closure _ + | Flags _ | Mutable _ + | Path _ | SockAddrAndLen _ | StringList _ + | UInt _ | UInt32 _ -> assert false + ) cbargs; + pr " return ret;\n"; + pr "}\n"; + pr "\n" | _ -> () ) args; @@ -3996,11 +3966,8 @@ let print_python_binding name { args; ret } = pr " PyObject *%s; /* PyCapsule pointing to struct py_aio_buffer */\n" n; pr " struct py_aio_buffer *%s_buf;\n" n - | Closure (false, cls) -> - pr " struct %s_user_data _user_data;\n" name; - pr " struct %s_user_data *user_data = &_user_data;\n" name - | Closure (true, cls) -> - pr " struct %s_user_data *user_data;\n" name + | Closure (_, { cbname }) -> + pr " PyObject *%s_user_data;\n" cbname | Flags n -> pr " uint32_t %s_u32;\n" n; pr " unsigned int %s; /* really uint32_t */\n" n @@ -4031,19 +3998,6 @@ let print_python_binding name { args; ret } = ) args; pr "\n"; - (* Allocate the persistent closure user_data. *) - List.iter ( - function - | Closure (false, _) -> () - | Closure (true, cls) -> - pr " user_data = malloc (sizeof *user_data);\n"; - pr " if (user_data == NULL) {\n"; - pr " PyErr_NoMemory ();\n"; - pr " return NULL;\n"; - pr " }\n" - | _ -> () - ) args; - (* Parse the Python parameters. *) pr " if (!PyArg_ParseTuple (args, (char *) \"O\""; List.iter ( @@ -4054,7 +4008,7 @@ let print_python_binding name { args; ret } = | BytesPersistIn (n, _) -> pr " \"O\"" | BytesOut (_, count) -> pr " \"n\"" | BytesPersistOut (_, count) -> pr " \"O\"" - | Closure (_, cls) -> List.iter (fun _ -> pr " \"O\"") cls + | Closure _ -> pr " \"O\"" | Flags n -> pr " \"I\"" | Int n -> pr " \"i\"" | Int64 n -> pr " \"L\"" @@ -4078,8 +4032,7 @@ let print_python_binding name { args; ret } = | BytesIn (n, _) | BytesPersistIn (n, _) | BytesPersistOut (n, _) -> pr ", &%s" n | BytesOut (_, count) -> pr ", &%s" count - | Closure (_, cls) -> - List.iter (fun { cbname } -> pr ", &user_data->%s" cbname) cls + | Closure (_, { cbname }) -> pr ", &%s_user_data" cbname | Flags n -> pr ", &%s" n | Int n -> pr ", &%s" n | Int64 n -> pr ", &%s" n @@ -4101,11 +4054,8 @@ let print_python_binding name { args; ret } = List.iter ( function | Closure (false, _) -> () - | Closure (true, cls) -> - List.iter ( - fun { cbname } -> - pr " Py_INCREF (user_data->%s);\n" cbname - ) cls + | Closure (true, { cbname }) -> + pr " Py_INCREF (%s_user_data);\n" cbname | _ -> () ) args; @@ -4136,15 +4086,12 @@ let print_python_binding name { args; ret } = pr " %s = malloc (%s);\n" n count | BytesPersistIn (n, _) | BytesPersistOut (n, _) -> pr " %s_buf = nbd_internal_py_get_aio_buffer (%s);\n" n n - | Closure (_, cls) -> - List.iter ( - fun { cbname } -> - pr " if (!PyCallable_Check (user_data->%s)) {\n" cbname; - pr " PyErr_SetString (PyExc_TypeError,\n"; - pr " \"callback parameter %s is not callable\");\n" cbname; - pr " return NULL;\n"; - pr " }\n" - ) cls + | Closure (_, { cbname }) -> + pr " if (!PyCallable_Check (%s_user_data)) {\n" cbname; + pr " PyErr_SetString (PyExc_TypeError,\n"; + pr " \"callback parameter %s is not callable\");\n" cbname; + pr " return NULL;\n"; + pr " }\n" | Flags n -> pr " %s_u32 = %s;\n" n n | Int _ -> () | Int64 n -> pr " %s_i64 = %s;\n" n n @@ -4175,12 +4122,9 @@ let print_python_binding name { args; ret } = | BytesOut (n, count) -> pr ", %s, %s" n count | BytesPersistIn (n, _) | BytesPersistOut (n, _) -> pr ", %s_buf->data, %s_buf->len" n n - | Closure (_, cls) -> - List.iter ( - fun { cbname } -> - pr ", %s_%s_wrapper" name cbname - ) cls; - pr ", user_data" + | Closure (_, { cbname }) -> + pr ", %s_%s_wrapper" name cbname; + pr ", %s_user_data" cbname | Flags n -> pr ", %s_u32" n | Int n -> pr ", %s" n | Int64 n -> pr ", %s_i64" n @@ -4257,9 +4201,10 @@ let print_python_binding name { args; ret } = | BytesIn (n, _) -> pr " PyBuffer_Release (&%s);\n" n | BytesPersistIn _ | BytesOut _ | BytesPersistOut _ -> () | Closure (false, _) -> () - | Closure (true, _) -> + | Closure (true, { cbname }) -> pr " /* This ensures the callback data is freed eventually. */\n"; - pr " nbd_add_close_callback (h, free_%s_user_data, user_data);\n" name + pr " nbd_add_close_callback (h, free_%s_%s_user_data, %s_user_data);\n" + name cbname cbname | Flags _ -> () | Int _ -> () | Int64 _ -> () @@ -4406,8 +4351,7 @@ class NBD (object): | BytesIn (n, _) | BytesPersistIn (n, _) -> [n, None] | BytesPersistOut (n, _) -> [n, None] | BytesOut (_, count) -> [count, None] - | Closure (_, cls) -> - List.map (fun { cbname } -> cbname, None) cls + | Closure (_, { cbname }) -> [cbname, None] | Flags n -> [n, Some "0"] | Int n -> [n, None] | Int64 n -> [n, None] @@ -4471,7 +4415,6 @@ end = struct type ocaml_arg = | OCamlHandle (* The NBD handle (NBD.t) *) | OCamlFlags of string (* Optional ?flags parameter *) - | OCamlClosure of bool * closure (* Single closure parameter *) | OCamlArg of arg (* Other arg (string = name). *) let args_to_ocaml_args args = @@ -4483,8 +4426,6 @@ let args_to_ocaml_args args = let args = List.map ( function - | Closure (persistent, cls) -> - List.map (fun cl -> OCamlClosure (persistent, cl)) cls | a -> [OCamlArg a] ) args in let args = List.flatten args in @@ -4502,10 +4443,6 @@ let rec ocaml_fundecl_to_string args ret = and ocaml_arg_to_string = function | OCamlHandle -> "t" | OCamlFlags n -> sprintf "?%s:int32 list" n - | OCamlClosure (_, { cbargs }) -> - sprintf "(%s)" - (ocaml_fundecl_to_string (List.map (fun a -> OCamlArg a) cbargs) - RErr) | OCamlArg (ArrayAndLen (t, _)) -> sprintf "%s array" (ocaml_arg_to_string (OCamlArg t)) | OCamlArg (Bool _) -> "bool" @@ -4513,7 +4450,10 @@ and ocaml_arg_to_string = function | OCamlArg (BytesPersistIn _) -> "Buffer.t" | OCamlArg (BytesOut _) -> "bytes" | OCamlArg (BytesPersistOut _) -> "Buffer.t" - | OCamlArg (Closure _) -> assert false (* see above *) + | OCamlArg (Closure (_, { cbargs })) -> + sprintf "(%s)" + (ocaml_fundecl_to_string (List.map (fun a -> OCamlArg a) cbargs) + RErr) | OCamlArg (Flags _) -> assert false (* see above *) | OCamlArg (Int _) -> "int" | OCamlArg (Int64 _) -> "int64" @@ -4538,7 +4478,6 @@ and ocaml_ret_to_string = function let rec name_of_ocaml_arg = function | OCamlHandle -> "h" | OCamlFlags n -> n - | OCamlClosure (_, { cbname }) -> cbname | OCamlArg a -> match a with | ArrayAndLen (arg, n) -> name_of_ocaml_arg (OCamlArg arg) @@ -4547,7 +4486,7 @@ let rec name_of_ocaml_arg = function | BytesOut (n, len) -> n | BytesPersistIn (n, len) -> n | BytesPersistOut (n, len) -> n - | Closure (_, closures) -> assert false + | Closure (_, { cbname }) -> cbname | Flags n -> n | Int n -> n | Int64 n -> n @@ -4705,42 +4644,31 @@ let print_ocaml_binding (name, { args; ret }) = (* Functions with a callback parameter require special handling. *) List.iter ( function - | Closure (persistent, cls) -> - pr "struct %s_user_data {\n" name; - List.iter (fun { cbname } -> pr " value %s;\n" cbname) cls; - pr "};\n"; - pr "\n"; - + | Closure (persistent, { cbname; cbargs }) -> (* Persistent closures need an explicit function to remove * the global root and free the user_data struct. *) if persistent then ( pr "static void\n"; - pr "free_%s_user_data (void *vp)\n" name; + pr "free_%s_%s_user_data (void *vp)\n" name cbname; pr "{\n"; - pr " struct %s_user_data *user_data = vp;\n" name; + pr " value *user_data = vp;\n"; pr "\n"; - List.iter ( - fun { cbname } -> - pr " caml_remove_generational_global_root (&user_data->%s);\n" - cbname - ) cls; + pr " caml_remove_generational_global_root (user_data);\n"; pr " free (user_data);\n"; pr "}\n"; pr "\n" ); - List.iter ( - fun { cbname; cbargs } -> - let argnames = - List.map ( - function - | ArrayAndLen (UInt32 n, _) | BytesIn (n, _) + let argnames = + List.map ( + function + | ArrayAndLen (UInt32 n, _) | BytesIn (n, _) | Int n | Int64 n | Mutable (Int n) | String n | UInt64 n -> - n ^ "v" - (* The following not yet implemented for callbacks XXX *) - | ArrayAndLen _ | Bool _ | BytesOut _ + n ^ "v" + (* The following not yet implemented for callbacks XXX *) + | ArrayAndLen _ | Bool _ | BytesOut _ | BytesPersistIn _ | BytesPersistOut _ | Closure _ | Flags _ | Path _ | Mutable _ @@ -4748,105 +4676,105 @@ let print_ocaml_binding (name, { args; ret }) = | UInt _ | UInt32 _ -> assert false ) cbargs in - pr "static int\n"; - pr "%s_%s_wrapper_locked " name cbname; - C.print_arg_list ~user_data:true cbargs; - pr "\n"; - pr "{\n"; - pr " CAMLparam0 ();\n"; - assert (List.length argnames <= 5); - pr " CAMLlocal%d (%s);\n" (List.length argnames) - (String.concat ", " argnames); - pr " CAMLlocal2 (fnv, rv);\n"; - pr " value args[%d];\n" (List.length argnames); - pr "\n"; + pr "/* Wrapper for %s callback of %s. */\n" cbname name; + pr "static int\n"; + pr "%s_%s_wrapper_locked " name cbname; + C.print_arg_list ~user_data:true cbargs; + pr "\n"; + pr "{\n"; + pr " CAMLparam0 ();\n"; + assert (List.length argnames <= 5); + pr " CAMLlocal%d (%s);\n" (List.length argnames) + (String.concat ", " argnames); + pr " CAMLlocal2 (fnv, rv);\n"; + pr " value args[%d];\n" (List.length argnames); + pr "\n"; - List.iter ( - function - | ArrayAndLen (UInt32 n, count) -> - pr " %sv = nbd_internal_ocaml_alloc_int32_array (%s, %s);\n" - n n count; - | BytesIn (n, len) -> - pr " %sv = caml_alloc_string (%s);\n" n len; - pr " memcpy (String_val (%sv), %s, %s);\n" n n len - | Int n -> - pr " %sv = Val_int (%s);\n" n n - | Int64 n -> - pr " %sv = caml_copy_int64 (%s);\n" n n - | String n -> - pr " %sv = caml_copy_string (%s);\n" n n - | UInt64 n -> - pr " %sv = caml_copy_int64 (%s);\n" n n - | Mutable (Int n) -> - pr " %sv = caml_alloc_tuple (1);\n" n; - pr " Store_field (%sv, 0, Val_int (*%s));\n" n n - (* The following not yet implemented for callbacks XXX *) - | ArrayAndLen _ | Bool _ | BytesOut _ - | BytesPersistIn _ | BytesPersistOut _ - | Closure _ - | Flags _ | Mutable _ - | Path _ | SockAddrAndLen _ | StringList _ - | UInt _ | UInt32 _ -> assert false - ) cbargs; + List.iter ( + function + | ArrayAndLen (UInt32 n, count) -> + pr " %sv = nbd_internal_ocaml_alloc_int32_array (%s, %s);\n" + n n count; + | BytesIn (n, len) -> + pr " %sv = caml_alloc_string (%s);\n" n len; + pr " memcpy (String_val (%sv), %s, %s);\n" n n len + | Int n -> + pr " %sv = Val_int (%s);\n" n n + | Int64 n -> + pr " %sv = caml_copy_int64 (%s);\n" n n + | String n -> + pr " %sv = caml_copy_string (%s);\n" n n + | UInt64 n -> + pr " %sv = caml_copy_int64 (%s);\n" n n + | Mutable (Int n) -> + pr " %sv = caml_alloc_tuple (1);\n" n; + pr " Store_field (%sv, 0, Val_int (*%s));\n" n n + (* The following not yet implemented for callbacks XXX *) + | ArrayAndLen _ | Bool _ | BytesOut _ + | BytesPersistIn _ | BytesPersistOut _ + | Closure _ + | Flags _ | Mutable _ + | Path _ | SockAddrAndLen _ | StringList _ + | UInt _ | UInt32 _ -> assert false + ) cbargs; - List.iteri (fun i n -> pr " args[%d] = %s;\n" i n) argnames; + List.iteri (fun i n -> pr " args[%d] = %s;\n" i n) argnames; - pr " fnv = ((struct %s_user_data *)user_data)->%s;\n" name cbname; + pr " fnv = * (value *) user_data;\n"; - pr " rv = caml_callbackN_exn (fnv, %d, args);\n" - (List.length argnames); + pr " rv = caml_callbackN_exn (fnv, %d, args);\n" + (List.length argnames); - List.iter ( - function - | ArrayAndLen (UInt32 n, count) -> () - | BytesIn (n, len) -> () - | Int n -> () - | Int64 n -> () - | String n -> () - | UInt64 n -> () - | Mutable (Int n) -> - pr " *%s = Int_val (Field (%sv, 0));\n" n n - (* The following not yet implemented for callbacks XXX *) - | ArrayAndLen _ | Bool _ | BytesOut _ - | BytesPersistIn _ | BytesPersistOut _ - | Closure _ - | Flags _ | Mutable _ - | Path _ | SockAddrAndLen _ | StringList _ - | UInt _ | UInt32 _ -> assert false - ) cbargs; + List.iter ( + function + | ArrayAndLen (UInt32 n, count) -> () + | BytesIn (n, len) -> () + | Int n -> () + | Int64 n -> () + | String n -> () + | UInt64 n -> () + | Mutable (Int n) -> + pr " *%s = Int_val (Field (%sv, 0));\n" n n + (* The following not yet implemented for callbacks XXX *) + | ArrayAndLen _ | Bool _ | BytesOut _ + | BytesPersistIn _ | BytesPersistOut _ + | Closure _ + | Flags _ | Mutable _ + | Path _ | SockAddrAndLen _ | StringList _ + | UInt _ | UInt32 _ -> assert false + ) cbargs; - pr " if (Is_exception_result (rv)) {\n"; - pr " /* XXX This is not really an error as callbacks can return\n"; - pr " * an error indication. But perhaps we should direct this\n"; - pr " * to a more suitable place or formalize what exception\n"; - pr " * means error versus unexpected failure.\n"; - pr " */\n"; - pr " fprintf (stderr,\n"; - pr " \"libnbd: uncaught OCaml exception: %%s\\n\",\n"; - pr " caml_format_exception (Extract_exception (rv)));\n"; - pr " CAMLreturnT (int, -1);\n"; - pr " }\n"; + pr " if (Is_exception_result (rv)) {\n"; + pr " /* XXX This is not really an error as callbacks can return\n"; + pr " * an error indication. But perhaps we should direct this\n"; + pr " * to a more suitable place or formalize what exception\n"; + pr " * means error versus unexpected failure.\n"; + pr " */\n"; + pr " fprintf (stderr,\n"; + pr " \"libnbd: uncaught OCaml exception: %%s\\n\",\n"; + pr " caml_format_exception (Extract_exception (rv)));\n"; + pr " CAMLreturnT (int, -1);\n"; + pr " }\n"; - pr "\n"; - pr " CAMLreturnT (int, 0);\n"; - pr "}\n"; - pr "\n"; - pr "static int\n"; - pr "%s_%s_wrapper " name cbname; - C.print_arg_list ~user_data:true cbargs; - pr "\n"; - pr "{\n"; - pr " int ret;\n"; - pr "\n"; - pr " caml_leave_blocking_section ();\n"; - pr " ret = %s_%s_wrapper_locked " name cbname; - C.print_arg_list ~user_data:true ~types:false cbargs; - pr ";\n"; - pr " caml_enter_blocking_section ();\n"; - pr " return ret;\n"; - pr "}\n"; - pr "\n" - ) cls + pr "\n"; + pr " CAMLreturnT (int, 0);\n"; + pr "}\n"; + pr "\n"; + pr "static int\n"; + pr "%s_%s_wrapper " name cbname; + C.print_arg_list ~user_data:true cbargs; + pr "\n"; + pr "{\n"; + pr " int ret;\n"; + pr "\n"; + pr " caml_leave_blocking_section ();\n"; + pr " ret = %s_%s_wrapper_locked " name cbname; + C.print_arg_list ~user_data:true ~types:false cbargs; + pr ";\n"; + pr " caml_enter_blocking_section ();\n"; + pr " return ret;\n"; + pr "}\n"; + pr "\n" | _ -> () ) args; @@ -4889,32 +4817,6 @@ let print_ocaml_binding (name, { args; ret }) = pr " CAMLlocal1 (rv);\n"; pr "\n"; - (* Set up user_data for closures. *) - let has_closures = - try - let cl = List.find (function Closure _ -> true | _ -> false) args in - match cl with - | Closure (b, _) -> Some b - | _ -> assert false - with - Not_found -> None in - (match has_closures with - | None -> () - | Some false -> - pr " /* This is safe because we only call this while this stack\n"; - pr " * frame is live.\n"; - pr " */\n"; - pr " struct %s_user_data _user_data;\n" name; - pr " struct %s_user_data *user_data = &_user_data;\n" name - | Some true -> - pr " /* The function may save a reference to the closure, so we\n"; - pr " * must treat it as a possible GC root.\n"; - pr " */\n"; - pr " struct %s_user_data *user_data;\n" name; - pr " user_data = malloc (sizeof *user_data);\n"; - pr " if (user_data == NULL) caml_raise_out_of_memory ();\n" - ); - List.iter ( function | OCamlHandle -> @@ -4927,14 +4829,6 @@ let print_ocaml_binding (name, { args; ret }) = pr " %s = Flags_val (Field (%sv, 0));\n" n n; pr " else /* None */\n"; pr " %s = 0;\n" n - | OCamlClosure (false, { cbname }) -> - pr " user_data->%s = %sv;\n" cbname cbname; - pr " const void *%s = %s_%s_wrapper;\n" cbname name cbname - | OCamlClosure (true, { cbname }) -> - pr " user_data->%s = %sv;\n" cbname cbname; - pr " caml_register_generational_global_root (&user_data->%s);\n" - cbname; - pr " const void *%s = %s_%s_wrapper;\n" cbname name cbname | OCamlArg (ArrayAndLen (t, _)) -> (* XXX *) () | OCamlArg (Bool n) -> pr " bool %s = Bool_val (%sv);\n" n n @@ -4952,7 +4846,26 @@ let print_ocaml_binding (name, { args; ret }) = pr " struct nbd_buffer %s_buf = NBD_buffer_val (%sv);\n" n n; pr " void *%s = %s_buf.data;\n" n n; pr " size_t %s = %s_buf.len;\n" count n - | OCamlArg (Closure _) -> assert false (* see above *) + | OCamlArg (Closure (false, { cbname })) -> + pr " /* This is safe because we only call this while this stack\n"; + pr " * frame is live.\n"; + pr " */\n"; + pr " CAMLlocal1 (_%s_user_data);\n" cbname; + pr " value *%s_user_data = &_%s_user_data;\n" cbname cbname; + pr " _%s_user_data = %sv;\n" cbname cbname; + pr " const void *%s = %s_%s_wrapper;\n" cbname name cbname + | OCamlArg (Closure (true, { cbname })) -> + pr " /* The function may save a reference to the closure, so we\n"; + pr " * must treat it as a possible GC root.\n"; + pr " */\n"; + pr " value *%s_user_data;\n" cbname; + pr " %s_user_data = malloc (sizeof (value));\n" cbname; + pr " if (%s_user_data == NULL) caml_raise_out_of_memory ();\n" cbname; + pr " caml_register_generational_global_root (%s_user_data);\n" cbname; + pr " *%s_user_data = %sv;\n" cbname cbname; + pr " const void *%s = %s_%s_wrapper;\n" cbname name cbname; + pr " nbd_add_close_callback (h, free_%s_%s_user_data, %s_user_data);\n" + name cbname cbname | OCamlArg (Flags _) -> assert false (* see above *) | OCamlArg (Int n) -> pr " int %s = Int_val (%sv);\n" n n @@ -4975,12 +4888,6 @@ let print_ocaml_binding (name, { args; ret }) = pr " uint64_t %s = Int64_val (%sv);\n" n n ) oargs; - (match has_closures with - | None | Some false -> () - | Some true -> - pr " nbd_add_close_callback (h, free_%s_user_data, user_data);\n" name - ); - let errcode = match ret with | RBool | RErr | RFd | RInt | RInt64 -> pr " int r;\n"; "-1" @@ -5014,7 +4921,6 @@ let print_ocaml_binding (name, { args; ret }) = | OCamlArg (StringList n) -> pr " free (%s);\n" n | OCamlHandle | OCamlFlags _ - | OCamlClosure _ | OCamlArg (ArrayAndLen _) | OCamlArg (Bool _) | OCamlArg (BytesIn _) diff --git a/generator/states-reply-simple.c b/generator/states-reply-simple.c index f1770fc..94875aa 100644 --- a/generator/states-reply-simple.c +++ b/generator/states-reply-simple.c @@ -64,7 +64,7 @@ int error = 0; assert (cmd->error == 0); - if (cmd->cb.fn.read (cmd->cb.user_data, cmd->data, cmd->count, + if (cmd->cb.fn.read (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 25987ed..f60232e 100644 --- a/generator/states-reply-structured.c +++ b/generator/states-reply-structured.c @@ -298,7 +298,7 @@ * 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.read (cmd->cb.user_data, + if (cmd->cb.fn.read (cmd->cb.fn_user_data, cmd->data + (offset - cmd->offset), 0, offset, LIBNBD_READ_ERROR, &scratch) == -1) if (cmd->error == 0) @@ -385,7 +385,7 @@ if (cmd->cb.fn.read) { int error = cmd->error; - if (cmd->cb.fn.read (cmd->cb.user_data, + if (cmd->cb.fn.read (cmd->cb.fn_user_data, cmd->data + (offset - cmd->offset), length - sizeof offset, offset, LIBNBD_READ_DATA, &error) == -1) @@ -447,7 +447,7 @@ if (cmd->cb.fn.read) { int error = cmd->error; - if (cmd->cb.fn.read (cmd->cb.user_data, + if (cmd->cb.fn.read (cmd->cb.fn_user_data, cmd->data + offset, length, cmd->offset + offset, LIBNBD_READ_HOLE, &error) == -1) @@ -499,7 +499,7 @@ /* Call the caller's extent function. */ int error = cmd->error; - if (cmd->cb.fn.extent (cmd->cb.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) diff --git a/lib/internal.h b/lib/internal.h index 3f2d3f8..b2a65bc 100644 --- a/lib/internal.h +++ b/lib/internal.h @@ -269,8 +269,9 @@ struct command_cb { extent_fn extent; read_fn read; } fn; + void *fn_user_data; /* associated with one of the fn callbacks above */ callback_fn callback; - void *user_data; + void *user_data; /* associated with the callback function */ }; struct command { diff --git a/lib/rw.c b/lib/rw.c index 680b81a..7999a86 100644 --- a/lib/rw.c +++ b/lib/rw.c @@ -280,18 +280,24 @@ nbd_unlocked_aio_pread_structured (struct nbd_handle *h, void *buf, uint32_t flags) { return nbd_unlocked_aio_pread_structured_callback (h, buf, count, offset, - read, NULL, user_data, + read, user_data, + NULL, NULL, flags); } int64_t nbd_unlocked_aio_pread_structured_callback (struct nbd_handle *h, void *buf, size_t count, uint64_t offset, - read_fn read, callback_fn callback, - void *user_data, uint32_t flags) + read_fn read, + void *read_user_data, + callback_fn callback, + void *callback_user_data, + uint32_t flags) { - struct command_cb cb = { .fn.read = read, .callback = callback, - .user_data = user_data, }; + struct command_cb cb = { .fn.read = read, + .fn_user_data = read_user_data, + .callback = callback, + .user_data = callback_user_data, }; if ((flags & ~LIBNBD_CMD_FLAG_DF) != 0) { set_error (EINVAL, "invalid flag: %" PRIu32, flags); @@ -493,18 +499,24 @@ nbd_unlocked_aio_block_status (struct nbd_handle *h, uint32_t flags) { return nbd_unlocked_aio_block_status_callback (h, count, offset, - extent, NULL, user_data, + extent, user_data, + NULL, NULL, flags); } int64_t nbd_unlocked_aio_block_status_callback (struct nbd_handle *h, uint64_t count, uint64_t offset, - extent_fn extent, callback_fn callback, - void *user_data, uint32_t flags) + extent_fn extent, + void *extent_user_data, + callback_fn callback, + void *callback_user_data, + uint32_t flags) { - struct command_cb cb = { .fn.extent = extent, .callback = callback, - .user_data = user_data, }; + struct command_cb cb = { .fn.extent = extent, + .fn_user_data = extent_user_data, + .callback = callback, + .user_data = callback_user_data }; if (!h->structured_replies) { set_error (ENOTSUP, "server does not support structured replies"); -- 2.22.0

On Wed, Jul 24, 2019 at 03:30:51PM -0500, Eric Blake wrote: Yes absolutely it must do this, and it's a bug in my v2 patch that it doesn't. I'll try to fix all this up tomorrow. It's basically free so why not. OK Rich. -- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones Read my programming and virtualization blog: http://rwmj.wordpress.com Fedora Windows cross-compiler. Compile Windows programs, test, and build Windows installers. Over 100 libraries supported. http://fedoraproject.org/wiki/MinGW

Thanks: Eric Blake. --- docs/libnbd.pod | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/docs/libnbd.pod b/docs/libnbd.pod index 0ce4d32..6c8d962 100644 --- a/docs/libnbd.pod +++ b/docs/libnbd.pod @@ -487,7 +487,9 @@ C<nbd_set_debug_callback>, C<nbd_pread_callback>). Libnbd can call these functions while processing. Callbacks have an opaque C<void *user_data> pointer. This is passed -as the second parameter to the callback. +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 @@ -539,6 +541,9 @@ write this: // 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 -- 2.22.0

On 7/24/19 11:54 AM, Richard W.M. Jones wrote: A bit of churn here that could have been squashed into patch 2. But at the end of the day, whether to spend more time polishing the patch when the end result is fine is up to you. ACK series. I'll rebase my auto-retire patches on top of this once you've pushed. -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3226 Virtualization: qemu.org | libvirt.org