Re: [evolution-patches] patch for #62030 (calendar)

[Rodrigo Moya <rodrigo novell com>]

On Thu, 2004-08-05 at 11:23 -0400, Jeffrey Stedfast wrote:
> On Thu, 2004-08-05 at 11:16, Rodrigo Moya wrote:
> > On Thu, 2004-08-05 at 16:31 +0200, Rodrigo Moya wrote:
> > > On Thu, 2004-08-05 at 10:09 -0400, Jeffrey Stedfast wrote:
> > > > eh? you don't need to g_free() alloca()'d memory.
> > > > 
> > > right, valgrind was bailing about this, and didn't see the g_alloca
> > > call. So, yes, discard this patch.
> > > 
> > > The real fix for the bug I was trying to fix is attached.
> > >
> > ok, hopefully the last patch, now including a fix in e-icon-factory.c
> > and some cleanup in the alarm daemon.
> 
> only commenting on the icon_factory bit
> 
> > Index: e-util/e-icon-factory.c
> > ===================================================================
> > RCS file: /cvs/gnome/evolution/e-util/e-icon-factory.c,v
> > retrieving revision 1.9
> > diff -u -p -r1.9 e-icon-factory.c
> > --- e-util/e-icon-factory.c     3 Jun 2004 15:01:33 -0000       1.9
> > +++ e-util/e-icon-factory.c     5 Aug 2004 15:09:05 -0000
> > @@ -196,6 +196,7 @@ e_icon_factory_init (void)
> >  static void
> >  icon_foreach_free (gpointer key, gpointer value, gpointer user_data)
> >  {
> > +       g_free (key);
> >         icon_free (value);
> >  }
> 
> I'd prefer that we simply used icon->name as the key like we currently
> do, then this wouldn't be necessary.
> 
> >  
> > @@ -287,7 +288,7 @@ e_icon_factory_get_icon (const char *ico
> >                         return gdk_pixbuf_scale_simple
> > (broken16_pixbuf, size, size, GDK_INTERP_NEAREST);
> >         }
> >         
> > -       icon_key = g_alloca (strlen (icon_name) + 7);
> > +       icon_key = g_malloc (strlen (icon_name) + 7);
> >         sprintf (icon_key, "%dx%d/%s", size, size, icon_name);
> 
> if we're going to malloc, might as well just use g_strdup_printf here.
> however, I'm not sure we really want to malloc. I think the better
> solution is to simply pass icon_key to load_icon() as well. Then
> load_icon() can pass icon_key to icon_new() rather than icon_name like
> it currently does (which is the bit that causes the leak).
> 
> if we do it this way, then very few lines of code need to change :)
> 
right, updated patch for e-icon-factory part attached
-- 
Rodrigo Moya <rodrigo novell com>

Index: ChangeLog
===================================================================
RCS file: /cvs/gnome/evolution/e-util/ChangeLog,v
retrieving revision 1.473
diff -u -p -r1.473 ChangeLog
--- ChangeLog	2 Aug 2004 18:34:55 -0000	1.473
+++ ChangeLog	5 Aug 2004 15:46:41 -0000
@@ -1,3 +1,12 @@
+2004-08-05  Rodrigo Moya <rodrigo novell com>
+
+	Fixes #62030
+
+	* e-icon-factory.c (load_icon): get the icon_key as a new argument
+	and use that in the call to icon_new().
+	(e_icon_factory_get_icon): pass icon_key to load_icon().
+	(e_icon_factory_get_icon_list): ditto.
+
 2004-08-01  JP Rosevear  <jpr novell com>
 
 	* e-pilot-util.c (e_pilot_get_sync_source): find the source with
Index: e-icon-factory.c
===================================================================
RCS file: /cvs/gnome/evolution/e-util/e-icon-factory.c,v
retrieving revision 1.9
diff -u -p -r1.9 e-icon-factory.c
--- e-icon-factory.c	3 Jun 2004 15:01:33 -0000	1.9
+++ e-icon-factory.c	5 Aug 2004 15:46:41 -0000
@@ -86,7 +86,7 @@ icon_free (Icon *icon)
 }
 
 static Icon *
-load_icon (const char *icon_name, int size, int scale)
+load_icon (const char *icon_key, const char *icon_name, int size, int scale)
 {
 	GdkPixbuf *pixbuf, *unscaled = NULL;
 	char *filename = NULL;
@@ -153,7 +153,7 @@ load_icon (const char *icon_name, int si
 		pixbuf = NULL;
 	}
 	
-	return icon_new (icon_name, pixbuf);
+	return icon_new (icon_key, pixbuf);
 }
 
 
@@ -293,7 +293,7 @@ e_icon_factory_get_icon (const char *ico
 	pthread_mutex_lock (&lock);
 	
 	if (!(icon = g_hash_table_lookup (name_to_icon, icon_key))) {
-		if (!(icon = load_icon (icon_name, size, TRUE))) {
+		if (!(icon = load_icon (icon_key, icon_name, size, TRUE))) {
 			g_warning ("Icon not found -- %s", icon_name);
 			
 			/* Create an empty icon so that we don't keep spitting
@@ -350,7 +350,7 @@ e_icon_factory_get_icon_list (const char
 		sprintf (icon_key, "%dx%d/%s", size, size, icon_name);
 		
 		if (!(icon = g_hash_table_lookup (name_to_icon, icon_key))) {
-			if ((icon = load_icon (icon_name, size, FALSE)))
+			if ((icon = load_icon (icon_key, icon_name, size, FALSE)))
 				g_hash_table_insert (name_to_icon, icon->name, icon);
 		}