Re: [PATCH v4 3/5] staging: rtl8192e: fix coding style errors (macros in parentheses)

[Mateusz Kulikowski <mateusz.kulikowski@gmail.com>]

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