Re: [PATCH v2 2/2] wifi: rtlwifi: rtl8821ae: phy: fix an undefined bitwise shift behavior

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 2023/11/24 19:19, Ping-Ke Shih wrote:
On Fri, 2023-11-24 at 18:06 +0800, Su Hui wrote:
On 2023/11/24 16:51, Ping-Ke Shih wrote:
Subject: [PATCH v2 2/2] wifi: rtlwifi: rtl8821ae: phy: fix an undefined bitwise shift behavior

[...]
diff --git a/drivers/net/wireless/realtek/rtlwifi/rtl8821ae/phy.c
b/drivers/net/wireless/realtek/rtlwifi/rtl8821ae/phy.c
index 6df270e29e66..52ab1b0761c0 100644
--- a/drivers/net/wireless/realtek/rtlwifi/rtl8821ae/phy.c
+++ b/drivers/net/wireless/realtek/rtlwifi/rtl8821ae/phy.c
@@ -31,7 +31,12 @@ static u32 _rtl8821ae_phy_calculate_bit_shift(u32 bitmask)
   {
          u32 i = ffs(bitmask);

-       return i ? i - 1 : 32;
+       if (!i) {
+               WARN_ON_ONCE(1);
+               return 0;
+       }
+
+       return i - 1;
   }
Personally, I prefer to use __ffs(), because in normal case no need additional '-1',
and abnormal cases should not happen.
Hi,  Ping-Ke

Replace _rtl8821ae_phy_calculate_bit_shift() by __ffs(bitmask) is better,
but I'm not sure what callers should do when callers check bitmask is 0 before calling.
Maybe this check is useless?

I can send a v3 patch if using  __ffs(bitmask) and no check for bitmask is fine.
Or could you send this patch if you have a better idea?
Thanks for your suggestion!

Can this work to you?
Looks good to me, briefer and better!
I will send v3 soon.
static u32 _rtl8821ae_phy_calculate_bit_shift(u32 bitmask)
{
	if (WARN_ON_ONCE(!bitmask))
		return 0;

	return __ffs(bitmask);
}





[Index of Archives]     [Kernel Development]     [Kernel Announce]     [Kernel Newbies]     [Linux Networking Development]     [Share Photos]     [IDE]     [Security]     [Git]     [Netfilter]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Device Mapper]

  Powered by Linux