* [PATCH 0/2] fallthrough related handling
@ 2024-11-14 0:36 Stephen Hemminger
2024-11-14 0:36 ` [PATCH 1/2] net/e1000: fix incorrect fallthrough in switch Stephen Hemminger
` (2 more replies)
0 siblings, 3 replies; 9+ messages in thread
From: Stephen Hemminger @ 2024-11-14 0:36 UTC (permalink / raw)
To: dev; +Cc: Stephen Hemminger
Fix lack of fallthrough checking and bug in e1000 driver.
Stephen Hemminger (2):
net/e1000: fix incorrect fallthrough in switch
common/dpaax: do not disable fallthrough warnings
drivers/common/dpaax/caamflib/rta/operation_cmd.h | 4 ----
drivers/net/e1000/base/e1000_82575.c | 3 +--
drivers/net/e1000/base/e1000_api.c | 2 +-
drivers/net/e1000/base/meson.build | 1 -
4 files changed, 2 insertions(+), 8 deletions(-)
--
2.45.2
^ permalink raw reply [flat|nested] 9+ messages in thread* [PATCH 1/2] net/e1000: fix incorrect fallthrough in switch 2024-11-14 0:36 [PATCH 0/2] fallthrough related handling Stephen Hemminger @ 2024-11-14 0:36 ` Stephen Hemminger 2024-11-14 9:03 ` Bruce Richardson 2024-11-14 0:36 ` [PATCH 2/2] common/dpaax: do not disable fallthrough warnings Stephen Hemminger 2024-11-14 17:11 ` [PATCH v2 0/2] re-enable implicit fallthrough checks Stephen Hemminger 2 siblings, 1 reply; 9+ messages in thread From: Stephen Hemminger @ 2024-11-14 0:36 UTC (permalink / raw) To: dev; +Cc: Stephen Hemminger, qiming.yang, Nir Efrati, Qi Zhang There is an incorrect fallthrough identified by PVS studio. Even though this is in base code it should be fixed, and the warning should be re-enabled to prevent future bugs. Link: https://pvs-studio.com/en/blog/posts/cpp/1183/ Fixes: f2553cb9eba6 ("net/e1000/base: add new I219 devices") Cc: qiming.yang@intel.com Signed-off-by: Stephen Hemminger <stephen@networkplumber.org> --- drivers/net/e1000/base/e1000_82575.c | 3 +-- drivers/net/e1000/base/e1000_api.c | 2 +- drivers/net/e1000/base/meson.build | 1 - 3 files changed, 2 insertions(+), 4 deletions(-) diff --git a/drivers/net/e1000/base/e1000_82575.c b/drivers/net/e1000/base/e1000_82575.c index 7c78649393..bcc695e8a1 100644 --- a/drivers/net/e1000/base/e1000_82575.c +++ b/drivers/net/e1000/base/e1000_82575.c @@ -1721,7 +1721,7 @@ STATIC s32 e1000_get_media_type_82575(struct e1000_hw *hw) dev_spec->sgmii_active = true; break; } - /* Fall through for I2C based SGMII */ + /* fallthrough */ case E1000_CTRL_EXT_LINK_MODE_PCIE_SERDES: /* read media type from SFP EEPROM */ ret_val = e1000_set_sfp_media_type_82575(hw); @@ -3585,4 +3585,3 @@ void e1000_i2c_bus_clear(struct e1000_hw *hw) /* Put the i2c bus back to default state */ e1000_i2c_stop(hw); } - diff --git a/drivers/net/e1000/base/e1000_api.c b/drivers/net/e1000/base/e1000_api.c index 0f6e5afa3b..61b3ead469 100644 --- a/drivers/net/e1000/base/e1000_api.c +++ b/drivers/net/e1000/base/e1000_api.c @@ -295,6 +295,7 @@ s32 e1000_set_mac_type(struct e1000_hw *hw) case E1000_DEV_ID_PCH_RPL_I219_LM23: case E1000_DEV_ID_PCH_RPL_I219_V23: mac->type = e1000_pch_tgp; + break; case E1000_DEV_ID_PCH_ADL_I219_LM17: case E1000_DEV_ID_PCH_ADL_I219_V17: case E1000_DEV_ID_PCH_RPL_I219_LM22: @@ -1368,4 +1369,3 @@ void e1000_shutdown_fiber_serdes_link(struct e1000_hw *hw) if (hw->mac.ops.shutdown_serdes) hw->mac.ops.shutdown_serdes(hw); } - diff --git a/drivers/net/e1000/base/meson.build b/drivers/net/e1000/base/meson.build index 6d6048488f..e73f3d6d55 100644 --- a/drivers/net/e1000/base/meson.build +++ b/drivers/net/e1000/base/meson.build @@ -24,7 +24,6 @@ sources = [ error_cflags = [ '-Wno-unused-parameter', - '-Wno-implicit-fallthrough', ] c_args = cflags foreach flag: error_cflags -- 2.45.2 ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 1/2] net/e1000: fix incorrect fallthrough in switch 2024-11-14 0:36 ` [PATCH 1/2] net/e1000: fix incorrect fallthrough in switch Stephen Hemminger @ 2024-11-14 9:03 ` Bruce Richardson 0 siblings, 0 replies; 9+ messages in thread From: Bruce Richardson @ 2024-11-14 9:03 UTC (permalink / raw) To: Stephen Hemminger; +Cc: dev, qiming.yang, Nir Efrati, Qi Zhang On Wed, Nov 13, 2024 at 04:36:13PM -0800, Stephen Hemminger wrote: > There is an incorrect fallthrough identified by PVS studio. > Even though this is in base code it should be fixed, and > the warning should be re-enabled to prevent future bugs. > > Link: https://pvs-studio.com/en/blog/posts/cpp/1183/ > > Fixes: f2553cb9eba6 ("net/e1000/base: add new I219 devices") > Cc: qiming.yang@intel.com > > Signed-off-by: Stephen Hemminger <stephen@networkplumber.org> Acked-by: Bruce Richardson <bruce.richardson@intel.com> Thanks. Two minor comments below. > --- > drivers/net/e1000/base/e1000_82575.c | 3 +-- > drivers/net/e1000/base/e1000_api.c | 2 +- > drivers/net/e1000/base/meson.build | 1 - > 3 files changed, 2 insertions(+), 4 deletions(-) > > diff --git a/drivers/net/e1000/base/e1000_82575.c b/drivers/net/e1000/base/e1000_82575.c > index 7c78649393..bcc695e8a1 100644 > --- a/drivers/net/e1000/base/e1000_82575.c > +++ b/drivers/net/e1000/base/e1000_82575.c > @@ -1721,7 +1721,7 @@ STATIC s32 e1000_get_media_type_82575(struct e1000_hw *hw) > dev_spec->sgmii_active = true; > break; > } > - /* Fall through for I2C based SGMII */ > + /* fallthrough */ Can we keep the old comment as well as the new? > case E1000_CTRL_EXT_LINK_MODE_PCIE_SERDES: > /* read media type from SFP EEPROM */ > ret_val = e1000_set_sfp_media_type_82575(hw); > @@ -3585,4 +3585,3 @@ void e1000_i2c_bus_clear(struct e1000_hw *hw) > /* Put the i2c bus back to default state */ > e1000_i2c_stop(hw); > } > - Unrelated whitespace change, and also below. No big deal though. > diff --git a/drivers/net/e1000/base/e1000_api.c b/drivers/net/e1000/base/e1000_api.c > index 0f6e5afa3b..61b3ead469 100644 > --- a/drivers/net/e1000/base/e1000_api.c > +++ b/drivers/net/e1000/base/e1000_api.c > @@ -295,6 +295,7 @@ s32 e1000_set_mac_type(struct e1000_hw *hw) > case E1000_DEV_ID_PCH_RPL_I219_LM23: > case E1000_DEV_ID_PCH_RPL_I219_V23: > mac->type = e1000_pch_tgp; > + break; > case E1000_DEV_ID_PCH_ADL_I219_LM17: > case E1000_DEV_ID_PCH_ADL_I219_V17: > case E1000_DEV_ID_PCH_RPL_I219_LM22: > @@ -1368,4 +1369,3 @@ void e1000_shutdown_fiber_serdes_link(struct e1000_hw *hw) > if (hw->mac.ops.shutdown_serdes) > hw->mac.ops.shutdown_serdes(hw); > } > - > diff --git a/drivers/net/e1000/base/meson.build b/drivers/net/e1000/base/meson.build > index 6d6048488f..e73f3d6d55 100644 > --- a/drivers/net/e1000/base/meson.build > +++ b/drivers/net/e1000/base/meson.build > @@ -24,7 +24,6 @@ sources = [ > > error_cflags = [ > '-Wno-unused-parameter', > - '-Wno-implicit-fallthrough', > ] > c_args = cflags > foreach flag: error_cflags > -- > 2.45.2 > ^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH 2/2] common/dpaax: do not disable fallthrough warnings 2024-11-14 0:36 [PATCH 0/2] fallthrough related handling Stephen Hemminger 2024-11-14 0:36 ` [PATCH 1/2] net/e1000: fix incorrect fallthrough in switch Stephen Hemminger @ 2024-11-14 0:36 ` Stephen Hemminger 2024-11-14 6:06 ` Hemant Agrawal 2024-11-14 17:11 ` [PATCH v2 0/2] re-enable implicit fallthrough checks Stephen Hemminger 2 siblings, 1 reply; 9+ messages in thread From: Stephen Hemminger @ 2024-11-14 0:36 UTC (permalink / raw) To: dev; +Cc: Stephen Hemminger, thomas, Hemant Agrawal, Sachin Saxena Fallthrough warnings catch real bugs and should not be disabled. There are warnings in this driver in current build. The commit that added the disable is old, and the problematic code appears to have been already removed. Fixes: 2ab9a9483196 ("crypto/dpaa2_sec: fix build with GCC 7") Cc: thomas@monjalon.net Signed-off-by: Stephen Hemminger <stephen@networkplumber.org> --- drivers/common/dpaax/caamflib/rta/operation_cmd.h | 4 ---- 1 file changed, 4 deletions(-) diff --git a/drivers/common/dpaax/caamflib/rta/operation_cmd.h b/drivers/common/dpaax/caamflib/rta/operation_cmd.h index fe1ac37ee8..563735eb88 100644 --- a/drivers/common/dpaax/caamflib/rta/operation_cmd.h +++ b/drivers/common/dpaax/caamflib/rta/operation_cmd.h @@ -7,10 +7,6 @@ #ifndef __RTA_OPERATION_CMD_H__ #define __RTA_OPERATION_CMD_H__ -#if defined(RTE_TOOLCHAIN_GCC) && (GCC_VERSION >= 70000) -#pragma GCC diagnostic ignored "-Wimplicit-fallthrough" -#endif - extern enum rta_sec_era rta_sec_era; static inline int -- 2.45.2 ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 2/2] common/dpaax: do not disable fallthrough warnings 2024-11-14 0:36 ` [PATCH 2/2] common/dpaax: do not disable fallthrough warnings Stephen Hemminger @ 2024-11-14 6:06 ` Hemant Agrawal 0 siblings, 0 replies; 9+ messages in thread From: Hemant Agrawal @ 2024-11-14 6:06 UTC (permalink / raw) To: Stephen Hemminger, dev; +Cc: thomas, Hemant Agrawal, Sachin Saxena On 14-11-2024 06:06, Stephen Hemminger wrote: > Fallthrough warnings catch real bugs and should not be disabled. > There are warnings in this driver in current build. > > The commit that added the disable is old, and the problematic code > appears to have been already removed. > > Fixes: 2ab9a9483196 ("crypto/dpaa2_sec: fix build with GCC 7") > Cc: thomas@monjalon.net > > Signed-off-by: Stephen Hemminger <stephen@networkplumber.org> > --- > drivers/common/dpaax/caamflib/rta/operation_cmd.h | 4 ---- > 1 file changed, 4 deletions(-) > > diff --git a/drivers/common/dpaax/caamflib/rta/operation_cmd.h b/drivers/common/dpaax/caamflib/rta/operation_cmd.h > index fe1ac37ee8..563735eb88 100644 > --- a/drivers/common/dpaax/caamflib/rta/operation_cmd.h > +++ b/drivers/common/dpaax/caamflib/rta/operation_cmd.h > @@ -7,10 +7,6 @@ > #ifndef __RTA_OPERATION_CMD_H__ > #define __RTA_OPERATION_CMD_H__ > > -#if defined(RTE_TOOLCHAIN_GCC) && (GCC_VERSION >= 70000) > -#pragma GCC diagnostic ignored "-Wimplicit-fallthrough" > -#endif > - > extern enum rta_sec_era rta_sec_era; > > static inline int Acked-by: Hemant Agrawal <hemant.agrawal@nxp.com> ^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH v2 0/2] re-enable implicit fallthrough checks 2024-11-14 0:36 [PATCH 0/2] fallthrough related handling Stephen Hemminger 2024-11-14 0:36 ` [PATCH 1/2] net/e1000: fix incorrect fallthrough in switch Stephen Hemminger 2024-11-14 0:36 ` [PATCH 2/2] common/dpaax: do not disable fallthrough warnings Stephen Hemminger @ 2024-11-14 17:11 ` Stephen Hemminger 2024-11-14 17:11 ` [PATCH v2 1/2] common/dpaax: do not disable fallthrough warnings Stephen Hemminger ` (2 more replies) 2 siblings, 3 replies; 9+ messages in thread From: Stephen Hemminger @ 2024-11-14 17:11 UTC (permalink / raw) To: dev; +Cc: Stephen Hemminger Fix bug detected from bad fallthroug in E1000 code. And re-enable compiler flag in both e1000 and dpaax v2 - minor comment fix Stephen Hemminger (2): common/dpaax: do not disable fallthrough warnings net/e1000: fix incorrect fallthrough in switch drivers/common/dpaax/caamflib/rta/operation_cmd.h | 4 ---- drivers/net/e1000/base/e1000_82575.c | 2 +- drivers/net/e1000/base/e1000_api.c | 2 +- drivers/net/e1000/base/meson.build | 1 - 4 files changed, 2 insertions(+), 7 deletions(-) -- 2.45.2 ^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH v2 1/2] common/dpaax: do not disable fallthrough warnings 2024-11-14 17:11 ` [PATCH v2 0/2] re-enable implicit fallthrough checks Stephen Hemminger @ 2024-11-14 17:11 ` Stephen Hemminger 2024-11-14 17:11 ` [PATCH v2 2/2] net/e1000: fix incorrect fallthrough in switch Stephen Hemminger 2024-11-19 17:00 ` [PATCH v2 0/2] re-enable implicit fallthrough checks Thomas Monjalon 2 siblings, 0 replies; 9+ messages in thread From: Stephen Hemminger @ 2024-11-14 17:11 UTC (permalink / raw) To: dev; +Cc: Stephen Hemminger, thomas, Hemant Agrawal, Sachin Saxena Fallthrough warnings catch real bugs and should not be disabled. There are warnings in this driver in current build. The commit that added the disable is old, and the problematic code appears to have been already removed. Fixes: 2ab9a9483196 ("crypto/dpaa2_sec: fix build with GCC 7") Cc: thomas@monjalon.net Signed-off-by: Stephen Hemminger <stephen@networkplumber.org> Acked-by: Hemant Agrawal <hemant.agrawal@nxp.com> --- drivers/common/dpaax/caamflib/rta/operation_cmd.h | 4 ---- 1 file changed, 4 deletions(-) diff --git a/drivers/common/dpaax/caamflib/rta/operation_cmd.h b/drivers/common/dpaax/caamflib/rta/operation_cmd.h index fe1ac37ee8..563735eb88 100644 --- a/drivers/common/dpaax/caamflib/rta/operation_cmd.h +++ b/drivers/common/dpaax/caamflib/rta/operation_cmd.h @@ -7,10 +7,6 @@ #ifndef __RTA_OPERATION_CMD_H__ #define __RTA_OPERATION_CMD_H__ -#if defined(RTE_TOOLCHAIN_GCC) && (GCC_VERSION >= 70000) -#pragma GCC diagnostic ignored "-Wimplicit-fallthrough" -#endif - extern enum rta_sec_era rta_sec_era; static inline int -- 2.45.2 ^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH v2 2/2] net/e1000: fix incorrect fallthrough in switch 2024-11-14 17:11 ` [PATCH v2 0/2] re-enable implicit fallthrough checks Stephen Hemminger 2024-11-14 17:11 ` [PATCH v2 1/2] common/dpaax: do not disable fallthrough warnings Stephen Hemminger @ 2024-11-14 17:11 ` Stephen Hemminger 2024-11-19 17:00 ` [PATCH v2 0/2] re-enable implicit fallthrough checks Thomas Monjalon 2 siblings, 0 replies; 9+ messages in thread From: Stephen Hemminger @ 2024-11-14 17:11 UTC (permalink / raw) To: dev Cc: Stephen Hemminger, qiming.yang, Bruce Richardson, Qi Zhang, Nir Efrati There is an incorrect fallthrough identified by PVS studio. Even though this is in base code it should be fixed, and the warning should be re-enabled to prevent future bugs. Link: https://pvs-studio.com/en/blog/posts/cpp/1183/ Fixes: f2553cb9eba6 ("net/e1000/base: add new I219 devices") Cc: qiming.yang@intel.com Signed-off-by: Stephen Hemminger <stephen@networkplumber.org> Acked-by: Bruce Richardson <bruce.richardson@intel.com> --- drivers/net/e1000/base/e1000_82575.c | 2 +- drivers/net/e1000/base/e1000_api.c | 2 +- drivers/net/e1000/base/meson.build | 1 - 3 files changed, 2 insertions(+), 3 deletions(-) diff --git a/drivers/net/e1000/base/e1000_82575.c b/drivers/net/e1000/base/e1000_82575.c index 7c78649393..ccc7aeab21 100644 --- a/drivers/net/e1000/base/e1000_82575.c +++ b/drivers/net/e1000/base/e1000_82575.c @@ -1722,6 +1722,7 @@ STATIC s32 e1000_get_media_type_82575(struct e1000_hw *hw) break; } /* Fall through for I2C based SGMII */ + /* Fall through */ case E1000_CTRL_EXT_LINK_MODE_PCIE_SERDES: /* read media type from SFP EEPROM */ ret_val = e1000_set_sfp_media_type_82575(hw); @@ -3585,4 +3586,3 @@ void e1000_i2c_bus_clear(struct e1000_hw *hw) /* Put the i2c bus back to default state */ e1000_i2c_stop(hw); } - diff --git a/drivers/net/e1000/base/e1000_api.c b/drivers/net/e1000/base/e1000_api.c index 0f6e5afa3b..61b3ead469 100644 --- a/drivers/net/e1000/base/e1000_api.c +++ b/drivers/net/e1000/base/e1000_api.c @@ -295,6 +295,7 @@ s32 e1000_set_mac_type(struct e1000_hw *hw) case E1000_DEV_ID_PCH_RPL_I219_LM23: case E1000_DEV_ID_PCH_RPL_I219_V23: mac->type = e1000_pch_tgp; + break; case E1000_DEV_ID_PCH_ADL_I219_LM17: case E1000_DEV_ID_PCH_ADL_I219_V17: case E1000_DEV_ID_PCH_RPL_I219_LM22: @@ -1368,4 +1369,3 @@ void e1000_shutdown_fiber_serdes_link(struct e1000_hw *hw) if (hw->mac.ops.shutdown_serdes) hw->mac.ops.shutdown_serdes(hw); } - diff --git a/drivers/net/e1000/base/meson.build b/drivers/net/e1000/base/meson.build index 6d6048488f..e73f3d6d55 100644 --- a/drivers/net/e1000/base/meson.build +++ b/drivers/net/e1000/base/meson.build @@ -24,7 +24,6 @@ sources = [ error_cflags = [ '-Wno-unused-parameter', - '-Wno-implicit-fallthrough', ] c_args = cflags foreach flag: error_cflags -- 2.45.2 ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v2 0/2] re-enable implicit fallthrough checks 2024-11-14 17:11 ` [PATCH v2 0/2] re-enable implicit fallthrough checks Stephen Hemminger 2024-11-14 17:11 ` [PATCH v2 1/2] common/dpaax: do not disable fallthrough warnings Stephen Hemminger 2024-11-14 17:11 ` [PATCH v2 2/2] net/e1000: fix incorrect fallthrough in switch Stephen Hemminger @ 2024-11-19 17:00 ` Thomas Monjalon 2 siblings, 0 replies; 9+ messages in thread From: Thomas Monjalon @ 2024-11-19 17:00 UTC (permalink / raw) To: dev; +Cc: Stephen Hemminger, Stephen Hemminger 14/11/2024 18:11, Stephen Hemminger: > Fix bug detected from bad fallthroug in E1000 code. > And re-enable compiler flag in both e1000 and dpaax > > v2 - minor comment fix > > Stephen Hemminger (2): > common/dpaax: do not disable fallthrough warnings > net/e1000: fix incorrect fallthrough in switch Applied, thanks. ^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2024-11-19 17:00 UTC | newest] Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2024-11-14 0:36 [PATCH 0/2] fallthrough related handling Stephen Hemminger 2024-11-14 0:36 ` [PATCH 1/2] net/e1000: fix incorrect fallthrough in switch Stephen Hemminger 2024-11-14 9:03 ` Bruce Richardson 2024-11-14 0:36 ` [PATCH 2/2] common/dpaax: do not disable fallthrough warnings Stephen Hemminger 2024-11-14 6:06 ` Hemant Agrawal 2024-11-14 17:11 ` [PATCH v2 0/2] re-enable implicit fallthrough checks Stephen Hemminger 2024-11-14 17:11 ` [PATCH v2 1/2] common/dpaax: do not disable fallthrough warnings Stephen Hemminger 2024-11-14 17:11 ` [PATCH v2 2/2] net/e1000: fix incorrect fallthrough in switch Stephen Hemminger 2024-11-19 17:00 ` [PATCH v2 0/2] re-enable implicit fallthrough checks Thomas Monjalon
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).