GNOME Bugzilla – Bug 651558
array+length return values
Last modified: 2011-06-17 15:05:05 UTC
Given a method like: public string[] get_groups () { vala generates: gchar** caribou_keyboard_model_get_groups (CaribouKeyboardModel* self, int* result_length1) aka: <method c:identifier="caribou_keyboard_model_get_groups" name="get_groups"> <parameters> <parameter direction="out" name="result_length1" transfer-ownership="none"> <type c:type="gint" name="gint"/> </parameter> </parameters> <return-value transfer-ownership="full"> <array length="0"> <type c:type="gchar*" name="utf8"/> </array> </return-value> </method> gjs cannot currently handle this, and given the current code organization, there's no obvious way to fix it; gjs_value_from_g_argument() is called to process the <return-value>, and can see the length="0", but it has no way to access the argument list to extract that value. Thoughts?
See also https://bugzilla.gnome.org/show_bug.cgi?id=646632
OK... so I guess we'd want JSBool gjs_value_from_explicit_array (JSContext *context, jsval *value_p, GITypeInfo *type_info, GArgument *arg, int length); (which would just be another small wrapper around gjs_array_from_carray_internal()). And then gjs_invoke_c_function() would notice that the return value was an array-with-length-arg, extract the length, and call gjs_value_from_explicit_array() instead of gjs_value_from_g_argument(). And suppress the out length arg.
Created attachment 189113 [details] [review] gimarshallingtests: add a few more array tests Add some tests with parameters on either side of an out array length parameter, to ensure that bindings that omit the length parameter don't mess up any other parameters.
Created attachment 189114 [details] [review] gi: simplify gjs_invoke_c_function() argument bookkeeping Make the out_arg_cvalues and inout_original_arg_cvalues arrays be the same length as in_arg_cvalues, so that we don't need to keep a separate counter for each. Rename the counter and length variables to make it clearer what each one is counting.
Created attachment 189115 [details] [review] gi: implement array+length for return value, out, and inout
Created attachment 189162 [details] [review] gimarshallingtests: add a few more array tests Add some tests with parameters on either side of an out array length parameter, to ensure that bindings that omit the length parameter don't mess up any other parameters. Add some tests with returning arrays (with lengths) of structs, to test the transfer/cleanup in that case.
Created attachment 189163 [details] [review] gi: simplify gjs_invoke_c_function() argument bookkeeping rebase for Giovanni's latest patch
Created attachment 189164 [details] [review] gi: implement array+length for return value, out, and inout rebase and add gjs_g_argument_release_out_array() (along with marshalling tests to exercise it)
Review of attachment 189162 [details] [review]: ::: tests/gimarshallingtests.c @@ +1219,3 @@ +const gint * +gi_marshalling_tests_array_return_etc (gint first, gint *length, gint last, gint *sum) +{ I'd prefer an explicit (out) annotation for @length. Actually it'd be nice to at least stub out the gtk-doc parameters, like: @first: @length: (out): @last: ...etc This applies for the rest of the functions below too.
Created attachment 189657 [details] [review] gimarshallingtests: add a few more array tests Add some tests with parameters on either side of an out array length parameter, to ensure that bindings that omit the length parameter don't mess up any other parameters.
Review of attachment 189657 [details] [review]: LGTM
Created attachment 190083 [details] [review] Handle inout array/length pairs more correctly In particular if we saw "null" for an inout array, we need to also pass null for the length argument.
Created attachment 190084 [details] [review] testGIMarshalling: Comment out nonexistent test cases
Created attachment 190113 [details] [review] gi: simplify gjs_invoke_c_function() argument bookkeeping rebase
Created attachment 190114 [details] [review] gi: implement array+length for return value, out, and inout rebase, remove erroneous tests from a bad rebase from an old version of Giovanni's patch
Ok, I didn't review each individual change here in close detail; they look right at a high level. I'm going to focus now on fixing leaks in valgrind and cleaning up the current code to understand it better.
Attachment 190083 [details] pushed as c501e3d - Handle inout array/length pairs more correctly Attachment 190113 [details] pushed as a964d78 - gi: simplify gjs_invoke_c_function() argument bookkeeping Attachment 190114 [details] pushed as 0cd4538 - gi: implement array+length for return value, out, and inout