[Bugfix] Mergesort used in clist

[Thomas Schultz <tststs gmx de>]

Hi,

In case this has not yet been fixed: Using GTK 1.2.8, I was experiencing
strange segfaults in a GNOME application and when I investigated into it,
I found that there is a problem with gtk_clist_sort(): After calling it,
row_list->prev will not be NULL, as it ought to be.

I believe that in gtkclist.c, z.next->prev should be set to NULL before
returning. I also think I found a few statements that do nothing but waste
processor cycles:

<---->
       {
         c->next = a;
         a->prev = c;
         c = a;
         a = a->next;
         break;
       }
<---->

As the "break;" statement leaves the loop and clearly neither c nor a are
used outside of it, changing them doesn't make sense here (it does in
other places, where this fragment seems to have been copy-pasted from).
You'll find that the same is true in the next block.

<---->
         if ((cmp >= 0 && clist->sort_type == GTK_SORT_DESCENDING) ||
             (cmp <= 0 && clist->sort_type == GTK_SORT_ASCENDING) ||
             (a && !b))
<---->

If you look at the code, you'll notice that this point can never be
reached when (a && !b) is true, so testing on it doesn't make sense
either.

For your convenience, I attached a patch as well as a small example
program to reproduce the bug.

BTW, thanks for a great product that's getting better all the time :-)

Kind regards,
    Thomas Schultz

P.S.: I just subscribed to this list; if it is not the right place for
such bugreports, I'd be grateful for a pointer. I also could not find a
"known bugs" page for GTK 1.2.8 - in case this problem is already fixed,
it would certainly have saved me several hours of debugging. Does
something like this exist?
--- gtk/gtkclist.c.orig	Thu Aug 31 22:01:28 2000
+++ gtk/gtkclist.c	Thu Aug 31 22:24:50 2000
@@ -7350,24 +7350,19 @@
 	{
 	  c->next = a;
 	  a->prev = c;
-	  c = a;
-	  a = a->next;
 	  break;
 	}
       else if (!a && b)
 	{
 	  c->next = b;
 	  b->prev = c;
-	  c = b;
-	  b = b->next;
 	  break;
 	}
       else /* a && b */
 	{
 	  cmp = clist->compare (clist, GTK_CLIST_ROW (a), GTK_CLIST_ROW (b));
 	  if ((cmp >= 0 && clist->sort_type == GTK_SORT_DESCENDING) ||
-	      (cmp <= 0 && clist->sort_type == GTK_SORT_ASCENDING) ||
-	      (a && !b))
+	      (cmp <= 0 && clist->sort_type == GTK_SORT_ASCENDING))
 	    {
 	      c->next = a;
 	      a->prev = c;
@@ -7384,6 +7379,7 @@
 	}
     }
 
+  z.next->prev = NULL;
   return z.next;
 }
 
#include <gtk/gtk.h>

/* This callback will crash the application. */
void crash(gpointer data)
{
    gtk_clist_sort(GTK_CLIST(data));
    while (GTK_CLIST(data)->rows)
    {
	gtk_clist_remove((GtkCList *) data, 0);
    }
}

int main(int argc, gchar *argv[])
{
    GtkWidget *window, *vbox, *clist, *button;
    gchar *clist_data[2][1] = {{"A"},{"Z"}};
    int i;
    gtk_init(&argc, &argv);
    window = gtk_window_new(GTK_WINDOW_TOPLEVEL);
    vbox = gtk_vbox_new(FALSE, 0);
    gtk_container_add(GTK_CONTAINER(window), vbox);
    gtk_widget_show(vbox);
    clist = gtk_clist_new(1);
    for (i = 0; i < 2; ++i)
      gtk_clist_append(GTK_CLIST(clist), clist_data[i]);
    gtk_box_pack_start(GTK_BOX(vbox), clist, FALSE, FALSE, 0);
    gtk_widget_show(clist);
    button = gtk_button_new_with_label("Crash");
    gtk_box_pack_start(GTK_BOX(vbox), button, FALSE, FALSE, 0);
    gtk_signal_connect_object(GTK_OBJECT(button), "clicked",
			      GTK_SIGNAL_FUNC(crash), (gpointer) clist);
    gtk_widget_show(button);
    gtk_widget_show(window);
    gtk_main();
    return(0);
}