similar to: [libnbd PATCH] generator: Add #define witnesses for all API

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

This just defines the namespace, its contexts and related constants and the only supported one is currently base:allocation. The names of the constants are not very future-proof, but shorter than LIBNBD_META_NS_CONTEXT_BASE_ALLOCATION or similar. Currently the output looks like this: /* "base" namespace */ /* "base" namespace contexts */ /* "base:allocation"

These are some simplifications to the generator. They don't probably make much sense on their own, but they are preparatory to better handling of enums, and or'd lists of flags. Rich.

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

We have a memory leak when a function with a closure exits early prior to registering that closure through some path that will guarantee cleanup. The easiest way to fix it is to guarantee that once a closure is passed into a public API, it will be cleaned regardless of whether that API succeeds or fails. But to avoid cleaning the closure more than once, we need to refactor our internal handling,

On Mon, Sep 07, 2020 at 04:46:39PM -0500, Eric Blake wrote: > We have a memory leak when a function with a closure exits early prior > to registering that closure through some path that will guarantee > cleanup. The easiest way to fix it is to guarantee that once a > closure is passed into a public API, it will be cleaned regardless of > whether that API succeeds or fails. But to

As promised in my earlier thread on libnbd completion callback question. Eric Blake (2): generator: Refactor handling of closures in unlocked functions generator: Free closures on failure docs/libnbd.pod | 2 +- generator/C.ml | 48 +++++++++++------ generator/C.mli | 1 + lib/debug.c | 7 +-- lib/opt.c | 31 ++++++-----

This isn't quite finished because not all of the tests or examples have been updated, but it demonstrates an idea: Should we forget about the concept of having multiple connections managed under a single handle? In this patch there is a single ‘struct nbd_handle *’ which manages a single state machine and connection (and therefore no nbd_connection). To connect to a multi-conn server you must

Apply the ideas in the previous patch to the C-language bindings generator. Signed-off-by: Laszlo Ersek <lersek at redhat.com> --- generator/C.ml | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/generator/C.ml b/generator/C.ml index f9171996dde0..07c924c48ccf 100644 --- a/generator/C.ml +++ b/generator/C.ml @@ -250,15 +250,15 @@ let *)

We previously needed nbd_add_close_callback to do cleanup from language bindings. However now we have closure lifetimes this is no longer needed and can be removed. See also: https://www.redhat.com/archives/libguestfs/2019-July/msg00213.html --- generator/generator | 18 ------------------ lib/handle.c | 35 ----------------------------------- lib/internal.h | 10 ---------- 3 files

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 |

Still not working, but I took the latest patch and: - rebased it against libnbd 1.0 - fixed it so it handles new args and cbargs The generator now runs without warnings. This patch doesn't handle optargs at all. In C these are converted to non-optional parameter. Rust doesn't (AFAIK) have optional or labelled arguments unfortunately. Rich.

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

This is just a basic ugly support, not meant to be pushed (at least for now). There is quite a lot missing, but there is an example which shows that it really works. And valgrind reports that all allocations were freed. The way the code is generated is also not nice, I wish there was more code actually written in some files and not generated by the generator (as much hard-coded static strings as

Exploration of what it would take to treat the nbd_pread_callback status parameter as a language-appropriate enum rather than a bare integer. While I've identified a rough set of the places to change, I have not actually got it working with Python or OCaml bindings. --- Posting this more for an idea of whether we want to pursue it further. I have no hard feelings if we decide to ditch it;

On Mon, Jun 17, 2019 at 08:39:43PM -0500, Eric Blake wrote: > Exploration of what it would take to treat the nbd_pread_callback > status parameter as a language-appropriate enum rather than a bare > integer. While I've identified a rough set of the places to change, I > have not actually got it working with Python or OCaml bindings. > --- > > Posting this more for an

Patches 1 & 2 are rather complex, but the end result is that we pass closures + user_data + free function in single struct parameters as I described previously in this email: https://www.redhat.com/archives/libguestfs/2019-August/msg00210.html Patch 3 adds a convenient FREE_CALLBACK macro which seems a worthwhile simplification if you buy into 1 & 2. Patch 4 adds another macro which is

I think I've addressed everything that was raised in review. Some of the highlights: - Callbacks should be freed reliably along all exit paths. - There's a simple test of closure lifetimes. - I've tried to use VALID|FREE in all the places where I'm confident that it's safe and correct to do. There may be more places. Note this is an optimization and shouldn't

This adds a C-only semi-private function for freeing various types of persistent data passed to libnbd. There are some similarities with nbd_add_close_callback which we removed in commit 7f191b150b52ed50098976309a6af883d245fc56. --- generator/generator | 46 +++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 46 insertions(+) diff --git a/generator/generator b/generator/generator

As proposed here: https://www.redhat.com/archives/libguestfs/2019-August/msg00130.html I didn't actually read Eric's replies to that yet because I've been concentrating on writing these patches all day. Anyway here they are and I'll look at what Eric said about the proposal next. Rich.