mbox series

[v4,0/7] add checking of header includes

Message ID 20210126141857.1029916-1-bruce.richardson@intel.com (mailing list archive)
Headers
Series add checking of header includes |

Message

Bruce Richardson Jan. 26, 2021, 2:18 p.m. UTC
  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.

V4:
* Fixed build errors with clang and arm builds
* Added support for running chkincs as part of github actions CI

V3:
* Shrunk patchset as most header fixes already applied
* Moved chkincs from "apps" to the "buildtools" directory, which is a
  better location for something not for installation for end-user use.
* Added patch to drop check-includes script.

V2:
* Add maintainers file entry for new app
* Drop patch for c11 ring header
* Use build variable "headers_no_chkincs" for tracking exceptions


Bruce Richardson (7):
  eal: add missing include to mcslock
  eal: fix error attribute use for clang
  rib: fix missing header include
  build: separate out headers for include checking
  buildtools/chkincs: add app to verify header includes
  devtools: remove check-includes script
  ci: add checking of includes to CI builds

 .ci/linux-build.sh                           |   1 +
 MAINTAINERS                                  |   5 +-
 buildtools/chkincs/gen_c_file_for_header.py  |  12 +
 buildtools/chkincs/main.c                    |   4 +
 buildtools/chkincs/meson.build               |  41 +++
 devtools/check-includes.sh                   | 259 -------------------
 devtools/test-meson-builds.sh                |   2 +-
 doc/guides/contributing/coding_style.rst     |  12 +
 lib/librte_eal/include/generic/rte_mcslock.h |   1 +
 lib/librte_eal/include/meson.build           |   2 +-
 lib/librte_eal/include/rte_compat.h          |   8 +-
 lib/librte_eal/x86/include/meson.build       |  14 +-
 lib/librte_ethdev/meson.build                |   4 +-
 lib/librte_hash/meson.build                  |   4 +-
 lib/librte_ipsec/meson.build                 |   3 +-
 lib/librte_lpm/meson.build                   |   2 +-
 lib/librte_regexdev/meson.build              |   2 +-
 lib/librte_rib/rte_rib6.h                    |   1 +
 lib/librte_ring/meson.build                  |   4 +-
 lib/librte_stack/meson.build                 |   4 +-
 lib/librte_table/meson.build                 |   7 +-
 lib/meson.build                              |   3 +
 meson.build                                  |   6 +
 meson_options.txt                            |   2 +
 24 files changed, 122 insertions(+), 281 deletions(-)
 create mode 100755 buildtools/chkincs/gen_c_file_for_header.py
 create mode 100644 buildtools/chkincs/main.c
 create mode 100644 buildtools/chkincs/meson.build
 delete mode 100755 devtools/check-includes.sh

--
2.27.0