Re: [PATCH 16/28] libxl: ocaml: use the "string option" type for IDL strings

[David Scott <dave.scott@eu.citrix.com>]

On 25/03/13 14:45, Rob Hoes wrote:
[ snip ]
> +static value Val_string_option(char *c_val)
> +{
> +	CAMLparam0();
> +	if (c_val)
> +		CAMLreturn(Val_some(caml_copy_string(c_val)));

A bad sequence is:

   1. caml_copy_string() allocates a string successfully
   2. Val_some() calls the allocator to allocate a block but the minor 
heap is full so it triggers a GC
   3. the GC deletes the string from (1) since it can't find any 
references to it

Personally I always force myself to write very basic code with lots of 
explicit temporaries, just to be totally safe. It feels strange because 
it's the complete opposite of good functional style (particularly if you 
believe in point-free programming!).

So I would write:

   CAMLparam0()
   CAMLlocal2(tmp1, tmp2)
   if (c_val) {
     tmp1 = caml_copy_string(c_val);
     tmp2 = Val_some(tmp1);
     CAMLreturn(tmp2)
   }
   ...

It's almost embarrassing to write code like that, but at least it's 
safe! :-)

> +	else
> +		CAMLreturn(Val_none);
> +}
> +
> +static char *String_option_val(value v)
> +{
> +	char *s = NULL;
> +	if (v != Val_none)
> +		s = dup_String_val(Some_val(v));
> +	return s;
> +}
> +
>   #include "_libxl_types.inc"
>
>   #define _STRINGIFY(x) #x
>