Re: [PATCH 1/2] net/e1000: fix incorrect fallthrough in switch

[Bruce Richardson <bruce.richardson@intel.com>]

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
>