On Wed, 14 Nov 2012, Christophe escribió:

> 
> ----- Carlos R. Mafra <[email protected]> a écrit :
> > 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 :-)
> 
> Hi Carlos, Hi Rodolfo,
> 
> From my understanding of the problem, it is because when the memory is freed 
> the first time the pointer is kept to the freed location, and then it looks 
> like the memory is still there, so a new call to wfree is possible.
> 
> When setting the pointer to NULL, this actually stores the information that 
> this memory is not reserved, so things works better. However the workaround 
> proposed is not great because the pointer reset is not done at the right 
> place. I'll send a patch to propose a different solution which I think should 
> be safer.
 
Thanks.
 
> > 
> > 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 [email protected].
> 
> 
> --
> To unsubscribe, send mail to [email protected].

-- 
||// //\\// Rodolfo "kix" Garcia
||\\// //\\ http://www.kix.es/


-- 
To unsubscribe, send mail to [email protected].

Reply via email to