14/03/2020 00:38, Dmitry Kozlyuk: > > I suggest this change (I can send a patch fixing the issue in other .h > > files): > > > > +/* > > + * RTE_TOOLCHAIN_GCC is true if the target is built with GCC, > > + * while a host application (like pmdinfogen) may have another compiler. > > + * RTE_CC_IS_GNU is true if the file is compiled with GCC, > > + * no matter it is a target or host application. > > + */ > > +#if defined __GNUC__ && !defined __clang__ && !defined __INTEL_COMPILER > > +#define RTE_CC_IS_GNU > > +#endif > > + > > +#ifdef RTE_CC_IS_GNU > > -/** Define GCC_VERSION **/ > > -#ifdef RTE_TOOLCHAIN_GCC > > #define GCC_VERSION (__GNUC__ * 10000 + __GNUC_MINOR__ * 100 + \ > > __GNUC_PATCHLEVEL__) > > #endif > > @@ -96,7 +105,7 @@ typedef uint16_t unaligned_uint16_t; > > * even if the underlying stdio implementation is ANSI-compliant, > > * so this must be overridden. > > */ > > -#if defined(RTE_TOOLCHAIN_GCC) > > +#ifdef RTE_CC_IS_GNU > > #define __rte_format_printf(format_index, first_arg) \ > > __attribute__((format(gnu_printf, format_index, first_arg))) > > #else > > The code you propose LGTM itself. If you think it's a better solution than > the one proposed below, I see no problem going with it. > > What I wonder is whether pmdinfogen should include the problematic code in the > first place. The errors come from declarations in rte_debug.h, but pmdinfogen > really can't use them, because definitions are compiled for different > machine. Pmdinfogen pulls rte_debug.h via rte_pci.h, which is only needed for > struct rte_pci_id. Shouldn't we instead break this bogus dependency chain by > moving struct rte_pci_id to a separate header?
Splitting headers to avoid EAL dependency looks to be a bad precedent to me. > EAL defines are tied to the target toolchain and are consistent with each > other, from this point of view RTE_CC_IS_GNU looks like a workaround. Yes there are some target-arch-dependent code in EAL. But a host application is not supposed to use arch-dependent code. > Another > reason why pmdinfogen should not depend on EAL is that its code will differ > considerably for POSIX and Windows (ELF vs COFF, mmap vs Win32 API). I believe managing some OS differences should be helped with EAL.
