[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Xen-devel] [PATCH 08/28] libxc: ocaml: add simple binding for xentoollog (output only).



On 25/03/13 14:45, Rob Hoes wrote:
[snip]
+/* external _create_logger: (string * string) -> handle = 
"stub_xtl_create_logger" */
+CAMLprim value stub_xtl_create_logger(value cbs)
+{
+       CAMLparam1(cbs);
+       struct caml_xtl *xtl = malloc(sizeof(*xtl));
+       if (xtl == NULL)
+               caml_raise_out_of_memory();
+
+       memset(xtl, 0, sizeof(*xtl));
+
+       xtl->vtable.vmessage = &stub_xtl_ocaml_vmessage;
+       xtl->vtable.progress = &stub_xtl_ocaml_progress;
+       xtl->vtable.destroy = &xtl_destroy;
+
+       xtl->vmessage_cb = dup_String_val(Field(cbs, 0));
+       xtl->progress_cb = dup_String_val(Field(cbs, 1));
+       CAMLreturn((value)xtl);
+}

I think we should avoid returning "bare pointers" to the OCaml heap for two reasons:

Firstly it makes us vulnerable to a sequence like the following:

  1. malloc() something out of heap
  2. return the "bare pointer" to the heap
  3. GC runs, ignores the bare pointer because it points out of heap
  ... some time later ...
  4. we call free() on the out of heap thing
  ... some time later ...
  5. GC runs, ignores the bare pointer because it points out of heap
  ... some time and many heap allocations later ...
  6. GC expands
  <-- the heap now includes the old address used by the bare pointer
7. GC runs, follows the bare pointer because it points inside the heap and segfaults

Secondly it prevents some heap optimisations that are being experimented with by people at OCamlLabs, so we'd be storing up some incompatibility for the future.

Instead of returning a "bare pointer" I think we should use a "Custom" value. This involves declaring a "struct custom_operations" like this:

  static struct custom_operations foo_custom_operations = {
     "foo_custom_operations",
     custom_finalize_default,
     custom_compare_default,
     custom_hash_default,
     custom_serialize_default,
     custom_deserialize_default
  };

And then wrapping and unwrapping "Custom" blocks using something like:

  #define Foo_val(x) (*((struct foo *)Data_custom_val(x)))

  static value
  Val_foo (struct foo *x)
  {
    CAMLparam0 ();
    CAMLlocal1 (result);
    result = caml_alloc_custom (&foo_custom_operations,
                               sizeof (struct foo*), 0, 1);
    Foo_val (result) = x;
    CAMLreturn (result);
 }

There's more information here:

http://caml.inria.fr/pub/docs/manual-ocaml-4.00/manual033.html#toc150

The ocaml-libvirt bindings are a good example of this pattern:

http://git.annexia.org/?p=ocaml-libvirt.git;a=summary

It's also worth considering whether to use the finalizer support to automatically free the underlying C resource when the last reference to it from OCaml has been GCed. This would be safer than exposing a direct "free" function in the OCaml interface, since it would prevent use-after-free.

Cheers,
Dave

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel


 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.