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

[Dmitry Bely <dmitry.bely@gmail.com>]

On Tue, Aug 16, 2011 at 12:57 PM, Christophe TROESTLER
<Christophe.Troestler@umons.ac.be> wrote:
> On Tue, 16 Aug 2011 11:37:03 +0400, 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 = 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". Of course, the
>> stack copy of "list" is not registered as a global root so wrp_ml_cons
>> gets an invalid value.
>
> Are you sure it is not because, in this way, “list” is being
> registered twice?

Where?

>  IMHO, wrp_ml_cons does not need to register values.

Let me explain again. Before wrp_ml_cons() is called, C compiler

1. Pushes a _copy_ of list value onto the stack.
2. Calls caml_copy_string(*s) and pushes its result onto the stack also.
3. Calls wrp_ml_cons().

After (2) the second parameter of wrp_ml_cons() becomes invalid: it is
a _copy_ of list, not registered as a local root.

Already two persons don't see the problem at first glance; It proves
that I was right starting the topic ;-)

- Dmitry Bely