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! Su Hui
