| Message ID | 20210116161246.67075-1-alobakin@pm.me (mailing list archive) |
|---|---|
| State | Accepted |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | [net-next] mdio, phy: fix -Wshadow warnings triggered by nested container_of() | expand |
| Context | Check | Description |
|---|---|---|
| netdev/cover_letter | success | Link |
| netdev/fixes_present | success | Link |
| netdev/patch_count | success | Link |
| netdev/tree_selection | success | Clearly marked for net-next |
| netdev/subject_prefix | success | Link |
| netdev/cc_maintainers | success | CCed 4 of 4 maintainers |
| netdev/source_inline | success | Was 0 now: 0 |
| netdev/verify_signedoff | success | Link |
| netdev/module_param | success | Was 0 now: 0 |
| netdev/build_32bit | success | Errors and warnings before: 845 this patch: 845 |
| netdev/kdoc | success | Errors and warnings before: 0 this patch: 0 |
| netdev/verify_fixes | success | Link |
| netdev/checkpatch | success | total: 0 errors, 0 warnings, 0 checks, 54 lines checked |
| netdev/build_allmodconfig_warn | success | Errors and warnings before: 745 this patch: 745 |
| netdev/header_inline | success | Link |
| netdev/stable | success | Stable not CCed |
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@pm.me> Reviewed-by: Andrew Lunn <andrew@lunn.ch> 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 <alobakin@pm.me> > > Reviewed-by: Andrew Lunn <andrew@lunn.ch> Applied, thanks!
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
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@pm.me> --- include/linux/mdio.h | 23 ++++++++++++++++++----- include/linux/phy.h | 7 +++++-- 2 files changed, 23 insertions(+), 7 deletions(-)