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 681714 - incorrect introspection annotation for vte_terminal_new
incorrect introspection annotation for vte_terminal_new
Status: RESOLVED FIXED
Product: vte
Classification: Core
Component: general
0.32.x
Other Linux
: Normal normal
: ---
Assigned To: VTE Maintainers
VTE Maintainers
Depends on:
Blocks:
 
 
Reported: 2012-08-13 02:17 UTC by Phil Clayton
Modified: 2012-08-24 19:17 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Correct introspection annotation for vte_terminal_new (781 bytes, patch)
2012-08-13 02:34 UTC, Phil Clayton
reviewed Details | Review
Correct introspection annotation for vte_terminal_new (797 bytes, patch)
2012-08-13 20:47 UTC, Phil Clayton
accepted-commit_now Details | Review

Description Phil Clayton 2012-08-13 02:17:07 UTC
As a general rule, the introspection annotations for constructor functions that return floating objects must effect transfer-ownership="none".  (Otherwise, transfer-ownership="full" will cause the caller will assume that they own a reference to the returned object and so unref it later.  However, the reference is actually floating and will likely become owned by someone else, e.g. a widget that does ref_sink.  This leads to a spurious unref.  transfer-ownership="none" ensures that the caller does a ref_sink.)

VteTerminal is a descendant of GInitiallyUnowned, so the object returned by vte_terminal_new is floating.  Thus the introspection annotation for vte_terminal_new should not contain (transfer full).
Comment 1 Phil Clayton 2012-08-13 02:34:04 UTC
Created attachment 220958 [details] [review]
Correct introspection annotation for vte_terminal_new
Comment 2 Christian Persch 2012-08-13 18:44:44 UTC
Looks ok to me, but I'm not an expert on introspection. Get someone from gobject-introspection to review; ok to commit if they think it's fine too.
Comment 3 Colin Walters 2012-08-13 19:30:10 UTC
Review of attachment 220958 [details] [review]:

The comment about floating is right, but:

::: src/vte.c
@@ +2488,3 @@
  * Creates a new terminal widget.
  *
+ * Returns: (type Vte.Terminal): a new #VteTerminal object

This should include (transfer none)
Comment 4 Phil Clayton 2012-08-13 20:47:57 UTC
Created attachment 221057 [details] [review]
Correct introspection annotation for vte_terminal_new

Patch in response to review comment.

However, as just discussed on #introspection, either patch is acceptable: the correct transfer mode for constructor functions is automatically worked out based on whether objects derive from GInitiallyUnowned.  (See _get_transfer_default_return in maintransformer.py .)  This explains why many, if not most, constructor functions in GTK+ have no (transfer ...) annotation.  I can't commit, so I leave the choice of which patch to apply to whoever commits.
Comment 5 Colin Walters 2012-08-13 20:52:55 UTC
Review of attachment 221057 [details] [review]:

Looks good, thanks!
Comment 6 Christian Persch 2012-08-24 19:17:29 UTC
Fixed on 0-34 and next.