search for: close_callbacks

..._close (struct nbd_handle *h) { - struct close_callback *cc, *cc_next; struct meta_context *m, *m_next; if (h == NULL) @@ -100,12 +99,6 @@ nbd_close (struct nbd_handle *h) if (h->debug_fn) h->debug_fn (LIBNBD_CALLBACK_FREE, h->debug_data, NULL, NULL); - for (cc = h->close_callbacks; cc != NULL; cc = cc_next) { - cc_next = cc->next; - cc->cb (cc->user_data); - free (cc); - } - free (h->bs_entries); for (m = h->meta_contexts; m != NULL; m = m_next) { m_next = m->next; @@ -202,34 +195,6 @@ nbd_unlocked_add_meta_context (struct nbd_handle *h...

...uot;}\n"; > pr "\n"; > diff --git a/lib/connect.c b/lib/connect.c > index b889f80..4e3141f 100644 > --- a/lib/connect.c > +++ b/lib/internal.h > @@ -80,7 +80,17 @@ struct nbd_handle { > /* Linked list of close callbacks. */ > struct close_callback *close_callbacks; > > - _Atomic enum state state; /* State machine. */ > + /* State machine. > + * > + * The actual current state is ‘next_state’. ‘state’ is updated > + * before we release the lock. > + * > + * Note don't access these fields directly, use the SET_NEXT...

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

** INCOMPLETE ** This is the generator change as discussed on the list already. The Python and OCaml bindings are not yet done. It passes all [C only] tests and valgrind. Note that nbd_add_close_callback is inconsistent with other closure types because it passes the user_data parameter after the function. (This is not caused by the current patch, it was already inconsistent). We decided that

...t;; > > diff --git a/lib/connect.c b/lib/connect.c > > index b889f80..4e3141f 100644 > > --- a/lib/connect.c > > > +++ b/lib/internal.h > > @@ -80,7 +80,17 @@ struct nbd_handle { > > /* Linked list of close callbacks. */ > > struct close_callback *close_callbacks; > > > > - _Atomic enum state state; /* State machine. */ > > + /* State machine. > > + * > > + * The actual current state is ‘next_state’. ‘state’ is updated > > + * before we release the lock. > > + * > > + * Note don't access...

...TE_START; h->pid = -1; h->export_name = strdup (""); diff --git a/lib/internal.h b/lib/internal.h index 7290247..1bef1c2 100644 --- a/lib/internal.h +++ b/lib/internal.h @@ -80,7 +80,17 @@ struct nbd_handle { /* Linked list of close callbacks. */ struct close_callback *close_callbacks; - _Atomic enum state state; /* State machine. */ + /* State machine. + * + * The actual current state is ‘next_state’. ‘state’ is updated + * before we release the lock. + * + * Note don't access these fields directly, use the SET_NEXT_STATE + * macro in generator/states*...

Hi, this series cleans up the Python sources, either static or generated, including also tests, to make them PEP 8 compliant; see https://www.python.org/dev/peps/pep-0008/ and tools like pep8. Almost all the issues reported by pep8 are fixed, reducing the issues from 3818 to 7. The changes should have no effect on the actual code, while it will help Python users with consistency with other

Instead of running all the tests manually, the unittest module has a 'discovery' mode to run tests by importing them from a directory: this requires the tests to have different filenames, since they need to be imported as modules now (hence an empty __init__.py is added), and the current naming does not match the convention. Using unittest as loader/runner brings another change: tests

...88ca0 100644 --- a/lib/internal.h +++ b/lib/internal.h @@ -80,8 +80,8 @@ struct nbd_handle { /* For debugging. */ bool debug; - void *debug_data; int (*debug_fn) (void *, const char *, const char *); + void *debug_data; /* Linked list of close callbacks. */ struct close_callback *close_callbacks; @@ -249,12 +249,12 @@ typedef int (*read_fn) (void *user_data, const void *buf, size_t count, typedef int (*callback_fn) (void *user_data, int64_t cookie, int *error); struct command_cb { - void *user_data; union { extent_fn extent; read_fn read; } fn; callback_fn callback;...

...b/lib/internal.h @@ -75,7 +75,7 @@ struct nbd_handle { /* For debugging. */ bool debug; void *debug_data; - void (*debug_fn) (void *, const char *, const char *); + int (*debug_fn) (void *, const char *, const char *); /* Linked list of close callbacks. */ struct close_callback *close_callbacks; @@ -212,7 +212,7 @@ struct socket { const struct socket_ops *ops; }; -typedef void (*extent_fn) (void *data, const char *metacontext, uint64_t offset, uint32_t *entries, size_t nr_entries); +typedef int (*extent_fn) (void *data, const char *metacontext, uint64_t offset, uint32_t *entries, si...

As before, but this one has working Python bindings. OCaml still TBD. Rich.

...&error) == -1 && error) cmd->error = error; } if (cmd->error == 0) diff --git a/lib/handle.c b/lib/handle.c index cbe7e8a..5003227 100644 --- a/lib/handle.c +++ b/lib/handle.c @@ -98,7 +98,7 @@ nbd_close (struct nbd_handle *h) for (cc = h->close_callbacks; cc != NULL; cc = cc_next) { cc_next = cc->next; - cc->cb (cc->data); + cc->cb (cc->user_data); free (cc); } @@ -202,7 +202,8 @@ nbd_unlocked_add_meta_context (struct nbd_handle *h, const char *name) * programming languages. */ int -nbd_add_close_callback (s...

...TE_START; h->pid = -1; h->export_name = strdup (""); diff --git a/lib/internal.h b/lib/internal.h index 61ddbde..503bf34 100644 --- a/lib/internal.h +++ b/lib/internal.h @@ -80,7 +80,17 @@ struct nbd_handle { /* Linked list of close callbacks. */ struct close_callback *close_callbacks; - _Atomic enum state state; /* State machine. */ + /* State machine. + * + * The actual current state is ‘state’. ‘public_state’ is updated + * before we release the lock. + * + * Note don't access these fields directly, use the SET_NEXT_STATE + * macro in generator/states...

v1 was here: https://www.redhat.com/archives/libguestfs/2019-July/thread.html#00231 The changes address everything that Eric picked up in his review of the first two patches. I have also added two more patches (4 and 5) which respectively fix docs and change int status -> unsigned status, as discussed. Passes make, check, check-valgrind. Rich.

This works. I'm in the middle of testing whether there is any noticable benefit. Rich.

...TE_START; h->pid = -1; h->export_name = strdup (""); diff --git a/lib/internal.h b/lib/internal.h index 5b6152b..ce4bf5b 100644 --- a/lib/internal.h +++ b/lib/internal.h @@ -80,7 +80,17 @@ struct nbd_handle { /* Linked list of close callbacks. */ struct close_callback *close_callbacks; - _Atomic enum state state; /* State machine. */ + /* State machine. + * + * The actual current state is ‘state’. ‘public_state’ is updated + * before we release the lock. + * + * Note don't access these fields directly, use the SET_NEXT_STATE + * macro in generator/states...

The API changes from: int nbd_add_close_callback (struct nbd_handle *h, nbd_close_callback cb, void *user_data); to: int nbd_add_close_callback (struct nbd_handle *h, void *user_data, nbd_close_callback cb); The second way is consistent with how other callbacks work

This implements most of what I wrote here: https://www.redhat.com/archives/libguestfs/2019-July/msg00213.html

Just a simple refactoring in preparation for forthcoming work. --- generator/states-reply.c | 2 +- generator/states.c | 4 ++-- lib/crypto.c | 5 +++-- lib/internal.h | 6 ++++-- lib/socket.c | 5 +++-- 5 files changed, 13 insertions(+), 9 deletions(-) diff --git a/generator/states-reply.c b/generator/states-reply.c index 5be3431..f0ef47c 100644 ---

I need to think about this patch series a bit more, but it does at least pass the tests. Rich.