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 137956 - gtkpathbar -- incorrect object disposal
gtkpathbar -- incorrect object disposal
Status: RESOLVED FIXED
Product: gtk+
Classification: Platform
Component: Widget: GtkFileChooser
unspecified
Other All
: Normal major
: ---
Assigned To: gtk-bugs
gtk-bugs
Depends on:
Blocks: 137950
 
 
Reported: 2004-03-22 18:08 UTC by Morten Welinder
Modified: 2011-02-04 16:16 UTC
See Also:
GNOME target: ---
GNOME version: ---



Description Morten Welinder 2004-03-22 18:08:28 UTC
This is wrong for all platforms, but for reasons unknown always triggers
on win32.

Index: gtkpathbar.c
===================================================================
RCS file: /cvs/gnome/gtk+/gtk/gtkpathbar.c,v
retrieving revision 1.23
diff -s -u -r1.23 gtkpathbar.c
--- gtkpathbar.c	15 Mar 2004 18:12:51 -0000	1.23
+++ gtkpathbar.c	22 Mar 2004 18:07:15 -0000
@@ -1045,15 +1068,7 @@
     }
   else
     {
-      GList *l;
-
-      for (l = new_buttons; l; l = l->next)
-	{
-	  GtkWidget *button = BUTTON_DATA (l->data)->button;
-	  gtk_widget_destroy (button);
-	  gtk_widget_unref (button);
-	}
-
+      g_list_foreach (new_buttons, (GFunc)gtk_object_sink, NULL);
       g_list_free (new_buttons);
     }
Comment 1 Morten Welinder 2004-03-22 18:08:58 UTC
Please test -- this is intended for 2.4.1.
Comment 2 Morten Welinder 2004-03-22 18:44:16 UTC
Take 2 per comments from owen:

Index: gtkpathbar.c
===================================================================
RCS file: /cvs/gnome/gtk+/gtk/gtkpathbar.c,v
retrieving revision 1.23
diff -s -u -r1.23 gtkpathbar.c
--- gtkpathbar.c	15 Mar 2004 18:12:51 -0000	1.23
+++ gtkpathbar.c	22 Mar 2004 18:43:11 -0000
@@ -872,6 +895,9 @@
   button_data->type = find_button_type (path_bar, path);
   button_data->button = gtk_toggle_button_new ();
 
+  gtk_widget_ref (button_data->button);
+  gtk_object_sink (GTK_OBJECT (button_data->button));
+
   switch (button_data->type)
     {
     case ROOT_BUTTON:
@@ -1041,6 +1067,7 @@
 	{
 	  GtkWidget *button = BUTTON_DATA (l->data)->button;
 	  gtk_container_add (GTK_CONTAINER (path_bar), button);
+	  g_object_unref (button);
 	}
     }
   else
@@ -1050,8 +1077,7 @@
       for (l = new_buttons; l; l = l->next)
 	{
 	  GtkWidget *button = BUTTON_DATA (l->data)->button;
-	  gtk_widget_destroy (button);
-	  gtk_widget_unref (button);
+	  g_object_unref (button);
 	}
 
       g_list_free (new_buttons);

Comment 3 Owen Taylor 2004-03-22 19:03:35 UTC
The one further comment I'd make is that I think it might
be a bit clearer to put the ref/sink _after_ the call
to make_directory_button() not inside it ... keeping
all the non-standard memory management in a single place
rather than scattering at different levels makes it
less likely that cut-and-paste or other later changes
will introduce problems.
Comment 4 Morten Welinder 2004-03-22 19:07:00 UTC
Take 3 -- now with leak fix.

Index: gtkpathbar.c
===================================================================
RCS file: /cvs/gnome/gtk+/gtk/gtkpathbar.c,v
retrieving revision 1.23
diff -s -u -r1.23 gtkpathbar.c
--- gtkpathbar.c	15 Mar 2004 18:12:51 -0000	1.23
+++ gtkpathbar.c	22 Mar 2004 19:06:01 -0000
@@ -952,7 +975,7 @@
   GtkFilePath *path;
   gboolean first_directory = TRUE;
   gboolean result;
-  GList *new_buttons = NULL;
+  GList *new_buttons = NULL, *l;
 
   g_return_val_if_fail (GTK_IS_PATH_BAR (path_bar), FALSE);
   g_return_val_if_fail (file_path != NULL, FALSE);
@@ -1018,6 +1041,8 @@
       gtk_file_info_free (file_info);
       gtk_file_path_free (path);
 
+      gtk_widget_ref (button_data->button);
+      gtk_object_sink (GTK_OBJECT (button_data->button));
       new_buttons = g_list_prepend (new_buttons, button_data);
 
       if (button_data->type != NORMAL_BUTTON)
@@ -1031,32 +1056,25 @@
     }
 
   if (result)
-    {
-      GList *l;
-
-      gtk_path_bar_clear_buttons (path_bar);
-      path_bar->button_list = g_list_reverse (new_buttons);
+    gtk_path_bar_clear_buttons (path_bar);
 
-      for (l = path_bar->button_list; l; l = l->next)
-	{
-	  GtkWidget *button = BUTTON_DATA (l->data)->button;
-	  gtk_container_add (GTK_CONTAINER (path_bar), button);
-	}
-    }
-  else
+  new_buttons = g_list_reverse (new_buttons);
+  for (l = new_buttons; l; l = l->next)
     {
-      GList *l;
-
-      for (l = new_buttons; l; l = l->next)
-	{
-	  GtkWidget *button = BUTTON_DATA (l->data)->button;
-	  gtk_widget_destroy (button);
-	  gtk_widget_unref (button);
-	}
-
-      g_list_free (new_buttons);
+      ButtonData *button_data = l->data;
+      GtkWidget *button = button_data->button;
+      if (result)
+	gtk_container_add (GTK_CONTAINER (path_bar), button);
+      else
+	button_data_free (button_data);
+      gtk_widget_unref (button);
     }
 
+  if (result)
+    path_bar->button_list = new_buttons;
+  else
+    g_list_free (new_buttons);
+
   gtk_widget_pop_composite_child ();
 
   return result;
Comment 5 Federico Mena Quintero 2004-04-06 01:23:46 UTC
Committed the patch with some changes.

Next time, please submit the patch as an attachment and include a ChangeLog. 
Thanks!