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 627236 - Auxiliary argument for array
Auxiliary argument for array
Status: RESOLVED FIXED
Product: pygobject
Classification: Bindings
Component: introspection
Git master
Other Linux
: Normal normal
: ---
Assigned To: Nobody's working on this now (help wanted and appreciated)
Python bindings maintainers
Depends on:
Blocks:
 
 
Reported: 2010-08-18 08:01 UTC by Damien Caliste
Modified: 2011-10-21 18:23 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Add a warning in case of double count on an auxiliary argument (518 bytes, patch)
2010-08-18 08:01 UTC, Damien Caliste
needs-work Details | Review
only update the arg counts once if child arg comes before parent arg (6.20 KB, patch)
2011-09-15 04:17 UTC, johnp
none Details | Review
we now assume that C arrays of structs are flat so memcpy them when marshalling (3.15 KB, patch)
2011-09-15 04:17 UTC, johnp
none Details | Review
fix tests to correctly construct a dummy Gtk.TargetEntry (1.90 KB, patch)
2011-09-15 04:17 UTC, johnp
none Details | Review

Description Damien Caliste 2010-08-18 08:01:12 UTC
Created attachment 168176 [details] [review]
Add a warning in case of double count on an auxiliary argument

When using several array arguments with the same auxciliary argument for length in GI annotations (array length=ARG), the number of arguments generated for the Python routine is wrong because of double counting in pygi-invoke.c, see:
http://www.mail-archive.com/pygtk%40daa.com.au/msg19542.html

As a first correction, I suggest the attached patch to warn that something goes wrong.

As suggest in the mail cited before, one may go further and try to avoid the double counting issue.
Comment 1 johnp 2010-09-07 01:58:59 UTC
Comment on attachment 168176 [details] [review]
Add a warning in case of double count on an auxiliary argument

I'm hesitant to add this patch.  It should be asserted on and fixed.  I think the correct fix is to have a bit field which marks which parameters have been marked as aux and only decrements if the bit is not flipped.  If we don't care about memory we could create an array with n elements where n is the number of parameters.  Each item can be a bitfield that we can use flags to mark different attributes for a parameter, not just aux parameters.
Comment 2 Bug flys 2011-05-11 05:06:04 UTC
Another use case in clutter:

  http://lists.clutter-project.org/pipermail/clutter-app-devel-list/2011-March/000547.html

Please fix the bug.
Comment 3 johnp 2011-05-12 14:17:42 UTC
Yes, this is being fixed in the invoke-rewrite branch.  Plans are for it to be released with PyGObject 3.
Comment 4 johnp 2011-09-15 03:44:22 UTC
I have a fix for this but it broke drag and drop so I need to track that down before committing.  Basically the API requires a flat array of GValues which means we need to marshal the GValue and then memcpy it into the array.  At issue is that some APIs may require an array of pointers, not a flat array of stucts.  There is no way to indicate this in GI.  We ran into this issue while marshalling the other way (from C to Python).  In that case we assume that struct arrays are all flat and object arrays are pointers.  It may not be a clean cut for marshalling to C.  I'll investigate more.
Comment 5 johnp 2011-09-15 04:01:32 UTC
Ah so it just exposed an error with the tests and the segfault is to be expected when passing a struct which hasn't been filled.  In fact it fixed some drag and drop issues that weren't showing up because we were passing the wrong data in the arrays.
Comment 6 johnp 2011-09-15 04:17:01 UTC
Created attachment 196582 [details] [review]
only update the arg counts once if child arg comes before parent arg

* if the child arg comes before the parent arg we need to update the
   argument counts and take the child arg out of the marshalling lists
   since it is handled by the parent
 * when two parents reference the same child arg as is the case with
   two arrays which have a single length argument we only want to update
   the count once
 * to do this we introduce the PYGI_META_ARG_CHILD_NEEDS_UPDATE meta type
   and only do the count update if this is set
 * APIs should keep in mind that this take extra processing so child args
   should really come after their parents
Comment 7 johnp 2011-09-15 04:17:11 UTC
Created attachment 196583 [details] [review]
we now assume that C arrays of structs are flat so memcpy them when marshalling

* there is no way in GI to tell if a C array is flat or an array of pointers
   so we assume that all arrays of structs are flat and all arrays of objects
   are pointer arrays.
Comment 8 johnp 2011-09-15 04:17:22 UTC
Created attachment 196584 [details] [review]
fix tests to correctly construct a dummy Gtk.TargetEntry

* structs are sometimes a pain in gi.  Simply constructing them using the
   the standard constructor (e.g. Gtk.TargetEntry()) will malloc the struct
   but not correctly initialize the fields which can cause a crash.
 * tests didn't crash before because they were sending in bogus data that
   somehow did not trigger the issue
 * now with the C struct array marshallers doing the right thing, the incorrect
   use of TargetEntry was causing a crash
Comment 9 johnp 2011-09-15 04:19:17 UTC
I need to write some tests in GObject-Introspection before committing this
Comment 10 johnp 2011-09-15 19:34:45 UTC
committed