On Wed, 14 Nov 2012 at 20:18:11 +0100, Rodolfo García Peñas wrote:
> On Wed, 14 Nov 2012, Carlos R. Mafra escribió:
> 
> > On Wed, 14 Nov 2012 at 19:34:42 +0100, Rodolfo García Peñas wrote:
> > > Ok,
> > > 
> > > seems to be ok now :-)
> > > 
> > > --- a/src/icon.c
> > > +++ b/src/icon.c
> > > @@ -778,8 +778,10 @@ static int get_rimage_icon_from_wm_hints(WIcon *icon)
> > >         /* Resize the icon to the wPreferences.icon_size size */
> > >         image = wIconValidateIconSize(image, wPreferences.icon_size);
> > >  
> > > -       /* FIXME: If unset_icon_image, pointer double free then crash 
> > > -       unset_icon_image(icon); */
> > > +       unset_icon_image(icon);
> > > +
> > > +       /* Set the new info */
> > > +       icon->file = NULL;
> > >         icon->file_image = image;
> > 
> > But why setting icon->file to NULL helps? What goes wrong otherwise?
> 
> Really I don't have idea, but now is fine. Problem in wfree()?
> 
> See that wfree sets it to NULL. Needs the "if (ptr)" a bracket?
> 
> 
> void wfree(void *ptr)
> {
>         if (ptr)
> #ifdef USE_BOEHM_GC
>                 /* This should eventually be removed, once the criss-cross
>                  * of wmalloc()d memory being free()d, malloc()d memory being
>                  * wfree()d, various misuses of calling wfree() on objects
>                  * allocated by libc malloc() and calling libc free() on
>                  * objects allocated by Boehm GC (think external libraries)
>                  * is cleaned up.
>                  */
>                 if (GC_base(ptr) != 0)
>                         GC_FREE(ptr);
>                 else
>                         free(ptr);
> #else
>                 free(ptr);
> #endif
>         ptr = NULL;
> }

I guess the bracket is not strictly necessary here under the rules,
but it's better to have it because it's fragile otherwise.

I still don't see why your later patch is fine, but I haven't read
the code. I was hoping it would be clear to you so that's why I asked.

Since I don't want to think about all patches in detail, I try to
enforce the rule "try to look like you know what you're doing" in the
changelog. So that's why changelogs are important, I want to understand
what's going on with minimal effort :-)

So when I read your email saying that it works, I want to know why.
Otherwise many monkeys might be typing stuff and at some point things
work too. No offense intended! I just want to emphasize the point :-)

Perhaps this is all obvious and I'm embarrassing myself. But I don't
mind (too much).


-- 
To unsubscribe, send mail to wmaker-dev-unsubscr...@lists.windowmaker.org.

Reply via email to