On 2/5/2024 9:07 PM, Morten Brørup wrote: >> 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. >
Should we document the proper extern "C" usage somewhere?
