Re: [PATCH net-next 1/1] drivers/phylib: fix coverity issue

[Jacob Keller]

On 1/17/2023 1:47 PM, Piergiorgio Beruto wrote:
> Coverity reported the following:
> 
> *** CID 1530573:    (UNINIT)
> drivers/net/phy/phy-c45.c:1036 in genphy_c45_plca_set_cfg()
> 1030     				return ret;
> 1031
> 1032     			val = ret;
> 1033     		}
> 1034
> 1035     		if (plca_cfg->node_cnt >= 0)
> vvv     CID 1530573:    (UNINIT)
> vvv     Using uninitialized value "val".
> 1036     			val = (val & ~MDIO_OATC14_PLCA_NCNT) |
> 1037     			      (plca_cfg->node_cnt << 8);
> 1038
> 1039     		if (plca_cfg->node_id >= 0)
> 1040     			val = (val & ~MDIO_OATC14_PLCA_ID) |
> 1041     			      (plca_cfg->node_id);
> drivers/net/phy/phy-c45.c:1076 in genphy_c45_plca_set_cfg()
> 1070     				return ret;
> 1071
> 1072     			val = ret;
> 1073     		}
> 1074
> 1075     		if (plca_cfg->burst_cnt >= 0)
> vvv     CID 1530573:    (UNINIT)
> vvv     Using uninitialized value "val".
> 1076     			val = (val & ~MDIO_OATC14_PLCA_MAXBC) |
> 1077     			      (plca_cfg->burst_cnt << 8);
> 1078
> 1079     		if (plca_cfg->burst_tmr >= 0)
> 1080     			val = (val & ~MDIO_OATC14_PLCA_BTMR) |
> 1081     			      (plca_cfg->burst_tmr);
> 
> This is not actually creating a real problem because the path leading to
> 'val' being used uninitialized will eventually override the full content
> of that variable before actually using it for writing the register.
> However, the fix is simple and comes at basically no cost.
> 

Makes sense, and its better to be clear, and prevent the introduction of
a bug later if somehow it refactored such that the value is not
re-initialized before use in that case.

Reviewed-by: Jacob Keller <jacob.e.keller@xxxxxxxxx>

> Signed-off-by: Piergiorgio Beruto <piergiorgio.beruto@xxxxxxxxx>
> Reported-by: coverity-bot <keescook+coverity-bot@xxxxxxxxxxxx>
> Addresses-Coverity-ID: 1530573 ("UNINIT")
> Fixes: 493323416fed ("drivers/net/phy: add helpers to get/set PLCA configuration")
> ---
>  drivers/net/phy/phy-c45.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/net/phy/phy-c45.c b/drivers/net/phy/phy-c45.c
> index cff83220595c..9f9565a4819d 100644
> --- a/drivers/net/phy/phy-c45.c
> +++ b/drivers/net/phy/phy-c45.c
> @@ -999,8 +999,8 @@ EXPORT_SYMBOL_GPL(genphy_c45_plca_get_cfg);
>  int genphy_c45_plca_set_cfg(struct phy_device *phydev,
>  			    const struct phy_plca_cfg *plca_cfg)
>  {
> +	u16 val = 0;
>  	int ret;
> -	u16 val;
>  
>  	// PLCA IDVER is read-only
>  	if (plca_cfg->version >= 0)