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 00814459CC; Wed, 18 Sep 2024 14:09:32 +0200 (CEST) Received: from mails.dpdk.org (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id BCB464027C; Wed, 18 Sep 2024 14:09:32 +0200 (CEST) Received: from mail.lysator.liu.se (mail.lysator.liu.se [130.236.254.3]) by mails.dpdk.org (Postfix) with ESMTP id 7FA384025C for ; Wed, 18 Sep 2024 14:09:31 +0200 (CEST) Received: from mail.lysator.liu.se (localhost [127.0.0.1]) by mail.lysator.liu.se (Postfix) with ESMTP id C881E1E943 for ; Wed, 18 Sep 2024 14:09:30 +0200 (CEST) Received: by mail.lysator.liu.se (Postfix, from userid 1004) id BC85F1E942; Wed, 18 Sep 2024 14:09:30 +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 7B5C41E941; Wed, 18 Sep 2024 14:09:27 +0200 (CEST) Message-ID: <6730daa9-bbd5-471a-8161-b7d4ecb98196@lysator.liu.se> Date: Wed, 18 Sep 2024 14:09:26 +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 , Bruce Richardson , Tyler Retzlaff Cc: =?UTF-8?Q?Mattias_R=C3=B6nnblom?= , dev@dpdk.org, Heng Wang , Stephen Hemminger , =?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> <8a06d4ce-0058-4a16-b89e-4a8b2bed77f3@lysator.liu.se> 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-18 13:15, David Marchand wrote: > On Tue, Sep 17, 2024 at 11:30 AM Mattias Rönnblom wrote: >> >> 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? > > This check was added with 1ee492bdc4ff ("buildtools/chkincs: check > missing C++ guards"). > The check is too naive, and I am not sure we can actually make a better one... > > I would remove this check, if no better option. > Just to be clear: what you are suggesting is removing the check as a part of this patch set? I think I was wrong saying the dummy C++ programs already detect omissions of C linkage. I'll leave for Bruce to comment on this before I do anything. > >>>> 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. > > I hear you about how the marking was done but it already has some > manual edits (seeing how some fixes were needed). > > I was not arguing against manual edits. I was arguing against inconsistent placement of the #ifdefs. That said, I don't know what the purpose of the ifdef DOXYGEN is.