Re: [Caml-list] Interfacing with C: bad practice

["Török Edwin" <edwintorok@gmail.com>]

On 08/16/2011 11:25 AM, Dmitry Bely wrote:
> 2011/8/16 Török Edwin <edwintorok@gmail.com>:
>> On 08/16/2011 10:37 AM, Dmitry Bely wrote:
>>> I would like to share my experience of writing bad C bindings. The
>>> following code is wrong, although no "living in harmony with the
>>> garbage collector" rule seems to be violated:
>>>
>>> value wrp_ml_cons (value v, value l)
>>> {
>>>   CAMLparam2(v, l);
>>>   CAMLlocal1(cell);
>>>   cell = caml_alloc_small(2, Tag_cons);
>>>   Field(cell, 0) = v;
>>>   Field(cell, 1) = l;
>>>   CAMLreturn(cell);
>>> }
>>>
>>> value string_list(const char ** s)
>>> {
>>>     CAMLparam0();
>>>     CAMLlocal1(list);

list is registered there, is that not enough? (the macro registers address of list AFAIK,
so even if you change its value, it'll stay registered as a local root)

>>>     list = Val_emptylist;
>>>     while (*s != NULL) {
>>>         list = wrp_ml_cons(caml_copy_string(*s), list); /* bug! */
>>>     }
>>>     CAMLreturn(list);
>>> }
>>>
>>> In the line
>>>
>>>         list = wrp_ml_cons(caml_copy_string(*s), list); /* bug! */
>>>
>>> C compiler first puts "list" pointer on stack and then calls
>>> caml_copy_string(*s), potentially invalidating "list".
>>
>> list is a local root though, shouldn't that prevent the invalidation?
> 
> Unfortunately not because wrp_ml_cons() second parameter is not
> registered. 

> So wrp_ml_cons() gets an invalid value.

'list' should be reachable via caml_local_roots, so if it really gets an invalid value
it sounds like a bug to me.

Best regards,
--Edwin