Re: [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 <hof...@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.ronnb...@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).