search for: nbd_buffer

...%s_user_data = malloc (sizeof (value));\n" n; + pr " if (%s_user_data == NULL) caml_raise_out_of_memory ();\n" n; + pr " *%s_user_data = %sv;\n" n n; + pr " caml_register_generational_global_root (%s_user_data);\n" n; pr " struct nbd_buffer *%s_buf = NBD_buffer_val (%sv);\n" n n; pr " const void *%s = %s_buf->data;\n" n n; - pr " size_t %s = %s_buf->len;\n" count n - | BytesOut (n, count) -> - pr " void *%s = Bytes_val (%sv);\n" n n; - pr " size_t %s = c...

...pr " caml_remove_generational_global_root (&data->bufv);\n"; pr " free (data);\n"; pr "}\n"; pr "\n"; diff --git a/ocaml/buffer.c b/ocaml/buffer.c index 837eece..0c0fe00 100644 --- a/ocaml/buffer.c +++ b/ocaml/buffer.c @@ -36,10 +36,6 @@ nbd_buffer_finalize (value bv) { struct nbd_buffer *b = NBD_buffer_val (bv); - /* Usually if this frees the buffer it indicates a bug in the - * program. The buffer should have been manually freed before - * this. But there's nothing we can do here, so free it. - */ free (b->data); }...

In this patch series we use the newly introduced free callback on the completion function to dererence the OCaml NBD.Buffer. I will make the same kind of change for Python later in a separate series. The completion function is always called at the C level, even if the OCaml program didn't use the optional argument. That's because the free callback doesn't run otherwise. There is a

...* must treat it as a possible GC root. > */ > value *buf_user_data; > buf_user_data = malloc (sizeof (value)); > if (buf_user_data == NULL) caml_raise_out_of_memory (); > *buf_user_data = bufv; > caml_register_generational_global_root (buf_user_data); > struct nbd_buffer *buf_buf = NBD_buffer_val (bufv); > const void *buf = buf_buf->data; > size_t count = buf_buf->len; > if (nbd_add_free_callback (h, (void *)buf, > free_root, buf_user_data) == -1) > caml_raise_out_of_memory (); > > Notice that the f...

On 8/12/19 11:08 AM, Richard W.M. Jones wrote: > 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. > --- > +=head1 FREE CALLBACKS > + > +B<Note:> The API described in this

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.

...e a reference to the Buffer, so we * must treat it as a possible GC root. */ value *buf_user_data; buf_user_data = malloc (sizeof (value)); if (buf_user_data == NULL) caml_raise_out_of_memory (); *buf_user_data = bufv; caml_register_generational_global_root (buf_user_data); struct nbd_buffer *buf_buf = NBD_buffer_val (bufv); const void *buf = buf_buf->data; size_t count = buf_buf->len; if (nbd_add_free_callback (h, (void *)buf, free_root, buf_user_data) == -1) caml_raise_out_of_memory (); Notice that the free_root function needs the pointer...

This is a combination of these two earlier series: https://www.redhat.com/archives/libguestfs/2019-August/msg00235.html https://www.redhat.com/archives/libguestfs/2019-August/msg00240.html plus changes to allow .callback = NULL / .free != NULL, and to reduce the complexity of freeing callbacks. Although it's rather long there's nothing complex here. We might consider squashing some

...count)) -> + | BytesIn (n, count) -> pr " const void *%s = Bytes_val (%sv);\n" n n; pr " size_t %s = caml_string_length (%sv);\n" count n - | OCamlArg (BytesPersistIn (n, count)) -> + | BytesPersistIn (n, count) -> pr " struct nbd_buffer *%s_buf = NBD_buffer_val (%sv);\n" n n; pr " const void *%s = %s_buf->data;\n" n n; pr " size_t %s = %s_buf->len;\n" count n - | OCamlArg (BytesOut (n, count)) -> + | BytesOut (n, count) -> pr " void *%s = Bytes_val (%sv);\n...

...quot;-Wmissing-prototypes\"\n"; pr "\n"; + List.iter print_ocaml_flag_val all_flags; List.iter print_ocaml_binding handle_calls end diff --git a/ocaml/nbd-c.h b/ocaml/nbd-c.h index 0c12dc2..ffd51d2 100644 --- a/ocaml/nbd-c.h +++ b/ocaml/nbd-c.h @@ -99,21 +99,6 @@ Val_nbd_buffer (struct nbd_buffer b) CAMLreturn (rv); } -/* Convert flags to uint32_t. This is simple because flags are just - * lists of int32 values so we only have to add them together. - */ -static inline uint32_t -Flags_val (value v) -{ - CAMLparam1 (v); - uint32_t r = 0; - - for (; v != Val_emptyl...

...ocking_section ();\n"; pr " return ret;\n"; pr "}\n" - | _ -> () + *) + | _ -> () ) args; (* Convert the generic args to OCaml args. *) @@ -4760,6 +4755,7 @@ 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 (Callback (n, _)) -> pr " /* This is safe because we only call this while this stack\n";...

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.

...t;; + pr " free (user_data);\n"; + pr " }\n"; + pr "\n"; pr " return ret;\n"; pr "}\n"; pr "\n" @@ -4846,15 +4817,7 @@ 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 (false, { cbname })) -> - pr " /* This is safe because we only call this while this stack\n"; -...

Posting now that I got something to compile (at the expense of breaking OCaml bindings), but I'm open to ideas on how to improve it. Eric Blake (2): generator: Tweak print_c_arg_list to take alternate first arg RFC: generator: Handle shared callbacks in Python generator/generator | 556 ++++++++++++++++++++++---------------------- 1 file changed, 280 insertions(+), 276 deletions(-) --

...t;; + pr " free (user_data);\n"; + pr " }\n"; + pr "\n"; pr " return ret;\n"; pr "}\n"; pr "\n" @@ -4846,15 +4808,7 @@ 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 (false, { cbname })) -> - pr " /* This is safe because we only call this while this stack\n"; -...

...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 " /* Thi...

...t;; + pr " free (user_data);\n"; + pr " }\n"; + pr "\n"; pr " return ret;\n"; pr "}\n"; pr "\n" @@ -4768,15 +4730,7 @@ 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 (false, { cbname })) -> - pr " /* This is safe because we only call this while this stack\n"; -...

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

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