search for: debug_fn

...n commands stranded by abrupt close to allow the user to still avoid memory leaks. > +++ b/lib/debug.c > @@ -40,9 +40,11 @@ nbd_unlocked_get_debug (struct nbd_handle *h) > > int > nbd_unlocked_set_debug_callback (struct nbd_handle *h, > - int (*debug_fn) (void *, const char *, const char *), > - void *data) > + debug_fn debug_fn, void *data) > { > + if (h->debug_fn) > + /* ignore return value */ > + h->debug_fn (LIBNBD_CALLBACK_FREE, h->debug_data, N...

...ack registers a debug closure which is saved in the handle. The closure is freed either when nbd_set_debug_callback is called again, or the handle is closed. The sequence of events would look like this: Caller Callback nbd_set_debug_callback # a new debug_fn is registered calls any nbd function debug_fn (valid_flag == VALID) debug_fn (valid_flag == VALID) debug_fn (valid_flag == VALID) ... nbd_set_debug_callback # the old debug_fn is freed...

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

...ack registers a debug closure which is saved in the handle. The closure is freed either when nbd_set_debug_callback is called again, or the handle is closed. The sequence of events would look like this: Caller Callback nbd_set_debug_callback # a new debug_fn is registered calls any nbd function debug_fn (valid_flag == VALID) debug_fn (valid_flag == VALID) debug_fn (valid_flag == VALID) ... nbd_set_debug_callback # the old debug_fn is freed...

...ack registers a debug closure which is saved in the handle. The closure is freed either when nbd_set_debug_callback is called again, or the handle is closed. The sequence of events would look like this: Caller Callback nbd_set_debug_callback # a new debug_fn is registered calls any nbd function debug_fn (valid_flag == VALID) debug_fn (valid_flag == VALID) debug_fn (valid_flag == VALID) ... nbd_set_debug_callback # the old debug_fn is freed...

...debug.c b/lib/debug.c index 8e2ff76..02d49f7 100644 --- a/lib/debug.c +++ b/lib/debug.c @@ -41,7 +41,7 @@ nbd_unlocked_get_debug (struct nbd_handle *h) int nbd_unlocked_set_debug_callback (struct nbd_handle *h, void *data, - void (*debug_fn) (void *, const char *, const char *)) + int (*debug_fn) (void *, const char *, const char *)) { h->debug_fn = debug_fn; h->debug_data = data; @@ -75,6 +75,7 @@ nbd_internal_debug (struct nbd_handle *h, const char *fs, ...) goto out; if (h->...

...enerator/generator b/generator/generator index 2a952cc..6f89792 100755 --- a/generator/generator +++ b/generator/generator @@ -924,7 +924,7 @@ Return the state of the debug flag on this handle."; "set_debug_callback", { default_call with - args = [ Closure { cbname="debug_fn"; + args = [ Closure { cbname="debug"; cbargs=[String "context"; String "msg"] } ]; ret = RErr; shortdesc = "set the debug callback"; @@ -1731,7 +1731,7 @@ C<nbd_pread>."; "aio_pread_callback",...

...a4d6..840702a 100644 --- a/lib/handle.c +++ b/lib/handle.c @@ -90,7 +90,6 @@ nbd_create (void) void nbd_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;...

...lib/debug.c b/lib/debug.c > index 12c10f7..56a9455 100644 > --- a/lib/debug.c > +++ b/lib/debug.c > @@ -40,9 +40,12 @@ nbd_unlocked_get_debug (struct nbd_handle *h) > > int > nbd_unlocked_set_debug_callback (struct nbd_handle *h, > - int (*debug_fn) (void *, const char *, const char *), > + int (*debug_fn) (int, void *, const char *, const char *), > void *data) > { > + if (h->debug_fn) > + /* ignore return value */ > + h->debug_fn (LIBNBD_CALLBA...

...x 12c10f7..56a9455 100644 > > --- a/lib/debug.c > > +++ b/lib/debug.c > > @@ -40,9 +40,12 @@ nbd_unlocked_get_debug (struct nbd_handle *h) > > > > int > > nbd_unlocked_set_debug_callback (struct nbd_handle *h, > > - int (*debug_fn) (void *, const char *, const char *), > > + int (*debug_fn) (int, void *, const char *, const char *), > > void *data) > > { > > + if (h->debug_fn) > > + /* ignore return value */ > > +...

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 implements most of what I wrote here: https://www.redhat.com/archives/libguestfs/2019-July/msg00213.html

...mall 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"] }) ]; Keeping the record type means that you...

...e 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 fre...

...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"] }) ]; > > Keeping the r...

..., free_closure); which calls free_closure (user_data) as soon as the closure will no longer be called by the library. This function would be used to decrement the refcount from Python or remove the global root from OCaml. Note this is a family of functions, eg: nbd_set_free_set_debug_callback_debug_fn corresponding to the debug_fn arg of nbd_set_debug_callback. Luckily they can all be generated along with the internal machinery to call them. Buffer lifetimes ---------------- Similar to the above, persistent buffers (BytesPersist*) can have lifetimes. Remove nbd_add_close_callback ----------...

...debug.c b/lib/debug.c index 02d49f7..12c10f7 100644 --- a/lib/debug.c +++ b/lib/debug.c @@ -40,8 +40,8 @@ nbd_unlocked_get_debug (struct nbd_handle *h) int nbd_unlocked_set_debug_callback (struct nbd_handle *h, - void *data, - int (*debug_fn) (void *, const char *, const char *)) + int (*debug_fn) (void *, const char *, const char *), + void *data) { h->debug_fn = debug_fn; h->debug_data = data; diff --git a/lib/handle.c b/lib/handle.c index 00d0ac2..5003227 1...

...es the API a bit but not as invasively. We overload the callback so that it can either be a callback function or a "free function". They are distinguished by an extra flag argument passed to the callback: extern int nbd_set_debug_callback ( struct nbd_handle *h, int (*debug_fn) (int valid_flag, // <-- note void *user_data, const char *context, const char *msg), void *user_data); The extra ‘valid_flag’ contains one or both of LIBNBD_CALLBACK_VALID and LIBNBD_CALLBACK_FREE. Callback code would look like: int my_d...

...0, NULL); > + > + free (cmd); > +} But as written, the function operates correctly. So up to you if you want to tweak it. > @@ -96,6 +96,10 @@ nbd_close (struct nbd_handle *h) > if (h == NULL) > return; > > + /* Free user callbacks first. */ > + 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) { I recommend either setting h->debug_fn = NULL here, or deferring the FREE callback to after the h->sock->ops->close (h->sock) b...

Hi Rich, I tried to implement the logging feature, but I can't though compiling with this patch now, could you please give me some comments? The error message is below, --- ocamlfind ocamlopt -g -warn-error CDEFLMPSUVYZX -package unix -I ../src/.libs -I ../ocaml -c sysprep_operation.ml -o sysprep_operation.cmx File "sysprep_operation.ml", line 1, characters 0-1: Error: The