! Please note that this is a snapshot of our old Bugzilla server, which is read only since May 29, 2020. Please go to gitlab.xfce.org for our new server !
menueditor segfaults due to freeing unitialized pointers
Status:
RESOLVED: FIXED
Product:
Xfce4-menueditor
Component:
General

Comments

Description Kevin Day 2007-12-14 00:50:37 CET
I sometimes start Windowmaker or another non-xfce desktio and then run xfsession so that xfce can manage my desktop for me. (ussualy just for kicks).
Because I am not starting the full blown xfce and my pixmaps and icons are at an unusual location, these pointers end up not being allocated to anything.
The end result is the "gtk free" command free's an unallocated pointer an an instant segfault. This segfault is noticed when I try to run xfce4-menueditor while under a non-xfce window manager.

there may be a better way than the if exist patch I have below. (possibly using some sort of gtk function to test for uninitialized pointers).

--- xfdesktop-4.4.2/menueditor/menueditor-main-window.c 2007-12-08 16:35:00 -0600
+++ xfdesktop-4.4.2/menueditor/menueditor-main-window.c.orig    2007-12-08 16:33:25 -0600
@@ -256,7 +256,7 @@
   /* Set default icon */
   icon = xfce_themed_icon_load ("xfce4-menueditor", 48);
   gtk_window_set_icon (GTK_WINDOW (mainwin), icon);
-  g_object_unref (icon);
+  if (icon) g_object_unref (icon);

   /* create ui manager */
   priv->action_group = gtk_action_group_new ("menueditor-main-window");
@@ -316,7 +316,7 @@
   gtk_box_pack_start (GTK_BOX (vbox), scrolledwindow, TRUE, TRUE, 0);

   priv->treeview = gtk_tree_view_new_with_model (GTK_TREE_MODEL (treestore));
-  g_object_unref (treestore);
+  if (treestore) g_object_unref (treestore);
   gtk_container_add (GTK_CONTAINER (scrolledwindow), priv->treeview);
   gtk_widget_show (priv->treeview);
Comment 1 Jean-François Wauthy editbugs 2007-12-14 07:50:30 CET
(In reply to comment #0)
>    /* Set default icon */
>    icon = xfce_themed_icon_load ("xfce4-menueditor", 48);
>    gtk_window_set_icon (GTK_WINDOW (mainwin), icon);
> -  g_object_unref (icon);
> +  if (icon) g_object_unref (icon);

I agree on this one (fixed with revision 26473).
 
>    priv->treeview = gtk_tree_view_new_with_model (GTK_TREE_MODEL (treestore));
> -  g_object_unref (treestore);
> +  if (treestore) g_object_unref (treestore);

But this one, if there is no treestore, what's the point of going further in the app ? 
 

Comment 2 Kevin Day 2007-12-14 23:01:16 CET
(In reply to comment #1)
> (In reply to comment #0)
> >    /* Set default icon */
> >    icon = xfce_themed_icon_load ("xfce4-menueditor", 48);
> >    gtk_window_set_icon (GTK_WINDOW (mainwin), icon);
> > -  g_object_unref (icon);
> > +  if (icon) g_object_unref (icon);
> 
> I agree on this one (fixed with revision 26473).
> 
> >    priv->treeview = gtk_tree_view_new_with_model (GTK_TREE_MODEL (treestore));
> > -  g_object_unref (treestore);
> > +  if (treestore) g_object_unref (treestore);
> 
> But this one, if there is no treestore, what's the point of going further in
> the app ? 
> 
> 

Ah, I had searched the file for other g_object_unref's to see if they weren't being checked.
I always see it as good practice to always check before freeing dynamic memory.
Comment 3 Jean-François Wauthy editbugs 2007-12-15 10:55:54 CET
(In reply to comment #2)
> Ah, I had searched the file for other g_object_unref's to see if they weren't
> being checked.
> I always see it as good practice to always check before freeing dynamic memory.

Yes but in this case I don't mind having the menueditor crashing when treestore could not be initialized. What's the point of running the menueditor without a working treeview ? 

Comment 4 Kevin Day 2007-12-15 16:13:27 CET
(In reply to comment #3)
> (In reply to comment #2)
> > Ah, I had searched the file for other g_object_unref's to see if they weren't
> > being checked.
> > I always see it as good practice to always check before freeing dynamic memory.
> 
> Yes but in this case I don't mind having the menueditor crashing when treestore
> could not be initialized. What's the point of running the menueditor without a
> working treeview ? 
> 
I can't argue there, its up to you.
You already accepted the important one anyway, so I believe this bug is almost solved (pending the commit).
I will leave the resolution options of this bug to your discretion.
Comment 5 Jean-François Wauthy editbugs 2007-12-15 16:35:15 CET
(In reply to comment #4)
> I can't argue there, its up to you.
> You already accepted the important one anyway, so I believe this bug is almost
> solved (pending the commit).
It has already been committed.

> I will leave the resolution options of this bug to your discretion.

I went for the cleaner way, now I force the menueditor to fail with an error message if treestore is not initialized.

Index: menueditor-main-window.c
===================================================================
--- menueditor-main-window.c	(révision 26479)
+++ menueditor-main-window.c	(copie de travail)
@@ -312,6 +312,9 @@
   treestore =
     gtk_tree_store_new (COLUMNS, GDK_TYPE_PIXBUF, G_TYPE_STRING, G_TYPE_STRING, G_TYPE_BOOLEAN, G_TYPE_INT,
                         G_TYPE_STRING, G_TYPE_STRING, G_TYPE_STRING);
+  if (G_UNLIKELY (!G_IS_OBJECT (treestore))) {
+    g_error ("Could not initialize the treestore");
+  }
   scrolledwindow = gtk_scrolled_window_new (NULL, NULL);
   gtk_widget_show (scrolledwindow);
   gtk_scrolled_window_set_policy (GTK_SCROLLED_WINDOW (scrolledwindow), GTK_POLICY_AUTOMATIC, GTK_POLICY_AUTOMATIC);

Committed with revision 26480

Bug #3747

Reported by:
Kevin Day
Reported on: 2007-12-14
Last modified on: 2009-07-14

People

Assignee:
Xfce Bug Triage
CC List:
0 users

Version

Attachments

Additional information