On Tue, Sep 17, 2024 at 11:30 AM Mattias Rönnblom <hof...@lysator.liu.se> wrote:
>
> On 2024-09-16 14:05, David Marchand wrote:
> > Hello,
> >
> > On Tue, Sep 10, 2024 at 10:41 AM Mattias Rönnblom
> > <mattias.ronnb...@ericsson.com> 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 <rte_cpuflags.h>
> >> #include <rte_debug.h>
> >>
> >> +#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.
> >> 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 <rte_common.h>
> >> #include <rte_stdatomic.h>
> >>
> >> +#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).
--
David Marchand
