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 685922 - [gobject-introspection] Add regress testing method for scope notified without user data
[gobject-introspection] Add regress testing method for scope notified without...
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)
Depends on:
Blocks: 685598
 
 
Reported: 2012-10-10 23:11 UTC by Simon Feltman
Modified: 2015-02-07 16:56 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Add test method for GDestroy with no user data (3.49 KB, patch)
2012-10-10 23:29 UTC, Simon Feltman
none Details | Review
Updated doc string for new method (3.52 KB, patch)
2012-10-11 22:38 UTC, Simon Feltman
committed Details | Review

Description Simon Feltman 2012-10-10 23:11:10 UTC
Although I could only find one occurance in all of glib and gtk+ which uses scope notified (gbinding) and it has a user data arg, a new regress testing method should be added for robustness of testing the following example:

/* callback: (scope notified): */
int method(SomeCallback callback, GDestroyNotify notify);

If we don't want to support methods which accept callbacks with GDestroyNotify and no user data, then gir scanning methods should error out when they find one.
Comment 1 Simon Feltman 2012-10-10 23:29:14 UTC
Created attachment 226220 [details] [review]
Add test method for GDestroy with no user data

Added regress_test_callback_destroy_notify_no_user_data.
Updated Regress-1.0-expected.gir
Comment 2 Martin Pitt 2012-10-11 14:15:24 UTC
Hello Simon,

for the record, this will provide test API for bug 685598.

(In reply to comment #0)
> Although I could only find one occurance in all of glib and gtk+ which uses
> scope notified (gbinding) and it has a user data arg

I guess you mean "no user data arg"?

+ * Notified - callback persists until a DestroyNotify delegate
+ * is invoked version without user_data argument.

Can you please add some interpunction in the last line, so that this is easier to read?

This is a curious API, what do we need a GDestroyNotify for if there is no user data to destroy? But if that API exists in GLib, then by all means let's add it.

The only API with a destroy notify that I found in GBindings is g_object_bind_property_full(), but that does have an user_data. Which one did you find?
Comment 3 Simon Feltman 2012-10-11 22:30:52 UTC
(In reply to comment #2)
> Hello Simon,
> 
> for the record, this will provide test API for bug 685598.
> 
> (In reply to comment #0)
> > Although I could only find one occurance in all of glib and gtk+ which uses
> > scope notified (gbinding) and it has a user data arg
> 
> I guess you mean "no user data arg"?

I meant it does have a user data arg. Just noting it is the only usage of scope notified I found and follows the expected protocol. This was the g_object_bind_property_full function you found.

> + * Notified - callback persists until a DestroyNotify delegate
> + * is invoked version without user_data argument.
> 
> Can you please add some interpunction in the last line, so that this is easier
> to read?

Sure thing.

> This is a curious API, what do we need a GDestroyNotify for if there is no user
> data to destroy? But if that API exists in GLib, then by all means let's add
> it.

We don't, this may never be a valid use case related for scope notified callbacks (there are cases where GDestroyNotify is used without user data like g_hash_table_new_full). However, this setup is allowed by gobject-introspection, and we have specific error logic in pygobject for using GDestroyNotify without user data. This error logic is what needs unit testing, hence the addition of this particular test method in order to do that. I will add this additional information to the methods comments.
Comment 4 Simon Feltman 2012-10-11 22:38:21 UTC
Created attachment 226303 [details] [review]
Updated doc string for new method
Comment 5 Martin Pitt 2012-10-12 05:06:43 UTC
Comment on attachment 226303 [details] [review]
Updated doc string for new method

Thanks Simon, please go ahead and commit.
Comment 6 André Klapper 2015-02-07 16:56:58 UTC
[Mass-moving gobject-introspection tickets to its own Bugzilla product - see bug 708029. Mass-filter your bugmail for this message: introspection20150207 ]