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

[Alexander Larsson <alexl redhat com>]

On Fri, 17 Jan 2003, Alexander Larsson wrote:

> On Fri, 17 Jan 2003, Alexander Larsson wrote:
> 
> > 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.
> 
> Oops. Sorry about this, i see i was mistaken about how g_int_hash works.

Side note: 
This means my patch was broken and needed this part too:

Index: eel/eel-preferences-glade.c
===================================================================
RCS file: /cvs/gnome/eel/eel/eel-preferences-glade.c,v
retrieving revision 1.2
diff -u -p -r1.2 eel-preferences-glade.c
--- eel/eel-preferences-glade.c 17 Jan 2003 09:15:53 -0000      1.2
+++ eel/eel-preferences-glade.c 17 Jan 2003 09:34:30 -0000
@@ -249,7 +249,7 @@ eel_preferences_glade_connect_int_enum (
 
        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 (g_direct_hash, g_direct_equal);
        value_list = NULL;
 

 

-- 
=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=
 Alexander Larsson                                            Red Hat, Inc 
                   alexl redhat com    alla lysator liu se 
He's a superhumanly strong pirate card sharp for the 21st century. She's a 
cynical Buddhist college professor operating on the wrong side of the law. 
They fight crime!