From mboxrd@z Thu Jan 1 00:00:00 1970 From: Bruce Richardson Subject: Re: [PATCH v1 5/6] fix missing includes in exported headers Date: Thu, 21 Dec 2017 14:12:57 +0000 Message-ID: <20171221141257.GB8256@bricha3-MOBL3.ger.corp.intel.com> References: <20171221122458.811-1-adrien.mazarguil@6wind.com> <20171221122458.811-6-adrien.mazarguil@6wind.com> Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit Cc: dev@dpdk.org, Thomas Monjalon To: Adrien Mazarguil Return-path: Received: from mga05.intel.com (mga05.intel.com [192.55.52.43]) by dpdk.org (Postfix) with ESMTP id 1D0391B282 for ; Thu, 21 Dec 2017 15:13:01 +0100 (CET) Content-Disposition: inline In-Reply-To: <20171221122458.811-6-adrien.mazarguil@6wind.com> List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" On Thu, Dec 21, 2017 at 02:00:04PM +0100, Adrien Mazarguil wrote: > Many exported headers rely on definitions found in rte_config.h without > including it, as shown by the following command: > > grep -L '^#include ' -- \ > $(grep -Rl \ > $(sed -n '/^#define \([^ ]\+\).*$/{s//\1/;H;};${x;s/\n//;s/\n/\\|/g;p;}' \ > build/include/rte_config.h) \ > -- build/include/) > > We cannot assume external applications will include rte_config.h on their > own, neither directly nor through a -include parameter like DPDK does > internally. > > This not only causes obvious compilation failures that can be reproduced > with check-includes.sh such as: > > [...]/rte_memory.h:88:43: error: ‘RTE_CACHE_LINE_SIZE’ was not declared in > this scope > #define __rte_cache_aligned __rte_aligned(RTE_CACHE_LINE_SIZE) > ^ > > It also results in less visible issues, for instance rte_hash_crc.h relying > on RTE_ARCH_X86_64's presence to provide dedicated inline functions. > > This patch partially reverts the commit below and adds missing include > lines to the remaining files. > > Fixes: f1a7a5c5f404 ("remove include of generated config header") > Cc: Thomas Monjalon > > Signed-off-by: Adrien Mazarguil > --- Hi Adrien, Just FYI, when we move to the new DPDK build system and pass the necessary build meta-data to the application using pkg-config, this should be a non-issue, as the pkg-config information will include the "-include rte_config.h" parameter. When investigating that, I also tried this approach of adding rte_config to files explicitly but it did not work for me as expected, as there were cases where the build was depending upon the rte_config.h always being the first include in the file. Normally, the rte_* headers should be last included, so putting it at the top just didn't seem right to me. I don't remember the specifics, but it was something like using the RTE_ defines to determine which system header file to use e.g. BSD vs Linux. However, this may be an internal DPDK-build restriction rather than one that would affect user-apps our or examples. So, with transitioning to meson and pkg-config, this issue becomes less significant. /Bruce