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 581687 - Support GArray parameters.
Support GArray parameters.
Status: RESOLVED FIXED
Product: gobject-introspection
Classification: Platform
Component: general
unspecified
Other Linux
: Normal normal
: ---
Assigned To: gobject-introspection Maintainer(s)
gobject-introspection Maintainer(s)
: 615231 (view as bug list)
Depends on:
Blocks: 581696
 
 
Reported: 2009-05-07 04:04 UTC by C. Scott Ananian
Modified: 2015-02-07 16:53 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Support-GArray-parameters.patch (8.39 KB, patch)
2009-05-07 04:04 UTC, C. Scott Ananian
none Details | Review
Support-GArray-parameters.patch (8.81 KB, patch)
2009-05-08 20:25 UTC, C. Scott Ananian
none Details | Review
Support-GArray-parameters.patch (8.80 KB, patch)
2009-06-10 18:42 UTC, C. Scott Ananian
none Details | Review
Support-GArray-parameters.patch (7.64 KB, patch)
2009-06-12 17:15 UTC, C. Scott Ananian
needs-work Details | Review
Support GArray parameters (7.33 KB, patch)
2009-12-09 14:30 UTC, Johan (not receiving bugmail) Dahlin
reviewed Details | Review
Add support for GArrays: add g_type_info_get_array_type() and properly scan GArray args (19.35 KB, patch)
2010-04-28 08:44 UTC, Tomeu Vizoso
reviewed Details | Review
Add support for GArrays: add g_type_info_get_array_type() and properly scan GArray args (19.39 KB, patch)
2010-04-30 16:17 UTC, Tomeu Vizoso
reviewed Details | Review
Add support for GArrays: add g_type_info_get_array_type() and properly scan GArray args (22.11 KB, patch)
2010-05-04 13:48 UTC, Tomeu Vizoso
none Details | Review
Add support for GArrays: add g_type_info_get_array_type() and properly scan GArray args (24.13 KB, patch)
2010-05-04 14:58 UTC, Tomeu Vizoso
committed Details | Review

Description C. Scott Ananian 2009-05-07 04:04:09 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.)
Comment 1 C. Scott Ananian 2009-05-07 04:04:34 UTC
Created attachment 134157 [details] [review]
Support-GArray-parameters.patch
Comment 2 Matthias Clasen 2009-05-07 16:29:52 UTC
Not sure if it is relevant here, but note that gobject just grew boxed types for arrays. 
Comment 3 C. Scott Ananian 2009-05-08 20:24:20 UTC
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).
Comment 4 C. Scott Ananian 2009-05-08 20:25:45 UTC
Created attachment 134285 [details] [review]
Support-GArray-parameters.patch

Update the patch to reflect the new patch for bug 581685
Comment 5 C. Scott Ananian 2009-06-10 18:42:46 UTC
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.
Comment 6 C. Scott Ananian 2009-06-12 17:15:30 UTC
Created attachment 136458 [details] [review]
Support-GArray-parameters.patch

Refreshed patch against git HEAD.  Lonely patch wants review.
Comment 7 Johan (not receiving bugmail) Dahlin 2009-06-12 17:19:07 UTC
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.
Comment 8 C. Scott Ananian 2009-06-12 17:52:22 UTC
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).
Comment 9 Johan (not receiving bugmail) Dahlin 2009-12-09 14:30:48 UTC
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.
Comment 10 Johan (not receiving bugmail) Dahlin 2009-12-09 14:31:33 UTC
I updated the patch for HEAD of gobject-introspection.
Comment 11 Colin Walters 2009-12-09 18:31:05 UTC
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.
Comment 12 Alan Knowles 2010-04-15 23:37:05 UTC
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
Comment 13 Tomeu Vizoso 2010-04-27 13:27:01 UTC
*** Bug 615231 has been marked as a duplicate of this bug. ***
Comment 14 Tomeu Vizoso 2010-04-28 08:44:28 UTC
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.
Comment 15 Colin Walters 2010-04-28 13:45:57 UTC
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.
Comment 16 Tomeu Vizoso 2010-04-29 16:28:36 UTC
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.
Comment 17 Tomeu Vizoso 2010-04-30 16:17:25 UTC
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>
Comment 18 Colin Walters 2010-05-03 17:07:12 UTC
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*">
Comment 19 Tomeu Vizoso 2010-05-04 13:48:12 UTC
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.
Comment 20 Tomeu Vizoso 2010-05-04 14:58:22 UTC
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>
Comment 21 Colin Walters 2010-05-04 15:01:10 UTC
Review of attachment 160264 [details] [review]:

Looks good, thanks a lot!
Comment 22 Tomeu Vizoso 2010-05-04 15:09:31 UTC
Attachment 160264 [details] pushed as f84400b - Add support for GArrays: add g_type_info_get_array_type() and properly scan GArray args
Comment 23 André Klapper 2015-02-07 16:53:40 UTC
[Mass-moving gobject-introspection tickets to its own Bugzilla product - see bug 708029. Mass-filter your bugmail for this message: introspection20150207 ]