Re: [PATCH 00/19] Enable -Wshadow=local for kernel/sched

From: Peter Zijlstra
Date: Wed Mar 02 2022 - 14:18:26 EST


On Wed, Mar 02, 2022 at 06:43:57PM +0000, Matthew Wilcox wrote:
> ie "__ret = freezable_schedule_timeout(__ret)" is supposed to refer to
> the inner __ret, not the outer __ret. Which was the opposite of what
> I thought was supposed to happen.
>
> We can fix this, of course. Something like ...
>
> #define ___wait_event_freezable_timeout(wq_head, condition, timeout, ret) \
> ___wait_event(wq_head, ___wait_cond_timeout(condition, ret), \
> TASK_INTERRUPTIBLE, 0, timeout, \
> ret = freezable_schedule_timeout(ret), ret)
>
> #define __wait_event_freezable_timeout(wq_head, condition, timeout) \
> ___wait_event_freezable_timeout(wq_head, condition, timeout, UNIQUE_ID)
>
> ... and now all the 'ret' refer to the thing that they look like they're
> referring to.

Right; so the trick is to make sure all ___wait_event() users will have
a ret and then the inner ret can go away. The interruptible/timeout
variants all already have a ret variable, but the unconditional things
like wait_event() do not (which is where all the trouble started).

By simply adding a ret variable, even to the variants without return
value, the inner variable can go away and the shadowing goes away.