Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Date:   Tue, 19 Jan 2021 11:48:46 -0800
From:   Jakub Kicinski <kuba@kernel.org>
To:     Andrew Lunn <andrew@lunn.ch>, Alexander Lobakin <alobakin@pm.me>
Cc:     "David S. Miller" <davem@davemloft.net>,
        Heiner Kallweit <hkallweit1@gmail.com>,
        Russell King <linux@armlinux.org.uk>, netdev@vger.kernel.org,
        linux-kernel@vger.kernel.org
Subject: Re: [PATCH net-next] mdio, phy: fix -Wshadow warnings triggered by
 nested container_of()
Message-ID: <20210119114846.20b844b2@kicinski-fedora-pc1c0hjn.dhcp.thefacebook.com>
In-Reply-To: <YAb4/7Nb1qaGiS0f@lunn.ch>
References: <20210116161246.67075-1-alobakin@pm.me>
        <YAb4/7Nb1qaGiS0f@lunn.ch>
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: quoted-printable
Precedence: bulk

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:
> >=20
> > 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 =E2=80=98phy_device_release=
=E2=80=99:
> > ./include/linux/kernel.h:693:8: warning: declaration of =E2=80=98__mptr=
=E2=80=99 shadows a previous local [-Wshadow]
> >   693 |  void *__mptr =3D (void *)(ptr);     \
> >       |        ^~~~~~
> > ./include/linux/phy.h:647:26: note: in expansion of macro =E2=80=98cont=
ainer_of=E2=80=99
> >   647 | #define to_phy_device(d) container_of(to_mdio_device(d), \
> >       |                          ^~~~~~~~~~~~
> > ./include/linux/mdio.h:52:27: note: in expansion of macro =E2=80=98cont=
ainer_of=E2=80=99
> >    52 | #define to_mdio_device(d) container_of(d, struct mdio_device, d=
ev)
> >       |                           ^~~~~~~~~~~~
> > ./include/linux/phy.h:647:39: note: in expansion of macro =E2=80=98to_m=
dio_device=E2=80=99
> >   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 =E2=80=
=98to_phy_device=E2=80=99
> >   217 |  kfree(to_phy_device(dev));
> >       |        ^~~~~~~~~~~~~
> > ./include/linux/kernel.h:693:8: note: shadowed declaration is here
> >   693 |  void *__mptr =3D (void *)(ptr);     \
> >       |        ^~~~~~
> > ./include/linux/phy.h:647:26: note: in expansion of macro =E2=80=98cont=
ainer_of=E2=80=99
> >   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 =E2=80=
=98to_phy_device=E2=80=99
> >   217 |  kfree(to_phy_device(dev));
> >       |        ^~~~~~~~~~~~~
> >=20
> > As they are declared in header files, these warnings are highly
> > repetitive and very annoying (along with the one from linux/pci.h).
> >=20
> > 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.
> >=20
> > Signed-off-by: Alexander Lobakin <alobakin@pm.me> =20
>=20
> Reviewed-by: Andrew Lunn <andrew@lunn.ch>

Applied, thanks!