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 <[email protected]>
---
include/linux/mdio.h | 23 ++++++++++++++++++-----
include/linux/phy.h | 7 +++++--
2 files changed, 23 insertions(+), 7 deletions(-)
diff --git a/include/linux/mdio.h b/include/linux/mdio.h
index dbd69b3d170b..ffb787d5ebde 100644
--- a/include/linux/mdio.h
+++ b/include/linux/mdio.h
@@ -49,7 +49,11 @@ struct mdio_device {
unsigned int reset_assert_delay;
unsigned int reset_deassert_delay;
};
-#define to_mdio_device(d) container_of(d, struct mdio_device, dev)
+
+static inline struct mdio_device *to_mdio_device(const struct device *dev)
+{
+ return container_of(dev, struct mdio_device, dev);
+}
/* struct mdio_driver_common: Common to all MDIO drivers */
struct mdio_driver_common {
@@ -57,8 +61,12 @@ struct mdio_driver_common {
int flags;
};
#define MDIO_DEVICE_FLAG_PHY 1
-#define to_mdio_common_driver(d) \
- container_of(d, struct mdio_driver_common, driver)
+
+static inline struct mdio_driver_common *
+to_mdio_common_driver(const struct device_driver *driver)
+{
+ return container_of(driver, struct mdio_driver_common, driver);
+}
/* struct mdio_driver: Generic MDIO driver */
struct mdio_driver {
@@ -73,8 +81,13 @@ struct mdio_driver {
/* Clears up any memory if needed */
void (*remove)(struct mdio_device *mdiodev);
};
-#define to_mdio_driver(d) \
- container_of(to_mdio_common_driver(d), struct mdio_driver, mdiodrv)
+
+static inline struct mdio_driver *
+to_mdio_driver(const struct device_driver *driver)
+{
+ return container_of(to_mdio_common_driver(driver), struct mdio_driver,
+ mdiodrv);
+}
/* device driver data */
static inline void mdiodev_set_drvdata(struct mdio_device *mdio, void *data)
diff --git a/include/linux/phy.h b/include/linux/phy.h
index 24fcc6456a9e..bc323fbdd21e 100644
--- a/include/linux/phy.h
+++ b/include/linux/phy.h
@@ -648,8 +648,11 @@ struct phy_device {
const struct macsec_ops *macsec_ops;
#endif
};
-#define to_phy_device(d) container_of(to_mdio_device(d), \
- struct phy_device, mdio)
+
+static inline struct phy_device *to_phy_device(const struct device *dev)
+{
+ return container_of(to_mdio_device(dev), struct phy_device, mdio);
+}
/**
* struct phy_tdr_config - Configuration of a TDR raw test
--
2.30.0
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 <[email protected]>
Reviewed-by: Andrew Lunn <[email protected]>
Andrew
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 <[email protected]>
>
> Reviewed-by: Andrew Lunn <[email protected]>
Applied, thanks!