Created attachment 166880 [details] [review]
patch

not sure about the name...

Created attachment 167317 [details] [review]
patch

also add testcase for fixed-size bytearray, as requested by pygobject maintainers.

Review of attachment 167317 [details] [review]:

First, I think those tests would better go to gir/gimarshallingtests.c.

Secondly, returning the thing that was passed is harder to debug than asserting an expected value inside the (then) procedure, not to say that both the marshalling of the input argument and the one of the output argument may fail, and that the test could still pass. Actually, this happened. So, please assert a value inside the procedure.

Wouldn't we add tests out arguments, and return values, by the way?

::: gir/everything.h
@@ +55,3 @@
+/* not null-terminated strings */
+char *test_bytearray_in (const char *str, int len);
+char *test_bytearray_fixed_size_in (const char *str);

I think the name is misleading. GByteArray exists as well.

Created attachment 168319 [details] [review]
patch

Sorry for the delay, I was on vacation.

updated patch, I renamed it to "chararray", I am open to other suggestions.



I did not add the tests for the output case: not sure what you had in mind there... do you mean a function filling a **char str and *len?

(In reply to comment #4)
> Created an attachment (id=168319) [details] [review]
> patch
> 
> Sorry for the delay, I was on vacation.
> 
> updated patch, I renamed it to "chararray", I am open to other suggestions.
> 
> 
> 
> I did not add the tests for the output case: not sure what you had in mind
> there... do you mean a function filling a **char str and *len?

This needs to be updated to HEAD which generates code for gimarshallingtests.c

any news for this report?

Created attachment 179705 [details] [review]
updated patch

Review of attachment 179705 [details] [review]:

::: tests/gimarshallingtests.c
@@ +3372,3 @@
+/**
+ * gi_marshalling_tests_chararray_in:
+ * @str: (array length=len) (element-type guint8):

Not a big fan of overriding the C type this way via (element-type); why didn't you write:

@@ +3375,3 @@
+ */
+void
+gi_marshalling_tests_chararray_in (const char *str, int len)

const guint8* str?

What real world API is this mirroring?

@@ +3383,3 @@
+/**
+ * gi_marshalling_tests_fixed_chararray_in:
+ * @str: (array fixed-size=5) (element-type guint8):

Is there really an API out there somewhere that does this?

(In reply to comment #8)
> Review of attachment 179705 [details] [review]:
> 
> ::: tests/gimarshallingtests.c
> @@ +3372,3 @@
> +/**
> + * gi_marshalling_tests_chararray_in:
> + * @str: (array length=len) (element-type guint8):
> 
> Not a big fan of overriding the C type this way via (element-type); why didn't
> you write:
> 
> @@ +3375,3 @@
> + */
> +void
> +gi_marshalling_tests_chararray_in (const char *str, int len)
> 
> const guint8* str?
> 
> What real world API is this mirroring?

I think Paolo was intending to mirror gtk_text_buffer_insert.

I'm doing this with the aim of getting gdk_pixbuf_loader_write working, which doesn't require an element-type annotation.

> @@ +3383,3 @@
> +/**
> + * gi_marshalling_tests_fixed_chararray_in:
> + * @str: (array fixed-size=5) (element-type guint8):
> 
> Is there really an API out there somewhere that does this?

The closest things I can find:

void dacp_share_pair (DACPShare *share, gchar *service_name, gchar passcode[4]);

void            gst_netaddress_set_ip6_address   (GstNetAddress *naddr, guint8 address[16], guint16 port);

[Mass-moving gobject-introspection tickets to its own Bugzilla product - see bug 708029. Mass-filter your bugmail for this message: introspection20150207 ]

-- GitLab Migration Automatic Message --

This bug has been migrated to GNOME's GitLab instance and has been closed from further activity.

You can subscribe and participate further through the new bug through this link to our GitLab instance: https://gitlab.gnome.org/GNOME/gobject-introspection/issues/30.