Re: [dpdk-dev] [PATCH v2 1/3] spinlock: add support for HTM lock elision for x86

[Adrien Mazarguil <adrien.mazarguil@6wind.com>]

On Fri, Jun 19, 2015 at 03:35:38PM +0200, Thomas Monjalon wrote:
> 2015-06-18 11:00, Bruce Richardson:
> > On Wed, Jun 17, 2015 at 11:29:49PM +0200, Thomas Monjalon wrote:
> > > Introducing rte_cpuflags.h in this header breaks the compilation of
> > > the mlx4 pmd with CONFIG_RTE_LIBRTE_MLX4_DEBUG=y.
> > > Indeed, it triggers the -pedantic flag which is not supported by rte_cpuflags.h.
> > > Maybe it's time to fix this header?
> > 
> > Do all our headers need to support the pedantic C flag? I don't believe this
> > was a previous requirement for header files. The mlx4 driver appears to be the
> > only place in the dpdk.org codebase where the flag actually appears - and even
> > then the flag disabled in mlx.c where the dpdk headers are actually included.
> > 
> >   73 /* DPDK headers don't like -pedantic. */$
> >   74 #ifdef PEDANTIC$
> >   75 #pragma GCC diagnostic ignored "-pedantic"$
> >   76 #endif$
> >   77 #include <rte_config.h>$
> > 	.....
> 
> You're right. It seems this disabling doesn't work.

Well, it used to work, at least sufficiently until now.

The mlx4 driver started as an out of tree development long ago, this flag
is here from the beginning and was left around to maintain a clean code base
in the PMD itself. Unfortunately, it had to include a few headers that were
not quite ready to handle such constraints, hence the somewhat ugly #pragma
workarounds left until these headers could be fixed someday.

> > I'm just not convinced that rte_cpuflags needs to be fixed at all here.
> 
> Yes, it's probably simpler to remove the -pedantic flag.

I'm not going to argue against that, as a PMD's Makefile is obviously not
the right place to add a -pedantic parameter anyway.

However outside of PMD usage, I think public API headers (I'm not talking
about the entire DPDK code base, just headers) should handle all kind of
warnings a user application might throw at it for its own use (-pedantic and
other -Wstuff, I'd even say -std=c99 for strict ISO C compliance), as is the
case for the C library and most, if not all system-wide headers.

-- 
Adrien Mazarguil
6WIND