Re: [patch] fix int-enum preference update in eel

[Alexander Larsson <alexl redhat com>]

On 17 Jan 2003, Jan Arne Petersen wrote:

> Hi,
> 
> Here is a fix for a bug in eel-preferences-glade.c.

Uhm. This looks quite wrong:

@@ -249,13 +248,17 @@
 	
 	option_menu = GTK_OPTION_MENU (glade_xml_get_widget (dialog, component));
 
-	map = g_hash_table_new (g_int_hash, g_int_equal);
+	map = g_hash_table_new_full(g_int_hash, g_int_equal, (GDestroyNotify) g_free, NULL);
 	value_list = NULL;
 
 	for (i = 0; values[i] != -1; i++) {
-		value = values[i];
-		value_list = g_slist_append (value_list, GINT_TO_POINTER (value));
-		g_hash_table_insert (map, &value, GINT_TO_POINTER (i));
+		value = g_malloc (sizeof (int));
+		if (value == NULL) {
+			return;
+		}

g_malloc never returns NULL.

+		*value = values[i];
+		value_list = g_slist_append (value_list, GINT_TO_POINTER (*value));
+		g_hash_table_insert (map, value, GINT_TO_POINTER (i));

This hash table is quite useless, as it maps from "address of some memory 
that nobody knows about" and the index i. Of course the previous code was 
bogus too, mapping from "some address on the stack not valid anymore" to 
the index i. Not to mention that the hash table uses g_int_hash/equal and 
not the pointer versions.

 	}
 
 	g_object_set_data_full (G_OBJECT (option_menu), EEL_PREFERENCES_GLADE_DATA_MAP, map,


I've commited the following fix instead:

Index: eel/eel-preferences-glade.c
===================================================================
RCS file: /cvs/gnome/eel/eel/eel-preferences-glade.c,v
retrieving revision 1.1
diff -u -p -r1.1 eel-preferences-glade.c
--- eel/eel-preferences-glade.c 31 Oct 2002 19:00:49 -0000      1.1
+++ eel/eel-preferences-glade.c 17 Jan 2003 09:10:53 -0000
@@ -226,7 +226,7 @@ eel_preferences_glade_int_enum_update (G
        value = eel_preferences_get_integer (g_object_get_data (G_OBJECT (option_menu),
                                                                
EEL_PREFERENCES_GLADE_DATA_KEY));
 
-       eel_preferences_glade_option_menu_update (option_menu, &value,
+       eel_preferences_glade_option_menu_update (option_menu, GINT_TO_POINTER (value),
                                                  G_CALLBACK (eel_preferences_glade_int_enum_changed));
 }
 
@@ -255,7 +255,7 @@ eel_preferences_glade_connect_int_enum (
        for (i = 0; values[i] != -1; i++) {
                value = values[i];
                value_list = g_slist_append (value_list, GINT_TO_POINTER (value));
-               g_hash_table_insert (map, &value, GINT_TO_POINTER (i));
+               g_hash_table_insert (map, GINT_TO_POINTER (value), GINT_TO_POINTER (i));
        }
 
        g_object_set_data_full (G_OBJECT (option_menu), EEL_PREFERENCES_GLADE_DATA_MAP, map,



-- 
=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=
 Alexander Larsson                                            Red Hat, Inc 
                   alexl redhat com    alla lysator liu se 
He's an immortal small-town master criminal on the run. She's a wealthy 
psychic bodyguard operating on the wrong side of the law. They fight crime!