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

[Jeffrey Stedfast <fejj ximian com>]

yea, this looks good :)

Jeff

On Thu, 2004-08-05 at 11:47, Rodrigo Moya wrote:
> 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