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 1B82143A73 for ; Mon, 5 Feb 2024 18:36:57 +0100 (CET) Received: from mails.dpdk.org (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 0B67440DDA; Mon, 5 Feb 2024 18:36:57 +0100 (CET) Received: from linux.microsoft.com (linux.microsoft.com [13.77.154.182]) by mails.dpdk.org (Postfix) with ESMTP id EDBE840269; Mon, 5 Feb 2024 18:36:53 +0100 (CET) Received: by linux.microsoft.com (Postfix, from userid 1086) id E644520B2000; Mon, 5 Feb 2024 09:36:52 -0800 (PST) DKIM-Filter: OpenDKIM Filter v2.11.0 linux.microsoft.com E644520B2000 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linux.microsoft.com; s=default; t=1707154612; bh=rLp9Ru82cfyVlDnSAuCmWAZ/f4ujzdDmptUey1I66Go=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=T7fq3DjTmjoBmqseSyCwgRE9i70DMaZ1C0CmZc6/alxX8A7kHr5mbpPdWBqfwlFJG i3eVVJDRZwOX6Hla1jHanD0WORncBGlq5FGPKEachb8G8e89aFUnZbFFqU1NvtZdns NsGVxNvpiRH4ktr7zCSuN9aWVpr8ImAz4bg7lfK8= Date: Mon, 5 Feb 2024 09:36:52 -0800 From: Tyler Retzlaff To: Bruce Richardson Cc: Thomas Monjalon , Ashish Sadanandan , dev@dpdk.org, nelio.laranjeiro@6wind.com, stable@dpdk.org, honnappa.nagarahalli@arm.com, konstantin.v.ananyev@yandex.ru, david.marchand@redhat.com, ruifeng.wang@arm.com Subject: Re: [PATCH 1/1] eal: add C++ include guard in generic/rte_vect.h Message-ID: <20240205173652.GA2935@linuxonhyperv3.guj3yctzbm1etfxqx2vob5hsef.xx.internal.cloudapp.net> References: <20240202051335.776290-1-ashish.sadanandan@gmail.com> <5751086.DvuYhMxLoT@thomas> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.5.21 (2010-09-15) X-BeenThere: stable@dpdk.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: patches for DPDK stable branches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: stable-bounces@dpdk.org On Fri, Feb 02, 2024 at 09:40:59AM +0000, Bruce Richardson wrote: > On Fri, Feb 02, 2024 at 10:18:23AM +0100, Thomas Monjalon wrote: > > 02/02/2024 06:13, Ashish Sadanandan: > > > The header was missing the extern "C" directive which causes name > > > mangling of functions by C++ compilers, leading to linker errors > > > complaining of undefined references to these functions. > > > > > > Fixes: 86c743cf9140 ("eal: define generic vector types") > > > Cc: nelio.laranjeiro@6wind.com > > > Cc: stable@dpdk.org > > > > > > Signed-off-by: Ashish Sadanandan > > > > Thank you for improving C++ compatibility. > > > > I'm not sure what is best to fix it. > > You are adding extern "C" in a file which is not directly included > > by the user app. The same was done for rte_rwlock.h. > > The other way is to make sure this include is in an extern "C" block > > in lib/eal/*/include/rte_vect.h (instead of being before the block). > > > > I would like we use the same approach for all files. > > Opinions? > > > I think just having the extern "C" guard in all files is the safest choice, > because it's immediately obvious in each and every file that it is correct. > Taking the other option, to check any indirect include file you need to go > finding what other files include it and check there that a) they have > include guards and b) the include for the indirect header is contained > within it. > > Adopting the policy of putting the guard in each and every header is also a > lot easier to do basic automated sanity checks on. If the file ends in .h, > we just use grep to quickly verify it's not missing the guards. [Naturally, > we can do more complete checks than that if we want, but 99% percent of > misses can be picked up by a grep for the 'extern "C"' bit] so first, i agree with what you say here. but one downside i've seen is that non-public symbols may end up as extern "C". i've also been unsatisfied with the inconsistency of either having includes in or outside of the guards. a lot of dpdk headers follow this pattern. // foo.h #ifdef __cplusplus extern "C" { #endif #include ... but some dpdk headers follow this pattern. // foo.h #include #ifdef __cplusplus extern "C" #endif ... standard C headers include the guards so don't need to be inside the extern "C" block. one minor annoyance with always including inside the block is we can't reliably provide a offer a C++-only header without doing extern "C++". please bear in mind i do not mean to suggest implementing any dpdk in C++ with this comment, merely that there are advantages to occasionally offering C++-only header content to applications should we wever want to. in some cases for harmony between C and C++ it may be easier to interoperate by supplying some basic C++-only headers, this may become more important as it there appears to be increasing divergance between the C and C++ standards and interoperability. for full disclosure i do anticipate having to provide some small bits of header only C++ for msvc which is why this is top of my mind. finally, i'll also note that we could again be explicit in our intent to declare what is extern "C" / exported by instead marking the declared names (functions and variables) themselves in a more precise manner. i.e. __rte__export // extern "C" or __declspec(dllexport) extern "C" void some_public_symbol(void); you'll recall we had a related discussion about symbol visibility here which is somewhat a similar problem to being solved to symbol naming. if we were defaulting visibility to hidden and using a single mechanism to guarantee extern "C" linkage and public visibility exposure we could catch all "missed" symbols for C++ without having to build as C++ and reference the symbols. https://mails.dpdk.org/archives/dev/2024-January/285109.html i still intend to put forward an RFC for the discussion resulting from that thread (just haven't had time yet). > > /Bruce