diff mbox series

wifi: rtw89: chan: Use swap() instead of open coding it

Message ID 20240524075819.2789-1-jiapeng.chong@linux.alibaba.com (mailing list archive)
State Changes Requested
Delegated to: Ping-Ke Shih
Headers show
Series wifi: rtw89: chan: Use swap() instead of open coding it | expand

Commit Message

Jiapeng Chong May 24, 2024, 7:58 a.m. UTC
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 <abaci@linux.alibaba.com>
Closes: https://bugzilla.openanolis.cn/show_bug.cgi?id=9174
Signed-off-by: Jiapeng Chong <jiapeng.chong@linux.alibaba.com>
---
 drivers/net/wireless/realtek/rtw89/chan.c | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

Comments

Ping-Ke Shih May 24, 2024, 9:47 a.m. UTC | #1
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 <abaci@linux.alibaba.com>
> Closes: https://bugzilla.openanolis.cn/show_bug.cgi?id=9174
> Signed-off-by: Jiapeng Chong <jiapeng.chong@linux.alibaba.com>
>
Markus Elfring May 24, 2024, 10:18 a.m. UTC | #2
> 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 <abaci@linux.alibaba.com>
> 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
diff mbox series

Patch

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)