[dpdk-dev] [PATCH v2 0/7] Various fixes to compile with gcc6

[dpdk-dev] [PATCH v2 0/7] Various fixes to compile with gcc6

[Aaron Conole]

This series brings a number of code cleanups to allow building using gcc6,
with various legitimate warnings being fixed.

Some of these fixes are to the drivers area, making this series a bit
atypical. However, the fixes identified in patches 2 and 3 are actual
bugs and should be fixed.

The first patch from the original series has been dropped. It is no
longer needed, after commit 5ecdeba601d1 ("lpm: merge tbl24 and tbl8
structures").


Aaron Conole (7):
  app/test/test: Fix missing brackets
  drivers/net/e1000: Fix missing brackets
  drivers/net/e1000: Fix missing lsc interrupt check brackets
  drivers/net/ixgbe: Fix vlan filter missing brackets
  drivers/net/e1000/igb: Signed left shift operator
  drivers/net/ixgbe: Signed left shift operator
  drivers/net/ixgbe: Fix uninitialized warning

 app/test/test.c                    |  3 ++-
 drivers/net/e1000/base/e1000_phy.c | 12 ++++++------
 drivers/net/e1000/em_ethdev.c      |  3 ++-
 drivers/net/e1000/igb_pf.c         |  4 ++--
 drivers/net/ixgbe/ixgbe_ethdev.c   |  3 ++-
 drivers/net/ixgbe/ixgbe_pf.c       |  4 ++--
 drivers/net/ixgbe/ixgbe_rxtx.c     |  4 ++--
 7 files changed, 18 insertions(+), 15 deletions(-)

-- 
2.8.0.rc2.35.gc2c5f6b.dirty

[dpdk-dev] [PATCH v2 1/7] app/test/test: Fix missing brackets

[Aaron Conole]

The test application calls printf(...) with the suite->suite_name argument.
The intent (based on whitespace) in the printf is to check suite->suite_name
first and then apply the printf. This doesn't happen due to missing brackets.

Signed-off-by: Aaron Conole <aconole@redhat.com>
Acked-by: Panu Matilainen <pmatilai@redhat.com>
---
v2:
* No change (apart from adding ACKs)

 app/test/test.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/app/test/test.c b/app/test/test.c
index f35b304..ccad0e3 100644
--- a/app/test/test.c
+++ b/app/test/test.c
@@ -162,9 +162,10 @@ unit_test_suite_runner(struct unit_test_suite *suite)
 	int test_success;
 	unsigned total = 0, executed = 0, skipped = 0, succeeded = 0, failed = 0;
 
-	if (suite->suite_name)
+	if (suite->suite_name) {
 		printf(" + ------------------------------------------------------- +\n");
 		printf(" + Test Suite : %s\n", suite->suite_name);
+	}
 
 	if (suite->setup)
 		if (suite->setup() != 0)
-- 
2.8.0.rc2.35.gc2c5f6b.dirty

[dpdk-dev] [PATCH v2 2/7] drivers/net/e1000: Fix missing brackets

[Aaron Conole]

The register read/write mphy functions have misleading whitespace around
the locked check. This cleanup merely preserves the existing functionality
while improving the ready check.

Fixes commit 38db3f7f50bd ("e1000: update base driver")

Signed-off-by: Aaron Conole <aconole@redhat.com>
---
v2:
* Changed from "whitespace-only" fix to a functional change
  moving the phy writes into protection of the `if (locked)`
  code
* Added "Fixes" line.

 drivers/net/e1000/base/e1000_phy.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/net/e1000/base/e1000_phy.c b/drivers/net/e1000/base/e1000_phy.c
index d43b7ce..ad3fd58 100644
--- a/drivers/net/e1000/base/e1000_phy.c
+++ b/drivers/net/e1000/base/e1000_phy.c
@@ -4153,12 +4153,12 @@ s32 e1000_read_phy_reg_mphy(struct e1000_hw *hw, u32 address, u32 *data)
 	*data = E1000_READ_REG(hw, E1000_MPHY_DATA);
 
 	/* Disable access to mPHY if it was originally disabled */
-	if (locked)
+	if (locked) {
 		ready = e1000_is_mphy_ready(hw);
 		if (!ready)
 			return -E1000_ERR_PHY;
-		E1000_WRITE_REG(hw, E1000_MPHY_ADDR_CTRL,
-				E1000_MPHY_DIS_ACCESS);
+		E1000_WRITE_REG(hw, E1000_MPHY_ADDR_CTRL, E1000_MPHY_DIS_ACCESS);
+	}
 
 	return E1000_SUCCESS;
 }
