Re: [PATCH net-next] mdio, phy: fix -Wshadow warnings triggered by nested container_of()

From: Jakub Kicinski
Date: Tue Jan 19 2021 - 15:02:31 EST


On Tue, 19 Jan 2021 16:21:35 +0100 Andrew Lunn wrote:
> 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@xxxxx>
>
> Reviewed-by: Andrew Lunn <andrew@xxxxxxx>

Applied, thanks!