On Tue, Aug 16, 2011 at 12:57 PM, Christophe TROESTLER
<christophe.troest...@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
--
Caml-list mailing list. Subscription management and archives:
https://sympa-roc.inria.fr/wws/info/caml-list
Beginner's list: http://groups.yahoo.com/group/ocaml_beginners
Bug reports: http://caml.inria.fr/bin/caml-bugs
