After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 693217 - The GStrv-to-js conversion crashes
The GStrv-to-js conversion crashes
Status: RESOLVED FIXED
Product: seed
Classification: Bindings
Component: libseed
git master
Other Linux
: Normal major
: ---
Assigned To: seed-maint
seed-maint
Depends on:
Blocks:
 
 
Reported: 2013-02-05 17:40 UTC by Alban Crequy
Modified: 2013-02-06 07:32 UTC
See Also:
GNOME target: ---
GNOME version: ---

Attachments
[PATCH] seed-types.c: fix binding GStrv to javascript value (1.22 KB, patch)
2013-02-05 17:43 UTC, Alban Crequy 		none Details | Review

Description Alban Crequy 2013-02-05 17:40:16 UTC 
libseed crashes when converting a GStrv to a javascript value. It happens when the javascript code connects to the "g-properties-changed" GLib signal on GDBusProxy objects.

The attached patch fixes the crash.
Comment 1 Alban Crequy 2013-02-05 17:43:17 UTC 
Created attachment 235238 [details] [review]
[PATCH] seed-types.c: fix binding GStrv to javascript value
Comment 2 Simon McVittie 2013-02-05 18:27:36 UTC 
Review of attachment 235238 [details] [review]:

::: libseed/seed-types.c
@@ +2600,3 @@
 	{
+          JSValueRef js_value = seed_value_from_string (ctx, *val, exception);
+	  js_string_array = g_array_append_val(js_string_array, js_value);

I am not a Seed developer, but this seems correct to me. It changes js_string_array from effectively being a JSStringRef[] (i.e. struct String **) to effectively being a JSValueRef[] (i.e. struct Value **) as expected by JSObjectMakeArray. It's also consistent with seed_gi_make_jsarray() now.

I do wonder whether the intermediate JSValueRefs need to be freed, or whether JSObjectMakeArray has "take ownership" semantics; but if seed_gi_make_jsarray() is correct and non-leaky, then this function is correct too.

Whitespace nitpicking: "val (".
Comment 3 Alan Knowles 2013-02-06 07:32:16 UTC 
Thanks

good catch.