Re: [PATCH] build/eal: add OS defines for C conditional checks

[Bruce Richardson <bruce.richardson@intel.com>]

On Tue, Dec 14, 2021 at 01:58:03AM +0300, Dmitry Kozlyuk wrote:
> 2021-12-10 14:53 (UTC+0000), Bruce Richardson:
> [...]
> 
> Acked-by: Dmitry Kozlyuk <dmitry.kozliuk@gmail.com>
> with one typo below and some considerations for the future in the bottom.
> 
> > +Defines to Avoid Conditional Compilation
> > +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> > +
> > +In many cases in DPDK, one wants to optionally compile code based on the target platform,
> > +or runtime environment.
> 
> "Compile" -> "run", that's the point to use conditionals instead of macros.
> 
> > +While this can be done using the conditional compilation directives,
> > +e.g. ``#ifdef RTE_EXEC_ENV_LINUX``, present in DPDK for many releases,
> > +this can also be done in many cases using regular ``if`` statements and the following defines:
> 
> > +
> > +* ``RTE_ENV_FREEBSD``, ``RTE_ENV_LINUX``, ``RTE_ENV_WINDOWS`` - these define ids for each operating system environment.
> > +* ``RTE_EXEC_ENV`` - this defines the id of the current environment, i.e. one of the items in list above.
> > +* ``RTE_EXEC_ENV_IS_FREEBSD``, ``RTE_EXEC_ENV_IS_LINUX``, ``RTE_EXEC_ENV_IS_WINDOWS`` - 0/1 values indicating if the current environment is that specified,
> > +  shortcuts for checking e.g. ``RTE_EXEC_ENV == RTE_ENV_WINDOWS``
> [...]
> 
> I wonder whether #if RTE_EXEC_ENV_IS_xxx
> should be preferred over #ifdef RTE_EXEC_ENV_xxx,
> so that all checks use the same symbol
> (and then remove old macros one day).
> 
> Since C conditionals are preferred over #ifdef,
> I suggest to give pointers when to use one or another mechanism:
> 
> 	If a code fragment can compile on all platforms,
> 	but cannot run on some due to lack of support,
> 	branch on constants.
> 
> 	If a code fragment cannot compile on all platforms
> 	(e.g. use of OS-specific headers or macros),
> 	but constitutes only a small fraction of the file,
> 	use conditional compilation.
> 
> 	If a group of functions implement an interface
> 	in an OS- or platform-specific way,
> 	create a file for each of the supported environments
> 	and plug an appropriate file from ``meson.build``.

Good suggestions. I rework into a v2 when I get the chance.

/Bruce