From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mails.dpdk.org (mails.dpdk.org [217.70.189.124]) by inbox.dpdk.org (Postfix) with ESMTP id 706DCA09E4; Thu, 21 Jan 2021 10:33:59 +0100 (CET) Received: from [217.70.189.124] (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 33F26140EF0; Thu, 21 Jan 2021 10:33:59 +0100 (CET) Received: from mga09.intel.com (mga09.intel.com [134.134.136.24]) by mails.dpdk.org (Postfix) with ESMTP id EEE69140CC4 for ; Thu, 21 Jan 2021 10:33:57 +0100 (CET) IronPort-SDR: czrK0xUPyIRFAHQOAo31UA/puZRgmtc62dM2Dnx47DSsI1ob2q9V/rEnN80xJpGsHrpZYKtEQY wvMc4TjyKCQw== X-IronPort-AV: E=McAfee;i="6000,8403,9870"; a="179393875" X-IronPort-AV: E=Sophos;i="5.79,363,1602572400"; d="scan'208";a="179393875" Received: from orsmga008.jf.intel.com ([10.7.209.65]) by orsmga102.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 21 Jan 2021 01:33:56 -0800 IronPort-SDR: 14VyV2EqQ7bloktPkTsOH7lUzPx27+/1G3ml88pwyF2U/BEEs4Re6+PWnB1c18AdKxZGMh5tk6 edsftxR+KPsA== X-IronPort-AV: E=Sophos;i="5.79,363,1602572400"; d="scan'208";a="385233872" Received: from bricha3-mobl.ger.corp.intel.com ([10.252.19.120]) by orsmga008-auth.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-SHA; 21 Jan 2021 01:33:55 -0800 Date: Thu, 21 Jan 2021 09:33:52 +0000 From: Bruce Richardson To: David Marchand Cc: dev , Thomas Monjalon Message-ID: <20210121093352.GA258@bricha3-MOBL.ger.corp.intel.com> References: <20210114110606.21142-1-bruce.richardson@intel.com> <20210115111052.16437-1-bruce.richardson@intel.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: Subject: Re: [dpdk-dev] [PATCH v2 00/19] ensure headers have correct includes X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" On Thu, Jan 21, 2021 at 10:25:07AM +0100, David Marchand wrote: > On Fri, Jan 15, 2021 at 12:11 PM Bruce Richardson > 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.