similar to: [PATCH libnbd] lib: Remove cookie parameter from completion callbacks.

Previously closures had a crude flag which tells if they are persistent or transient. Transient closures (flag = false) last for the lifetime of the currently called libnbd function. Persistent closures had an indefinite lifetime which could last for as long as the handle. In language bindings handling persistent closures was wasteful as we needed to register a "close callback" to

Previously closures had a crude flag which tells if they are persistent or transient. Transient closures (flag = false) last for the lifetime of the currently called libnbd function. Persistent closures had an indefinite lifetime which could last for as long as the handle. In language bindings handling persistent closures was wasteful as we needed to register a "close callback" to

Previously closures had a crude flag which tells if they are persistent or transient. Transient closures (flag = false) last for the lifetime of the currently called libnbd function. Persistent closures had an indefinite lifetime which could last for as long as the handle. In language bindings handling persistent closures was wasteful as we needed to register a "close callback" to

If we accept that callbacks will never handle the full range of parameters then we can simplify the generator quite a bit by using a special type for closure args vs normal method args. This removes many asserts and quite a bit of unreachable code (eg. Python code for handling ArrayAndLen in normal methods that was never used). The output of the generator after this commit should be identical.

For example nbd_set_debug takes a callback function. Previously this was defined explicitly inside the function parameters. This commit defines a new public typedef: typedef int (*nbd_debug_callback) (unsigned valid_flag, void *user_data, const char *context, const char *msg); and then uses the typedef like this: extern int nbd_set_debug_callback

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

A Closure is a list of (usually one, but can be more) closures. In C there is also a singe ‘void *user_data’ parameter which is passed by the caller into the function and through as the first parameter of each callback invocation. By grouping the previously separate Opaque and Callback* parameters together we can avoid the awkward situation where we have to scan through the argument list to try

This is a bitmask so using an unsigned type is slightly safer. This is not an ABI change since the types are compatible. Thanks: Eric Blake. --- examples/strict-structured-reads.c | 2 +- generator/generator | 67 ++++++++++++++++-------------- interop/structured-read.c | 2 +- lib/internal.h | 2 +- tests/oldstyle.c | 2 +- 5

On 7/24/19 7:17 AM, Richard W.M. Jones wrote: > Previously closures had a crude flag which tells if they are > persistent or transient. Transient closures (flag = false) last for > the lifetime of the currently called libnbd function. Persistent > closures had an indefinite lifetime which could last for as long as > the handle. In language bindings handling persistent closures

This change does not affect the output of the generator. Note this requires that all Closure args have the same parameter name whichever method they appear in. An alternate refactoring could work the same way as Flags and Enum where the parameter name is part of the arg, eg: type arg = ... | Closure of string * closure (* name, type *) ... "set_debug_callback", {

On 7/30/19 10:36 AM, Richard W.M. Jones wrote: > As discussed in this thread, the parameter is an invitation to write > code with race conditions: > > https://www.redhat.com/archives/libguestfs/2019-July/thread.html#00309 > --- > +++ b/generator/generator > @@ -1729,7 +1729,7 @@ C<nbd_pread>."; > default_call with > args = [ BytesPersistOut

On Wed, Jul 24, 2019 at 10:18:20AM -0500, Eric Blake wrote: > On 7/24/19 7:17 AM, Richard W.M. Jones wrote: > > +=head2 Callback lifetimes > > + > > +All callbacks have an C<int valid_flag> parameter which is used to > > +help with the lifetime of the callback. C<valid_flag> contains the > > +I<logical or> of: > > Again, worth mentioning

On 7/25/19 8:07 AM, Richard W.M. Jones wrote: > Previously closures had a crude flag which tells if they are > persistent or transient. Transient closures (flag = false) last for > the lifetime of the currently called libnbd function. Persistent > closures had an indefinite lifetime which could last for as long as > the handle. In language bindings handling persistent closures

In libguestfs generator we have the concept of optargs which is different from plain arguments. These are mapped to optional arguments in languages that support them such as Python. This commit adds a new concept of optargs. At the moment it is simply limited to handling the optional (in some bindings) flags parameter which is used to handle the NBD_CMD_FLAG_* flags. If present, the old Flags

On 7/16/19 6:04 AM, Richard W.M. Jones wrote: > A Closure is a list of (usually one, but can be more) closures. In C > there is also a singe ‘void *user_data’ parameter which is passed by > the caller into the function and through as the first parameter of > each callback invocation. > > By grouping the previously separate Opaque and Callback* parameters > together we can

In a future commit we want to add (non-optional) Flags arg. As a step to doing this, generalize OFlags so it's not tied to just NBD_CMD_FLAG_*. This does not change the C API. It does introduce a small change to the OCaml API -- putting related flags into a submodule, basically renaming them. Note we don't provide guarantees for non-C APIs. --- generator/generator

Change the way that we deal with freeing closures in language bindings: * The valid_flag is removed (mostly reverting commit 2d9b98e96772e282d51dafac07f16387dadc8afa). * An extra ‘free’ parameter is added to all callback structures. This is called by the library whenever the closure won't be called again (so the user_data can be freed). This is analogous to valid_flag ==

Rather than having to open-code the list of accepted command flags in the unlocked version of each command, we can store that information in the generator to produce the check directly in the public API. --- generator/API.ml | 53 +++++++++++++++++++++++++++++---------------- generator/API.mli | 3 ++- generator/C.ml | 26 +++++++++++++++------- generator/GoLang.ml | 10 ++++-----

On 7/24/19 7:17 AM, Richard W.M. Jones wrote: > 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 +- >

[RFC because generated OCaml code needs the same treatment, but I ran out of time to play with that. At least 'make -C python check' passes, although coverage is not complete yet, as there is no python/t/5XX-pread-structured-callback.py...] [Also RFC because I'm not sure if the use of a record type for 'callback' is the best approach or most idiomatic OCaml - but hey, it