2021-12-10 14:53 (UTC+0000), Bruce Richardson:
[...]

Acked-by: Dmitry Kozlyuk <dmitry.kozl...@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``.

Reply via email to