RE: [PATCH 1/4] Adds -Wshadow=local on KBUILD_HOSTCFLAGS

From: David Laight
Date: Fri Oct 19 2018 - 07:28:22 EST


From: Masahiro Yamada
> Sent: 18 October 2018 17:39
>
> On Thu, Oct 18, 2018 at 6:18 PM Borislav Petkov <bp@xxxxxxxxx> wrote:
> >
> > On Wed, Oct 17, 2018 at 09:40:53PM -0300, Leonardo Bras wrote:
> > > The idea was to put it as default and fix all the shadowing warnings.
> > > What do you think? I am open to suggestions.
> >
> > That's Masahiro's call. In the rest of the kernel, those warnings are behind
> > the W=2 switch - i.e., not enabled by default.
>
>
> It is not realistic to enable this warning option by default.
> Even -Wshadow=local emits tons of warnings.
> (More with -Wshadow)

The question is, how many of them are actual bugs.
IMHO -Wshadow is a good idea.

> The problem of this flag is,
> it is false positive in macro expansions.

Right, but macro expansions inside macro definitions could accidentally
use the same local variable - leading to choas.

> For example, I think the following is a legitimate case.
...
> arch/arm64/kernel/fpsimd.c:713:2: note: in expansion of macro âwrite_sysregâ
> write_sysreg(read_sysreg(CPACR_EL1) | CPACR_EL1_ZEN_EL1EN, CPACR_EL1);
> ^~~~~~~~~~~~

Easily fixed by using different named temporaries in the two macros.
There probably aren't that many macro pairs where that happens.
Especially since many are now inlined functions.

It might be that a small number of changes get rid of most of the warnings.

David

-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)