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 35C52459BC; Tue, 17 Sep 2024 11:30:10 +0200 (CEST) Received: from mails.dpdk.org (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 0987F40261; Tue, 17 Sep 2024 11:30:10 +0200 (CEST) Received: from mail.lysator.liu.se (mail.lysator.liu.se [130.236.254.3]) by mails.dpdk.org (Postfix) with ESMTP id 20FF14025F for ; Tue, 17 Sep 2024 11:30:08 +0200 (CEST) Received: from mail.lysator.liu.se (localhost [127.0.0.1]) by mail.lysator.liu.se (Postfix) with ESMTP id 5E167197CC for ; Tue, 17 Sep 2024 11:30:06 +0200 (CEST) Received: by mail.lysator.liu.se (Postfix, from userid 1004) id 51A421975B; Tue, 17 Sep 2024 11:30:06 +0200 (CEST) X-Spam-Checker-Version: SpamAssassin 4.0.0 (2022-12-13) on hermod.lysator.liu.se X-Spam-Level: X-Spam-Status: No, score=-1.2 required=5.0 tests=ALL_TRUSTED,AWL, T_SCC_BODY_TEXT_LINE autolearn=disabled version=4.0.0 X-Spam-Score: -1.2 Received: from [192.168.1.86] (h-62-63-215-114.A163.priv.bahnhof.se [62.63.215.114]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by mail.lysator.liu.se (Postfix) with ESMTPSA id AA93F19758; Tue, 17 Sep 2024 11:30:02 +0200 (CEST) Message-ID: <8a06d4ce-0058-4a16-b89e-4a8b2bed77f3@lysator.liu.se> Date: Tue, 17 Sep 2024 11:30:02 +0200 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH v6 1/6] dpdk: do not force C linkage on include file dependencies To: David Marchand , =?UTF-8?Q?Mattias_R=C3=B6nnblom?= Cc: dev@dpdk.org, Heng Wang , Stephen Hemminger , Tyler Retzlaff , =?UTF-8?Q?Morten_Br=C3=B8rup?= , Jack Bond-Preston , Chengwen Feng References: <20240910062051.699096-2-mattias.ronnblom@ericsson.com> <20240910083139.699291-1-mattias.ronnblom@ericsson.com> <20240910083139.699291-2-mattias.ronnblom@ericsson.com> Content-Language: en-US From: =?UTF-8?Q?Mattias_R=C3=B6nnblom?= In-Reply-To: Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit X-Virus-Scanned: ClamAV using ClamSMTP 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 On 2024-09-16 14:05, David Marchand wrote: > Hello, > > On Tue, Sep 10, 2024 at 10:41 AM Mattias Rönnblom > wrote: >> diff --git a/lib/acl/rte_acl_osdep.h b/lib/acl/rte_acl_osdep.h >> index 3c1dc402ca..e4c7d07c69 100644 >> --- a/lib/acl/rte_acl_osdep.h >> +++ b/lib/acl/rte_acl_osdep.h >> @@ -5,10 +5,6 @@ >> #ifndef _RTE_ACL_OSDEP_H_ >> #define _RTE_ACL_OSDEP_H_ >> >> -#ifdef __cplusplus >> -extern "C" { >> -#endif >> - >> /** >> * @file >> * >> @@ -49,6 +45,10 @@ extern "C" { >> #include >> #include >> >> +#ifdef __cplusplus >> +extern "C" { >> +#endif >> + >> #ifdef __cplusplus >> } >> #endif > > This part is a NOOP, so we can just drop it. > I did try to drop such NOOPs, but then something called sanitycheckcpp.exe failed the build because it required 'extern "C"' in those header files. Isn't that check superfluous? A missing 'extern "C"' would be detected at a later stage, when the dummy C++ programs are compiled against the public header files. If we agree santifycheckcpp.exe should be fixed, is that a separate patch or need it be a part of this patch set? > I found this occurence in other files of the patch. > > $ git show lib/ | grep -E '^ .*__cplusplus|diff' | grep -B1 > __cplusplus | sed -ne 's/^diff --git a\/\(.*\) b\/.*$/\1/p' | while > read file; do git show -- $file | tr '\n' ' ' | grep -q ' +#ifdef > __cplusplus +extern "C" { +#endif + #ifdef __cplusplus' && echo > $file; done > lib/acl/rte_acl_osdep.h > lib/eal/arm/include/rte_cpuflags_32.h > lib/eal/arm/include/rte_cpuflags_64.h > lib/eal/arm/include/rte_power_intrinsics.h > lib/eal/loongarch/include/rte_cpuflags.h > lib/eal/loongarch/include/rte_power_intrinsics.h > lib/eal/ppc/include/rte_cpuflags.h > lib/eal/ppc/include/rte_power_intrinsics.h > lib/eal/riscv/include/rte_cpuflags.h > lib/eal/riscv/include/rte_power_intrinsics.h > lib/eal/x86/include/rte_cpuflags.h > lib/eal/x86/include/rte_power_intrinsics.h > lib/ipsec/rte_ipsec.h > lib/pdcp/rte_pdcp.h > lib/ring/rte_ring_elem.h > > >> diff --git a/lib/eal/arm/include/rte_io.h b/lib/eal/arm/include/rte_io.h >> index f4e66e6bad..658768697c 100644 >> --- a/lib/eal/arm/include/rte_io.h >> +++ b/lib/eal/arm/include/rte_io.h >> @@ -5,14 +5,14 @@ >> #ifndef _RTE_IO_ARM_H_ >> #define _RTE_IO_ARM_H_ >> >> -#ifdef __cplusplus >> -extern "C" { >> -#endif >> - >> #ifdef RTE_ARCH_64 >> #include "rte_io_64.h" >> #else >> #include "generic/rte_io.h" >> + >> +#ifdef __cplusplus >> +extern "C" { >> +#endif >> #endif > > I suspect it is the reason for the CI build error on ARM. > This block should be out of the #endif, but then with the next lines, > it ends up as a noop. > >> >> #ifdef __cplusplus > > >> diff --git a/lib/eal/arm/include/rte_pause.h b/lib/eal/arm/include/rte_pause.h >> index 6c7002ad98..8f35d60a6e 100644 >> --- a/lib/eal/arm/include/rte_pause.h >> +++ b/lib/eal/arm/include/rte_pause.h >> @@ -5,14 +5,14 @@ >> #ifndef _RTE_PAUSE_ARM_H_ >> #define _RTE_PAUSE_ARM_H_ >> >> -#ifdef __cplusplus >> -extern "C" { >> -#endif >> - >> #ifdef RTE_ARCH_64 >> #include >> #else >> #include >> + >> +#ifdef __cplusplus >> +extern "C" { >> +#endif >> #endif > > Idem, probably breaking build for ARM. > > >> diff --git a/lib/eal/include/generic/rte_atomic.h b/lib/eal/include/generic/rte_atomic.h >> index f859707744..0a4f3f8528 100644 >> --- a/lib/eal/include/generic/rte_atomic.h >> +++ b/lib/eal/include/generic/rte_atomic.h >> @@ -17,6 +17,10 @@ >> #include >> #include >> >> +#ifdef __cplusplus >> +extern "C" { >> +#endif >> + >> #ifdef __DOXYGEN__ >> >> /** @name Memory Barrier >> @@ -1156,4 +1160,8 @@ rte_atomic128_cmp_exchange(rte_int128_t *dst, >> >> #endif /* __DOXYGEN__ */ >> >> +#ifdef __cplusplus >> +} >> +#endif >> + > > I would move under #ifdef DOXYGEN. > Why? The pattern now is "almost always directly after the #includes". That is better than before, but not ideal. C linkage should only be covering functions and global variables declared, I think. > The rest looks good to me. > > Thanks for the help!