Re: [dpdk-dev] [PATCH v2 00/19] ensure headers have correct includes

On Thu, Jan 21, 2021 at 10:25:07AM +0100, David Marchand wrote:

On Fri, Jan 15, 2021 at 12:11 PM Bruce Richardson
[off-list ref] wrote:

quoted

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. This patchset fixes any missing includes in
public headers from the DPDK "lib" folder and then adds a "chkincs" app.
to verify this on an ongoing basis.

This chkincs app does nothing when run, 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,
for example.

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 (19):
  eal: fix missing header inclusion

I split this as two patches since the rte_thread.h only applies to 21.02.

quoted

  telemetry: fix missing header include
  ethdev: fix missing header include

Patch 3 seems unnecessary, we skip rte_eth_ctrl.h validation later but
I took it anyway.

quoted

  net: fix missing header include
  mbuf: fix missing header include

Is rte_byteorder.h inclusion in rte_mbuf.h still necessary?

Good question. The checks I was doing were only for missing headers.
Checking for superfluous headers is more complicated and not something I've
looked at. I know it was previously suggested to use the
"include-what-you-use" tool on DPDK, if anyone wants to attempt that.

quoted

  bitratestats: fix missing header include
  rib: fix missing header includes
  vhost: fix missing header includes
  ipsec: fix missing header include
  fib: fix missing header includes
  table: fix missing header include
  pipeline: fix missing header includes
  metrics: fix variable declaration in header
  node: fix missing header include
  app: fix extra include paths for app builds

Reviewed and applied patches 1-15, thanks Bruce.

Thanks for the bits of rework, David.