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 761658 - Out arrays of structs should not consist of identical elements
Out arrays of structs should not consist of identical elements
Status: RESOLVED FIXED
Product: gjs
Classification: Bindings
Component: general
1.45.x
Other Mac OS
: Normal normal
: ---
Assigned To: gjs-maint
gjs-maint
Depends on:
Blocks:
 
 
Reported: 2016-02-07 04:14 UTC by Philip Chimento
Modified: 2016-02-08 04:54 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
arg: Marshal all structs in out array correctly (1.88 KB, patch)
2016-02-07 04:15 UTC, Philip Chimento
none Details | Review
arg: Marshal all structs in out array correctly (2.67 KB, patch)
2016-02-07 05:48 UTC, Philip Chimento
committed Details | Review
tests: Add regression test for out array of struct (8.50 KB, patch)
2016-02-07 05:49 UTC, Philip Chimento
committed Details | Review

Description Philip Chimento 2016-02-07 04:14:42 UTC
See https://bugzilla.gnome.org/show_bug.cgi?id=704842, the patch committed there doesn't quite return the correct out array. Instead it places the same struct into each element of the returned array, so the array has the correct length but all the elements are identical.

e.g. to continue the example use case from the other bug

    let keymap = Gdk.Keymap.get_default();
    let [success, entries] = keymap.get_entries_for_keyval(Gdk.KEY_Return);
    print(entries.map(entry => entry.keycode));

would print 52,52 whereas it should print 36,52.
Comment 1 Philip Chimento 2016-02-07 04:15:32 UTC
Created attachment 320562 [details] [review]
arg: Marshal all structs in out array correctly

Previously, an out array of structs would have all its elements be the
same struct, since gjs_array_from_carray_internal() would not iterate
through the C array correctly. This fixes the iteration and adds a test
case (which is hopefully not locale- or keyboard-dependent!)
Comment 2 Jasper St. Pierre (not reading bugmail) 2016-02-07 04:23:52 UTC
Review of attachment 320562 [details] [review]:

OK. I would much prefer a test in Regress / Everything, but this is OK for now.
Comment 3 Philip Chimento 2016-02-07 04:30:38 UTC
Not a problem. I already wrote a Regress test for the other bug I just filed, this actually reminded me that I forgot to attach it there ... :-P
Comment 4 Philip Chimento 2016-02-07 05:48:58 UTC
Created attachment 320566 [details] [review]
arg: Marshal all structs in out array correctly

Previously, an out array of structs would have all its elements be the
same struct, since gjs_array_from_carray_internal() would not iterate
through the C array correctly. This fixes the iteration and expands the
test case to cover this as well. It also changes the old test case, that
used the Gdk function directly, to a Regress test case, newly added to
Regress in gobject-introspection.
Comment 5 Philip Chimento 2016-02-07 05:49:34 UTC
Created attachment 320567 [details] [review]
tests: Add regression test for out array of struct

This is a regression test for returning out arrays of structs, like
gdk_keymap_get_entries_for_keyval() for example.
Comment 6 Jasper St. Pierre (not reading bugmail) 2016-02-07 05:51:13 UTC
Review of attachment 320567 [details] [review]:

OK.
Comment 7 Jasper St. Pierre (not reading bugmail) 2016-02-07 05:51:28 UTC
Review of attachment 320566 [details] [review]:

Nice, thank you.
Comment 8 Philip Chimento 2016-02-08 04:53:14 UTC
Comment on attachment 320567 [details] [review]
tests: Add regression test for out array of struct

Attachment 320567 [details] pushed as b20ac73 - tests: Add regression test for out array of struct
Comment 9 Philip Chimento 2016-02-08 04:54:55 UTC
Attachment 320566 [details] pushed as b4061d4 - arg: Marshal all structs in out array correctly