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

[Bruce Richardson]

On Thu, Jan 21, 2021 at 10:25:07AM +0100, David Marchand wrote:
> On Fri, Jan 15, 2021 at 12:11 PM Bruce Richardson
> <bruce.richardson at 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. 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.
> 
> 
> >   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.
> 
> >   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.

> 
> >   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.