----- 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.


> 
> 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].

Reply via email to