> From: Tyler Retzlaff [mailto:roret...@linux.microsoft.com] > Sent: Monday, 5 February 2024 18.37 > > 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.laranje...@6wind.com > > > > Cc: sta...@dpdk.org > > > > > > > > Signed-off-by: Ashish Sadanandan <ashish.sadanan...@gmail.com> > > > > > > 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 <stdio.h> > > ... > > but some dpdk headers follow this pattern. > > // foo.h > #include <stdio.h> > > #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++".
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. > > 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_<lib>_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). With that RFC, please also mention if function pointers need any special/additional considerations. No need to think about it yet. :-) > > > > > /Bruce
