[Libguestfs] [PATCH libnbd 2/2] ocaml: Remove NBD.Buffer.free function, use the completion callback instead.

Eric Blake eblake at redhat.com
Wed Aug 14 21:29:59 UTC 2019


On 8/14/19 2:31 PM, Richard W.M. Jones wrote:
> By using the completion free callback function we don't need to
> manually free the buffer.
> 
> Note that after this patch OClosures are always called from OCaml,
> because we might need the side effect of freeing the buffer.
> 
> This reverts part of commit fef1c281a65d061127bf178e5f8cfca0a2475c56.
> ---
>  generator/generator              | 59 +++++++++++++++++++++-----------
>  ocaml/buffer.c                   | 22 ------------
>  ocaml/examples/asynch_copy.ml    |  4 +--
>  ocaml/libnbd-ocaml.pod           | 22 ------------
>  ocaml/tests/test_590_aio_copy.ml |  4 +--
>  5 files changed, 41 insertions(+), 70 deletions(-)

Series looks good to me.

> @@ -4945,6 +4940,12 @@ let print_ocaml_closure_wrapper { cbname; cbargs } =
>    pr "  int r;\n";
>    pr "  value args[%d];\n" (List.length argnames);
>    pr "\n";
> +  pr "  /* The C callback is always registered, even if there's no OCaml\n";
> +  pr "   * callback.  This is because we may need to unregister an\n";
> +  pr "   * associated persistent buffer.\n";
> +  pr "   */\n";
> +  pr "  if (data->fnv == 0)\n";
> +  pr "    CAMLreturnT (int, 0);\n";

This is what you would change if, per the cover letter question, we
decide to allow .callback=NULL + .free=something to call
free(.user_data).  But always calling .callback, and having it
short-circuit when OCaml did not provide a function to call, is sane too.

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3226
Virtualization:  qemu.org | libvirt.org

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 488 bytes
Desc: OpenPGP digital signature
URL: <http://listman.redhat.com/archives/libguestfs/attachments/20190814/5c3f2547/attachment.sig>


More information about the Libguestfs mailing list