The function efx_devlink_info_board_cfg() calls the function
devlink_info_serial_number_put(), but does not check its return
value.

Return the error code if either the devlink_info_serial_number_put()
or the efx_mcdi_get_board_cfg() fails.The control flow of the code is
changed a little bit to simplify the code. The functionality of the
code remain the same.

Fixes: 14743ddd2495 ("sfc: add devlink info support for ef100")
Cc: stable@vger.kernel.org # v6.3+
Signed-off-by: Wentao Liang <vulab@iscas.ac.cn>
---
v2: Simplify code logic.

 drivers/net/ethernet/sfc/efx_devlink.c | 11 ++++++-----
 1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/drivers/net/ethernet/sfc/efx_devlink.c b/drivers/net/ethernet/sfc/efx_devlink.c
index 3cd750820fdd..53b17cd252c8 100644
--- a/drivers/net/ethernet/sfc/efx_devlink.c
+++ b/drivers/net/ethernet/sfc/efx_devlink.c
@@ -584,11 +584,12 @@ static int efx_devlink_info_board_cfg(struct efx_nic *efx,
 	int rc;
 
 	rc = efx_mcdi_get_board_cfg(efx, (u8 *)mac_address, NULL, NULL);
-	if (!rc) {
-		snprintf(sn, EFX_MAX_SERIALNUM_LEN, "%pm", mac_address);
-		devlink_info_serial_number_put(req, sn);
-	}
-	return rc;
+	if (rc)
+		return rc;
+
+	snprintf(sn, EFX_MAX_SERIALNUM_LEN, "%pm", mac_address);
+
+	return devlink_info_serial_number_put(req, sn);
 }
 
 static int efx_devlink_info_get(struct devlink *devlink,
-- 
2.42.0.windows.2

+ Alejandro

On Mon, Apr 07, 2025 at 09:11:10PM +0800, Wentao Liang wrote:
> The function efx_devlink_info_board_cfg() calls the function
> devlink_info_serial_number_put(), but does not check its return
> value.
> 
> Return the error code if either the devlink_info_serial_number_put()
> or the efx_mcdi_get_board_cfg() fails.The control flow of the code is
> changed a little bit to simplify the code. The functionality of the
> code remain the same.
> 
> Fixes: 14743ddd2495 ("sfc: add devlink info support for ef100")
> Cc: stable@vger.kernel.org # v6.3+
> Signed-off-by: Wentao Liang <vulab@iscas.ac.cn>

Hi Wentao,

In his review of v1 Edward Cree said:

  "Looking at the rest of the file, all the calls to
    devlink_info_*_put() in this driver ignore the return value, not
    just this one.  I think this may have been an intentional decision
    to only report errors in getting the info from FW, which seems
    reasonable to me.
  "If not, then all the calls need fixing, not just this one.
    CCing Alejandro, original author of this code, for his opinion.

I have CCed Aljandro on this email to see if he can help.
And in any case, I think we need to come to a consensus on
Ed's point before moving forwards with this patch.

-- 
pw-bot: changes-requested