On Thu, Aug 04, 2016 at 03:30:53PM +0300, Alexander Monakov wrote:
> On Wed, 3 Aug 2016, Paul Menzel wrote:
> > dwm.c:480:4: warning: Use of memory after it is freed
> >                         unmanage(m->stack, 0);
> >                         ^~~~~~~~~~~~~~~~~~~~~
> 
> I think the warning is correct: after m->stack is free'd in unmanage, the 
> subsequent 'focus(NULL)' call in unmanage will dereference m->stack if
> 'm' happened to match 'selmon'.  It's possible that a similar dereference can
> happen via other paths if 'm' is different from 'selmon'.
> 
> In any case, you should be able to confirm it by running dwm under Address
> Sanitizer or Valgrind and invoking the 'quit' procedure (bound to Mod-Shift-q 
> by
> default), unless you hit some other error first.
> 
> Note,
> 
> > void
> > cleanup(void)
> > {
> [...]
> >         for (m = mons; m; m = m->next)
> >                 if (m->stack)
> >                         unmanage(m->stack, 0);
> 
> this bit seems to be misquoting dwm source: afaics dwm has 'while' rather than
> 'if', and the rest of your email is worded as if you (correctly) had a 'while'
> there.
> 
> Alexander
> 

Hi,

The while loop is correct, it needs to unmanage a linked-list of stacked 
clients.

I've looked into it but I don't think the dereference can happen. It is likely
a false positive from clang analyzer, this happends quite often depending how
the code is structured.

In unmanage() it calls detachstack() and removes the client from the stack and
updates m->stack (can be NULL):

        ...
        Client **tc, *t;

        for (tc = &c->mon->stack; *tc && *tc != c; tc = &(*tc)->snext);
        *tc = c->snext;
        ...

Later in unmanage() the client is free'd after that, but is not in the
linked-list anymore so it will not be used. After all that is done the monitor
resources (linked-list) is free'd.

I hope this explains it.

-- 
Kind regards,
Hiltjo

Reply via email to