Re: [PATCH v14 04/11] app/test: skip interrupt tests on Windows
From: Bruce Richardson <hidden>
Date: 2021-12-10 09:30:18
On Fri, Dec 10, 2021 at 12:23:59PM +0300, Dmitry Kozlyuk wrote:
2021-12-09 16:39 (UTC+0000), Bruce Richardson:quoted
On Thu, Dec 09, 2021 at 04:17:08PM +0000, Bruce Richardson wrote: [...]quoted
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