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 83E71A0093; Fri, 10 Dec 2021 10:30:18 +0100 (CET) Received: from [217.70.189.124] (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 06EAA4014F; Fri, 10 Dec 2021 10:30:18 +0100 (CET) Received: from mga05.intel.com (mga05.intel.com [192.55.52.43]) by mails.dpdk.org (Postfix) with ESMTP id 9752040041 for ; Fri, 10 Dec 2021 10:30:16 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=simple/simple; d=intel.com; i=@intel.com; q=dns/txt; s=Intel; t=1639128616; x=1670664616; h=date:from:to:cc:subject:message-id:references: mime-version:in-reply-to; bh=RdWlElxctQH88lf60cwNVnjRSBmN69vI9TNufz/5wlc=; b=ivQ8/Ku6HgSdyTbtPClZyXFHnfIGkvWOGnWlzVdwliqes3G9Ew6YuuOk +3gBp0k/945dd09ocLus5LPvn9j/pqxd5R75OLTJL8Rl2HwwklrYpHygU 00l96sxzw2dquRZciTigFCw+662TEamGyD2MG1ncU1Xiix/GnYcuvDCIN Tcvwiw3Q7IT2cwhgVx0eH1VR8VFO1FlpJSjEl0wPoJc5BeaAzhfSBuCby V5stdKMHovIIrLzJh7Qo4TiqVm0vmVJ3l99G4cGlzdxxlmJuFBXKRD79j 3qGephoDvC9XeXC0ZD3BjR/vaVxM+J9NSDCKTM5eG+Pay2hStVeJptZLc w==; X-IronPort-AV: E=McAfee;i="6200,9189,10193"; a="324578059" X-IronPort-AV: E=Sophos;i="5.88,195,1635231600"; d="scan'208";a="324578059" Received: from orsmga003.jf.intel.com ([10.7.209.27]) by fmsmga105.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 10 Dec 2021 01:30:15 -0800 X-IronPort-AV: E=Sophos;i="5.88,195,1635231600"; d="scan'208";a="462481505" Received: from moiraric-mobl.ger.corp.intel.com (HELO bricha3-MOBL.ger.corp.intel.com) ([10.252.20.13]) by orsmga003-auth.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-SHA; 10 Dec 2021 01:30:12 -0800 Date: Fri, 10 Dec 2021 09:30:09 +0000 From: Bruce Richardson To: Dmitry Kozlyuk Cc: Aaron Conole , Jerin Jacob , Jie Zhou , dpdk-dev , roretzla@microsoft.com, Narcisa Ana Maria Vasile , "Dmitry Malloy (MESHCHANINOV)" , Pallavi Kadam , talshn@nvidia.com, Thomas Monjalon Subject: Re: [PATCH v14 04/11] app/test: skip interrupt tests on Windows Message-ID: References: <1638928262-13177-1-git-send-email-jizh@linux.microsoft.com> <1638990000-3228-1-git-send-email-jizh@linux.microsoft.com> <1638990000-3228-5-git-send-email-jizh@linux.microsoft.com> <20211210122359.17a1be53@sovereign> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20211210122359.17a1be53@sovereign> 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 Fri, Dec 10, 2021 at 12:23:59PM +0300, Dmitry Kozlyuk wrote: > 2021-12-09 16:39 (UTC+0000), Bruce Richardson: > > On Thu, Dec 09, 2021 at 04:17:08PM +0000, Bruce Richardson wrote: > > [...] > > > I'm wondering if a reasonable compromise solution might be to have the > > > build system expose a usable RTE_EXEC_ENV symbol that can be used in C-code > > > if statements rather than just in ifdefs. That would allow us to easily add > > > e.g. > > > > > > if (RTE_EXEC_ENV == rte_env_linux) > > > return TEST_SKIPPED; > > > > > > into each test function needing it. Two lines of C-code is a lot easier to > > > add and manage than #ifdefs covering the whole file, or alternative lists > > > in meson. > > > > > Quick patch to allow C-code comparisons: > > > > diff --git a/lib/eal/meson.build b/lib/eal/meson.build > > index 1722924f67..b5b9fa14b4 100644 > > --- a/lib/eal/meson.build > > +++ b/lib/eal/meson.build > > @@ -10,6 +10,12 @@ if not is_windows > > subdir('unix') > > endif > > > > +exec_envs = {'freebsd': 0, 'linux': 1, 'windows': 2} > > +foreach env, id:exec_envs > > + dpdk_conf.set('RTE_ENV_' + env.to_upper(), id) > > +endforeach > > +dpdk_conf.set('RTE_EXEC_ENV', exec_envs[exec_env]) > > + > > dpdk_conf.set('RTE_EXEC_ENV_' + exec_env.to_upper(), 1) > > subdir(exec_env) > > > > A slightly simpler patch would just expose the environment as a string as > > e.g. "linux", but I think numeric ids just make the code better rather than > > having string comparisons. Alternatively, this could also be done via > > C-code with ifdefs in EAL, but as it stands this meson change allows: > > > > if (RTE_EXEC_ENV == RTE_ENV_WINDOWS) > > ... > > > > or: > > > > switch (RTE_EXEC_ENV) { > > case RTE_ENV_LINUX: ... ; break; > > case RTE_ENV_FREEBSD: ... ; break; > > case RTE_ENV_WINDOWS: ... ; break; > > } > > > > Thoughts? > > I like this. > Even outside of tests more code can be made to compile on all platforms > (e.g. ixgbe_wait_for_link_up). > Alternative naming: RTE_EXEC_ENV_IS_* (similar to RTE_CC_IS_*), > which does not allow switch statements, but shortens most practical cases. Sure. I wonder if it is worthwhile implementing both, since it's not a large amount of code. > Will Coverity understand that if a condition is always false, > variables beneath still may be used on another platform? That I don't know, unfortunately. Perhaps some coverity experts can weigh in. /Bruce