From: Bruce Richardson <bruce.richardson@intel.com>
To: Adrien Mazarguil <adrien.mazarguil@6wind.com>
Cc: dev@dpdk.org, Thomas Monjalon <thomas@monjalon.net>
Subject: Re: [dpdk-dev] [PATCH v1 5/6] fix missing includes in exported headers
Date: Thu, 21 Dec 2017 14:12:57 +0000 [thread overview]
Message-ID: <20171221141257.GB8256@bricha3-MOBL3.ger.corp.intel.com> (raw)
In-Reply-To: <20171221122458.811-6-adrien.mazarguil@6wind.com>
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 <rte_config.h>' -- \
> $(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 <thomas@monjalon.net>
>
> Signed-off-by: Adrien Mazarguil <adrien.mazarguil@6wind.com>
> ---
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
next prev parent reply other threads:[~2017-12-21 14:13 UTC|newest]
Thread overview: 17+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-12-21 12:59 [dpdk-dev] [PATCH v1 0/6] Address various issues with " Adrien Mazarguil
2017-12-21 12:59 ` [dpdk-dev] [PATCH v1 1/6] devtools: update check-includes exceptions Adrien Mazarguil
2017-12-21 12:59 ` [dpdk-dev] [PATCH v1 2/6] net/i40e: fix issue in exported header Adrien Mazarguil
2017-12-27 6:53 ` Xing, Beilei
2017-12-21 12:59 ` [dpdk-dev] [PATCH v1 3/6] flow_classify: " Adrien Mazarguil
2018-01-02 15:19 ` Iremonger, Bernard
2017-12-21 13:00 ` [dpdk-dev] [PATCH v1 4/6] member: " Adrien Mazarguil
2017-12-21 13:00 ` [dpdk-dev] [PATCH v1 5/6] fix missing includes in exported headers Adrien Mazarguil
2017-12-21 14:12 ` Bruce Richardson [this message]
2017-12-21 14:50 ` Adrien Mazarguil
2017-12-21 16:01 ` Bruce Richardson
2017-12-21 13:00 ` [dpdk-dev] [PATCH v1 6/6] net: fix rte_ether conflicts with libc Adrien Mazarguil
2017-12-22 13:34 ` Olivier MATZ
2017-12-22 14:25 ` Adrien Mazarguil
2018-01-16 13:04 ` Olivier Matz
2018-01-16 23:19 ` Thomas Monjalon
2018-01-16 23:18 ` [dpdk-dev] [PATCH v1 0/6] Address various issues with exported headers Thomas Monjalon
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20171221141257.GB8256@bricha3-MOBL3.ger.corp.intel.com \
--to=bruce.richardson@intel.com \
--cc=adrien.mazarguil@6wind.com \
--cc=dev@dpdk.org \
--cc=thomas@monjalon.net \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).