@@ -4218,12 +4218,12 @@ s32 e1000_write_phy_reg_mphy(struct e1000_hw *hw, u32 address, u32 data,
 	E1000_WRITE_REG(hw, E1000_MPHY_DATA, data);
 
 	/* Disable access to mPHY if it was originally disabled */
-	if (locked)
+	if (locked) {
 		ready = e1000_is_mphy_ready(hw);
 		if (!ready)
 			return -E1000_ERR_PHY;
-		E1000_WRITE_REG(hw, E1000_MPHY_ADDR_CTRL,
-				E1000_MPHY_DIS_ACCESS);
+		E1000_WRITE_REG(hw, E1000_MPHY_ADDR_CTRL, E1000_MPHY_DIS_ACCESS);
+	}
 
 	return E1000_SUCCESS;
 }
-- 
2.8.0.rc2.35.gc2c5f6b.dirty

[dpdk-dev] [PATCH v2 3/7] drivers/net/e1000: Fix missing lsc interrupt check brackets

[Aaron Conole]

The device lsc interrupt check has a misleading whitespace around it which
can be improved by adding appropriate braces to the check. Since the ret
variable was checked after previous assignment, this introduces no functional
change.

Fixes commit 921a72008f76 ("e1000: add Rx interrupt handler")

Signed-off-by: Aaron Conole <aconole@redhat.com>
Acked-by: Panu Matilainen <pmatilai@redhat.com>
---
v2:
* Added Fixes line.

 drivers/net/e1000/em_ethdev.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/net/e1000/em_ethdev.c b/drivers/net/e1000/em_ethdev.c
