Nicholas Clark <n...@ccl4.org> 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