On Wed, 6 Apr 2016 10:54:14 +0200 Adrien Mazarguil <adrien.mazarguil at 6wind.com> wrote:
> Hi Jan, > > Replying below as well. > [...] > > > --- a/lib/librte_eal/common/include/arch/arm/rte_byteorder.h > > > +++ b/lib/librte_eal/common/include/arch/arm/rte_byteorder.h > > > @@ -37,6 +37,9 @@ > > > # error Platform must be built with CONFIG_RTE_FORCE_INTRINSICS > > > #endif > > > > > > +#include <stdint.h> > > > +#include <rte_common.h> > > > > Why not to place it into the extern "C" { block? There is already: > > > > #include "generic/rte_byteorder.h" > > Right, I did not do it because headers may eventually contain C++ > compatibility code someday, so I think we should avoid #includes inside > extern "C" blocks. C++ compliant headers should provide their own blocks, > also I'm not sure how well it mixes with system includes having their own > compatibility layer. > > I agree we need consistency, so what about a commit to move all #includes > outside of such blocks instead? Yes, I agree. > > > > +#include <rte_common.h> > > > > I don't see any reason for this. The header does not use anything > > special. Just "asm", but that should be a keyword... > > Unfortunately it's a nonstandard keyword which is defined as __asm__ in > rte_common.h, itself an extension keyword compilers will swallow without > complaining thanks to these "__". OK. > > > > #ifdef __cplusplus > > > extern "C" { > > > #endif > > > diff --git a/lib/librte_eal/common/include/arch/arm/rte_prefetch_64.h > > > b/lib/librte_eal/common/include/arch/arm/rte_prefetch_64.h > > > index 3ed46a4..600c6f0 100644 > > > --- a/lib/librte_eal/common/include/arch/arm/rte_prefetch_64.h > > > +++ b/lib/librte_eal/common/include/arch/arm/rte_prefetch_64.h > > > @@ -33,6 +33,8 @@ > > > #ifndef _RTE_PREFETCH_ARM_64_H_ > > > #define _RTE_PREFETCH_ARM_64_H_ > > > > > > +#include <rte_common.h> > > > > Same here. > > Same reason here. OK. Regards Jan