| 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 |
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> >
> 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 --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)
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(-)