Hi Jan,
Replying below as well.
On Tue, Apr 05, 2016 at 10:23:04PM +0200, Jan Viktorin wrote:
> Hello Adrien,
>
> just quickly skimming through the ARM fixes...
>
> On Tue, 5 Apr 2016 16:08:07 +0200
> Adrien Mazarguil <adrien.mazarguil at 6wind.com> wrote:
>
> > Exported header files for use by applications should be self sufficient and
> > allow out of order inclusion. Moreover, they must include all the system
> > headers they need for types and macros.
> >
> > This commit prevents the following errors:
> >
> > error: `RTE_MAX_LCORE' undeclared here (not in a function)
> > error: `RTE_LPM_VALID_EXT_ENTRY_BITMASK' undeclared (first use in this
> > function)
> > error: #error "Unsupported cache line size"
> > error: `asm' undeclared (first use in this function)
> > error: implicit declaration of function `[...]'
> > error: unknown type name `[...]'
> > error: field `mac_addr' has incomplete type
> > error: `CHAR_BIT' undeclared here (not in a function)
> > error: `struct timespec' declared inside parameter list
> >
> > Signed-off-by: Adrien Mazarguil <adrien.mazarguil at 6wind.com>
> >
> > ---
> [...]
> > +
> > #ifdef __cplusplus
> > extern "C" {
> > #endif
> > diff --git a/lib/librte_eal/common/include/arch/arm/rte_byteorder.h
> > b/lib/librte_eal/common/include/arch/arm/rte_byteorder.h
> > index 3f2dd1f..c2078e7 100644
> > --- 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?
> > +#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 "__".
> > #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.
--
Adrien Mazarguil
6WIND
