On Fri, Jan 29, 2021 at 5:48 PM Bruce Richardson <bruce.richard...@intel.com> wrote: > > As a general principle, each header file should include any other > headers it needs to provide data type definitions or macros. For > example, any header using the uintX_t types in structures or function > prototypes should include "stdint.h" to provide those type definitions. > > In practice, while many, but not all, headers in DPDK did include all > necessary headers, it was never actually checked that each header could > be included in a C file and compiled without having any compiler errors > about missing definitions. The script "check-includes.sh" could be used > for this job, but it was not called out in the documentation, so many > contributors may not have been aware of it's existance. It also was > difficult to run from a source-code directory, as the script did not > automatically allow finding of headers from one DPDK library directory > to another [this was probably based on running it on a build created by > the "make" build system, where all headers were in a single directory]. > To attempt to have a build-system integrated replacement, this patchset > adds a "chkincs" app in the buildtools directory to verify this on an > ongoing basis. > > This chkincs app does nothing when run, and is not installed as part of > a DPDK "ninja install", it's for build-time checking only. Its source > code consists of one C file per public DPDK header, where that C file > contains nothing except an include for that header. Therefore, if any > header is added to the lib folder which fails to compile when included > alone, the build of chkincs will fail with a suitable error message. > Since this compile checking is not needed on most builds of DPDK, the > building of chkincs is disabled by default, but can be enabled by the > "test_includes" meson option. To catch errors with patch submissions, > the final patch of this series enables it for a single build in > test-meson-builds script. > > Future work could involve doing similar checks on headers for C++ > compatibility, which was something done by the check-includes.sh script > but which is missing here. > > V7: > * fixed issue with compilers not supporting __has_attribute > * added patch to add missing headers to power library .h file > * added patch to make eventdev headers non-public > * renamed headers_no_chkincs to indirect_headers
I squashed patch 7/8 as commented by Thomas. The series looks good. Series applied, thanks Bruce. -- David Marchand