2018-11-24 10:21:41

by Yue Haibing

Subject: [PATCH net-next] net: aquantia: return 'err' if set MPI_DEINIT state fails

Fixes gcc '-Wunused-but-set-variable' warning:

drivers/net/ethernet/aquantia/atlantic/hw_atl/hw_atl_utils.c:260:7:
warning: variable 'err' set but not used [-Wunused-but-set-variable]

'err' should be returned while set MPI_DEINIT state fails
in hw_atl_utils_soft_reset.

Fixes: cce96d1883da ("net: aquantia: Regression on reset with 1.x firmware")
Signed-off-by: YueHaibing <[email protected]>
---
drivers/net/ethernet/aquantia/atlantic/hw_atl/hw_atl_utils.c | 2 ++
1 file changed, 2 insertions(+)

diff --git a/drivers/net/ethernet/aquantia/atlantic/hw_atl/hw_atl_utils.c b/drivers/net/ethernet/aquantia/atlantic/hw_atl/hw_atl_utils.c
index 1af6606..9b74a31 100644
--- a/drivers/net/ethernet/aquantia/atlantic/hw_atl/hw_atl_utils.c
+++ b/drivers/net/ethernet/aquantia/atlantic/hw_atl/hw_atl_utils.c
@@ -263,6 +263,8 @@ int hw_atl_utils_soft_reset(struct aq_hw_s *self)
AQ_HW_WAIT_FOR((aq_hw_read_reg(self, HW_ATL_MPI_STATE_ADR) &
HW_ATL_MPI_STATE_MSK) == MPI_DEINIT,
10, 1000U);
+ if (err)
+ return err;
}

if (self->rbl_enabled)
--
2.7.0

2018-11-27 23:28:14

by David Miller

Subject: Re: [PATCH net-next] net: aquantia: return 'err' if set MPI_DEINIT state fails

From: YueHaibing <[email protected]>
Date: Sat, 24 Nov 2018 18:16:41 +0800

> Fixes gcc '-Wunused-but-set-variable' warning:
>
> drivers/net/ethernet/aquantia/atlantic/hw_atl/hw_atl_utils.c:260:7:
> warning: variable 'err' set but not used [-Wunused-but-set-variable]
>
> 'err' should be returned while set MPI_DEINIT state fails
> in hw_atl_utils_soft_reset.
>
> Fixes: cce96d1883da ("net: aquantia: Regression on reset with 1.x firmware")
> Signed-off-by: YueHaibing <[email protected]>

Applied.

But this AQ_HW_STATE_FOR macro _MUST_ be fixed.

No macro should have invisible references to variables like this, and
that is exactly what leads to bugs like this. CPP macros written this
way make it impossible to properly audit code.

'err' should be an explicit argument given to this function instead of
how it works now.

2018-11-28 09:41:28

by Igor Russkikh

Subject: Re: [PATCH net-next] net: aquantia: return 'err' if set MPI_DEINIT state fails

>> drivers/net/ethernet/aquantia/atlantic/hw_atl/hw_atl_utils.c:260:7:
>> warning: variable 'err' set but not used [-Wunused-but-set-variable]
>>
>> 'err' should be returned while set MPI_DEINIT state fails
>> in hw_atl_utils_soft_reset.
>>
>> Fixes: cce96d1883da ("net: aquantia: Regression on reset with 1.x firmware")
>> Signed-off-by: YueHaibing <[email protected]>
>
> Applied.
>
> But this AQ_HW_STATE_FOR macro _MUST_ be fixed.
>
> No macro should have invisible references to variables like this, and
> that is exactly what leads to bugs like this. CPP macros written this
> way make it impossible to properly audit code.
>
> 'err' should be an explicit argument given to this function instead of
> how it works now.

Hi David,

Agreed, I'll clean this up.

Regards,
Igor