| Message ID | 20231124020352.1660621-2-suhui@nfschina.com |
|---|---|
| State | New |
| Headers | show |
| Series | [v2,1/2] wifi: rtlwifi: rtl8821ae: phy: remove some useless code | expand |
> -----Original Message----- > From: Su Hui <suhui@nfschina.com> > Sent: Friday, November 24, 2023 10:04 AM > To: dan.carpenter@linaro.org; Ping-Ke Shih <pkshih@realtek.com>; kvalo@kernel.org; nathan@kernel.org; > ndesaulniers@google.com; trix@redhat.com > Cc: Su Hui <suhui@nfschina.com>; lizetao1@huawei.com; linville@tuxdriver.com; Larry.Finger@lwfinger.net; > linux-wireless@vger.kernel.org; linux-kernel@vger.kernel.org; llvm@lists.linux.dev; > kernel-janitors@vger.kernel.org > 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.
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; } static bool _rtl8821ae_phy_bb8821a_config_parafile(struct ieee80211_hw *hw); /*static bool _rtl8812ae_phy_config_mac_with_headerfile(struct ieee80211_hw *hw);*/
Clang staic checker warning: drivers/net/wireless/realtek/rtlwifi/rtl8821ae/phy.c:184:49: The result of the left shift is undefined due to shifting by '32', which is greater or equal to the width of type 'u32'. [core.UndefinedBinaryOperatorResult] If the value of the right operand is negative or is greater than or equal to the width of the promoted left operand, the behavior is undefined.[1][2] For example, when using different gcc's compilation optimizaation options (-O0 or -O2), the result of '(u32)data << 32' is different. One is 0, the other is old value of data. Let _rtl8821ae_phy_calculate_bit_shift()'s return value less than 32 to fix this problem. Warn if bitmask is zero. [1]:https://stackoverflow.com/questions/11270492/what-does-the-c- standard-say-about-bitshifting-more-bits-than-the-width-of-type [2]:https://www.open-std.org/jtc1/sc22/wg14/www/docs/n1256.pdf Fixes: 21e4b0726dc6 ("rtlwifi: rtl8821ae: Move driver from staging to regular tree") Signed-off-by: Su Hui <suhui@nfschina.com> --- v2: - fix the subject prefix problem - silence the warning by not return 32 bits rather than adding a type cast.(Thanks to Dan and Ping-Ke) By the way, there some similar problems in _rtl88e_phy_calculate_bit_shift(), _rtl92c_phy_calculate_bit_shift() and so on... drivers/net/wireless/realtek/rtlwifi/rtl8821ae/phy.c | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-)