[PATCH wireless-next] wlcore: debugfs: add an error code check in beacon_filtering_write

[PATCH wireless-next] wlcore: debugfs: add an error code check in beacon_filtering_write

[Su Hui]

wl1271_acx_beacon_filter_opt() return error code if failed, add a check
for this is better and safer.

Signed-off-by: Su Hui <suhui@nfschina.com>
---
 drivers/net/wireless/ti/wlcore/debugfs.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/net/wireless/ti/wlcore/debugfs.c b/drivers/net/wireless/ti/wlcore/debugfs.c
index eb3d3f0e0b4d..d4071291a8cd 100644
--- a/drivers/net/wireless/ti/wlcore/debugfs.c
+++ b/drivers/net/wireless/ti/wlcore/debugfs.c
@@ -932,13 +932,15 @@ static ssize_t beacon_filtering_write(struct file *file,
 
 	wl12xx_for_each_wlvif(wl, wlvif) {
 		ret = wl1271_acx_beacon_filter_opt(wl, wlvif, !!value);
+		if (ret < 0)
+			break;
 	}
 
 	pm_runtime_mark_last_busy(wl->dev);
 	pm_runtime_put_autosuspend(wl->dev);
 out:
 	mutex_unlock(&wl->mutex);
-	return count;
+	return ret < 0 ? ret : count;
 }
 
 static const struct file_operations beacon_filtering_ops = {
-- 
2.30.2

Re: [PATCH wireless-next] wlcore: debugfs: add an error code check in beacon_filtering_write

[Kalle Valo]

Su Hui <suhui@nfschina.com> writes:

> wl1271_acx_beacon_filter_opt() return error code if failed, add a check
> for this is better and safer.
>
> Signed-off-by: Su Hui <suhui@nfschina.com>

How did you test this?

-- 
https://patchwork.kernel.org/project/linux-wireless/list/

https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches

Re: [PATCH wireless-next] wlcore: debugfs: add an error code check in beacon_filtering_write

[Su Hui]

On 2023/11/13 14:16, Kalle Valo wrote:

> Su Hui <suhui@nfschina.com> writes:
>
>> wl1271_acx_beacon_filter_opt() return error code if failed, add a check
>> for this is better and safer.
>>
>> Signed-off-by: Su Hui <suhui@nfschina.com>
> How did you test this?
>
Hi，

Only compile test.
Clang static checker complains about this code that  value stored to 
'ret' is never read.
And most of the caller check  the error code of 
wl1271_acx_beacon_filter_opt().

Such like this:
--
drivers/net/wireless/ti/wlcore/init.c:277:      ret = 
wl1271_acx_beacon_filter_opt(wl, wlvif, false);
drivers/net/wireless/ti/wlcore/init.c-278-      if (ret < 0)
drivers/net/wireless/ti/wlcore/init.c-279-              return ret;
--
drivers/net/wireless/ti/wlcore/init.c:410:      ret = 
wl1271_acx_beacon_filter_opt(wl, wlvif, false);
drivers/net/wireless/ti/wlcore/init.c-411-      if (ret < 0)
drivers/net/wireless/ti/wlcore/init.c-412-              return ret;
--
drivers/net/wireless/ti/wlcore/main.c:1652:     ret = 
wl1271_acx_beacon_filter_opt(wl, wlvif, true);
drivers/net/wireless/ti/wlcore/main.c-1653-     if (ret < 0)
drivers/net/wireless/ti/wlcore/main.c-1654-             goto out;

Sorry for the few test.

Su Hui

Re: [PATCH wireless-next] wlcore: debugfs: add an error code check in beacon_filtering_write

[Kalle Valo]

Su Hui <suhui@nfschina.com> writes:

> On 2023/11/13 14:16, Kalle Valo wrote:
>
>> Su Hui <suhui@nfschina.com> writes:
>>
>>> wl1271_acx_beacon_filter_opt() return error code if failed, add a check
>>> for this is better and safer.
>>>
>>> Signed-off-by: Su Hui <suhui@nfschina.com>
>> How did you test this?
>
> Only compile test.

If you have only compile tested please document that clearly in the
commit message.

> Clang static checker complains about this code that  value stored to
> 'ret' is never read.

This would be good to also have in the commit message.

> And most of the caller check  the error code of
> wl1271_acx_beacon_filter_opt().

This might still break something so I would prefer to test this in a
real device before taking it.

-- 
https://patchwork.kernel.org/project/linux-wireless/list/

https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches

Re: [PATCH wireless-next] wlcore: debugfs: add an error code check in beacon_filtering_write

[Su Hui]

On 2023/11/13 20:07, Kalle Valo wrote:
> Su Hui <suhui@nfschina.com> writes:
>
>> On 2023/11/13 14:16, Kalle Valo wrote:
>>
>>> Su Hui <suhui@nfschina.com> writes:
>>>
>>>> wl1271_acx_beacon_filter_opt() return error code if failed, add a check
>>>> for this is better and safer.
>>>>
>>>> Signed-off-by: Su Hui <suhui@nfschina.com>
>>> How did you test this?
>> Only compile test.
> If you have only compile tested please document that clearly in the
> commit message.
Sorry for the unclear commit message.
>> Clang static checker complains about this code that  value stored to
>> 'ret' is never read.
> This would be good to also have in the commit message.
I will add this to v2 patch if pass a test  in a real device.
>> And most of the caller check  the error code of
>> wl1271_acx_beacon_filter_opt().
> This might still break something so I would prefer to test this in a
> real device before taking it.
This might take some time, I try to find a wlcore device to test this.
Thanks for your reply!

Su Hui