From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mails.dpdk.org (mails.dpdk.org [217.70.189.124]) by inbox.dpdk.org (Postfix) with ESMTP id 9D38F4577B; Fri, 9 Aug 2024 18:57:09 +0200 (CEST) Received: from mails.dpdk.org (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 3440D42EE4; Fri, 9 Aug 2024 18:57:09 +0200 (CEST) Received: from mail.lysator.liu.se (mail.lysator.liu.se [130.236.254.3]) by mails.dpdk.org (Postfix) with ESMTP id 34D7B42DE9 for ; Fri, 9 Aug 2024 18:57:08 +0200 (CEST) Received: from mail.lysator.liu.se (localhost [127.0.0.1]) by mail.lysator.liu.se (Postfix) with ESMTP id 92BE2154A4 for ; Fri, 9 Aug 2024 18:57:07 +0200 (CEST) Received: by mail.lysator.liu.se (Postfix, from userid 1004) id 83DDF154A3; Fri, 9 Aug 2024 18:57:07 +0200 (CEST) X-Spam-Checker-Version: SpamAssassin 4.0.0 (2022-12-13) on hermod.lysator.liu.se X-Spam-Level: X-Spam-Status: No, score=-1.3 required=5.0 tests=ALL_TRUSTED,AWL, T_SCC_BODY_TEXT_LINE autolearn=disabled version=4.0.0 X-Spam-Score: -1.3 Received: from [192.168.1.86] (h-62-63-215-114.A163.priv.bahnhof.se [62.63.215.114]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by mail.lysator.liu.se (Postfix) with ESMTPSA id 31E191535B; Fri, 9 Aug 2024 18:57:05 +0200 (CEST) Message-ID: <4484c356-aa2d-4bd0-848b-23226c1d0abb@lysator.liu.se> Date: Fri, 9 Aug 2024 18:57:04 +0200 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH 2/5] eal: add unit tests for bit operations To: Stephen Hemminger Cc: =?UTF-8?Q?Mattias_R=C3=B6nnblom?= , dev@dpdk.org, Heng Wang , Joyce Kong , Tyler Retzlaff , =?UTF-8?Q?Morten_Br=C3=B8rup?= References: <20240505083737.118649-2-mattias.ronnblom@ericsson.com> <20240809090439.589295-1-mattias.ronnblom@ericsson.com> <20240809090439.589295-3-mattias.ronnblom@ericsson.com> <20240809080314.51637b87@hermes.local> <201141b3-d8f2-4445-88d3-d8480b1c8761@lysator.liu.se> <20240809093136.487249e7@hermes.local> Content-Language: en-US From: =?UTF-8?Q?Mattias_R=C3=B6nnblom?= In-Reply-To: <20240809093136.487249e7@hermes.local> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit X-Virus-Scanned: ClamAV using ClamSMTP X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org On 2024-08-09 18:31, Stephen Hemminger wrote: > On Fri, 9 Aug 2024 17:37:08 +0200 > Mattias Rönnblom wrote: > >> On 2024-08-09 17:03, Stephen Hemminger wrote: >>> On Fri, 9 Aug 2024 11:04:36 +0200 >>> Mattias Rönnblom 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).