rt2x00: Fix power of 2 check
PATCH 4/4 rt2x00: Fix power of 2 check
Cleanup code, make gcc happy, make sparse happy. What a happy patch ;) This cleans up the code used to check the validity of the register fields, this will also reduce the required memory usage while working with sparse. This patch was inspired on the codesuggestions from Linus.
Signed-off-by Ivo van Doorn <[email protected]>
---
drivers/net/wireless/mac80211/rt2x00/rt2x00.h +22 -193
@@ -206,171 +206,10 @@ enum cipher { };/* -* Macros for determining which is the lowest or highest bit -* set in a 16 or 32 bit variable. -*/ -#define BIT_SET (__val, __bit) (__val & (1 << __bit)) - -#define BIT_OK (__val, __bit, __low, __high) \ - (__bit < __low ? 1 : \ - (__bit > __high ? 1 : \ - BIT_SET(__val, __bit) ? 1 : 0)) - -#define LOWEST_BIT16 (__val) \ - (BIT_SET(__val, 0) ? 0 : (BIT_SET(__val, 1) ? 1 : \ - (BIT_SET(__val, 2) ? 2 : (BIT_SET(__val, 3) ? 3 : \ - (BIT_SET(__val, 4) ? 4 : (BIT_SET(__val, 5) ? 5 : \ - (BIT_SET(__val, 6) ? 6 : (BIT_SET(__val, 7) ? 7 : \ - (BIT_SET(__val, 8) ? 8 : (BIT_SET(__val, 9) ? 9 : \ - (BIT_SET(__val, 10) ? 10 : (BIT_SET(__val, 11) ? 11 : \ - (BIT_SET(__val, 12) ? 12 : (BIT_SET(__val, 13) ? 13 : \ - (BIT_SET(__val, 14) ? 14 : (BIT_SET(__val, 15) ? 15 : \ - -EINVAL)))))))))))))))) - -#define LOWEST_BIT32 (__val) \ - (BIT_SET(__val, 0) ? 0 : (BIT_SET(__val, 1) ? 1 : \ - (BIT_SET(__val, 2) ? 2 : (BIT_SET(__val, 3) ? 3 : \ - (BIT_SET(__val, 4) ? 4 : (BIT_SET(__val, 5) ? 5 : \ - (BIT_SET(__val, 6) ? 6 : (BIT_SET(__val, 7) ? 7 : \ - (BIT_SET(__val, 8) ? 8 : (BIT_SET(__val, 9) ? 9 : \ - (BIT_SET(__val, 10) ? 10 : (BIT_SET(__val, 11) ? 11 : \ - (BIT_SET(__val, 12) ? 12 : (BIT_SET(__val, 13) ? 13 : \ - (BIT_SET(__val, 14) ? 14 : (BIT_SET(__val, 15) ? 15 : \ - (BIT_SET(__val, 16) ? 16 : (BIT_SET(__val, 17) ? 17 : \ - (BIT_SET(__val, 18) ? 18 : (BIT_SET(__val, 19) ? 19 : \ - (BIT_SET(__val, 20) ? 20 : (BIT_SET(__val, 21) ? 21 : \ - (BIT_SET(__val, 22) ? 22 : (BIT_SET(__val, 23) ? 23 : \ - (BIT_SET(__val, 24) ? 24 : (BIT_SET(__val, 25) ? 25 : \ - (BIT_SET(__val, 26) ? 27 : (BIT_SET(__val, 27) ? 27 : \ - (BIT_SET(__val, 28) ? 28 : (BIT_SET(__val, 29) ? 29 : \ - (BIT_SET(__val, 30) ? 30 : (BIT_SET(__val, 31) ? 31 : \ - -EINVAL)))))))))))))))))))))))))))))))) - -#define HIGHEST_BIT16 (__val) \ - (BIT_SET(__val, 15) ? 15 : (BIT_SET(__val, 14) ? 14 : \ - (BIT_SET(__val, 13) ? 13 : (BIT_SET(__val, 12) ? 12 : \ - (BIT_SET(__val, 11) ? 11 : (BIT_SET(__val, 10) ? 10 : \ - (BIT_SET(__val, 9) ? 9 : (BIT_SET(__val, 8) ? 8 : \ - (BIT_SET(__val, 7) ? 7 : (BIT_SET(__val, 6) ? 6 : \ - (BIT_SET(__val, 5) ? 5 : (BIT_SET(__val, 4) ? 4 : \ - (BIT_SET(__val, 3) ? 3 : (BIT_SET(__val, 2) ? 2 : \ - (BIT_SET(__val, 1) ? 1 : (BIT_SET(__val, 0) ? 0 : \ - -EINVAL)))))))))))))))) - -#define HIGHEST_BIT32 (__val) \ - (BIT_SET(__val, 31) ? 31 : (BIT_SET(__val, 30) ? 30 : \ - (BIT_SET(__val, 29) ? 29 : (BIT_SET(__val, 28) ? 28 : \ - (BIT_SET(__val, 27) ? 27 : (BIT_SET(__val, 26) ? 26 : \ - (BIT_SET(__val, 25) ? 25 : (BIT_SET(__val, 24) ? 24 : \ - (BIT_SET(__val, 23) ? 23 : (BIT_SET(__val, 22) ? 22 : \ - (BIT_SET(__val, 21) ? 21 : (BIT_SET(__val, 20) ? 20 : \ - (BIT_SET(__val, 19) ? 19 : (BIT_SET(__val, 18) ? 18 : \ - (BIT_SET(__val, 17) ? 17 : (BIT_SET(__val, 16) ? 16 : \ - (BIT_SET(__val, 15) ? 15 : (BIT_SET(__val, 14) ? 14 : \ - (BIT_SET(__val, 13) ? 13 : (BIT_SET(__val, 12) ? 12 : \ - (BIT_SET(__val, 11) ? 11 : (BIT_SET(__val, 10) ? 10 : \ - (BIT_SET(__val, 9) ? 9 : (BIT_SET(__val, 8) ? 8 : \ - (BIT_SET(__val, 7) ? 7 : (BIT_SET(__val, 6) ? 6 : \ - (BIT_SET(__val, 5) ? 5 : (BIT_SET(__val, 4) ? 4 : \ - (BIT_SET(__val, 3) ? 3 : (BIT_SET(__val, 2) ? 2 : \ - (BIT_SET(__val, 1) ? 1 : (BIT_SET(__val, 0) ? 0 : \ - -EINVAL)))))))))))))))))))))))))))))))) - -#define BITRANGE_OK16 (__val, __low, __high) \ - ((BIT_OK(__val, 0, __low, __high) && \ - BIT_OK(__val, 1, __low, __high) && \ - BIT_OK(__val, 2, __low, __high) && \ - BIT_OK(__val, 3, __low, __high) && \ - BIT_OK(__val, 4, __low, __high) && \ - BIT_OK(__val, 5, __low, __high) && \ - BIT_OK(__val, 6, __low, __high) && \ - BIT_OK(__val, 7, __low, __high) && \ - BIT_OK(__val, 8, __low, __high) && \ - BIT_OK(__val, 9, __low, __high) && \ - BIT_OK(__val, 10, __low, __high) && \ - BIT_OK(__val, 11, __low, __high) && \ - BIT_OK(__val, 12, __low, __high) && \ - BIT_OK(__val, 13, __low, __high) && \ - BIT_OK(__val, 14, __low, __high) && \ - BIT_OK(__val, 15, __low, __high)) ? \ - 0 : -EINVAL) - -#define BITRANGE_OK32 (__val, __low, __high) \ - ((BIT_OK(__val, 0, __low, __high) && \ - BIT_OK(__val, 1, __low, __high) && \ - BIT_OK(__val, 2, __low, __high) && \ - BIT_OK(__val, 3, __low, __high) && \ - BIT_OK(__val, 4, __low, __high) && \ - BIT_OK(__val, 5, __low, __high) && \ - BIT_OK(__val, 6, __low, __high) && \ - BIT_OK(__val, 7, __low, __high) && \ - BIT_OK(__val, 8, __low, __high) && \ - BIT_OK(__val, 9, __low, __high) && \ - BIT_OK(__val, 10, __low, __high) && \ - BIT_OK(__val, 11, __low, __high) && \ - BIT_OK(__val, 12, __low, __high) && \ - BIT_OK(__val, 13, __low, __high) && \ - BIT_OK(__val, 14, __low, __high) && \ - BIT_OK(__val, 15, __low, __high) && \ - BIT_OK(__val, 16, __low, __high) && \ - BIT_OK(__val, 17, __low, __high) && \ - BIT_OK(__val, 18, __low, __high) && \ - BIT_OK(__val, 19, __low, __high) && \ - BIT_OK(__val, 20, __low, __high) && \ - BIT_OK(__val, 21, __low, __high) && \ - BIT_OK(__val, 22, __low, __high) && \ - BIT_OK(__val, 23, __low, __high) && \ - BIT_OK(__val, 24, __low, __high) && \ - BIT_OK(__val, 25, __low, __high) && \ - BIT_OK(__val, 26, __low, __high) && \ - BIT_OK(__val, 27, __low, __high) && \ - BIT_OK(__val, 28, __low, __high) && \ - BIT_OK(__val, 29, __low, __high) && \ - BIT_OK(__val, 30, __low, __high) && \ - BIT_OK(__val, 31, __low, __high)) ? \ - 0 : -EINVAL) - -extern int error_lowest_bit_not_constant; -extern int error_highest_bit_not_constant; -extern int error_bitrange_not_constant; -extern int error_bitrange_bad; - -#define BUILD_LOWEST_BIT16 (__val) \ - (!__builtin_constant_p(__val) ? error_lowest_bit_not_constant : \ - LOWEST_BIT16(__val)) - -#define BUILD_LOWEST_BIT32 (__val) \ - (!__builtin_constant_p(__val) ? error_lowest_bit_not_constant : \ - LOWEST_BIT32(__val)) - -#define BUILD_HIGHEST_BIT16 (__val) \ - (!__builtin_constant_p(__val) ? error_highest_bit_not_constant : \ - HIGHEST_BIT16(__val)) - -#define BUILD_HIGHEST_BIT32 (__val) \ - (!__builtin_constant_p(__val) ? error_highest_bit_not_constant : \ - HIGHEST_BIT32(__val)) - -#define BUILD_BITRANGE_OK16 (__val, __low, __high) \ - ((!__builtin_constant_p(__val) || !__builtin_constant_p(__low) || \ - !__builtin_constant_p(__high)) ? error_bitrange_not_constant : \ - BITRANGE_OK16(__val, __low, __high)) - -#define BUILD_BITRANGE_OK32 (__val, __low, __high) \ - ((!__builtin_constant_p(__val) || !__builtin_constant_p(__low) || \ - !__builtin_constant_p(__high)) ? error_bitrange_not_constant : \ - BITRANGE_OK16(__val, __low, __high)) - -/* * Register handlers. * We store the position of a register field inside a field structure, * This will simplify the process of setting and reading a certain field * inside the register while making sure the process remains byte order safe. -* Before setting the value into the structure we use macros to determine -* whether all bits in the field are contineous and valid. -* These additional checks will be optimized away at compile time, -* but do have a major impact on compile speed, therefor we only make this -* check when compiling with debug enabled. */ struct rt2x00_field16 {u16 bit_offset;
@@ -383,39 +222,29 @@ struct rt2x00_field32 { };/* -* Before intitializing the rt2x00_field# structures, -* we will check if the bitmask is correct and does -* not contain any gaps. -* This check is only done in debug mode, since it severely -* impacts compilation speed. +* Power of two check from Linus Torvalds, +* this will check if the mask that has been +* given contains and contiguous set of bits. */ -#ifdef CONFIG_RT2X00_DEBUG -#define FIELD16 (__mask) \ - ( (struct rt2x00_field16) { \ - (BUILD_BITRANGE_OK16(__mask, BUILD_LOWEST_BIT16(__mask), \ - BUILD_HIGHEST_BIT16(__mask)) == 0) ? \ - BUILD_LOWEST_BIT16(__mask) : error_bitrange_bad, \ - (__mask) \ - } ) - -#define FIELD32 (__mask) \ - ( (struct rt2x00_field32) { \ - (BUILD_BITRANGE_OK32(__mask, BUILD_LOWEST_BIT32(__mask), \ - BUILD_HIGHEST_BIT32(__mask)) == 0) ? \ - BUILD_LOWEST_BIT32(__mask) : error_bitrange_bad, \ - (__mask) \ - } ) -#else /* CONFIG_RT2X00_DEBUG */ -#define FIELD16 (__mask) \ - ( (struct rt2x00_field16) { \ - BUILD_LOWEST_BIT16(__mask), (__mask) \ - } ) - -#define FIELD32 (__mask) \ - ( (struct rt2x00_field32) { \ - BUILD_LOWEST_BIT32(__mask), (__mask) \ - } ) -#endif /* CONFIG_RT2X00_DEBUG */ +#define is_power_of_two (x) ( !((x) & ((x)-1)) ) +#define low_bit_mask (x) ( ((x)-1) & ~(x) ) +#define is_valid_mask (x) is_power_of_two(1 + (x) + low_bit_mask(x)) + +#define FIELD16 (__mask) \ +({ \ + BUILD_BUG_ON(!__mask || !is_valid_mask(__mask));\ + (struct rt2x00_field16) { \ + (__scanbit((__mask), 16) - 1), (__mask) \ + }; \ +}) + +#define FIELD32 (__mask) \ +({ \ + BUILD_BUG_ON(!__mask || !is_valid_mask(__mask));\ + (struct rt2x00_field32) { \ + (__scanbit((__mask), 32) - 1), (__mask) \ + }; \ +})static inline void rt2x00_set_field32 (u32 *reg,const struct rt2x00_field32 field,const u32 value)
Quoted 4 lines · Ivo van Doorn
Cleanup code, make gcc happy, make sparse happy. What a happy patch ;) This cleans up the code used to check the validity of the register fields, this will also reduce the required memory usage while working with sparse. This patch was inspired on the codesuggestions from Linus.
Please cancel this 4th patch, the previous 3 were correct but this one is terribly wrong.
Quoted 9 lines
drivers/net/wireless/mac80211/rt2x00/rt2x00.h +9
-0+#define is_power_of_two (x) ( !((x) & ((x)-1)) ) +#define low_bit_mask (x) ( ((x)-1) & ~(x) ) +#define is_valid_mask (x) is_power_of_two(1 + (x) + low_bit_mask(x)) + +#define FIELD16 (__mask) \ +({ \ + BUILD_BUG_ON(!__mask || !is_valid_mask(__mask));\ + (struct rt2x00_field16) { \ + (__scanbit((__mask), 16) - 1), (__mask) \
drivers/net/wireless/mac80211/rt2x00/rt2x00.h struct rt2x00_field32 { · line 247
Quoted 10 lines
drivers/net/wireless/mac80211/rt2x00/rt2x00.h +10
-0+ }; \ +}) + +#define FIELD32 (__mask) \ +({ \ + BUILD_BUG_ON(!__mask || !is_valid_mask(__mask));\ + (struct rt2x00_field32) { \ + (__scanbit((__mask), 32) - 1), (__mask) \ + }; \ +})
The problem is here, I have used __scanbit to determine the first bit that is set. __scanbit was used since this doesn't require a pointer to the value while the other functions do require the pointer (and the macro receives the direct value hence the reason for the __scanbit choice).
Unfortunately I did not check if __scanbit was available on all arches, and this one seems to only excist for x86_64.. :S
I'll try to fix this asap and come with a new patch.
Ivo
PATCH v2 1/1 rt2x00: Fix power of 2 check
In reply to Ivo van DoornCleanup code, make gcc happy, make sparse happy. What a happy patch ;) This cleans up the code used to check the validity of the register fields, this will also reduce the required memory usage while working with sparse. This patch was inspired on the codesuggestions from Linus.
This update also includes a check if the first found bit does not exceed the 16bit or 32 bit limit.
Signed-off-by Ivo van Doorn <[email protected]>
---
drivers/net/wireless/mac80211/rt2x00/rt2x00.h +26 -193
@@ -206,171 +206,10 @@ enum cipher { };/* -* Macros for determining which is the lowest or highest bit -* set in a 16 or 32 bit variable. -*/ -#define BIT_SET (__val, __bit) (__val & (1 << __bit)) - -#define BIT_OK (__val, __bit, __low, __high) \ - (__bit < __low ? 1 : \ - (__bit > __high ? 1 : \ - BIT_SET(__val, __bit) ? 1 : 0)) - -#define LOWEST_BIT16 (__val) \ - (BIT_SET(__val, 0) ? 0 : (BIT_SET(__val, 1) ? 1 : \ - (BIT_SET(__val, 2) ? 2 : (BIT_SET(__val, 3) ? 3 : \ - (BIT_SET(__val, 4) ? 4 : (BIT_SET(__val, 5) ? 5 : \ - (BIT_SET(__val, 6) ? 6 : (BIT_SET(__val, 7) ? 7 : \ - (BIT_SET(__val, 8) ? 8 : (BIT_SET(__val, 9) ? 9 : \ - (BIT_SET(__val, 10) ? 10 : (BIT_SET(__val, 11) ? 11 : \ - (BIT_SET(__val, 12) ? 12 : (BIT_SET(__val, 13) ? 13 : \ - (BIT_SET(__val, 14) ? 14 : (BIT_SET(__val, 15) ? 15 : \ - -EINVAL)))))))))))))))) - -#define LOWEST_BIT32 (__val) \ - (BIT_SET(__val, 0) ? 0 : (BIT_SET(__val, 1) ? 1 : \ - (BIT_SET(__val, 2) ? 2 : (BIT_SET(__val, 3) ? 3 : \ - (BIT_SET(__val, 4) ? 4 : (BIT_SET(__val, 5) ? 5 : \ - (BIT_SET(__val, 6) ? 6 : (BIT_SET(__val, 7) ? 7 : \ - (BIT_SET(__val, 8) ? 8 : (BIT_SET(__val, 9) ? 9 : \ - (BIT_SET(__val, 10) ? 10 : (BIT_SET(__val, 11) ? 11 : \ - (BIT_SET(__val, 12) ? 12 : (BIT_SET(__val, 13) ? 13 : \ - (BIT_SET(__val, 14) ? 14 : (BIT_SET(__val, 15) ? 15 : \ - (BIT_SET(__val, 16) ? 16 : (BIT_SET(__val, 17) ? 17 : \ - (BIT_SET(__val, 18) ? 18 : (BIT_SET(__val, 19) ? 19 : \ - (BIT_SET(__val, 20) ? 20 : (BIT_SET(__val, 21) ? 21 : \ - (BIT_SET(__val, 22) ? 22 : (BIT_SET(__val, 23) ? 23 : \ - (BIT_SET(__val, 24) ? 24 : (BIT_SET(__val, 25) ? 25 : \ - (BIT_SET(__val, 26) ? 27 : (BIT_SET(__val, 27) ? 27 : \ - (BIT_SET(__val, 28) ? 28 : (BIT_SET(__val, 29) ? 29 : \ - (BIT_SET(__val, 30) ? 30 : (BIT_SET(__val, 31) ? 31 : \ - -EINVAL)))))))))))))))))))))))))))))))) - -#define HIGHEST_BIT16 (__val) \ - (BIT_SET(__val, 15) ? 15 : (BIT_SET(__val, 14) ? 14 : \ - (BIT_SET(__val, 13) ? 13 : (BIT_SET(__val, 12) ? 12 : \ - (BIT_SET(__val, 11) ? 11 : (BIT_SET(__val, 10) ? 10 : \ - (BIT_SET(__val, 9) ? 9 : (BIT_SET(__val, 8) ? 8 : \ - (BIT_SET(__val, 7) ? 7 : (BIT_SET(__val, 6) ? 6 : \ - (BIT_SET(__val, 5) ? 5 : (BIT_SET(__val, 4) ? 4 : \ - (BIT_SET(__val, 3) ? 3 : (BIT_SET(__val, 2) ? 2 : \ - (BIT_SET(__val, 1) ? 1 : (BIT_SET(__val, 0) ? 0 : \ - -EINVAL)))))))))))))))) - -#define HIGHEST_BIT32 (__val) \ - (BIT_SET(__val, 31) ? 31 : (BIT_SET(__val, 30) ? 30 : \ - (BIT_SET(__val, 29) ? 29 : (BIT_SET(__val, 28) ? 28 : \ - (BIT_SET(__val, 27) ? 27 : (BIT_SET(__val, 26) ? 26 : \ - (BIT_SET(__val, 25) ? 25 : (BIT_SET(__val, 24) ? 24 : \ - (BIT_SET(__val, 23) ? 23 : (BIT_SET(__val, 22) ? 22 : \ - (BIT_SET(__val, 21) ? 21 : (BIT_SET(__val, 20) ? 20 : \ - (BIT_SET(__val, 19) ? 19 : (BIT_SET(__val, 18) ? 18 : \ - (BIT_SET(__val, 17) ? 17 : (BIT_SET(__val, 16) ? 16 : \ - (BIT_SET(__val, 15) ? 15 : (BIT_SET(__val, 14) ? 14 : \ - (BIT_SET(__val, 13) ? 13 : (BIT_SET(__val, 12) ? 12 : \ - (BIT_SET(__val, 11) ? 11 : (BIT_SET(__val, 10) ? 10 : \ - (BIT_SET(__val, 9) ? 9 : (BIT_SET(__val, 8) ? 8 : \ - (BIT_SET(__val, 7) ? 7 : (BIT_SET(__val, 6) ? 6 : \ - (BIT_SET(__val, 5) ? 5 : (BIT_SET(__val, 4) ? 4 : \ - (BIT_SET(__val, 3) ? 3 : (BIT_SET(__val, 2) ? 2 : \ - (BIT_SET(__val, 1) ? 1 : (BIT_SET(__val, 0) ? 0 : \ - -EINVAL)))))))))))))))))))))))))))))))) - -#define BITRANGE_OK16 (__val, __low, __high) \ - ((BIT_OK(__val, 0, __low, __high) && \ - BIT_OK(__val, 1, __low, __high) && \ - BIT_OK(__val, 2, __low, __high) && \ - BIT_OK(__val, 3, __low, __high) && \ - BIT_OK(__val, 4, __low, __high) && \ - BIT_OK(__val, 5, __low, __high) && \ - BIT_OK(__val, 6, __low, __high) && \ - BIT_OK(__val, 7, __low, __high) && \ - BIT_OK(__val, 8, __low, __high) && \ - BIT_OK(__val, 9, __low, __high) && \ - BIT_OK(__val, 10, __low, __high) && \ - BIT_OK(__val, 11, __low, __high) && \ - BIT_OK(__val, 12, __low, __high) && \ - BIT_OK(__val, 13, __low, __high) && \ - BIT_OK(__val, 14, __low, __high) && \ - BIT_OK(__val, 15, __low, __high)) ? \ - 0 : -EINVAL) - -#define BITRANGE_OK32 (__val, __low, __high) \ - ((BIT_OK(__val, 0, __low, __high) && \ - BIT_OK(__val, 1, __low, __high) && \ - BIT_OK(__val, 2, __low, __high) && \ - BIT_OK(__val, 3, __low, __high) && \ - BIT_OK(__val, 4, __low, __high) && \ - BIT_OK(__val, 5, __low, __high) && \ - BIT_OK(__val, 6, __low, __high) && \ - BIT_OK(__val, 7, __low, __high) && \ - BIT_OK(__val, 8, __low, __high) && \ - BIT_OK(__val, 9, __low, __high) && \ - BIT_OK(__val, 10, __low, __high) && \ - BIT_OK(__val, 11, __low, __high) && \ - BIT_OK(__val, 12, __low, __high) && \ - BIT_OK(__val, 13, __low, __high) && \ - BIT_OK(__val, 14, __low, __high) && \ - BIT_OK(__val, 15, __low, __high) && \ - BIT_OK(__val, 16, __low, __high) && \ - BIT_OK(__val, 17, __low, __high) && \ - BIT_OK(__val, 18, __low, __high) && \ - BIT_OK(__val, 19, __low, __high) && \ - BIT_OK(__val, 20, __low, __high) && \ - BIT_OK(__val, 21, __low, __high) && \ - BIT_OK(__val, 22, __low, __high) && \ - BIT_OK(__val, 23, __low, __high) && \ - BIT_OK(__val, 24, __low, __high) && \ - BIT_OK(__val, 25, __low, __high) && \ - BIT_OK(__val, 26, __low, __high) && \ - BIT_OK(__val, 27, __low, __high) && \ - BIT_OK(__val, 28, __low, __high) && \ - BIT_OK(__val, 29, __low, __high) && \ - BIT_OK(__val, 30, __low, __high) && \ - BIT_OK(__val, 31, __low, __high)) ? \ - 0 : -EINVAL) - -extern int error_lowest_bit_not_constant; -extern int error_highest_bit_not_constant; -extern int error_bitrange_not_constant; -extern int error_bitrange_bad; - -#define BUILD_LOWEST_BIT16 (__val) \ - (!__builtin_constant_p(__val) ? error_lowest_bit_not_constant : \ - LOWEST_BIT16(__val)) - -#define BUILD_LOWEST_BIT32 (__val) \ - (!__builtin_constant_p(__val) ? error_lowest_bit_not_constant : \ - LOWEST_BIT32(__val)) - -#define BUILD_HIGHEST_BIT16 (__val) \ - (!__builtin_constant_p(__val) ? error_highest_bit_not_constant : \ - HIGHEST_BIT16(__val)) - -#define BUILD_HIGHEST_BIT32 (__val) \ - (!__builtin_constant_p(__val) ? error_highest_bit_not_constant : \ - HIGHEST_BIT32(__val)) - -#define BUILD_BITRANGE_OK16 (__val, __low, __high) \ - ((!__builtin_constant_p(__val) || !__builtin_constant_p(__low) || \ - !__builtin_constant_p(__high)) ? error_bitrange_not_constant : \ - BITRANGE_OK16(__val, __low, __high)) - -#define BUILD_BITRANGE_OK32 (__val, __low, __high) \ - ((!__builtin_constant_p(__val) || !__builtin_constant_p(__low) || \ - !__builtin_constant_p(__high)) ? error_bitrange_not_constant : \ - BITRANGE_OK16(__val, __low, __high)) - -/* * Register handlers. * We store the position of a register field inside a field structure, * This will simplify the process of setting and reading a certain field * inside the register while making sure the process remains byte order safe. -* Before setting the value into the structure we use macros to determine -* whether all bits in the field are contineous and valid. -* These additional checks will be optimized away at compile time, -* but do have a major impact on compile speed, therefor we only make this -* check when compiling with debug enabled. */ struct rt2x00_field16 {u16 bit_offset;
@@ -383,39 +222,33 @@ struct rt2x00_field32 { };/* -* Before intitializing the rt2x00_field# structures, -* we will check if the bitmask is correct and does -* not contain any gaps. -* This check is only done in debug mode, since it severely -* impacts compilation speed. +* Power of two check from Linus Torvalds, +* this will check if the mask that has been +* given contains and contiguous set of bits. */ -#ifdef CONFIG_RT2X00_DEBUG -#define FIELD16 (__mask) \ - ( (struct rt2x00_field16) { \ - (BUILD_BITRANGE_OK16(__mask, BUILD_LOWEST_BIT16(__mask), \ - BUILD_HIGHEST_BIT16(__mask)) == 0) ? \ - BUILD_LOWEST_BIT16(__mask) : error_bitrange_bad, \ - (__mask) \ - } ) - -#define FIELD32 (__mask) \ - ( (struct rt2x00_field32) { \ - (BUILD_BITRANGE_OK32(__mask, BUILD_LOWEST_BIT32(__mask), \ - BUILD_HIGHEST_BIT32(__mask)) == 0) ? \ - BUILD_LOWEST_BIT32(__mask) : error_bitrange_bad, \ - (__mask) \ - } ) -#else /* CONFIG_RT2X00_DEBUG */ -#define FIELD16 (__mask) \ - ( (struct rt2x00_field16) { \ - BUILD_LOWEST_BIT16(__mask), (__mask) \ - } ) - -#define FIELD32 (__mask) \ - ( (struct rt2x00_field32) { \ - BUILD_LOWEST_BIT32(__mask), (__mask) \ - } ) -#endif /* CONFIG_RT2X00_DEBUG */ +#define is_power_of_two (x) ( !((x) & ((x)-1)) ) +#define low_bit_mask (x) ( ((x)-1) & ~(x) ) +#define is_valid_mask (x) is_power_of_two(1 + (x) + low_bit_mask(x)) + +#define FIELD16 (__mask) \ +({ \ + BUILD_BUG_ON(!(__mask) || \ + !is_valid_mask(__mask) || \ + (__ffs((__mask) > 16))); \ + (struct rt2x00_field16) { \ + (__ffs(__mask) - 1), (__mask) \ + }; \ +}) + +#define FIELD32 (__mask) \ +({ \ + BUILD_BUG_ON(!(__mask) || \ + !is_valid_mask(__mask) || \ + (__ffs((__mask) > 32))); \ + (struct rt2x00_field32) { \ + (__ffs(__mask) - 1), (__mask) \ + }; \ +})static inline void rt2x00_set_field32 (u32 *reg,const struct rt2x00_field32 field,const u32 value)
Cleanup code, make gcc happy, make sparse happy. What a happy patch ;) This cleans up the code used to check the validity of the register fields, this will also reduce the required memory usage while working with sparse. This patch was inspired on the codesuggestions from Linus.
This update also includes a check if the first found bit does not exceed the 16bit or 32 bit limit.
Signed-off-by Ivo van Doorn <[email protected]>
---
Sorry for all the spamming, this is the correct version of the patch.
drivers/net/wireless/mac80211/rt2x00/rt2x00.h +26 -193
@@ -206,171 +206,10 @@ enum cipher { };/* -* Macros for determining which is the lowest or highest bit -* set in a 16 or 32 bit variable. -*/ -#define BIT_SET (__val, __bit) (__val & (1 << __bit)) - -#define BIT_OK (__val, __bit, __low, __high) \ - (__bit < __low ? 1 : \ - (__bit > __high ? 1 : \ - BIT_SET(__val, __bit) ? 1 : 0)) - -#define LOWEST_BIT16 (__val) \ - (BIT_SET(__val, 0) ? 0 : (BIT_SET(__val, 1) ? 1 : \ - (BIT_SET(__val, 2) ? 2 : (BIT_SET(__val, 3) ? 3 : \ - (BIT_SET(__val, 4) ? 4 : (BIT_SET(__val, 5) ? 5 : \ - (BIT_SET(__val, 6) ? 6 : (BIT_SET(__val, 7) ? 7 : \ - (BIT_SET(__val, 8) ? 8 : (BIT_SET(__val, 9) ? 9 : \ - (BIT_SET(__val, 10) ? 10 : (BIT_SET(__val, 11) ? 11 : \ - (BIT_SET(__val, 12) ? 12 : (BIT_SET(__val, 13) ? 13 : \ - (BIT_SET(__val, 14) ? 14 : (BIT_SET(__val, 15) ? 15 : \ - -EINVAL)))))))))))))))) - -#define LOWEST_BIT32 (__val) \ - (BIT_SET(__val, 0) ? 0 : (BIT_SET(__val, 1) ? 1 : \ - (BIT_SET(__val, 2) ? 2 : (BIT_SET(__val, 3) ? 3 : \ - (BIT_SET(__val, 4) ? 4 : (BIT_SET(__val, 5) ? 5 : \ - (BIT_SET(__val, 6) ? 6 : (BIT_SET(__val, 7) ? 7 : \ - (BIT_SET(__val, 8) ? 8 : (BIT_SET(__val, 9) ? 9 : \ - (BIT_SET(__val, 10) ? 10 : (BIT_SET(__val, 11) ? 11 : \ - (BIT_SET(__val, 12) ? 12 : (BIT_SET(__val, 13) ? 13 : \ - (BIT_SET(__val, 14) ? 14 : (BIT_SET(__val, 15) ? 15 : \ - (BIT_SET(__val, 16) ? 16 : (BIT_SET(__val, 17) ? 17 : \ - (BIT_SET(__val, 18) ? 18 : (BIT_SET(__val, 19) ? 19 : \ - (BIT_SET(__val, 20) ? 20 : (BIT_SET(__val, 21) ? 21 : \ - (BIT_SET(__val, 22) ? 22 : (BIT_SET(__val, 23) ? 23 : \ - (BIT_SET(__val, 24) ? 24 : (BIT_SET(__val, 25) ? 25 : \ - (BIT_SET(__val, 26) ? 27 : (BIT_SET(__val, 27) ? 27 : \ - (BIT_SET(__val, 28) ? 28 : (BIT_SET(__val, 29) ? 29 : \ - (BIT_SET(__val, 30) ? 30 : (BIT_SET(__val, 31) ? 31 : \ - -EINVAL)))))))))))))))))))))))))))))))) - -#define HIGHEST_BIT16 (__val) \ - (BIT_SET(__val, 15) ? 15 : (BIT_SET(__val, 14) ? 14 : \ - (BIT_SET(__val, 13) ? 13 : (BIT_SET(__val, 12) ? 12 : \ - (BIT_SET(__val, 11) ? 11 : (BIT_SET(__val, 10) ? 10 : \ - (BIT_SET(__val, 9) ? 9 : (BIT_SET(__val, 8) ? 8 : \ - (BIT_SET(__val, 7) ? 7 : (BIT_SET(__val, 6) ? 6 : \ - (BIT_SET(__val, 5) ? 5 : (BIT_SET(__val, 4) ? 4 : \ - (BIT_SET(__val, 3) ? 3 : (BIT_SET(__val, 2) ? 2 : \ - (BIT_SET(__val, 1) ? 1 : (BIT_SET(__val, 0) ? 0 : \ - -EINVAL)))))))))))))))) - -#define HIGHEST_BIT32 (__val) \ - (BIT_SET(__val, 31) ? 31 : (BIT_SET(__val, 30) ? 30 : \ - (BIT_SET(__val, 29) ? 29 : (BIT_SET(__val, 28) ? 28 : \ - (BIT_SET(__val, 27) ? 27 : (BIT_SET(__val, 26) ? 26 : \ - (BIT_SET(__val, 25) ? 25 : (BIT_SET(__val, 24) ? 24 : \ - (BIT_SET(__val, 23) ? 23 : (BIT_SET(__val, 22) ? 22 : \ - (BIT_SET(__val, 21) ? 21 : (BIT_SET(__val, 20) ? 20 : \ - (BIT_SET(__val, 19) ? 19 : (BIT_SET(__val, 18) ? 18 : \ - (BIT_SET(__val, 17) ? 17 : (BIT_SET(__val, 16) ? 16 : \ - (BIT_SET(__val, 15) ? 15 : (BIT_SET(__val, 14) ? 14 : \ - (BIT_SET(__val, 13) ? 13 : (BIT_SET(__val, 12) ? 12 : \ - (BIT_SET(__val, 11) ? 11 : (BIT_SET(__val, 10) ? 10 : \ - (BIT_SET(__val, 9) ? 9 : (BIT_SET(__val, 8) ? 8 : \ - (BIT_SET(__val, 7) ? 7 : (BIT_SET(__val, 6) ? 6 : \ - (BIT_SET(__val, 5) ? 5 : (BIT_SET(__val, 4) ? 4 : \ - (BIT_SET(__val, 3) ? 3 : (BIT_SET(__val, 2) ? 2 : \ - (BIT_SET(__val, 1) ? 1 : (BIT_SET(__val, 0) ? 0 : \ - -EINVAL)))))))))))))))))))))))))))))))) - -#define BITRANGE_OK16 (__val, __low, __high) \ - ((BIT_OK(__val, 0, __low, __high) && \ - BIT_OK(__val, 1, __low, __high) && \ - BIT_OK(__val, 2, __low, __high) && \ - BIT_OK(__val, 3, __low, __high) && \ - BIT_OK(__val, 4, __low, __high) && \ - BIT_OK(__val, 5, __low, __high) && \ - BIT_OK(__val, 6, __low, __high) && \ - BIT_OK(__val, 7, __low, __high) && \ - BIT_OK(__val, 8, __low, __high) && \ - BIT_OK(__val, 9, __low, __high) && \ - BIT_OK(__val, 10, __low, __high) && \ - BIT_OK(__val, 11, __low, __high) && \ - BIT_OK(__val, 12, __low, __high) && \ - BIT_OK(__val, 13, __low, __high) && \ - BIT_OK(__val, 14, __low, __high) && \ - BIT_OK(__val, 15, __low, __high)) ? \ - 0 : -EINVAL) - -#define BITRANGE_OK32 (__val, __low, __high) \ - ((BIT_OK(__val, 0, __low, __high) && \ - BIT_OK(__val, 1, __low, __high) && \ - BIT_OK(__val, 2, __low, __high) && \ - BIT_OK(__val, 3, __low, __high) && \ - BIT_OK(__val, 4, __low, __high) && \ - BIT_OK(__val, 5, __low, __high) && \ - BIT_OK(__val, 6, __low, __high) && \ - BIT_OK(__val, 7, __low, __high) && \ - BIT_OK(__val, 8, __low, __high) && \ - BIT_OK(__val, 9, __low, __high) && \ - BIT_OK(__val, 10, __low, __high) && \ - BIT_OK(__val, 11, __low, __high) && \ - BIT_OK(__val, 12, __low, __high) && \ - BIT_OK(__val, 13, __low, __high) && \ - BIT_OK(__val, 14, __low, __high) && \ - BIT_OK(__val, 15, __low, __high) && \ - BIT_OK(__val, 16, __low, __high) && \ - BIT_OK(__val, 17, __low, __high) && \ - BIT_OK(__val, 18, __low, __high) && \ - BIT_OK(__val, 19, __low, __high) && \ - BIT_OK(__val, 20, __low, __high) && \ - BIT_OK(__val, 21, __low, __high) && \ - BIT_OK(__val, 22, __low, __high) && \ - BIT_OK(__val, 23, __low, __high) && \ - BIT_OK(__val, 24, __low, __high) && \ - BIT_OK(__val, 25, __low, __high) && \ - BIT_OK(__val, 26, __low, __high) && \ - BIT_OK(__val, 27, __low, __high) && \ - BIT_OK(__val, 28, __low, __high) && \ - BIT_OK(__val, 29, __low, __high) && \ - BIT_OK(__val, 30, __low, __high) && \ - BIT_OK(__val, 31, __low, __high)) ? \ - 0 : -EINVAL) - -extern int error_lowest_bit_not_constant; -extern int error_highest_bit_not_constant; -extern int error_bitrange_not_constant; -extern int error_bitrange_bad; - -#define BUILD_LOWEST_BIT16 (__val) \ - (!__builtin_constant_p(__val) ? error_lowest_bit_not_constant : \ - LOWEST_BIT16(__val)) - -#define BUILD_LOWEST_BIT32 (__val) \ - (!__builtin_constant_p(__val) ? error_lowest_bit_not_constant : \ - LOWEST_BIT32(__val)) - -#define BUILD_HIGHEST_BIT16 (__val) \ - (!__builtin_constant_p(__val) ? error_highest_bit_not_constant : \ - HIGHEST_BIT16(__val)) - -#define BUILD_HIGHEST_BIT32 (__val) \ - (!__builtin_constant_p(__val) ? error_highest_bit_not_constant : \ - HIGHEST_BIT32(__val)) - -#define BUILD_BITRANGE_OK16 (__val, __low, __high) \ - ((!__builtin_constant_p(__val) || !__builtin_constant_p(__low) || \ - !__builtin_constant_p(__high)) ? error_bitrange_not_constant : \ - BITRANGE_OK16(__val, __low, __high)) - -#define BUILD_BITRANGE_OK32 (__val, __low, __high) \ - ((!__builtin_constant_p(__val) || !__builtin_constant_p(__low) || \ - !__builtin_constant_p(__high)) ? error_bitrange_not_constant : \ - BITRANGE_OK16(__val, __low, __high)) - -/* * Register handlers. * We store the position of a register field inside a field structure, * This will simplify the process of setting and reading a certain field * inside the register while making sure the process remains byte order safe. -* Before setting the value into the structure we use macros to determine -* whether all bits in the field are contineous and valid. -* These additional checks will be optimized away at compile time, -* but do have a major impact on compile speed, therefor we only make this -* check when compiling with debug enabled. */ struct rt2x00_field16 {u16 bit_offset;
@@ -383,39 +222,33 @@ struct rt2x00_field32 { };/* -* Before intitializing the rt2x00_field# structures, -* we will check if the bitmask is correct and does -* not contain any gaps. -* This check is only done in debug mode, since it severely -* impacts compilation speed. +* Power of two check from Linus Torvalds, +* this will check if the mask that has been +* given contains and contiguous set of bits. */ -#ifdef CONFIG_RT2X00_DEBUG -#define FIELD16 (__mask) \ - ( (struct rt2x00_field16) { \ - (BUILD_BITRANGE_OK16(__mask, BUILD_LOWEST_BIT16(__mask), \ - BUILD_HIGHEST_BIT16(__mask)) == 0) ? \ - BUILD_LOWEST_BIT16(__mask) : error_bitrange_bad, \ - (__mask) \ - } ) - -#define FIELD32 (__mask) \ - ( (struct rt2x00_field32) { \ - (BUILD_BITRANGE_OK32(__mask, BUILD_LOWEST_BIT32(__mask), \ - BUILD_HIGHEST_BIT32(__mask)) == 0) ? \ - BUILD_LOWEST_BIT32(__mask) : error_bitrange_bad, \ - (__mask) \ - } ) -#else /* CONFIG_RT2X00_DEBUG */ -#define FIELD16 (__mask) \ - ( (struct rt2x00_field16) { \ - BUILD_LOWEST_BIT16(__mask), (__mask) \ - } ) - -#define FIELD32 (__mask) \ - ( (struct rt2x00_field32) { \ - BUILD_LOWEST_BIT32(__mask), (__mask) \ - } ) -#endif /* CONFIG_RT2X00_DEBUG */ +#define is_power_of_two (x) ( !((x) & ((x)-1)) ) +#define low_bit_mask (x) ( ((x)-1) & ~(x) ) +#define is_valid_mask (x) is_power_of_two(1 + (x) + low_bit_mask(x)) + +#define FIELD16 (__mask) \ +({ \ + BUILD_BUG_ON(!(__mask) || \ + !is_valid_mask(__mask) || \ + (__ffs((__mask) >= 16))); \ + (struct rt2x00_field16) { \ + __ffs(__mask), (__mask) \ + }; \ +}) + +#define FIELD32 (__mask) \ +({ \ + BUILD_BUG_ON(!(__mask) || \ + !is_valid_mask(__mask) || \ + (__ffs((__mask) >= 32))); \ + (struct rt2x00_field32) { \ + __ffs(__mask), (__mask) \ + }; \ +})static inline void rt2x00_set_field32 (u32 *reg,const struct rt2x00_field32 field,const u32 value)