GNOME Bugzilla – Bug 702508
Issue when marshaling C arrays
Last modified: 2014-08-08 01:24:27 UTC
Passing a C array to python using the gobject-introspection seem to have couple of issues when using pygobject-3.8.2. Everything works fine when the array is passed as an "in" parameter. But when it is passed as either "inout" or "out" there are couple of issues with marshaling parameters in c->python as well as python->c. 1. When the length parameter is passed as inout, the length value is not read correctly (in this case the length is set to the pointer) 2. In the python->c there are again two issues: a) A GArray is being returned when expecting C array b) The contents in the array are encapsulated as GIArgument, instead of the original data type of the array. The following patch addresses both issues described above: diff --git a/pygobject/pygobject-3.8.2/gi/pygi-argument.c b/pygobject/pygobject-3.8.2/gi/pygi-argument.c index 7de90a2..a5b7fc5 100644 --- a/pygobject/pygobject-3.8.2/gi/pygi-argument.c +++ b/pygobject/pygobject-3.8.2/gi/pygi-argument.c @@ -835,7 +835,12 @@ _pygi_argument_to_array (GIArgument *arg, g_arg_info_load_type (&length_arg_info, &length_type_info); if (args != NULL) { - if (!gi_argument_to_gssize (args[length_arg_pos], + GIArgument *p_length_arg = args[length_arg_pos]; + if (g_arg_info_get_direction (&length_arg_info) != GI_DIRECTION_IN) { + p_length_arg = (GIArgument *)p_length_arg->v_pointer; + } + + if (!gi_argument_to_gssize (p_length_arg, g_type_info_get_tag (&length_type_info), &length)) return NULL; @@ -1172,7 +1177,8 @@ _pygi_argument_from_object (PyObject *object, if (g_type_info_get_tag (item_type_info) == GI_TYPE_TAG_UINT8) item_size = 1; else - item_size = sizeof (GIArgument); + item_size = _pygi_g_type_info_size (item_type_info); + // item_size = sizeof (GIArgument); array = g_array_sized_new (is_zero_terminated, FALSE, item_size, length); if (array == NULL) { diff --git a/pygobject/pygobject-3.8.2/gi/pygi-closure.c b/pygobject/pygobject-3.8.2/gi/pygi-closure.c index f30fa52..03e4e87 100644 --- a/pygobject/pygobject-3.8.2/gi/pygi-closure.c +++ b/pygobject/pygobject-3.8.2/gi/pygi-closure.c @@ -193,8 +193,14 @@ _pygi_closure_assign_pyobj_to_out_argument (gpointer out_arg, PyObject *object, } default: - *((gpointer *) out_arg) = arg.v_pointer; - break; + if (g_type_info_get_array_type (type_info) == GI_ARRAY_TYPE_C) { + GArray *temp = (GArray *) arg.v_pointer; + *((gpointer *) out_arg) = temp->data; + g_array_free(temp, FALSE); + } else { + *((gpointer *) out_arg) = arg.v_pointer; + } + break; } }
Hi, Can you attach this as a .patch file as it will be easier to review? unittests will also help getting this pushed through. See: https://wiki.gnome.org/GnomeLove/SubmittingPatches Thanks
Created attachment 250884 [details] [review] Patch for issues with passing c arrays
Submitted the requested patch.
This looks like it might also be the same as bug #669496. I will test your patch against the tests in that ticket and see if things are fixed.
Review of attachment 250884 [details] [review]: Sorry for not reviewing this sooner. At this point the code path in question is going away as it's being unified with regular function call marshalling. This effort should also fix the bug reported here. ::: gi/pygi-closure.c @@ +208,2 @@ default: + if (g_type_info_get_array_type (type_info) == GI_ARRAY_TYPE_C) { This should be moved into a separate case for GI_TYPE_TAG_ARRAY.
Created attachment 282835 [details] [review] tests: Add regression test for callbacks with an inout array tests: Add regression test for callbacks with an inout array
Created attachment 282836 [details] [review] Add test for a callback with an inout array This was broken until the closures used the caches for marshaling. NOTE: This only works when the closures have been fully converted to using the caches for marshaling. https://bugzilla.gnome.org/show_bug.cgi?id=727004
Review of attachment 282835 [details] [review]: Looks good. Normally I see the bugzilla URL as the last line of commit messages. See: https://wiki.gnome.org/GnomeLove/SubmittingPatches
Review of attachment 282836 [details] [review]: LGTM. Same comment about bugzilla URL. Please also commit with a decorator so our test suite still works with older versions of GI: @unittest.skipUnless(hasattr(Everything, 'test_array_inout_callback'), 'Requires newer version of GI') def test_callback_scope_call_array_inout(self): ... Additionally if this is submitted before patches that fix it, add the @unittest.expectedFailure decorator, then remove that in the patch that fixes it.
Closing now that relevant patches in bug 727004 have landed.