index 1f0a7f4..9b5e1af 100644
--- a/drivers/net/e1000/em_ethdev.c
+++ b/drivers/net/e1000/em_ethdev.c
@@ -666,13 +666,14 @@ eth_em_start(struct rte_eth_dev *dev)
 
 	if (rte_intr_allow_others(intr_handle)) {
 		/* check if lsc interrupt is enabled */
-		if (dev->data->dev_conf.intr_conf.lsc != 0)
+		if (dev->data->dev_conf.intr_conf.lsc != 0) {
 			ret = eth_em_interrupt_setup(dev);
 			if (ret) {
 				PMD_INIT_LOG(ERR, "Unable to setup interrupts");
 				em_dev_clear_queues(dev);
 				return ret;
 			}
+		}
 	} else {
 		rte_intr_callback_unregister(intr_handle,
 						eth_em_interrupt_handler,
-- 
2.8.0.rc2.35.gc2c5f6b.dirty

[dpdk-dev] [PATCH v2 4/7] drivers/net/ixgbe: Fix vlan filter missing brackets

[Aaron Conole]

The ixgbe vlan filter code has an if check with an incorrect whitespace.

Signed-off-by: Aaron Conole <aconole@redhat.com>
Acked-by: Panu Matilainen <pmatilai@redhat.com>
Acked-by: Helin Zhang <helin.zhang@intel.com>
---
v2:
* No change (apart from adding ACKs)

 drivers/net/ixgbe/ixgbe_ethdev.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ixgbe/ixgbe_ethdev.c b/drivers/net/ixgbe/ixgbe_ethdev.c
index d4d883a..e290bce 100644
--- a/drivers/net/ixgbe/ixgbe_ethdev.c
+++ b/drivers/net/ixgbe/ixgbe_ethdev.c
@@ -4377,10 +4377,11 @@ ixgbe_set_pool_vlan_filter(struct rte_eth_dev *dev, uint16_t vlan,
 	if (ixgbe_vmdq_mode_check(hw) < 0)
 		return -ENOTSUP;
 	for (pool_idx = 0; pool_idx < ETH_64_POOLS; pool_idx++) {
-		if (pool_mask & ((uint64_t)(1ULL << pool_idx)))
+		if (pool_mask & ((uint64_t)(1ULL << pool_idx))) {
 			ret = hw->mac.ops.set_vfta(hw,vlan,pool_idx,vlan_on);
 			if (ret < 0)
 				return ret;
+		}
 	}
 
 	return ret;
-- 
2.8.0.rc2.35.gc2c5f6b.dirty

[dpdk-dev] [PATCH v2 5/7] drivers/net/e1000/igb: Signed left shift operator

[Aaron Conole]

Tell the compiler to use an unsigned constant for the config shifts.

Signed-off-by: Aaron Conole <aconole@redhat.com>
Acked-by: Panu Matilainen <pmatilai@redhat.com>
Acked-by: Helin Zhang <helin.zhang@intel.com>
---
v2:
* No change (apart from adding ACKs)

 drivers/net/e1000/igb_pf.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/net/e1000/igb_pf.c b/drivers/net/e1000/igb_pf.c
index 95204e9..7029251 100644
--- a/drivers/net/e1000/igb_pf.c
+++ b/drivers/net/e1000/igb_pf.c
@@ -172,8 +172,8 @@ int igb_pf_host_configure(struct rte_eth_dev *eth_dev)
 	E1000_WRITE_REG(hw, E1000_VT_CTL, vtctl);
 
 	/* Enable pools reserved to PF only */
-	E1000_WRITE_REG(hw, E1000_VFRE, (~0) << vf_num);
-	E1000_WRITE_REG(hw, E1000_VFTE, (~0) << vf_num);
+	E1000_WRITE_REG(hw, E1000_VFRE, (~0U) << vf_num);
+	E1000_WRITE_REG(hw, E1000_VFTE, (~0U) << vf_num);
 
 	/* PFDMA Tx General Switch Control Enables VMDQ loopback */
 	if (hw->mac.type == e1000_i350)
-- 
2.8.0.rc2.35.gc2c5f6b.dirty

[dpdk-dev] [PATCH v2 6/7] drivers/net/ixgbe: Signed left shift operator

[Aaron Conole]

Tell the compiler to use an unsigned constant for the config shifts.

Signed-off-by: Aaron Conole <aconole@redhat.com>
Acked-by: Panu Matilainen <pmatilai@redhat.com>
Acked-by: Helin Zhang <helin.zhang@intel.com>
---
v2:
* No change (apart from adding ACKs)

 drivers/net/ixgbe/ixgbe_pf.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ixgbe/ixgbe_pf.c b/drivers/net/ixgbe/ixgbe_pf.c
index b854c72..f61653b 100644
--- a/drivers/net/ixgbe/ixgbe_pf.c
+++ b/drivers/net/ixgbe/ixgbe_pf.c
@@ -236,9 +236,9 @@ int ixgbe_pf_host_configure(struct rte_eth_dev *eth_dev)
 	vfre_slot = (vf_num >> VFRE_SHIFT) > 0 ? 1 : 0;
 
 	/* Enable pools reserved to PF only */
-	IXGBE_WRITE_REG(hw, IXGBE_VFRE(vfre_slot), (~0) << vfre_offset);
+	IXGBE_WRITE_REG(hw, IXGBE_VFRE(vfre_slot), (~0U) << vfre_offset);
 	IXGBE_WRITE_REG(hw, IXGBE_VFRE(vfre_slot ^ 1), vfre_slot - 1);
-	IXGBE_WRITE_REG(hw, IXGBE_VFTE(vfre_slot), (~0) << vfre_offset);
+	IXGBE_WRITE_REG(hw, IXGBE_VFTE(vfre_slot), (~0U) << vfre_offset);
 	IXGBE_WRITE_REG(hw, IXGBE_VFTE(vfre_slot ^ 1), vfre_slot - 1);
 
 	/* PFDMA Tx General Switch Control Enables VMDQ loopback */
-- 
2.8.0.rc2.35.gc2c5f6b.dirty

[dpdk-dev] [PATCH v2 7/7] drivers/net/ixgbe: Fix uninitialized warning

[Aaron Conole]

Silence a compiler warning that this variable may be used uninitialized.

Signed-off-by: Aaron Conole <aconole@redhat.com>
Acked-by: Helin Zhang <helin.zhang@intel.com>
---
v2:
* No change (apart from adding ACK)

 drivers/net/ixgbe/ixgbe_rxtx.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ixgbe/ixgbe_rxtx.c b/drivers/net/ixgbe/ixgbe_rxtx.c
index ff6ddb8..885fbd7 100644
--- a/drivers/net/ixgbe/ixgbe_rxtx.c
+++ b/drivers/net/ixgbe/ixgbe_rxtx.c
@@ -1614,7 +1614,7 @@ ixgbe_recv_pkts_lro(void *rx_queue, struct rte_mbuf **rx_pkts, uint16_t nb_pkts,
 		struct ixgbe_rx_entry *rxe;
 		struct ixgbe_scattered_rx_entry *sc_entry;
 		struct ixgbe_scattered_rx_entry *next_sc_entry;
-		struct ixgbe_rx_entry *next_rxe;
+		struct ixgbe_rx_entry *next_rxe = NULL;
 		struct rte_mbuf *first_seg;
 		struct rte_mbuf *rxm;
 		struct rte_mbuf *nmb;
@@ -1791,7 +1791,7 @@ next_desc:
 		 * the pointer to the first mbuf at the NEXTP entry in the
 		 * sw_sc_ring and continue to parse the RX ring.
 		 */
-		if (!eop) {
+		if (!eop && next_rxe) {
 			rxm->next = next_rxe->mbuf;
 			next_sc_entry->fbuf = first_seg;
 			goto next_desc;
-- 
2.8.0.rc2.35.gc2c5f6b.dirty

Re: [dpdk-dev] [PATCH v2 2/7] drivers/net/e1000: Fix missing brackets

[Thomas Monjalon]

2016-03-22 17:37, Aaron Conole:
> The register read/write mphy functions have misleading whitespace around
> the locked check. This cleanup merely preserves the existing functionality
> while improving the ready check.
> 
> Fixes commit 38db3f7f50bd ("e1000: update base driver")
> 
> Signed-off-by: Aaron Conole <aconole@redhat.com>
> ---
> v2:
> * Changed from "whitespace-only" fix to a functional change
>   moving the phy writes into protection of the `if (locked)`
>   code
> * Added "Fixes" line.

Wenzhuo, do you agree to fix the base driver here?

Re: [dpdk-dev] [PATCH v2 2/7] drivers/net/e1000: Fix missing brackets

[Thomas Monjalon]

fixing Wenzhuo email address...

2016-03-23 11:38, Thomas Monjalon:
> 2016-03-22 17:37, Aaron Conole:
> > The register read/write mphy functions have misleading whitespace around
> > the locked check. This cleanup merely preserves the existing functionality
> > while improving the ready check.
> > 
> > Fixes commit 38db3f7f50bd ("e1000: update base driver")
> > 
> > Signed-off-by: Aaron Conole <aconole@redhat.com>
> > ---
> > v2:
> > * Changed from "whitespace-only" fix to a functional change
> >   moving the phy writes into protection of the `if (locked)`
> >   code
> > * Added "Fixes" line.
> 
> Wenzhuo, do you agree to fix the base driver here?

Re: [dpdk-dev] [PATCH v2 2/7] drivers/net/e1000: Fix missing brackets

[Lu, Wenzhuo]

Hi Thomas, Aaron,
> > Wenzhuo, do you agree to fix the base driver here?
Honestly I find the logic has some problem and maybe more changes needed. I'm talking with our kernel driver contactors to make sure they have no concern about  my idea.
And Aaron, do we really need to fix this code? For I find this function is not called. So it has no real impact to us. Could we just wait until kernel driver fixes it and leverage the new version? Thanks.

Re: [dpdk-dev] [PATCH v2 2/7] drivers/net/e1000: Fix missing brackets

[Panu Matilainen]

On 03/24/2016 02:35 AM, Lu, Wenzhuo wrote:
> Hi Thomas, Aaron,
>>> Wenzhuo, do you agree to fix the base driver here?
> Honestly I find the logic has some problem and maybe more changes needed. I'm talking with our kernel driver contactors to make sure they have no concern about  my idea.
> And Aaron, do we really need to fix this code? For I find this function is not called. So it has no real impact to us. Could we just wait until kernel driver fixes it and leverage the new version? Thanks.

It breaks the build with gcc >= 6.0 due to -Werror so yes we need to fix 
it, one way or the other.

As spelled out in an earlier mail 
(http://dpdk.org/ml/archives/dev/2016-March/034290.html), for somebody 
building with gcc >= 6.0 the options basically are:
1) disable the driver entirely
2) build with -Wno-error
3) paper over it with local warning disablers
4) patch the thing locally

If the offending function really is not used at all in DPDK context then 
3) is an entirely valid option for an upstream solution, ie something 
like (untested)

--- a/drivers/net/e1000/Makefile
+++ b/drivers/net/e1000/Makefile
@@ -54,6 +54,9 @@ else
  #
  CFLAGS_BASE_DRIVER = -Wno-uninitialized -Wno-unused-parameter
  CFLAGS_BASE_DRIVER += -Wno-unused-variable
+ifeq ($(shell test $(GCC_VERSION) -ge 60 && echo 1), 1)
+CFLAGS_BASE_DRIVER += -Wno-misleading-indentation
+endif
  endif

  #


	- Panu -

Re: [dpdk-dev] [PATCH v2 2/7] drivers/net/e1000: Fix missing brackets

[Thomas Monjalon]

2016-03-24 08:54, Panu Matilainen:
> --- a/drivers/net/e1000/Makefile
> +++ b/drivers/net/e1000/Makefile
> @@ -54,6 +54,9 @@ else
>   #
>   CFLAGS_BASE_DRIVER = -Wno-uninitialized -Wno-unused-parameter
>   CFLAGS_BASE_DRIVER += -Wno-unused-variable
> +ifeq ($(shell test $(GCC_VERSION) -ge 60 && echo 1), 1)
> +CFLAGS_BASE_DRIVER += -Wno-misleading-indentation
> +endif

Aaron, have you tested this solution?
Are you going to provide a v3?

Re: [dpdk-dev] [PATCH v2 2/7] drivers/net/e1000: Fix missing brackets

[Aaron Conole]

Thomas Monjalon <thomas.monjalon@6wind.com> writes:

> 2016-03-24 08:54, Panu Matilainen:
>> --- a/drivers/net/e1000/Makefile
>> +++ b/drivers/net/e1000/Makefile
>> @@ -54,6 +54,9 @@ else
>>   #
>>   CFLAGS_BASE_DRIVER = -Wno-uninitialized -Wno-unused-parameter
>>   CFLAGS_BASE_DRIVER += -Wno-unused-variable
>> +ifeq ($(shell test $(GCC_VERSION) -ge 60 && echo 1), 1)
>> +CFLAGS_BASE_DRIVER += -Wno-misleading-indentation
>> +endif
>
> Aaron, have you tested this solution?
> Are you going to provide a v3?

I haven't yet tested this solution, but if folks are that opposed to
changing the code, then I will test it and post a v3 of this particular
patch in the series. 

Thanks so much for the reviews and time on this (Panu AND Thomas :-)),
-Aaron

[dpdk-dev] [PATCH v3 2/7] drivers/net/e1000: Suppress misleading indentation warning

[Aaron Conole]

The register read/write mphy functions have misleading whitespace around
the `locked` check. This cleanup merely preserves the existing functionality
and suppresses future gcc versions' "misleading indentation" warning.

Suggested-by: Panu Matilainen <pmatilai@redhat.com>
Signed-off-by: Aaron Conole <aconole@redhat.com>
---
v2:
* Changed from "whitespace-only" fix to a functional change
  moving the phy writes into protection of the `if (locked)`
  code
* Added "Fixes" line.

v3:
* Instead of changing the code, change to suppress the compiler warning
  when using gcc6+. This was tested with both gcc6 and gcc5 using gnu
  make 4.0 and gnu bash 4.3.42 on a fedora 23 system.

 drivers/net/e1000/Makefile | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/net/e1000/Makefile b/drivers/net/e1000/Makefile
index ccd2b7b..f4879e6 100644
--- a/drivers/net/e1000/Makefile
+++ b/drivers/net/e1000/Makefile
@@ -54,6 +54,9 @@ else
 #
 CFLAGS_BASE_DRIVER = -Wno-uninitialized -Wno-unused-parameter
 CFLAGS_BASE_DRIVER += -Wno-unused-variable
+ifeq ($(shell test $(GCC_VERSION) -ge 60 && echo 1), 1)
+CFLAGS_BASE_DRIVER += -Wno-misleading-indentation
+endif
 endif
 
 #
-- 
2.5.5

Re: [dpdk-dev] [PATCH v3 2/7] drivers/net/e1000: Suppress misleading indentation warning

[Stephen Hemminger]

On Wed, 30 Mar 2016 10:06:36 -0400
Aaron Conole <aconole@redhat.com> wrote:

> The register read/write mphy functions have misleading whitespace around
> the `locked` check. This cleanup merely preserves the existing functionality
> and suppresses future gcc versions' "misleading indentation" warning.
> 
> Suggested-by: Panu Matilainen <pmatilai@redhat.com>
> Signed-off-by: Aaron Conole <aconole@redhat.com>
> ---
> v2:
> * Changed from "whitespace-only" fix to a functional change
>   moving the phy writes into protection of the `if (locked)`
>   code
> * Added "Fixes" line.
> 
> v3:
> * Instead of changing the code, change to suppress the compiler warning
>   when using gcc6+. This was tested with both gcc6 and gcc5 using gnu
>   make 4.0 and gnu bash 4.3.42 on a fedora 23 system.
> 
>  drivers/net/e1000/Makefile | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/drivers/net/e1000/Makefile b/drivers/net/e1000/Makefile
> index ccd2b7b..f4879e6 100644
> --- a/drivers/net/e1000/Makefile
> +++ b/drivers/net/e1000/Makefile
> @@ -54,6 +54,9 @@ else
>  #
>  CFLAGS_BASE_DRIVER = -Wno-uninitialized -Wno-unused-parameter
>  CFLAGS_BASE_DRIVER += -Wno-unused-variable
> +ifeq ($(shell test $(GCC_VERSION) -ge 60 && echo 1), 1)
> +CFLAGS_BASE_DRIVER += -Wno-misleading-indentation
> +endif
>  endif
>  
>  #

NAK, don't do it to the whole file.
Fix the code (best option)
or use a pragma for the small area which is broken for other reasons.

Re: [dpdk-dev] [PATCH v3 2/7] drivers/net/e1000: Suppress misleading indentation warning

[Thomas Monjalon]

2016-03-30 09:36, Stephen Hemminger:
> On Wed, 30 Mar 2016 10:06:36 -0400
> Aaron Conole <aconole@redhat.com> wrote:
> > --- a/drivers/net/e1000/Makefile
> > +++ b/drivers/net/e1000/Makefile
> > @@ -54,6 +54,9 @@ else
> >  #
> >  CFLAGS_BASE_DRIVER = -Wno-uninitialized -Wno-unused-parameter
> >  CFLAGS_BASE_DRIVER += -Wno-unused-variable
> > +ifeq ($(shell test $(GCC_VERSION) -ge 60 && echo 1), 1)
> > +CFLAGS_BASE_DRIVER += -Wno-misleading-indentation
> > +endif
> >  endif
> 
> NAK, don't do it to the whole file.
> Fix the code (best option)
> or use a pragma for the small area which is broken for other reasons.

Stephen, your solutions do not work because Aaron has not been allowed
to modify this file.
As long as we are not allowed to modify the Intel base drivers,
I don't see any problem to hide some warnings in them.
The warnings could help us to clean the code or fix some bugs but
we are not allowed to...
It is the responsibility of the driver maintainer to keep this nasty code.

Re: [dpdk-dev] [PATCH v3 2/7] drivers/net/e1000: Suppress misleading indentation warning

[Stephen Hemminger]

On Wed, 30 Mar 2016 19:12:39 +0200
Thomas Monjalon <thomas.monjalon@6wind.com> wrote:

> 2016-03-30 09:36, Stephen Hemminger:
> > On Wed, 30 Mar 2016 10:06:36 -0400
> > Aaron Conole <aconole@redhat.com> wrote:
> > > --- a/drivers/net/e1000/Makefile
> > > +++ b/drivers/net/e1000/Makefile
> > > @@ -54,6 +54,9 @@ else
> > >  #
> > >  CFLAGS_BASE_DRIVER = -Wno-uninitialized -Wno-unused-parameter
> > >  CFLAGS_BASE_DRIVER += -Wno-unused-variable
> > > +ifeq ($(shell test $(GCC_VERSION) -ge 60 && echo 1), 1)
> > > +CFLAGS_BASE_DRIVER += -Wno-misleading-indentation
> > > +endif
> > >  endif
> > 
> > NAK, don't do it to the whole file.
> > Fix the code (best option)
> > or use a pragma for the small area which is broken for other reasons.
> 
> Stephen, your solutions do not work because Aaron has not been allowed
> to modify this file.
> As long as we are not allowed to modify the Intel base drivers,
> I don't see any problem to hide some warnings in them.
> The warnings could help us to clean the code or fix some bugs but
> we are not allowed to...
> It is the responsibility of the driver maintainer to keep this nasty code.

ok, but the policy of "base drivers are allowed to be unmaintainable" is
an albatross around the neck of DPDK. There is a reason such a policy was rejected
in Linux.

Re: [dpdk-dev] [PATCH v3 2/7] drivers/net/e1000: Suppress misleading indentation warning

[Lu, Wenzhuo]

Hi,

> -----Original Message-----
> From: Aaron Conole [mailto:aconole@redhat.com]
> Sent: Wednesday, March 30, 2016 10:07 PM
> To: Thomas Monjalon
> Cc: Panu Matilainen; Lu, Wenzhuo; dev@dpdk.org; Richardson, Bruce
> Subject: [PATCH v3 2/7] drivers/net/e1000: Suppress misleading indentation
> warning
> 
> The register read/write mphy functions have misleading whitespace around the
> `locked` check. This cleanup merely preserves the existing functionality and
> suppresses future gcc versions' "misleading indentation" warning.
> 
> Suggested-by: Panu Matilainen <pmatilai@redhat.com>
> Signed-off-by: Aaron Conole <aconole@redhat.com>
Acked-by: Wenzhuo Lu <wenzhuo.lu@intel.com>

Re: [dpdk-dev] [PATCH v2 0/7] Various fixes to compile with gcc6

[Thomas Monjalon]

2016-03-22 17:37, Aaron Conole:
> This series brings a number of code cleanups to allow building using gcc6,
> with various legitimate warnings being fixed.
> 
> Some of these fixes are to the drivers area, making this series a bit
> atypical. However, the fixes identified in patches 2 and 3 are actual
> bugs and should be fixed.
> 
> The first patch from the original series has been dropped. It is no
> longer needed, after commit 5ecdeba601d1 ("lpm: merge tbl24 and tbl8
> structures").
> 
> 
> Aaron Conole (7):
>   app/test/test: Fix missing brackets
>   drivers/net/e1000: Fix missing brackets
>   drivers/net/e1000: Fix missing lsc interrupt check brackets
>   drivers/net/ixgbe: Fix vlan filter missing brackets
>   drivers/net/e1000/igb: Signed left shift operator
>   drivers/net/ixgbe: Signed left shift operator
>   drivers/net/ixgbe: Fix uninitialized warning

Applied with v3 of patch 2, thanks

Re: [dpdk-dev] [PATCH v2 0/7] Various fixes to compile with gcc6

[Panu Matilainen]

On 03/31/2016 06:02 PM, Thomas Monjalon wrote:
> 2016-03-22 17:37, Aaron Conole:
>> This series brings a number of code cleanups to allow building using gcc6,
>> with various legitimate warnings being fixed.
>>
>> Some of these fixes are to the drivers area, making this series a bit
>> atypical. However, the fixes identified in patches 2 and 3 are actual
>> bugs and should be fixed.
>>
>> The first patch from the original series has been dropped. It is no
>> longer needed, after commit 5ecdeba601d1 ("lpm: merge tbl24 and tbl8
>> structures").
>>
>>
>> Aaron Conole (7):
>>    app/test/test: Fix missing brackets
>>    drivers/net/e1000: Fix missing brackets
>>    drivers/net/e1000: Fix missing lsc interrupt check brackets
>>    drivers/net/ixgbe: Fix vlan filter missing brackets
>>    drivers/net/e1000/igb: Signed left shift operator
>>    drivers/net/ixgbe: Signed left shift operator
>>    drivers/net/ixgbe: Fix uninitialized warning
>
> Applied with v3 of patch 2, thanks
>

...and so we finally have a warning-free build with gcc 6.

Thanks,

	- Panu -