GNOME Bugzilla – Bug 761150
Fixing return type inside the seed_value_from_strv
Last modified: 2016-01-29 05:49:02 UTC
This one might generate a discussion... The following code is incorrect. JSStringRef js_string = seed_value_from_string (ctx, *val, exception); Two issues: First one is that seed_value_from_string returns a value and not a StringRef, and this is fixed by this patch (it adds a reference to JSStringRef in a header, if that's not wanted, I can make this a private method). Second issue, and it's also gets a compiling warning, is that there's a loop in a pointer to GStrv which is a bit confusing. seed_string_from_cstring expects a char *, but var is a char ***, however we are iterating over *var, so it might only be a casting issue. But I'm not totally sure about that... What do you think Alan? The error is: seed-types.c:2882:59: warning: incompatible pointer types passing 'GStrv' (aka 'char **') to parameter of type 'const gchar *' (aka 'const char *'); dereference with * [-Wincompatible-pointer-types] JSStringRef js_string = seed_string_from_cstring (ctx, *val, exception);
Created attachment 319796 [details] [review] patch 0003
Adding Alexandre Mazari, as he's the author of the patch... https://bugzilla.gnome.org/show_bug.cgi?id=657647
that code is a bit confusing.... caller is line 1410 return seed_value_from_strv(ctx, g_value_get_boxed (gval), exception); that should really be doing return seed_value_from_strv(ctx, (GStr*) g_value_get_boxed (gval), exception); The code is almost correct, but this JSStringRef js_string = seed_value_from_string (ctx, *val, exception); should be this. JSValueRef js_string = seed_value_from_string (ctx, *val, exception); The array that's being created is actually an array of JSValueRef's , not JSStringRef's I think there is also a casting missing. JSValueRef js_string = seed_value_from_string (ctx, (char *)*val, exception); see Alexandre's original patch https://git.gnome.org/browse/seed/commit/libseed/seed-types.c?id=bc3c00ee0b18d6fd13ee8d9766d53e443e57bf6c
I guess you're right. It should have been changed during https://bugzilla.gnome.org/show_bug.cgi?id=693217, perhaps Alban didn't see it... Anyway, I'm sending a new patch in a few minutes...
Created attachment 319949 [details] [review] 0001-Fixing-some-build-warnings-in-seed-types.c.patch new patch...
looks good - commited