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 E786B43A2A; Mon, 5 Feb 2024 22:07:49 +0100 (CET) Received: from mails.dpdk.org (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 4DFB440E09; Mon, 5 Feb 2024 22:07:49 +0100 (CET) Received: from dkmailrelay1.smartsharesystems.com (smartserver.smartsharesystems.com [77.243.40.215]) by mails.dpdk.org (Postfix) with ESMTP id 609D8402ED; Mon, 5 Feb 2024 22:07:48 +0100 (CET) Received: from smartserver.smartsharesystems.com (smartserver.smartsharesys.local [192.168.4.10]) by dkmailrelay1.smartsharesystems.com (Postfix) with ESMTP id 3078F20CF6; Mon, 5 Feb 2024 22:07:48 +0100 (CET) Content-class: urn:content-classes:message MIME-Version: 1.0 Content-Type: text/plain; charset="iso-8859-1" Content-Transfer-Encoding: quoted-printable Subject: RE: [PATCH 1/1] eal: add C++ include guard in generic/rte_vect.h X-MimeOLE: Produced By Microsoft Exchange V6.5 Date: Mon, 5 Feb 2024 22:07:41 +0100 Message-ID: <98CBD80474FA8B44BF855DF32C47DC35E9F1EA@smartserver.smartshare.dk> In-Reply-To: <20240205173652.GA2935@linuxonhyperv3.guj3yctzbm1etfxqx2vob5hsef.xx.internal.cloudapp.net> X-MS-Has-Attach: X-MS-TNEF-Correlator: Thread-Topic: [PATCH 1/1] eal: add C++ include guard in generic/rte_vect.h Thread-Index: AdpYWewOqAf1cayGRHeeVoRthXamNAAGoW8w References: <20240202051335.776290-1-ashish.sadanandan@gmail.com> <5751086.DvuYhMxLoT@thomas> <20240205173652.GA2935@linuxonhyperv3.guj3yctzbm1etfxqx2vob5hsef.xx.internal.cloudapp.net> From: =?iso-8859-1?Q?Morten_Br=F8rup?= To: "Tyler Retzlaff" , "Bruce Richardson" Cc: "Thomas Monjalon" , "Ashish Sadanandan" , , , , , , , 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 > From: Tyler Retzlaff [mailto:roretzla@linux.microsoft.com] > Sent: Monday, 5 February 2024 18.37 >=20 > 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] >=20 > 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". >=20 > i've also been unsatisfied with the inconsistency of either having > includes in or outside of the guards. >=20 > a lot of dpdk headers follow this pattern. >=20 > // foo.h > #ifdef __cplusplus > extern "C" { > #endif >=20 > #include >=20 > ... >=20 > but some dpdk headers follow this pattern. >=20 > // foo.h > #include >=20 > #ifdef __cplusplus > extern "C" > #endif >=20 > ... >=20 > 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++". I would say that the first of the two above patterns is not only = annoying, it is incorrect. A DPDK header file should not change the meaning of any other header = files it includes. And although the incorrectness currently only screws up any C++ in those = header files, I still consider it a bug. >=20 > 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. >=20 > 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. >=20 > 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. >=20 > 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. >=20 > i.e. > __rte__export // extern "C" or __declspec(dllexport) extern "C" > void some_public_symbol(void); >=20 > 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. >=20 > https://mails.dpdk.org/archives/dev/2024-January/285109.html >=20 > i still intend to put forward an RFC for the discussion resulting from > that thread (just haven't had time yet). With that RFC, please also mention if function pointers need any = special/additional considerations. No need to think about it yet. :-) >=20 > > > > /Bruce