On 16.03.2015 05:51, Joe Perches wrote:
> On Sun, 2015-03-15 at 21:39 +0100, Mateusz Kulikowski wrote:
>> Fix checkpatch.pl errors 'Macros with complex values should be enclosed in
>> parentheses'.
> []
>> diff --git a/drivers/staging/rtl8192e/rtl819x_HT.h
>> b/drivers/staging/rtl8192e/rtl819x_HT.h
> []
>> @@ -78,7 +78,7 @@ enum chnl_op {
>> };
>>
>> #define CHHLOP_IN_PROGRESS(_pHTInfo) \
>> - ((_pHTInfo)->ChnlOp > CHNLOP_NONE) ? true : false
>> + (((_pHTInfo)->ChnlOp > CHNLOP_NONE) ? true : false)
>
> This would perhaps be better without the ternary as
> the compiler might not optimize the ternary away
>
> #define CHHLOP_IN_PROGRESS(_pHTInfo) \
> ((_pHTInfo)->ChnOp > CHNLOP_NONE)
>
> This would also probably be better as a static inline
> not a macro, but as this is never used it's actually
> better just to remove it.
Thanks for the hints, done in v5 (other macros were also harmed)
>
>> @@ -385,8 +385,8 @@ extern u8 MCS_FILTER_1SS[16];
>> #define LEGACY_WIRELESS_MODE IEEE_MODE_MASK
>>
>> #define CURRENT_RATE(WirelessMode, LegacyRate, HTRate) \
>> - ((WirelessMode & (LEGACY_WIRELESS_MODE)) != 0) ? \
>> - (LegacyRate) : (PICK_RATE(LegacyRate, HTRate))
>> + (((WirelessMode & (LEGACY_WIRELESS_MODE)) != 0) ? \
>> + (LegacyRate) : (PICK_RATE(LegacyRate, HTRate)))
>
> This would be better with the various macro arguments
> parenthesized (especially WirelessMode)
>
> #define CURRENT_RATE(WirelessMode, LegacyRate, HTRate) \
> (((WirelessMode) & LEGACY_WIRELESS_MODE) ? \
> (LegacyRate) : PICK_RATE((LegacyRate), HTRate))
>
> As this is used exactly once, it's probably better
> expanded in that one place and the macro removed.
>
> That's true for PICK_RATE as well.
Will replace with static function (in .c file) - code that uses this macro is
nested enough
Regards
Mateusz
_______________________________________________
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
