* [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; 8+ 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] 8+ 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; 8+ 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] 8+ 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; 8+ 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] 8+ 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; 8+ 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] 8+ 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; 8+ 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] 8+ 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
2024-11-14 17:11 ` [PATCH v2 2/2] net/e1000: fix incorrect fallthrough in switch Stephen Hemminger
2 siblings, 2 replies; 8+ 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] 8+ 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
1 sibling, 0 replies; 8+ 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] 8+ 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
1 sibling, 0 replies; 8+ 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] 8+ messages in thread
end of thread, other threads:[~2024-11-14 17:14 UTC | newest]
Thread overview: 8+ 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
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).