| Message ID | 20220108155003.3991055-1-trix@redhat.com (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | net: ethernet: mtk_eth_soc: fix error checking in mtk_mac_config() | expand |
On Sat, 8 Jan 2022 07:50:03 -0800 trix@redhat.com wrote: > From: Tom Rix <trix@redhat.com> > > Clang static analysis reports this problem > mtk_eth_soc.c:394:7: warning: Branch condition evaluates > to a garbage value > if (err) > ^~~ > > err is not initialized and only conditionally set. > Check err consistently with the rest of mtk_mac_config(), > after even possible setting. > > Fixes: 7e538372694b ("net: ethernet: mediatek: Re-add support SGMII") > Signed-off-by: Tom Rix <trix@redhat.com> > --- > drivers/net/ethernet/mediatek/mtk_eth_soc.c | 12 +++++++----- > 1 file changed, 7 insertions(+), 5 deletions(-) > > diff --git a/drivers/net/ethernet/mediatek/mtk_eth_soc.c b/drivers/net/ethernet/mediatek/mtk_eth_soc.c > index b67b4323cff08..a27e548488584 100644 > --- a/drivers/net/ethernet/mediatek/mtk_eth_soc.c > +++ b/drivers/net/ethernet/mediatek/mtk_eth_soc.c > @@ -385,14 +385,16 @@ static void mtk_mac_config(struct phylink_config *config, unsigned int mode, > 0 : mac->id; > > /* Setup SGMIISYS with the determined property */ > - if (state->interface != PHY_INTERFACE_MODE_SGMII) > + if (state->interface != PHY_INTERFACE_MODE_SGMII) { > err = mtk_sgmii_setup_mode_force(eth->sgmii, sid, > state); > - else if (phylink_autoneg_inband(mode)) > + if (err) > + goto init_err; > + } else if (phylink_autoneg_inband(mode)) { > err = mtk_sgmii_setup_mode_an(eth->sgmii, sid); > - > - if (err) > - goto init_err; > + if (err) > + goto init_err; > + } > > regmap_update_bits(eth->ethsys, ETHSYS_SYSCFG0, > SYSCFG0_SGMII_MASK, val); Why not init err to 0 before the if or add an else err = 0; branch?
On 1/9/22 4:43 PM, Jakub Kicinski wrote: > On Sat, 8 Jan 2022 07:50:03 -0800 trix@redhat.com wrote: >> From: Tom Rix <trix@redhat.com> >> >> Clang static analysis reports this problem >> mtk_eth_soc.c:394:7: warning: Branch condition evaluates >> to a garbage value >> if (err) >> ^~~ >> >> err is not initialized and only conditionally set. >> Check err consistently with the rest of mtk_mac_config(), >> after even possible setting. >> >> Fixes: 7e538372694b ("net: ethernet: mediatek: Re-add support SGMII") >> Signed-off-by: Tom Rix <trix@redhat.com> >> --- >> drivers/net/ethernet/mediatek/mtk_eth_soc.c | 12 +++++++----- >> 1 file changed, 7 insertions(+), 5 deletions(-) >> >> diff --git a/drivers/net/ethernet/mediatek/mtk_eth_soc.c b/drivers/net/ethernet/mediatek/mtk_eth_soc.c >> index b67b4323cff08..a27e548488584 100644 >> --- a/drivers/net/ethernet/mediatek/mtk_eth_soc.c >> +++ b/drivers/net/ethernet/mediatek/mtk_eth_soc.c >> @@ -385,14 +385,16 @@ static void mtk_mac_config(struct phylink_config *config, unsigned int mode, >> 0 : mac->id; >> >> /* Setup SGMIISYS with the determined property */ >> - if (state->interface != PHY_INTERFACE_MODE_SGMII) >> + if (state->interface != PHY_INTERFACE_MODE_SGMII) { >> err = mtk_sgmii_setup_mode_force(eth->sgmii, sid, >> state); >> - else if (phylink_autoneg_inband(mode)) >> + if (err) >> + goto init_err; >> + } else if (phylink_autoneg_inband(mode)) { >> err = mtk_sgmii_setup_mode_an(eth->sgmii, sid); >> - >> - if (err) >> - goto init_err; >> + if (err) >> + goto init_err; >> + } >> >> regmap_update_bits(eth->ethsys, ETHSYS_SYSCFG0, >> SYSCFG0_SGMII_MASK, val); > Why not init err to 0 before the if or add an else err = 0; branch? This is the way I would have preferred to do it but the function's existing error handling does it this way. I'll respin the patch. Tom >
diff --git a/drivers/net/ethernet/mediatek/mtk_eth_soc.c b/drivers/net/ethernet/mediatek/mtk_eth_soc.c index b67b4323cff08..a27e548488584 100644 --- a/drivers/net/ethernet/mediatek/mtk_eth_soc.c +++ b/drivers/net/ethernet/mediatek/mtk_eth_soc.c @@ -385,14 +385,16 @@ static void mtk_mac_config(struct phylink_config *config, unsigned int mode, 0 : mac->id; /* Setup SGMIISYS with the determined property */ - if (state->interface != PHY_INTERFACE_MODE_SGMII) + if (state->interface != PHY_INTERFACE_MODE_SGMII) { err = mtk_sgmii_setup_mode_force(eth->sgmii, sid, state); - else if (phylink_autoneg_inband(mode)) + if (err) + goto init_err; + } else if (phylink_autoneg_inband(mode)) { err = mtk_sgmii_setup_mode_an(eth->sgmii, sid); - - if (err) - goto init_err; + if (err) + goto init_err; + } regmap_update_bits(eth->ethsys, ETHSYS_SYSCFG0, SYSCFG0_SGMII_MASK, val);