> 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? 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. 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). -- Dmitry Kozlyuk
