This pattern is repeated a couple of times in glib2 and gtk2:
for (i = 0; i < gargc; i++) {
if (TYPE(RARRAY_PTR(argary)[i]) == T_STRING) {
gargv[i+1] = RVAL2CSTR(RARRAY_PTR(argary)[i]);
}
else {
gargv[i+1] = g_strdup("");
}
}
The TYPE() == T_STRING test seems like a poor choice. Shouldn’t we at
least try using #to_str?
Also, as this pattern is repeated I really think that we should have a
utility function for it. I’ll write it, but I need input on the
TYPE() comparison above.
In [email protected]
“[ruby-gnome2-devel-en] Converting array of Strings to C array of
strings” on Fri, 9 Sep 2011 14:28:05 +0200,
Nikolai W. [email protected] wrote:
The TYPE() == T_STRING test seems like a poor choice. Shouldnt we at
least try using #to_str?
Also, as this pattern is repeated I really think that we should have a
utility function for it. Ill write it, but I need input on the
TYPE() comparison above.
Use the following code that uses #to_str:
for (i = 0; i < gargc; i++) {
gargv[i+1] = RVAL2CSTR(RARRAY_PTR(argary)[i]);
}
utility function for it. Ill write it, but I need input on the
TYPE() comparison above.
Use the following code that uses #to_str:
for (i = 0; i < gargc; i++) {
gargv[i+1] = RVAL2CSTR(RARRAY_PTR(argary)[i]);
}
BTW. The code is not GC-safe.
Suppose argary[0].to_str returns a newly allocated string object S and
garbage collection happens during execution of argary[1].to_str.
In this case, it is possible that S is reclaimed by GC and gargv[0]
becomes dangling pointer.
for (i = 0; i < gargc; i++) {
gargv[i+1] = RVAL2CSTR(RARRAY_PTR(argary)[i]);
}
BTW. The code is not GC-safe.
Suppose argary[0].to_str returns a newly allocated string object S and
garbage collection happens during execution of argary[1].to_str.
In this case, it is possible that S is reclaimed by GC and gargv[0]
becomes dangling pointer.
How do we solve this? This problem probably appears in a lot of places.
I’m sorry but I misunderstood.
RVAL2CSTR stores results of #to_str into RARRAY_PTR(argary)[i],
so that newly allocated objects are reachable and will not be GC’ed.
(But, of course, modifying RARRAY_PTR(argary)[i] directly is not a
good idea and causes problems in some cases. For example it modify
the array even if the array is frozen. Also it might affect other arrays
if contents of the array are shared with other arrays.)
I’m sorry but I misunderstood.
RVAL2CSTR stores results of #to_str into RARRAY_PTR(argary)[i],
so that newly allocated objects are reachable and will not be GC’ed.
Oh, yes, that’s true, even for arrays.
(But, of course, modifying RARRAY_PTR(argary)[i] directly is not a
good idea and causes problems in some cases. For example it modify
the array even if the array is frozen. Also it might affect other arrays
if contents of the array are shared with other arrays.)
Yes, this is probably even worse. Hm, I guess copying the Ruby array
and then using RVAL2CSTR on the copy is the best thing to do. I’ll
look into creating a patch. (I have some other things queued up
affecting this code anyway.)
if contents of the array are shared with other arrays.)
Yes, this is probably even worse. Hm, I guess copying the Ruby array
and then using RVAL2CSTR on the copy is the best thing to do. I’ll
look into creating a patch. (I have some other things queued up
affecting this code anyway.)
OK, so the new interface will be
const gchar **rbg_rval2strv(volatile VALUE *ary, long *n); #define RVAL2STRS(ary, n) rbg_rval2strv(&(ary), &(n))
Is that fine with everyone? I would have prefered
#define RVAL2STRS(ary, n) rbg_rval2strv(ary, n)
simply because it’s more explicit, but just passing in the variable
meshes with RVAL2STR/StringValue/StringValuePtr.
What do you think?
Once we agree on an interface I will apply all the necessary changes.
(The same interface will then apply to all RVAL2*(ary, n)
macros/functions.)
Note that the functionality will change so that
*ary = rb_ary_dup(rb_ary_to_ary(*ary));
will be performed before modifying the values or ary, thus making sure
that we don’t change either ary or the result of #to_ary.