[PATCH 2/5] eal: add unit tests for bit operations

[Mattias Rönnblom]

On 2024-08-09 18:31, Stephen Hemminger wrote:
> On Fri, 9 Aug 2024 17:37:08 +0200
> Mattias Rönnblom <hofors at lysator.liu.se> wrote:
> 
>> On 2024-08-09 17:03, Stephen Hemminger wrote:
>>> On Fri, 9 Aug 2024 11:04:36 +0200
>>> Mattias Rönnblom <mattias.ronnblom at ericsson.com> wrote:
>>>    
>>>> -uint32_t val32;
>>>> -uint64_t val64;
>>>> +#define GEN_TEST_BIT_ACCESS(test_name, set_fun, clear_fun, assign_fun,	\
>>>> +			    flip_fun, test_fun, size)			\
>>>> +	static int							\
>>>> +	test_name(void)							\
>>>> +	{								\
>>>> +		uint ## size ## _t reference = (uint ## size ## _t)rte_rand(); \
>>>> +		unsigned int bit_nr;					\
>>>> +		uint ## size ## _t word = (uint ## size ## _t)rte_rand(); \
>>>> +									\
>>>> +		for (bit_nr = 0; bit_nr < size; bit_nr++) {		\
>>>> +			bool reference_bit = (reference >> bit_nr) & 1;	\
>>>> +			bool assign = rte_rand() & 1;			\
>>>> +			if (assign)					\
>>>> +				assign_fun(&word, bit_nr, reference_bit); \
>>>> +			else {						\
>>>> +				if (reference_bit)			\
>>>> +					set_fun(&word, bit_nr);		\
>>>> +				else					\
>>>> +					clear_fun(&word, bit_nr);	\
>>>> +									\
>>>> +			}						\
>>>> +			TEST_ASSERT(test_fun(&word, bit_nr) == reference_bit, \
>>>> +				    "Bit %d had unexpected value", bit_nr); \
>>>> +			flip_fun(&word, bit_nr);			\
>>>> +			TEST_ASSERT(test_fun(&word, bit_nr) != reference_bit, \
>>>> +				    "Bit %d had unflipped value", bit_nr); \
>>>> +			flip_fun(&word, bit_nr);			\
>>>> +									\
>>>> +			const uint ## size ## _t *const_ptr = &word;	\
>>>> +			TEST_ASSERT(test_fun(const_ptr, bit_nr) ==	\
>>>> +				    reference_bit,			\
>>>> +				    "Bit %d had unexpected value", bit_nr); \
>>>> +		}							\
>>>> +									\
>>>> +		for (bit_nr = 0; bit_nr < size; bit_nr++) {		\
>>>> +			bool reference_bit = (reference >> bit_nr) & 1;	\
>>>> +			TEST_ASSERT(test_fun(&word, bit_nr) == reference_bit, \
>>>> +				    "Bit %d had unexpected value", bit_nr); \
>>>> +		}							\
>>>> +									\
>>>> +		TEST_ASSERT(reference == word, "Word had unexpected value"); \
>>>> +									\
>>>> +		return TEST_SUCCESS;					\
>>>> +	}
>>>> +
>>>> +GEN_TEST_BIT_ACCESS(test_bit_access32, rte_bit_set, rte_bit_clear,
>>>> +		    rte_bit_assign, rte_bit_flip, rte_bit_test, 32)
>>>> +
>>>> +GEN_TEST_BIT_ACCESS(test_bit_access64, rte_bit_set, rte_bit_clear,
>>>> +		    rte_bit_assign, rte_bit_flip, rte_bit_test, 64)
>>>
>>> Having large macro like this for two cases adds complexity without
>>> additional clarity. Just duplicate the code please.
>>
>> GEN_TEST_BIT_ACCESS is being used by six more test cases in later
>> patches in the series.
> 
> Would it be possible to make it a function and pass function pointers with
> Generic?

I'm not sure exactly what you are suggesting here, but a function can't 
do the job of GEN_TEST_BIT_ACCESS. You can't pass macros as parameters 
to functions, and also the signatures of the _Generic-macros-under-test 
(e.g., set_fun) various across different test cases.

I agree with what underlies your suggestion - prefer functions over 
macros when functions can do the job (reasonably well).