From mboxrd@z Thu Jan 1 00:00:00 1970 From: Adrien Mazarguil Subject: Re: [dpdk-dev, 07/10] lib: fix missing include dependencies Date: Wed, 6 Apr 2016 10:54:14 +0200 Message-ID: <20160406085414.GE28696@6wind.com> References: <1459865290-10248-8-git-send-email-adrien.mazarguil@6wind.com> <20160405222304.20a38b6d@pcviktorin.fit.vutbr.cz> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Cc: dev@dpdk.org, Bruce Richardson , Thomas Monjalon To: Jan Viktorin Return-path: Received: from mail-wm0-f48.google.com (mail-wm0-f48.google.com [74.125.82.48]) by dpdk.org (Postfix) with ESMTP id BCDD02C06 for ; Wed, 6 Apr 2016 10:54:27 +0200 (CEST) Received: by mail-wm0-f48.google.com with SMTP id u206so35778840wme.1 for ; Wed, 06 Apr 2016 01:54:27 -0700 (PDT) Content-Disposition: inline In-Reply-To: <20160405222304.20a38b6d@pcviktorin.fit.vutbr.cz> List-Id: patches and discussions about DPDK List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" 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 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 > > > > --- > [...] > > + > > #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 > > +#include > > 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 > > 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 > > Same here. Same reason here. -- Adrien Mazarguil 6WIND