netdev - Re: [PATCH net-next] mdio, phy: fix -Wshadow warnings triggered by nested container

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <YAb4/7Nb1qaGiS0f@lunn.ch>
Date:   Tue, 19 Jan 2021 16:21:35 +0100
From:   Andrew Lunn <andrew@...n.ch>
To:     Alexander Lobakin <alobakin@...me>
Cc:     "David S. Miller" <davem@...emloft.net>,
        Jakub Kicinski <kuba@...nel.org>,
        Heiner Kallweit <hkallweit1@...il.com>,
        Russell King <linux@...linux.org.uk>, netdev@...r.kernel.org,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH net-next] mdio, phy: fix -Wshadow warnings triggered by
 nested container_of()

On Sat, Jan 16, 2021 at 04:13:22PM +0000, Alexander Lobakin wrote:
> container_of() macro hides a local variable '__mptr' inside. This
> becomes a problem when several container_of() are nested in each
> other within single line or plain macros.
> As C preprocessor doesn't support generating random variable names,
> the sole solution is to avoid defining macros that consist only of
> container_of() calls, or they will self-shadow '__mptr' each time:
> 
> In file included from ./include/linux/bitmap.h:10,
>                  from drivers/net/phy/phy_device.c:12:
> drivers/net/phy/phy_device.c: In function ‘phy_device_release’:
> ./include/linux/kernel.h:693:8: warning: declaration of ‘__mptr’ shadows a previous local [-Wshadow]
>   693 |  void *__mptr = (void *)(ptr);     \
>       |        ^~~~~~
> ./include/linux/phy.h:647:26: note: in expansion of macro ‘container_of’
>   647 | #define to_phy_device(d) container_of(to_mdio_device(d), \
>       |                          ^~~~~~~~~~~~
> ./include/linux/mdio.h:52:27: note: in expansion of macro ‘container_of’
>    52 | #define to_mdio_device(d) container_of(d, struct mdio_device, dev)
>       |                           ^~~~~~~~~~~~
> ./include/linux/phy.h:647:39: note: in expansion of macro ‘to_mdio_device’
>   647 | #define to_phy_device(d) container_of(to_mdio_device(d), \
>       |                                       ^~~~~~~~~~~~~~
> drivers/net/phy/phy_device.c:217:8: note: in expansion of macro ‘to_phy_device’
>   217 |  kfree(to_phy_device(dev));
>       |        ^~~~~~~~~~~~~
> ./include/linux/kernel.h:693:8: note: shadowed declaration is here
>   693 |  void *__mptr = (void *)(ptr);     \
>       |        ^~~~~~
> ./include/linux/phy.h:647:26: note: in expansion of macro ‘container_of’
>   647 | #define to_phy_device(d) container_of(to_mdio_device(d), \
>       |                          ^~~~~~~~~~~~
> drivers/net/phy/phy_device.c:217:8: note: in expansion of macro ‘to_phy_device’
>   217 |  kfree(to_phy_device(dev));
>       |        ^~~~~~~~~~~~~
> 
> As they are declared in header files, these warnings are highly
> repetitive and very annoying (along with the one from linux/pci.h).
> 
> Convert the related macros from linux/{mdio,phy}.h to static inlines
> to avoid self-shadowing and potentially improve bug-catching.
> No functional changes implied.
> 
> Signed-off-by: Alexander Lobakin <alobakin@...me>

Reviewed-by: Andrew Lunn <andrew@...n.ch>

    Andrew