From mboxrd@z Thu Jan 1 00:00:00 1970 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) Received: from compute1.internal (compute1.nyi.internal [10.202.2.41]) by mailout.nyi.internal (Postfix) with ESMTP id 8623821F30; Fri, 26 Oct 2018 17:59:12 -0400 (EDT) Received: from mailfrontend2 ([10.202.2.163]) by compute1.internal (MEProxy); Fri, 26 Oct 2018 17:59:12 -0400 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=monjalon.net; h= from:to:cc:subject:date:message-id:in-reply-to:references :mime-version:content-transfer-encoding:content-type; s=mesmtp; bh=PZDhT7w+9/Uu4rBGNC+RyY0fs5grcjPQT5YZT+qNQJc=; b=E7lk3KMCQmWm XapTRz7hv5zpiZEVZcg6IdB4HFjZ6IXdNCaMa/B8e31cGL38P3PYhHWQSBOnK92t esvYdgLnIG2+vqYbytEvQ8mjpg77bYmimj260D7hlnojHPF3H4PRlXC9BL0D36b9 4g4RexPJvJufqyeMbyMyNIlhKBetkTo= DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d= messagingengine.com; h=cc:content-transfer-encoding:content-type :date:from:in-reply-to:message-id:mime-version:references :subject:to:x-me-proxy:x-me-proxy:x-me-sender:x-me-sender :x-sasl-enc; s=fm1; bh=PZDhT7w+9/Uu4rBGNC+RyY0fs5grcjPQT5YZT+qNQ Jc=; b=XBGQx6vphNeuK+TVEssB2dDcLqm+je6tPdK67lJFx2lfusrtWjwD3fprx fYg3K2qqiKJMgg03UH6Mo4NDHXt09siUxyVZ5bsJOCa5m/wah2QzvoCdjtnqkLvw LJ2pi211dt/zUvJ6JyR5MI7U5Q+ZPt3XQ5XmF3YMv12b/MScL6h2Qww70rK2hOl+ mKd3oShMnAYyBVA23pu6jfQiaW9RoxNlwLHNg4mcngfVmuL6s/KIPHm17ERCukwO BA/EbOZffayLJv44/VMtoHDPc2EehXdwD36Lx196OF1mLbDz6Vh8+ZLlX+wRD2f9 FOwCx2aOuv3lL0ovcIkW0rcImq4xA== X-ME-Sender: X-ME-Proxy: Received: from xps.localnet (184.203.134.77.rev.sfr.net [77.134.203.184]) by mail.messagingengine.com (Postfix) with ESMTPA id 05420102E8; Fri, 26 Oct 2018 17:59:10 -0400 (EDT) From: Thomas Monjalon To: Dharmik Thakkar , "Wang, Yipeng1" Cc: "Richardson, Bruce" , "De Lara Guarch, Pablo" , "dev@dpdk.org" , Honnappa Nagarahalli , nd Date: Fri, 26 Oct 2018 23:59:16 +0200 Message-ID: <4369948.DfioVmEDkk@xps> In-Reply-To: References: <1535379969-19642-1-git-send-email-dharmik.thakkar@arm.com> MIME-Version: 1.0 Content-Transfer-Encoding: quoted-printable Content-Type: text/plain; charset="UTF-8" Subject: Re: [dpdk-dev] [dpdk-stable] [PATCH v2] test/hash: solve unit test hash compilation error X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 26 Oct 2018 21:59:13 -0000 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.