GNOME Bugzilla – Bug 581687
Support GArray parameters.
Last modified: 2015-02-07 16:53:40 UTC
Add 'is_garray' property of array types, which indicates that the array is stored as a GArray, ie, with public length and data members in a structure. This implies that zero-terminated and has_length are false, since the length is stored in the GArray. This patch assumes that this support will be added at the same time as "extra type" support, in v2.1 of the typelib format, and so we use has_extra_types() as a proxy to determine whether the is_garray bit in the typelib is meaningful. (See bug 581686.)
Created attachment 134157 [details] [review] Support-GArray-parameters.patch
Not sure if it is relevant here, but note that gobject just grew boxed types for arrays.
Matthias: the G_TYPE_BYTE_ARRAY and friends work very nicely with this patch, which allows these to be represented as array types. See also bug 581686, which lets register the type name for G_TYPE_BYTE_ARRAY as a GLib.ByteArray, and bug 581696 (for example), which lets you pass values to/from GArrays just like you would to native arrays (actually easier, since the GArray contains an inherent length, which makes it easier to convert them to a native value).
Created attachment 134285 [details] [review] Support-GArray-parameters.patch Update the patch to reflect the new patch for bug 581685
Created attachment 136295 [details] [review] Support-GArray-parameters.patch Updated patch to git HEAD. Also ripped out the "has_garray" method which unnecessarily tied this support to bug 581686. On Colin's advice given in bug 581686, "let's just break the typelib format, we haven't done a stable release, so it's not a problem." I don't think we even break the format, actually, since the is_garray field should have been cleared to 0 in old typelibs anyway.
Created attachment 136458 [details] [review] Support-GArray-parameters.patch Refreshed patch against git HEAD. Lonely patch wants review.
Comment on attachment 136458 [details] [review] Support-GArray-parameters.patch >From 6a18978dab37b3bb53b6fb53ef1ae65489ce419a Mon Sep 17 00:00:00 2001 >From: C. Scott Ananian <cscott@litl.com> >Date: Wed, 6 May 2009 16:03:04 -0400 >Subject: [PATCH] Support GArray parameters. Missing: * documentation for newly added API * use-case, eg foo from library bar needs this * tests >+gboolean >+g_type_info_is_g_array (GITypeInfo *info) This needs documentation.
I'll add more docs and tests. For my future reference: * docs in girepository/gtypelib.h for ArrayTypeBlob. Info should be transferred over to accessors in ginfo.c, for all the g_type_info_* functions (see g_callable_info_get_arg, for a good example) * use cases: NetworkManager's libnm-util (everyone's favorite example of a not-designed-to-be-bindable library) has: gboolean nm_utils_same_ssid (const GByteArray * ssid1, const GByteArray * ssid2, gboolean ignore_trailing_null); which can be called from javascript as: js> imports.gi.NMUtil.same_ssid("abcd", "abcd\0", true) once the gjs support patch (bug 581696) is landed. Other uses, found in a quick grep of my /usr/include: /usr/include/libpurple/certificate.h: GByteArray * (* get_fingerprint_sha1)(PurpleCertificate *crt); /usr/include/libpurple/certificate.h:GByteArray * purple_certificate_get_fingerprint_sha1(PurpleCertificate *crt); //usr/include/pidgin/gtkutils.h:void pidgin_toggle_sensitive_array(GtkWidget *w, GPtrArray *data); usr/include/libnm-glib/nm-access-point.h:const GByteArray * nm_access_point_get_ssid (NMAccessPoint *ap); /usr/include/evolution-data-server-2.22/camel/camel-sasl.h:GByteArray *camel_sasl_challenge (CamelSasl *sasl, GByteArray *token, CamelException *ex); (and many other places in e-d-s/camel/*.h) /usr/include/gnome-keyring-1/gnome-keyring.h:typedef GArray GnomeKeyringAttribut eList; libdbus-glib specialized type support /usr/include/atk-1.0/atk/atkrelation.h:GPtrArray* atk_relation_get_ta rget (AtkRelation *relation); For newly-written bindable code, G*Array is much pleasanter than using separate array and length parameters, which aren't yet supported by gjs (I don't know about pybank).
Created attachment 149432 [details] [review] Support GArray parameters Add 'is_garray' property of array types, which indicates that the array is stored as a GArray, ie, with public length and data members in a structure. This implies that zero-terminated and has_length are false, since the length is stored in the GArray.
I updated the patch for HEAD of gobject-introspection.
Review of attachment 149432 [details] [review]: ::: girepository/girepository.h @@ +371,3 @@ gboolean g_type_info_is_zero_terminated (GITypeInfo *info); gint g_type_info_get_array_fixed_size(GITypeInfo *info); +gboolean g_type_info_is_g_array (GITypeInfo *info); Hmm, and then the consumer looks at the type name and does a strcmp? Ok, I guess, but I'd prefer to see something like: enum { GI_ARRAY_TYPE_C, GI_ARRAY_TYPE_ARRAY, GI_ARRAY_TYPE_PTR_ARRAY, GI_ARRAY_TYPE_BYTE_ARRAY } GIArrayType; g_type_info_get_array_type (GITypeInfo *info); Then we do the strcmps on the type name inside that function.
Can someone change the bug title to "Support for GArray and Binary Arrays" It also blocks these bugs - can someone add that to the blockers https://bugzilla.gnome.org/show_bug.cgi?id=612136 https://bugzilla.gnome.org/show_bug.cgi?id=607625
*** Bug 615231 has been marked as a duplicate of this bug. ***
Created attachment 159765 [details] [review] Add support for GArrays: add g_type_info_get_array_type() and properly scan GArray args Implements Colin's suggestion, also improves the scanner for GArrays. Extensive tests included.
Review of attachment 159765 [details] [review]: This patch reminds me we should probably add an -expected.gir for the marshalling tests. Don't need to do it in this patch though. Overall a good patch! Thanks a lot for diving into this. ::: gir/gimarshallingtests.c @@ +1329,3 @@ + */ + * Returns: (element-type gint) (transfer none): + * g_i_marshalling_tests_garray_int_none_return: Need space between identifier and paren (throughout most of this patch). We use the GNU style in g-i, and I'd like to keep things consistent. @@ +1424,3 @@ +/** + * g_i_marshalling_tests_garray_utf8_container_in: + * @array_: (element-type utf8) (transfer container): While I don't strictly mind supporting this, I think it's generally ill-advised to write C functions which take input parameters with a transfer. The major exception here is floating GObjects which are handled automatically by binding implementations. Not sure offhand if gjs actually handles transfer inputs right now or not. ::: girepository/ginfo.c @@ +1108,3 @@ +{ +g_type_info_get_array_type (GITypeInfo *info) +GIArrayType I'd somewhat prefer a: g_return_val_if_fail (blob->tag != GI_TYPE_TAG_ARRAY, GI_ARRAY_TYPE_C) (We're probably not consistent about this elsewhere in the code, but more assertions are good) ::: girepository/girparser.c @@ +1720,3 @@ + (strcmp (ctype, "GArray**") == 0)) { + if ((strcmp (ctype, "GArray*") == 0) || + ctype = find_attribute ("c:type", attribute_names, attribute_values); g_str_has_prefix? @@ +1737,3 @@ + size = find_attribute ("fixed-size", attribute_names, attribute_values); + zero = find_attribute ("zero-terminated", attribute_names, attribute_values); + if (typenode->array_type == GI_ARRAY_TYPE_C) { This line can be dropped since we assign just a bit below I believe.
Something I have thought afterwards is if we really need to store this information in the typelib or if we should just make sure that the type name mades the typelib and then check for GArray* etc in g_type_info_get_array_type(). This would make also easier to add more types of Arrays in the future.
Created attachment 159984 [details] [review] Add support for GArrays: add g_type_info_get_array_type() and properly scan GArray args Based on a previous patch by C. Scott Ananian <cscott@litl.com>
Review of attachment 159984 [details] [review]: One small comment for something that I didn't catch before. ::: girepository/girparser.c @@ +1727,3 @@ + } else { + typenode->array_type = GI_ARRAY_TYPE_C; + } I'd like to avoid having the gir parser scanning the c:type actually. I think this would just take a small modification in girwriter.py. ::: tests/scanner/foo-1.0-expected.gir @@ +792,3 @@ + <function name="test_array" c:identifier="foo_test_array"> + <return-value transfer-ownership="container"> + <array c:type="GArray*"> So this would be: <array type="GLib.Array" c:type="GArray*">
Created attachment 160261 [details] [review] Add support for GArrays: add g_type_info_get_array_type() and properly scan GArray args Based on a previous patch by C. Scott Ananian <cscott@litl.com> Only makes test pass, will change it to use type instead of c:type in a future update.
Created attachment 160264 [details] [review] Add support for GArrays: add g_type_info_get_array_type() and properly scan GArray args Based on a previous patch by C. Scott Ananian <cscott@litl.com>
Review of attachment 160264 [details] [review]: Looks good, thanks a lot!
Attachment 160264 [details] pushed as f84400b - Add support for GArrays: add g_type_info_get_array_type() and properly scan GArray args
[Mass-moving gobject-introspection tickets to its own Bugzilla product - see bug 708029. Mass-filter your bugmail for this message: introspection20150207 ]