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 618571 - Add gtk_radio_action_join_group method
Add gtk_radio_action_join_group method
Status: RESOLVED FIXED
Product: gtk+
Classification: Platform
Component: Class: UIManager / Actions
unspecified
Other Linux
: Normal normal
: ---
Assigned To: gtk-bugs
gtk-bugs
Depends on:
Blocks:
 
 
Reported: 2010-05-13 20:36 UTC by johnp
Modified: 2010-05-19 19:18 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
[PATCH] add the binding friendly join_group method to GtkRadioAction (3.96 KB, patch)
2010-05-13 20:36 UTC, johnp
none Details | Review
[PATCH] add the binding friendly join_group method to GtkRadioAction (3.96 KB, patch)
2010-05-13 20:39 UTC, johnp
accepted-commit_now Details | Review

Description johnp 2010-05-13 20:36:13 UTC
The current way to set a radio action as part of a group is to get the group from an action currently in the group and then setting that group on the action.  Because group is just a GSList containing all of the actions in the group, we run into reffing (ownership) issues in GI generated bindings causing segfaults.  Basically the list gets changed out from underneath the bindings when set_group is called causing issues later in the code.

join_group fixes this by keeping the group list internal when adding an action to a group.  It takes two actions (the second can be NULL) and adds the first action to the second action's group.  Since the bindings never see the group list it never has to deal with ownership issues for object in the list.
Comment 1 johnp 2010-05-13 20:36:48 UTC
Created attachment 161005 [details] [review]
[PATCH] add the binding friendly join_group method to GtkRadioAction


* Due to object ownership issues it is imposible to correctly use
  get_group/set_group from a GI binding
* join_group is safer because at the binding level it works with indvidual
  GtkRadioAction objects and not with the list of objects that gets
  modified in the library
---
 docs/reference/gtk/gtk3-sections.txt        |    1 +
 docs/reference/gtk/tmpl/gtkradioaction.sgml |    9 +++++
 gtk/gtk.symbols                             |    1 +
 gtk/gtkradioaction.c                        |   53 +++++++++++++++++++++++++++
 gtk/gtkradioaction.h                        |    2 +
 5 files changed, 66 insertions(+), 0 deletions(-)
Comment 2 johnp 2010-05-13 20:39:45 UTC
Created attachment 161006 [details] [review]
[PATCH] add the binding friendly join_group method to GtkRadioAction


* Due to object ownership issues it is impossible to correctly use
  get_group/set_group from a GI binding
* join_group is safer because at the binding level it works with individual
  GtkRadioAction objects and not with the list of objects that gets
  modified in the library
---
 docs/reference/gtk/gtk3-sections.txt        |    1 +
 docs/reference/gtk/tmpl/gtkradioaction.sgml |    9 +++++
 gtk/gtk.symbols                             |    1 +
 gtk/gtkradioaction.c                        |   53 +++++++++++++++++++++++++++
 gtk/gtkradioaction.h                        |    2 +
 5 files changed, 66 insertions(+), 0 deletions(-)
Comment 3 johnp 2010-05-13 20:40:39 UTC
fixed some spelling mistakes in the commit log
Comment 4 Matthias Clasen 2010-05-17 22:47:23 UTC
Review of attachment 161006 [details] [review]:

Looks like an ok api addition to me, in general.

::: gtk/gtkradioaction.c
@@ +460,3 @@
+ * gtk_radio_action_join_group:
+ * @action: the action object
+ * @group_source: (allow-none): a radio action object whos group we are joining

Should say ", or %NULL to remove the radio action from its group" here

@@ +464,3 @@
+ * Joins a radio action object to the group of another radio action object.
+ *
+ * Use this in GI scripting languages instead of the get_group and set_group method

Please use full function names with () here, to get proper links in the generated docs. And I would prefer 'language bindings' over 'GI scripting languages'.