2024-05-24 07:58:49

by Jiapeng Chong

[permalink] [raw]
Subject: [PATCH] wifi: rtw89: chan: Use swap() instead of open coding it

Swap is a function interface that provides exchange function. To avoid
code duplication, we can use swap function.

./drivers/net/wireless/realtek/rtw89/chan.c:2336:32-33: WARNING opportunity for swap().

Reported-by: Abaci Robot <[email protected]>
Closes: https://bugzilla.openanolis.cn/show_bug.cgi?id=9174
Signed-off-by: Jiapeng Chong <[email protected]>
---
drivers/net/wireless/realtek/rtw89/chan.c | 5 +----
1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/drivers/net/wireless/realtek/rtw89/chan.c b/drivers/net/wireless/realtek/rtw89/chan.c
index 051a3cad6101..3b1997223cc5 100644
--- a/drivers/net/wireless/realtek/rtw89/chan.c
+++ b/drivers/net/wireless/realtek/rtw89/chan.c
@@ -2322,7 +2322,6 @@ static void rtw89_swap_sub_entity(struct rtw89_dev *rtwdev,
enum rtw89_sub_entity_idx idx2)
{
struct rtw89_hal *hal = &rtwdev->hal;
- struct rtw89_sub_entity tmp;
struct rtw89_vif *rtwvif;
u8 cur;

@@ -2332,9 +2331,7 @@ static void rtw89_swap_sub_entity(struct rtw89_dev *rtwdev,
hal->sub[idx1].cfg->idx = idx2;
hal->sub[idx2].cfg->idx = idx1;

- tmp = hal->sub[idx1];
- hal->sub[idx1] = hal->sub[idx2];
- hal->sub[idx2] = tmp;
+ swap(hal->sub[idx1], hal->sub[idx2]);

rtw89_for_each_rtwvif(rtwdev, rtwvif) {
if (!rtwvif->chanctx_assigned)
--
2.20.1.7.g153144c



2024-05-24 09:49:20

by Ping-Ke Shih

[permalink] [raw]
Subject: Re: [PATCH] wifi: rtw89: chan: Use swap() instead of open coding it

Hi,

Subject "wifi: rtw89: chan: Use swap() instead of open coding it" is too common.
If more than one patch does similar thing, we can't know the change from
subject. It would be better to mention "swap sub_entity".

Others are good to me.


On Fri, 2024-05-24 at 15:58 +0800, Jiapeng Chong wrote:
>
> Swap is a function interface that provides exchange function. To avoid
> code duplication, we can use swap function.
>
> ./drivers/net/wireless/realtek/rtw89/chan.c:2336:32-33: WARNING opportunity for swap().
>
> Reported-by: Abaci Robot <[email protected]>
> Closes: https://bugzilla.openanolis.cn/show_bug.cgi?id=9174
> Signed-off-by: Jiapeng Chong <[email protected]>
>

2024-05-24 10:19:20

by Markus Elfring

[permalink] [raw]
Subject: Re: [PATCH] wifi: rtw89: chan: Use swap() instead of open coding it

> Swap is a function interface that provides exchange function. To avoid
> code duplication, we can use swap function.

Would a wording approach (like the following) be a bit nicer
for the second sentence?

Use existing swap() function instead of keeping duplicate source code.


How do you think about to apply the summary phrase “Use swap() in rtw89_swap_sub_entity()”?


> ./drivers/net/wireless/realtek/rtw89/chan.c:2336:32-33: WARNING opportunity for swap().
>
> Reported-by: Abaci Robot <[email protected]>
> Closes: https://bugzilla.openanolis.cn/show_bug.cgi?id=9174

Would another indication be helpful for the involved analysis tool?
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/scripts/coccinelle/misc/swap.cocci?h=v6.9

Regards,
Markus