GNOME Bugzilla – Bug 620170
GStrv args being interpreted as void*
Last modified: 2015-02-07 17:02:33 UTC
The patch attached special cases GStrv as an array of utf8. Another alternative is to make the C parser interpret correctly the typedef as gchar** and making sure the GIR parser generates the correct info on the typelib. It may imply adding support for annotations about types.
Created attachment 162394 [details] [review] Add a special case for GStrv so it's interpreted as an array of utf8.
Tested this patch and it fix segfault when calling a function that has a GStrv in its args. Thanks! Note that this bug is the same as described in bug #619799 comment #3.
Also note that this patch does not fix the case of GStrv inside a struct, I still get that error when running g-ir-compiler: ** WARNING **: field TelepathyGLib.AvatarRequirements.supported_mime_types has void type
Looks like we can't pass 'None' for a GStrv arg, is that normal? I get that backtrace: Traceback (most recent call last):
+ Trace 222173
print get_name();
field = TelepathyGLib.ContactInfoField.new ("n", None, None)
return info.invoke(cls, *args)
Passing '[]' works though.
(In reply to comment #4) > Looks like we can't pass 'None' for a GStrv arg, is that normal? Yes, unless the arg is annotated with (allow-none).
(In reply to comment #5) > (In reply to comment #4) > > Looks like we can't pass 'None' for a GStrv arg, is that normal? > > Yes, unless the arg is annotated with (allow-none). An empty list should always be accepted though, but that's up to each LB to implement.
Created attachment 162469 [details] [review] Fix marshalling of GStrv. * gir/gimarshallingtests.[hc]: Add a test for GStrv in function args and as struct fields. * girepository/giroffsets.c: Correctly compute the size of structs with array fields * girepository/girparser.c: Set is_pointer to FALSE for arrays with fixed size inside structs. * giscanner/glibtransformer.py: Special case GStrv as arrays of utf8.
*** Bug 616815 has been marked as a duplicate of this bug. ***
Comment on attachment 162469 [details] [review] Fix marshalling of GStrv. >diff --git a/giscanner/glibtransformer.py b/giscanner/glibtransformer.py >+ elif ptype.name == "GObject.Strv": >+ return Array(None, ptype.ctype, 'utf8') According to: http://library.gnome.org/devel/gobject/unstable/gobject-Boxed-Types.html#G-TYPE-STRV:CAPS G_TYPE_STRV: The GType for a boxed type holding a NULL-terminated array of strings. In other words, a GStrv should default to being zero-terminated. We should have a default transfer annotation as well, either container or full. The rest looks good.
(In reply to comment #9) [..] > We should have a default transfer annotation as well, either container or full. Should probably mimic the default GObject transfer annotation and our policy that functions without annotations should rather be "crashing" than "leaking", since otherwise it'll be much harder to track down wrong annotations. In other words, "full" should be the default transfer for GStrv.
*** Bug 619799 has been marked as a duplicate of this bug. ***
Created attachment 162472 [details] [review] Fix marshalling of GStrv. * gir/gimarshallingtests.[hc]: Add a test for GStrv in function args and as struct fields. * girepository/giroffsets.c: Correctly compute the size of structs with array fields * girepository/girparser.c: Set is_pointer to FALSE for arrays with fixed size inside structs. * giscanner/glibtransformer.py: Special case GStrv as arrays of utf8.
Comment on attachment 162472 [details] [review] Fix marshalling of GStrv. + elif isinstance(node.type, Array): + return PARAM_TRANSFER_FULL Let's be conservative instead and avoid potentially breaking current APIs. This should also apply only for return values. I can't see the zero-terminated=true part of the patch. To clarify myself: * GObject.Strv should set zero-terminated=True (do this in glibtransfer.py where you're creating the Array) * return values of GObject.Strv should set transfer=full, unsure if this needs to be done in annotationparser.py or if it can be in glibtransfer.py.
Created attachment 162550 [details] [review] Fix marshalling of GStrv. * gir/gimarshallingtests.[hc]: Add a test for GStrv in function args and as struct fields. * girepository/giroffsets.c: Correctly compute the size of structs with array fields * girepository/girparser.c: Set is_pointer to FALSE for arrays with fixed size that are inside structs. * giscanner/glibtransformer.py: Special case GStrv as arrays of utf8. * giscanner/annotationparser.py: Make full transfer the default for arrays of char* returned by functions.
(In reply to comment #13) > (From update of attachment 162472 [details] [review]) > + elif isinstance(node.type, Array): > + return PARAM_TRANSFER_FULL > > Let's be conservative instead and avoid potentially breaking current APIs. > This should also apply only for return values. I can't see the > zero-terminated=true part of the patch. > > To clarify myself: > > * GObject.Strv should set zero-terminated=True (do this in glibtransfer.py > where you're creating the Array) But all Arrays are zero-terminated by default: http://git.gnome.org/browse/gobject-introspection/tree/giscanner/ast.py#n306 http://git.gnome.org/browse/gobject-introspection/tree/giscanner/girwriter.py#n245 > * return values of GObject.Strv should set transfer=full, unsure if this needs > to be done in annotationparser.py or if it can be in glibtransfer.py. Done
Review of attachment 162550 [details] [review]: Looks good, thanks for solving that. ::: giscanner/annotationparser.py @@ +935,3 @@ return PARAM_TRANSFER_NONE elif isinstance(node, Return): + if isinstance(node.type, Array) and \ No need for \ if you put () around the whole if statement.
Created attachment 162565 [details] [review] Fix marshalling of GStrv. * gir/gimarshallingtests.[hc]: Add a test for GStrv in function args and as struct fields. * girepository/giroffsets.c: Correctly compute the size of structs with array fields * girepository/girparser.c: Set is_pointer to FALSE for arrays with fixed size that are inside structs. * giscanner/glibtransformer.py: Special case GStrv as arrays of utf8. * giscanner/annotationparser.py: Make full transfer the default for arrays of char* returned by functions.
[Mass-moving gobject-introspection tickets to its own Bugzilla product - see bug 708029. Mass-filter your bugmail for this message: introspection20150207 ]