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 BFB11A052A; Mon, 25 Jan 2021 16:51:40 +0100 (CET) Received: from [217.70.189.124] (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 4581E140FA9; Mon, 25 Jan 2021 16:51:40 +0100 (CET) Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [216.205.24.124]) by mails.dpdk.org (Postfix) with ESMTP id 328EC140F8C for ; Mon, 25 Jan 2021 16:51:37 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1611589896; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=qz6GRGSH/j+dzEsSyJF4tniu6LAw0BcHAnKInXBsQwg=; b=HbS2pF73hBXrTAAWvYpuoC58mLnFHOMHTnBv3q1JzdDS666LPJdjh/ZP97OS0inaWfrH+1 hVvN5LGXWKL5Qxle9af+YMBRWGzBWj913L+QCtwL8Qi2Z9c/Yg8SsSPXMUPhjBT4b23/ES 812zphTxCqKKenAcz6feAS7JuQuZ2Dk= Received: from mail-vk1-f198.google.com (mail-vk1-f198.google.com [209.85.221.198]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-341-U3hsv6N_ORajQRjPjPv3tA-1; Mon, 25 Jan 2021 10:51:32 -0500 X-MC-Unique: U3hsv6N_ORajQRjPjPv3tA-1 Received: by mail-vk1-f198.google.com with SMTP id o62so566572vka.6 for ; Mon, 25 Jan 2021 07:51:32 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc:content-transfer-encoding; bh=qz6GRGSH/j+dzEsSyJF4tniu6LAw0BcHAnKInXBsQwg=; b=ivoYN2nkyUIZAep1fZbDOyFjhEKkYRJRXD0W7yYsvDzcCe7BlQUlK0k33ZkAgvIq01 lN6VxYAIWOXB56VqHZhCUumJlAzb2X8qQSMjCwIRkwAiO0MYwH+YflTM7pNaPHvyfUEV J2qN972pr7UWlWjecEbLtV+0J3UEXe+1Ud/bNKxn2jQ5BV9wV59Z7cGS1PLdw3odNYCC tMM/VKKh31jdmi5WD9wQVvicwd4L1/vYhmvl3llBY+VnaEdgTS+aNJ0hoRKxNpOzGbkA 0jU7axuAKZUwp7B/vvxoURTq9gamNWDPsH6eEPHuJZ5yWqJRmRaTL9+4QtSwYdtlIHF2 AHgg== X-Gm-Message-State: AOAM530M2zZtixEFJeeX35fLiuvOqnEzBFYxZ6g1L1AbnVldThxMuAGS /RGk46MEkWz2NtKuxWXYqh8Rk4WTFCB9UFm/4mwA8fcT8YeOya3g4IuEUZlbpbw4+KCFM/7lpwe sfFa4iNcjpLrrYTTck3Q= X-Received: by 2002:a67:6943:: with SMTP id e64mr1304446vsc.10.1611589891784; Mon, 25 Jan 2021 07:51:31 -0800 (PST) X-Google-Smtp-Source: ABdhPJyG5P/iVOWp8nka0X2QpZqbzRU8IBAvdOgP8RUY4Qjf6m9C1F0EduGKEp9T2RB1pVLseebcr0/tELEpYVtqkOE= X-Received: by 2002:a67:6943:: with SMTP id e64mr1304424vsc.10.1611589891359; Mon, 25 Jan 2021 07:51:31 -0800 (PST) MIME-Version: 1.0 References: <20210114110606.21142-1-bruce.richardson@intel.com> <20210125141115.573122-1-bruce.richardson@intel.com> In-Reply-To: <20210125141115.573122-1-bruce.richardson@intel.com> From: David Marchand Date: Mon, 25 Jan 2021 16:51:19 +0100 Message-ID: To: Bruce Richardson Cc: dev , Thomas Monjalon Authentication-Results: relay.mimecast.com; auth=pass smtp.auth=CUSA124A263 smtp.mailfrom=dmarchan@redhat.com X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable Subject: Re: [dpdk-dev] [PATCH v3 0/4] add checking of header 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 Mon, Jan 25, 2021 at 3: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. 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.. > > 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 (4): > eal: add missing include to mcslock > build: separate out headers for include checking > buildtools/chkincs: add app to verify header includes > devtools: remove check-includes script > > MAINTAINERS | 5 +- > buildtools/chkincs/gen_c_file_for_header.py | 12 + > buildtools/chkincs/main.c | 4 + > buildtools/chkincs/meson.build | 40 +++ > 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/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_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 + > 21 files changed, 112 insertions(+), 280 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 - clang is not happy when enabling the check: $ meson configure $HOME/builds/build-clang-static -Dcheck_includes=3Dtrue $ devtools/test-meson-builds.sh ... [362/464] Compiling C object buildtools/chkincs/chkincs.p/meson-generated_rte_ethdev_vdev.c.o FAILED: buildtools/chkincs/chkincs.p/meson-generated_rte_ethdev_vdev.c.o clang -Ibuildtools/chkincs/chkincs.p -Ibuildtools/chkincs -I../../dpdk/buildtools/chkincs -Idrivers/bus/pci -I../../dpdk/drivers/bus/pci -Idrivers/bus/vdev -I../../dpdk/drivers/bus/vdev -I. -I../../dpdk -Iconfig -I../../dpdk/config -Ilib/librte_eal/include -I../../dpdk/lib/librte_eal/include -Ilib/librte_eal/linux/include -I../../dpdk/lib/librte_eal/linux/include -Ilib/librte_eal/x86/include -I../../dpdk/lib/librte_eal/x86/include -Ilib/librte_kvargs -I../../dpdk/lib/librte_kvargs -Ilib/librte_metrics -I../../dpdk/lib/librte_metrics -Ilib/librte_telemetry -I../../dpdk/lib/librte_telemetry -Ilib/librte_eal/common -I../../dpdk/lib/librte_eal/common -Ilib/librte_eal -I../../dpdk/lib/librte_eal -Ilib/librte_ring -I../../dpdk/lib/librte_ring -Ilib/librte_rcu -I../../dpdk/lib/librte_rcu -Ilib/librte_mempool -I../../dpdk/lib/librte_mempool -Ilib/librte_mbuf -I../../dpdk/lib/librte_mbuf -Ilib/librte_net -I../../dpdk/lib/librte_net -Ilib/librte_meter -I../../dpdk/lib/librte_meter -Ilib/librte_ethdev -I../../dpdk/lib/librte_ethdev -Ilib/librte_pci -I../../dpdk/lib/librte_pci -Ilib/librte_cmdline -I../../dpdk/lib/librte_cmdline -Ilib/librte_hash -I../../dpdk/lib/librte_hash -Ilib/librte_timer -I../../dpdk/lib/librte_timer -Ilib/librte_acl -I../../dpdk/lib/librte_acl -Ilib/librte_bbdev -I../../dpdk/lib/librte_bbdev -Ilib/librte_bitratestats -I../../dpdk/lib/librte_bitratestats -Ilib/librte_cfgfile -I../../dpdk/lib/librte_cfgfile -Ilib/librte_compressdev -I../../dpdk/lib/librte_compressdev -Ilib/librte_cryptodev -I../../dpdk/lib/librte_cryptodev -Ilib/librte_distributor -I../../dpdk/lib/librte_distributor -Ilib/librte_efd -I../../dpdk/lib/librte_efd -Ilib/librte_eventdev -I../../dpdk/lib/librte_eventdev -Ilib/librte_gro -I../../dpdk/lib/librte_gro -Ilib/librte_gso -I../../dpdk/lib/librte_gso -Ilib/librte_ip_frag -I../../dpdk/lib/librte_ip_frag -Ilib/librte_jobstats -I../../dpdk/lib/librte_jobstats -Ilib/librte_kni -I../../dpdk/lib/librte_kni -Ilib/librte_latencystats -I../../dpdk/lib/librte_latencystats -Ilib/librte_lpm -I../../dpdk/lib/librte_lpm -Ilib/librte_member -I../../dpdk/lib/librte_member -Ilib/librte_power -I../../dpdk/lib/librte_power -Ilib/librte_pdump -I../../dpdk/lib/librte_pdump -Ilib/librte_rawdev -I../../dpdk/lib/librte_rawdev -Ilib/librte_regexdev -I../../dpdk/lib/librte_regexdev -Ilib/librte_rib -I../../dpdk/lib/librte_rib -Ilib/librte_reorder -I../../dpdk/lib/librte_reorder -Ilib/librte_sched -I../../dpdk/lib/librte_sched -Ilib/librte_security -I../../dpdk/lib/librte_security -Ilib/librte_stack -I../../dpdk/lib/librte_stack -Ilib/librte_vhost -I../../dpdk/lib/librte_vhost -Ilib/librte_ipsec -I../../dpdk/lib/librte_ipsec -Ilib/librte_fib -I../../dpdk/lib/librte_fib -Ilib/librte_port -I../../dpdk/lib/librte_port -Ilib/librte_table -I../../dpdk/lib/librte_table -Ilib/librte_pipeline -I../../dpdk/lib/librte_pipeline -Ilib/librte_flow_classify -I../../dpdk/lib/librte_flow_classify -Ilib/librte_bpf -I../../dpdk/lib/librte_bpf -Ilib/librte_graph -I../../dpdk/lib/librte_graph -Ilib/librte_node -I../../dpdk/lib/librte_node -I/home/dmarchan/intel-ipsec-mb/install/include -Xclang -fcolor-diagnostics -pipe -D_FILE_OFFSET_BITS=3D64 -Wall -Winvalid-pch -Werror -O2 -g -include rte_config.h -Wextra -Wcast-qual -Wdeprecated -Wformat -Wformat-nonliteral -Wformat-security -Wmissing-declarations -Wmissing-prototypes -Wnested-externs -Wold-style-definition -Wpointer-arith -Wsign-compare -Wstrict-prototypes -Wundef -Wwrite-strings -Wno-address-of-packed-member -Wno-missing-field-initializers -D_GNU_SOURCE -march=3Dnative -Wno-unused-function -DALLOW_EXPERIMENTAL_API -MD -MQ buildtools/chkincs/chkincs.p/meson-generated_rte_ethdev_vdev.c.o -MF buildtools/chkincs/chkincs.p/meson-generated_rte_ethdev_vdev.c.o.d -o buildtools/chkincs/chkincs.p/meson-generated_rte_ethdev_vdev.c.o -c buildtools/chkincs/chkincs.p/rte_ethdev_vdev.c In file included from buildtools/chkincs/chkincs.p/rte_ethdev_vdev.c:1: In file included from /home/dmarchan/dpdk/lib/librte_ethdev/rte_ethdev_vdev.h:12: ../../dpdk/lib/librte_ethdev/rte_ethdev_driver.h:964:1: error: unknown attribute 'error' ignored [-Werror,-Wunknown-attributes] __rte_internal ^ ../../dpdk/lib/librte_eal/include/rte_compat.h:25:16: note: expanded from macro '__rte_internal' __attribute__((error("Symbol is not public ABI"), \ ^ - Other issues with ARM builds (arch-specific headers probably the reason): $ meson configure $HOME/builds/build-arm64-bluefield -Dcheck_includes=3Dtru= e $ devtools/test-meson-builds.sh ... In file included from buildtools/chkincs/chkincs.p/rte_rib6.c:1: /home/dmarchan/dpdk/lib/librte_rib/rte_rib6.h: In function =E2=80=98get_msk= _part=E2=80=99: /home/dmarchan/dpdk/lib/librte_rib/rte_rib6.h:112:10: error: implicit declaration of function =E2=80=98RTE_MIN=E2=80=99; did you mean =E2=80=98IN= T8_MIN=E2=80=99? [-Werror=3Dimplicit-function-declaration] depth =3D RTE_MIN(depth, 128); ^~~~~~~ INT8_MIN /home/dmarchan/dpdk/lib/librte_rib/rte_rib6.h:112:10: error: nested extern declaration of =E2=80=98RTE_MIN=E2=80=99 [-Werror=3Dnested-externs] /home/dmarchan/dpdk/lib/librte_rib/rte_rib6.h:113:9: error: implicit declaration of function =E2=80=98RTE_MAX=E2=80=99; did you mean =E2=80=98IN= T8_MAX=E2=80=99? [-Werror=3Dimplicit-function-declaration] part =3D RTE_MAX((int16_t)depth - (byte * 8), 0); ^~~~~~~ INT8_MAX /home/dmarchan/dpdk/lib/librte_rib/rte_rib6.h:113:9: error: nested extern declaration of =E2=80=98RTE_MAX=E2=80=99 [-Werror=3Dnested-externs] cc1: all warnings being treated as errors - This check should be enabled for x86 and aarch cross build in GHA. --=20 David Marchand