ath9k: cleanup: u32 => bool
PATCH 3/9 ath9k: cleanup: u32 => bool
This is used as a boolean variable so changing it to a boolean type is a cleanup.
drivers/net/wireless/ath/ath9k/ar5008_phy.c +1 -1
@@ -1082,7 +1082,7 @@ static bool ar5008_hw_ani_control(struct ath_hw *ah,const int m2Thresh[] = {127 ,0x40 };const int m2CountThr[] = {31 ,16 };const int m2CountThrLow[] = {63 ,48 }; -u32 on = param ?1 :0 ; +bool on = param ?1 :0 ; REG_RMW_FIELD(ah, AR_PHY_SFCORR_LOW, AR_PHY_SFCORR_LOW_M1_THRESH_LOW,
drivers/net/wireless/ath/ath9k/ar9003_phy.c +1 -1
@@ -771,7 +771,7 @@ static bool ar9003_hw_ani_control(struct ath_hw *ah,const int m2Thresh[] = {127 ,0x40 };const int m2CountThr[] = {31 ,16 };const int m2CountThrLow[] = {63 ,48 }; -u32 on = param ?1 :0 ; +bool on = param ?1 :0 ; REG_RMW_FIELD(ah, AR_PHY_SFCORR_LOW, AR_PHY_SFCORR_LOW_M1_THRESH_LOW,
PATCH 3/9 ath9k: cleanup: u32 => bool
Review
Quoted 1 line · Dan Carpenter
(quoted diff) +1
-0+ bool on = param ? 1 : 0;
umm, if you're going to use bool then use true/false too.
johannes
PATCH 3/9 ath9k: cleanup: u32 => bool
Quoted 7 lines · Johannes Berg
Review
Quoted 1 line · Dan Carpenter
(quoted diff) +1
-0+ bool on = param ? 1 : 0;umm, if you're going to use bool then use true/false too.
johannes
The 1 and 0 are used as array indexes so it looks funny to use true and false.
Let's just drop this patch. I wrote it to silence a smatch false positive but I should/will just fix smatch instead.
Smatch complains we negate a non-boolean type here:
if (!on != aniState->ofdmWeakSigDetectOff) {It's a quite common bug to forget that negation has higher precendence than compare operations. In this case, it's clear from code that "on" is either 1 or 0 so smatch should not complain.
regards,
dan carpenter
PATCH 3/9 ath9k: cleanup: u32 => bool
Quoting Dan Carpenter <[email protected]>:
Quoted 7 lines
Smatch complains we negate a non-boolean type here:
if (!on != aniState->ofdmWeakSigDetectOff) {It's a quite common bug to forget that negation has higher precendence than compare operations. In this case, it's clear from code that "on" is either 1 or 0 so smatch should not complain.
We could rename ofdmWeakSigDetectOff to ofdmWeakSigDetectOn and change the logic appropriately. Positive logic is easier to understand in the long run.
ofdmWeakSigDetectOn could be boolean and "on" could be int, since it's an index. The comparison could use !!on to convert int to boolean.
Signature · 2 lines
Regards, Pavel Roskin