From mboxrd@z Thu Jan 1 00:00:00 1970 From: Thomas Monjalon Subject: Re: [dpdk-stable] [PATCH v2] test/hash: solve unit test hash compilation error Date: Fri, 26 Oct 2018 23:59:16 +0200 Message-ID: <4369948.DfioVmEDkk@xps> References: <1535379969-19642-1-git-send-email-dharmik.thakkar@arm.com> Mime-Version: 1.0 Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable Cc: "Richardson, Bruce" , "De Lara Guarch, Pablo" , "dev@dpdk.org" , Honnappa Nagarahalli , nd To: Dharmik Thakkar , "Wang, Yipeng1" Return-path: Received: from out3-smtp.messagingengine.com (out3-smtp.messagingengine.com [66.111.4.27]) by dpdk.org (Postfix) with ESMTP id 0502BF94 for ; Fri, 26 Oct 2018 23:59:13 +0200 (CEST) In-Reply-To: List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" 26/10/2018 23:55, Dharmik Thakkar: >=20 > > On Oct 26, 2018, at 4:05 PM, Wang, Yipeng1 wro= te: > >=20 > >> -----Original Message----- > >> From: Thomas Monjalon [mailto:thomas@monjalon.net] > >> Sent: Friday, October 26, 2018 1:25 PM > >> To: Dharmik Thakkar > >> Cc: Richardson, Bruce ; De Lara Guarch, Pa= blo ; dev@dpdk.org; > >> honnappa.nagarahalli@arm.com; Wang, Yipeng1 > >> Subject: Re: [dpdk-stable] [PATCH v2] test/hash: solve unit test hash = compilation error > >>=20 > >> +Cc Yipeng > >>=20 > >> 18/09/2018 21:22, Dharmik Thakkar: > >>> Enable print_key_info() function compilation always. > >>=20 > >> Please see my first comment: you need to show the compilation error > >> in this message. Otherwise, we don't know what you are trying > >> to fix. > >>=20 > >>> Fixes: af75078fece36 ("first public release") > >>> Cc: stable@dpdk.org > >>>=20 > >>> Suggested-by: Honnappa Nagarahalli > >>> Signed-off-by: Dharmik Thakkar > >>> Reviewed-by: Honnappa Nagarahalli > >>> Reviewed-by: Gavin Hu > >>> --- > >>> v2: > >>> * Fix checkpatch coding style issue > >>> * Add "Fixes:" tag > >>> --- > >>> test/test/test_hash.c | 24 +++++++++--------------- > >>> 1 file changed, 9 insertions(+), 15 deletions(-) > >>>=20 > >>> diff --git a/test/test/test_hash.c b/test/test/test_hash.c > >>> index b3db9fd10547..db6442a2b101 100644 > >>> --- a/test/test/test_hash.c > >>> +++ b/test/test/test_hash.c > >>> +#define UNIT_TEST_HASH_VERBOSE 0 > >>> /* > >>> * Print out result of unit test hash operation. > >>> */ > >>> -#if defined(UNIT_TEST_HASH_VERBOSE) > >>> static void print_key_info(const char *msg, const struct flow_key *ke= y, > >>> int32_t pos) > >>> { > >>> - uint8_t *p =3D (uint8_t *)key; > >>> - unsigned i; > >>> - > >>> - printf("%s key:0x", msg); > >>> - for (i =3D 0; i < sizeof(struct flow_key); i++) { > >>> - printf("%02X", p[i]); > >>> + if (UNIT_TEST_HASH_VERBOSE) { > >>=20 > >> This is very suspicious. > >> Why keeping this code if it is never called? > >=20 > > [Wang, Yipeng] I assume this is for the convenience for debug. E.g. if = the unit test failed, > > developer can set the macro and print more information, but by default = the code is not used. > >=20 > > A quick grep I found the test_timer_racecond and efd unit test has sim= ilar macros. But could anyone > > let me know what is the best coding practice for such purpose in unit t= est? > Thank you bringing up the discussion, Yipeng. I, too, would like to know = the best coding practice for such purposes. >=20 > One disadvantage of such macros is: That section of the code is only comp= iled when the macro is defined.=20 > For eg., previously, =E2=80=98print_key_info()=E2=80=99 did not compile w= ithout defining UNIT_TEST_HASH_VERBOSE. > Thus, it=E2=80=99s compilation error(s) are not accounted for always. The compilation time options are generally bad. In this case, we could use the log level as a condition for printing.