diff mbox

[dpdk-dev,v2] net/vdev_netvsc: fix build using C11 mode and pedantic

Message ID 1516803133-17529-1-git-send-email-ophirmu@mellanox.com (mailing list archive)
State Accepted, archived
Delegated to: Ferruh Yigit
Headers

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/Intel-compilation fail Compilation issues

Commit Message

Ophir Munk Jan. 24, 2018, 2:12 p.m. UTC 
  Remove CFLAGS -std=c11 and -pedantic in order to guarantee
a successful vdev_netvsc compilation on old Linux distributions.
Otherwise old GCC compilers may complain as follows:
cc1: error: unrecognized command line option -std=c11

Fixes: 6086ab3bb3d2 ("net/vdev_netvsc: introduce Hyper-V platform driver")
Cc: stable@dpdk.org

Signed-off-by: Ophir Munk <ophirmu@mellanox.com>
---
 drivers/net/vdev_netvsc/Makefile | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Matan Azrad Jan. 24, 2018, 2:45 p.m. UTC | #1 
Hi Ophir

From: Ophir Munk, Wednesday, January 24, 2018 4:12 PM
> Remove CFLAGS -std=c11 and -pedantic in order to guarantee a successful
> vdev_netvsc compilation on old Linux distributions.
> Otherwise old GCC compilers may complain as follows:
> cc1: error: unrecognized command line option -std=c11
> 
> Fixes: 6086ab3bb3d2 ("net/vdev_netvsc: introduce Hyper-V platform
> driver")
> Cc: stable@dpdk.org

No need to backport this fix.

> Signed-off-by: Ophir Munk <ophirmu@mellanox.com>

Besides that,
Acked-by: Matan Azrad <matan@mellanox.com>
Stephen Hemminger Jan. 24, 2018, 3:39 p.m. UTC | #2 
On Wed, 24 Jan 2018 14:12:13 +0000
Ophir Munk <ophirmu@mellanox.com> wrote:

> Remove CFLAGS -std=c11 and -pedantic in order to guarantee
> a successful vdev_netvsc compilation on old Linux distributions.
> Otherwise old GCC compilers may complain as follows:
> cc1: error: unrecognized command line option -std=c11
> 
> Fixes: 6086ab3bb3d2 ("net/vdev_netvsc: introduce Hyper-V platform driver")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Ophir Munk <ophirmu@mellanox.com>
> ---
>  drivers/net/vdev_netvsc/Makefile | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/net/vdev_netvsc/Makefile b/drivers/net/vdev_netvsc/Makefile
> index f2b2ac5..45351b8 100644
> --- a/drivers/net/vdev_netvsc/Makefile
> +++ b/drivers/net/vdev_netvsc/Makefile
> @@ -12,7 +12,7 @@ EXPORT_MAP := rte_pmd_vdev_netvsc_version.map
>  # Additional compilation flags.
>  CFLAGS += -O3
>  CFLAGS += -g
> -CFLAGS += -std=c11 -pedantic -Wall -Wextra
> +CFLAGS += -Wall -Wextra
>  CFLAGS += -D_XOPEN_SOURCE=600
>  CFLAGS += -D_BSD_SOURCE
>  CFLAGS += -D_DEFAULT_SOURCE

Why did this driver not use $(WERROR) like rest of DPDK drivers.
Thomas Monjalon Jan. 24, 2018, 6:06 p.m. UTC | #3 
24/01/2018 15:45, Matan Azrad:
> Hi Ophir
> 
> From: Ophir Munk, Wednesday, January 24, 2018 4:12 PM
> > Remove CFLAGS -std=c11 and -pedantic in order to guarantee a successful
> > vdev_netvsc compilation on old Linux distributions.
> > Otherwise old GCC compilers may complain as follows:
> > cc1: error: unrecognized command line option -std=c11
> > 
> > Fixes: 6086ab3bb3d2 ("net/vdev_netvsc: introduce Hyper-V platform
> > driver")
> > Cc: stable@dpdk.org
> 
> No need to backport this fix.
> 
> > Signed-off-by: Ophir Munk <ophirmu@mellanox.com>
> 
> Besides that,
> Acked-by: Matan Azrad <matan@mellanox.com>

Applied, thanks
Thomas Monjalon Jan. 24, 2018, 6:08 p.m. UTC | #4 
24/01/2018 16:39, Stephen Hemminger:
> On Wed, 24 Jan 2018 14:12:13 +0000
> Ophir Munk <ophirmu@mellanox.com> wrote:
> > --- a/drivers/net/vdev_netvsc/Makefile
> > +++ b/drivers/net/vdev_netvsc/Makefile
> > @@ -12,7 +12,7 @@ EXPORT_MAP := rte_pmd_vdev_netvsc_version.map
> >  # Additional compilation flags.
> >  CFLAGS += -O3
> >  CFLAGS += -g
> > -CFLAGS += -std=c11 -pedantic -Wall -Wextra
> > +CFLAGS += -Wall -Wextra
> >  CFLAGS += -D_XOPEN_SOURCE=600
> >  CFLAGS += -D_BSD_SOURCE
> >  CFLAGS += -D_DEFAULT_SOURCE
> 
> Why did this driver not use $(WERROR) like rest of DPDK drivers.

It can be a separate patch.
Matan?
Stephen Hemminger Jan. 24, 2018, 6:27 p.m. UTC | #5 
On Wed, 24 Jan 2018 19:08:02 +0100
Thomas Monjalon <thomas@monjalon.net> wrote:

> 24/01/2018 16:39, Stephen Hemminger:
> > On Wed, 24 Jan 2018 14:12:13 +0000
> > Ophir Munk <ophirmu@mellanox.com> wrote:  
> > > --- a/drivers/net/vdev_netvsc/Makefile
> > > +++ b/drivers/net/vdev_netvsc/Makefile
> > > @@ -12,7 +12,7 @@ EXPORT_MAP := rte_pmd_vdev_netvsc_version.map
> > >  # Additional compilation flags.
> > >  CFLAGS += -O3
> > >  CFLAGS += -g
> > > -CFLAGS += -std=c11 -pedantic -Wall -Wextra
> > > +CFLAGS += -Wall -Wextra
> > >  CFLAGS += -D_XOPEN_SOURCE=600
> > >  CFLAGS += -D_BSD_SOURCE
> > >  CFLAGS += -D_DEFAULT_SOURCE  
> > 
> > Why did this driver not use $(WERROR) like rest of DPDK drivers.  
> 
> It can be a separate patch.
> Matan?

I meant that you should use:

CFLAGS += $(WERROR_FLAGS)

instead of

CFLAGS += -Wall -Wextra

in this patch.

Also, do you really  need all the other CFLAGS? Why?

This driver has no reason to be a special case different from what is done
in virtio, vmxnet3, ixgbe, e1000, ...
Matan Azrad Jan. 25, 2018, 8:24 a.m. UTC | #6 
Hi Stephan

From: Stephen Hemminger, Wednesday, January 24, 2018 8:28 PM
> On Wed, 24 Jan 2018 19:08:02 +0100
> Thomas Monjalon <thomas@monjalon.net> wrote:
> 
> > 24/01/2018 16:39, Stephen Hemminger:
> > > On Wed, 24 Jan 2018 14:12:13 +0000
> > > Ophir Munk <ophirmu@mellanox.com> wrote:
> > > > --- a/drivers/net/vdev_netvsc/Makefile
> > > > +++ b/drivers/net/vdev_netvsc/Makefile
> > > > @@ -12,7 +12,7 @@ EXPORT_MAP :=
> rte_pmd_vdev_netvsc_version.map  #
> > > > Additional compilation flags.
> > > >  CFLAGS += -O3
> > > >  CFLAGS += -g
> > > > -CFLAGS += -std=c11 -pedantic -Wall -Wextra
> > > > +CFLAGS += -Wall -Wextra
> > > >  CFLAGS += -D_XOPEN_SOURCE=600
> > > >  CFLAGS += -D_BSD_SOURCE
> > > >  CFLAGS += -D_DEFAULT_SOURCE
> > >
> > > Why did this driver not use $(WERROR) like rest of DPDK drivers.
> >
> > It can be a separate patch.
> > Matan?
> 
> I meant that you should use:
> 
> CFLAGS += $(WERROR_FLAGS)
> 

These line already exists.

> instead of
> 
> CFLAGS += -Wall -Wextra
> 

-Wall is already in $(WERROR_FLAGS).
-Wextra is called also -W in the old versions and it appears in  WERROR_FLAGS as -W.
So just need to remove this line in different patch.

> in this patch.
> 
> Also, do you really  need all the other CFLAGS? Why?
> 
If you mean to the -D_BSD_SOURCE, it is needed for clang compilation.
diff mbox

Patch

diff --git a/drivers/net/vdev_netvsc/Makefile b/drivers/net/vdev_netvsc/Makefile
index f2b2ac5..45351b8 100644
--- a/drivers/net/vdev_netvsc/Makefile
+++ b/drivers/net/vdev_netvsc/Makefile
@@ -12,7 +12,7 @@  EXPORT_MAP := rte_pmd_vdev_netvsc_version.map
 # Additional compilation flags.
 CFLAGS += -O3
 CFLAGS += -g
-CFLAGS += -std=c11 -pedantic -Wall -Wextra
+CFLAGS += -Wall -Wextra
 CFLAGS += -D_XOPEN_SOURCE=600
 CFLAGS += -D_BSD_SOURCE
 CFLAGS += -D_DEFAULT_SOURCE