Nicholas Clark <[email protected]> wrote ..
> I believe that MVM_file_set_encoding() needs to root oshandle (patch attached)
> as MVM_string_find_encoding() can allocate and hence can cause a GC run.
>
Patch applied; thanks.
> There are two other uses of MVM_string_find_encoding(). One is:
>
> /* At least find_encoding may allocate on first call, so root just
> * in case. */
> MVMROOT(tc, buf, {
> encoded = MVM_string_encode(tc, s, 0, NUM_GRAPHS(s), &output_size,
> MVM_string_find_encoding(tc, enc_name));
> });
>
Which is wrong because it puts s on the stack, as well as having failed to root
it. I've fixed that one.
> The other is:
>
> /* Decode. */
> return MVM_string_decode(tc, tc->instance->VMString,
> ((MVMArray *)buf)->body.slots.i8 + ((MVMArray *)buf)->body.start,
> ((MVMArray *)buf)->body.elems * elem_size,
> MVM_string_find_encoding(tc, enc_name));
>
>
> Is this one buggy, because tc->instance->VMString is a GC-able object, and
> so might move if MVM_string_find_encoding() triggers a GC collection?
>
> ie, should that MVM_string_find_encoding(tc, enc_name) be yanked to a line
> above and assigned to a local variable, so that any GC run has happened
> before the arguments to MVM_string_decode() start to be put onto the CPU
> stack for the C function call?
>
Yes; also fixed.
Thanks!
Jonathan
