On Fri, Dec 06, 2024 at 11:27:52AM -0800, Andre Muezerie wrote:
> MSVC compiler issues warnings like the one below:
>
> lib\dmadev\rte_dmadev_trace_fp.h(36):
> warning C5101: use of preprocessor directive in
> function-like macro argument list is undefined behavior
>
> Indeed, looking at C99 section 6.10.3 Macro replacement, paragraph 11:
> "If there are sequences of preprocessing tokens within the list of
> arguments that would otherwise act as preprocessing directives, the
> behavior is undefined."
>
> The fix proposed in this patch moves the ifdef to the outside.
> This results in no perf impact, but some lines end up being
> duplicated, which seems to be a reasonable trade-off.
>
> Signed-off-by: Andre Muezerie <andre...@linux.microsoft.com>
> ---
> lib/dmadev/rte_dmadev_trace.h | 62 ++++++++++++++++++++++++++++----
> lib/dmadev/rte_dmadev_trace_fp.h | 52 +++++++++++++++++++++++----
> 2 files changed, 102 insertions(+), 12 deletions(-)
>
> diff --git a/lib/dmadev/rte_dmadev_trace.h b/lib/dmadev/rte_dmadev_trace.h
> index be089c065c..264a464928 100644
> --- a/lib/dmadev/rte_dmadev_trace.h
> +++ b/lib/dmadev/rte_dmadev_trace.h
> @@ -19,13 +19,12 @@
> extern "C" {
> #endif
>
> +#ifdef _RTE_TRACE_POINT_REGISTER_H_
> RTE_TRACE_POINT(
> rte_dma_trace_info_get,
> RTE_TRACE_POINT_ARGS(int16_t dev_id, struct rte_dma_info *dev_info),
> -#ifdef _RTE_TRACE_POINT_REGISTER_H_
> struct rte_dma_info __dev_info = {0};
> dev_info = &__dev_info;
> -#endif /* _RTE_TRACE_POINT_REGISTER_H_ */
> rte_trace_point_emit_i16(dev_id);
> rte_trace_point_emit_string(dev_info->dev_name);
> rte_trace_point_emit_u64(dev_info->dev_capa);
> @@ -37,15 +36,30 @@ RTE_TRACE_POINT(
> rte_trace_point_emit_u16(dev_info->nb_vchans);
> rte_trace_point_emit_u16(dev_info->nb_priorities);
> )
> +#else
> +RTE_TRACE_POINT(
> + rte_dma_trace_info_get,
> + RTE_TRACE_POINT_ARGS(int16_t dev_id, struct rte_dma_info *dev_info),
> + rte_trace_point_emit_i16(dev_id);
> + rte_trace_point_emit_string(dev_info->dev_name);
> + rte_trace_point_emit_u64(dev_info->dev_capa);
> + rte_trace_point_emit_u16(dev_info->max_vchans);
> + rte_trace_point_emit_u16(dev_info->max_desc);
> + rte_trace_point_emit_u16(dev_info->min_desc);
> + rte_trace_point_emit_u16(dev_info->max_sges);
> + rte_trace_point_emit_i16(dev_info->numa_node);
> + rte_trace_point_emit_u16(dev_info->nb_vchans);
> + rte_trace_point_emit_u16(dev_info->nb_priorities);
> +)
> +#endif /* _RTE_TRACE_POINT_REGISTER_H_ */
>
+Jerin
I'm unfortunately not familiar with the internals of the traceing library.
Can someone perhaps explain (and maybe add in the code as a comment), why
we need conditional compilation here? Specifically, when would
_RTE_TRACE_POINT_REGISTER_H_ be defined, and when not defined? (or why not
defined?) How does having it defined or not affect the expansion of the
macro here?
Thanks,
/Bruce
