DPDK patches and discussions
 help / color / mirror / Atom feed
* [dpdk-dev] [PATCH] ixgbe: fix build with gcc 4.4
@ 2015-04-14  9:31 Thomas Monjalon
  2015-04-14 12:48 ` Vlad Zolotarov
  2015-04-14 12:51 ` [dpdk-dev] [PATCH] ixgbe: fix build with gcc 4.4 Vlad Zolotarov
  0 siblings, 2 replies; 33+ messages in thread
From: Thomas Monjalon @ 2015-04-14  9:31 UTC (permalink / raw)
  To: Vlad Zolotarov, Konstantin Ananyev, Helin Zhang; +Cc: dev

With GCC 4.4.7 from CentOS 6.5, the following errors arise:

lib/librte_pmd_ixgbe/ixgbe_rxtx.c: In function ‘ixgbe_dev_rx_queue_setup’:
lib/librte_pmd_ixgbe/ixgbe_rxtx.c:2509: error: missing initializer
lib/librte_pmd_ixgbe/ixgbe_rxtx.c:2509: error: (near initialization for ‘dev_info.driver_name’)

lib/librte_pmd_ixgbe/ixgbe_rxtx.c: In function ‘ixgbe_set_rsc’:
lib/librte_pmd_ixgbe/ixgbe_rxtx.c:4072: error: missing initializer
lib/librte_pmd_ixgbe/ixgbe_rxtx.c:4072: error: (near initialization for ‘dev_info.driver_name’)

lib/librte_pmd_ixgbe/ixgbe_rxtx.c: In function ‘ixgbe_recv_pkts_lro_single_alloc’:
lib/librte_pmd_ixgbe/ixgbe_rxtx.c:1479: error: ‘next_rsc_entry’ may be used uninitialized in this function
lib/librte_pmd_ixgbe/ixgbe_rxtx.c:1480: error: ‘next_rxe’ may be used uninitialized in this function

Fixes: 8eecb3295aed ("ixgbe: add LRO support")

Signed-off-by: Thomas Monjalon <thomas.monjalon@6wind.com>
---
 lib/librte_pmd_ixgbe/ixgbe_rxtx.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/lib/librte_pmd_ixgbe/ixgbe_rxtx.c b/lib/librte_pmd_ixgbe/ixgbe_rxtx.c
index f1da9ec..a2b8631 100644
--- a/lib/librte_pmd_ixgbe/ixgbe_rxtx.c
+++ b/lib/librte_pmd_ixgbe/ixgbe_rxtx.c
@@ -1476,8 +1476,8 @@ ixgbe_recv_pkts_lro(void *rx_queue, struct rte_mbuf **rx_pkts, uint16_t nb_pkts,
 		bool eop;
 		struct ixgbe_rx_entry *rxe;
 		struct ixgbe_rsc_entry *rsc_entry;
-		struct ixgbe_rsc_entry *next_rsc_entry;
-		struct ixgbe_rx_entry *next_rxe;
+		struct ixgbe_rsc_entry *next_rsc_entry = NULL;
+		struct ixgbe_rx_entry *next_rxe = NULL;
 		struct rte_mbuf *first_seg;
 		struct rte_mbuf *rxm;
 		struct rte_mbuf *nmb;
@@ -2506,7 +2506,7 @@ ixgbe_dev_rx_queue_setup(struct rte_eth_dev *dev,
 	struct ixgbe_rx_queue *rxq;
 	struct ixgbe_hw     *hw;
 	uint16_t len;
-	struct rte_eth_dev_info dev_info = { 0 };
+	struct rte_eth_dev_info dev_info = { .max_rx_queues = 0 };
 	struct rte_eth_rxmode *dev_rx_mode = &dev->data->dev_conf.rxmode;
 	bool rsc_requested = false;
 
@@ -4069,7 +4069,7 @@ ixgbe_set_rsc(struct rte_eth_dev *dev)
 {
 	struct rte_eth_rxmode *rx_conf = &dev->data->dev_conf.rxmode;
 	struct ixgbe_hw *hw = IXGBE_DEV_PRIVATE_TO_HW(dev->data->dev_private);
-	struct rte_eth_dev_info dev_info = { 0 };
+	struct rte_eth_dev_info dev_info = { .max_rx_queues = 0 };
 	bool rsc_capable = false;
 	uint16_t i;
 	uint32_t rdrxctl;
-- 
2.2.2

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [dpdk-dev] [PATCH] ixgbe: fix build with gcc 4.4
  2015-04-14  9:31 [dpdk-dev] [PATCH] ixgbe: fix build with gcc 4.4 Thomas Monjalon
@ 2015-04-14 12:48 ` Vlad Zolotarov
  2015-04-14 13:06   ` Ananyev, Konstantin
  2015-04-14 12:51 ` [dpdk-dev] [PATCH] ixgbe: fix build with gcc 4.4 Vlad Zolotarov
  1 sibling, 1 reply; 33+ messages in thread
From: Vlad Zolotarov @ 2015-04-14 12:48 UTC (permalink / raw)
  To: Thomas Monjalon, Konstantin Ananyev, Helin Zhang; +Cc: dev



On 04/14/15 12:31, Thomas Monjalon wrote:
> With GCC 4.4.7 from CentOS 6.5, the following errors arise:
>
> lib/librte_pmd_ixgbe/ixgbe_rxtx.c: In function ‘ixgbe_dev_rx_queue_setup’:
> lib/librte_pmd_ixgbe/ixgbe_rxtx.c:2509: error: missing initializer
> lib/librte_pmd_ixgbe/ixgbe_rxtx.c:2509: error: (near initialization for ‘dev_info.driver_name’)
>
> lib/librte_pmd_ixgbe/ixgbe_rxtx.c: In function ‘ixgbe_set_rsc’:
> lib/librte_pmd_ixgbe/ixgbe_rxtx.c:4072: error: missing initializer
> lib/librte_pmd_ixgbe/ixgbe_rxtx.c:4072: error: (near initialization for ‘dev_info.driver_name’)
>
> lib/librte_pmd_ixgbe/ixgbe_rxtx.c: In function ‘ixgbe_recv_pkts_lro_single_alloc’:
> lib/librte_pmd_ixgbe/ixgbe_rxtx.c:1479: error: ‘next_rsc_entry’ may be used uninitialized in this function
> lib/librte_pmd_ixgbe/ixgbe_rxtx.c:1480: error: ‘next_rxe’ may be used uninitialized in this function
>
> Fixes: 8eecb3295aed ("ixgbe: add LRO support")
>
> Signed-off-by: Thomas Monjalon <thomas.monjalon@6wind.com>
> ---
>   lib/librte_pmd_ixgbe/ixgbe_rxtx.c | 8 ++++----
>   1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/lib/librte_pmd_ixgbe/ixgbe_rxtx.c b/lib/librte_pmd_ixgbe/ixgbe_rxtx.c
> index f1da9ec..a2b8631 100644
> --- a/lib/librte_pmd_ixgbe/ixgbe_rxtx.c
> +++ b/lib/librte_pmd_ixgbe/ixgbe_rxtx.c
> @@ -1476,8 +1476,8 @@ ixgbe_recv_pkts_lro(void *rx_queue, struct rte_mbuf **rx_pkts, uint16_t nb_pkts,
>   		bool eop;
>   		struct ixgbe_rx_entry *rxe;
>   		struct ixgbe_rsc_entry *rsc_entry;
> -		struct ixgbe_rsc_entry *next_rsc_entry;
> -		struct ixgbe_rx_entry *next_rxe;
> +		struct ixgbe_rsc_entry *next_rsc_entry = NULL;
> +		struct ixgbe_rx_entry *next_rxe = NULL;
>   		struct rte_mbuf *first_seg;
>   		struct rte_mbuf *rxm;
>   		struct rte_mbuf *nmb;
> @@ -2506,7 +2506,7 @@ ixgbe_dev_rx_queue_setup(struct rte_eth_dev *dev,
>   	struct ixgbe_rx_queue *rxq;
>   	struct ixgbe_hw     *hw;
>   	uint16_t len;
> -	struct rte_eth_dev_info dev_info = { 0 };
> +	struct rte_eth_dev_info dev_info = { .max_rx_queues = 0 };
>   	struct rte_eth_rxmode *dev_rx_mode = &dev->data->dev_conf.rxmode;
>   	bool rsc_requested = false;
>   
> @@ -4069,7 +4069,7 @@ ixgbe_set_rsc(struct rte_eth_dev *dev)
>   {
>   	struct rte_eth_rxmode *rx_conf = &dev->data->dev_conf.rxmode;
>   	struct ixgbe_hw *hw = IXGBE_DEV_PRIVATE_TO_HW(dev->data->dev_private);
> -	struct rte_eth_dev_info dev_info = { 0 };
> +	struct rte_eth_dev_info dev_info = { .max_rx_queues = 0 };

Hmmm... Unless I miss something this and one above would zero only a 
single field - "max_rx_queues"; and would leave the rest uninitialized.
The original code intend to zero the whole struct. The alternative to 
the original lines could be usage of memset().

>   	bool rsc_capable = false;
>   	uint16_t i;
>   	uint32_t rdrxctl;

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [dpdk-dev] [PATCH] ixgbe: fix build with gcc 4.4
  2015-04-14  9:31 [dpdk-dev] [PATCH] ixgbe: fix build with gcc 4.4 Thomas Monjalon
  2015-04-14 12:48 ` Vlad Zolotarov
@ 2015-04-14 12:51 ` Vlad Zolotarov
  2015-04-14 13:23   ` Ananyev, Konstantin
  1 sibling, 1 reply; 33+ messages in thread
From: Vlad Zolotarov @ 2015-04-14 12:51 UTC (permalink / raw)
  To: Thomas Monjalon, Konstantin Ananyev, Helin Zhang; +Cc: dev



On 04/14/15 12:31, Thomas Monjalon wrote:
> With GCC 4.4.7 from CentOS 6.5, the following errors arise:
>
> lib/librte_pmd_ixgbe/ixgbe_rxtx.c: In function ‘ixgbe_dev_rx_queue_setup’:
> lib/librte_pmd_ixgbe/ixgbe_rxtx.c:2509: error: missing initializer
> lib/librte_pmd_ixgbe/ixgbe_rxtx.c:2509: error: (near initialization for ‘dev_info.driver_name’)
>
> lib/librte_pmd_ixgbe/ixgbe_rxtx.c: In function ‘ixgbe_set_rsc’:
> lib/librte_pmd_ixgbe/ixgbe_rxtx.c:4072: error: missing initializer
> lib/librte_pmd_ixgbe/ixgbe_rxtx.c:4072: error: (near initialization for ‘dev_info.driver_name’)
>
> lib/librte_pmd_ixgbe/ixgbe_rxtx.c: In function ‘ixgbe_recv_pkts_lro_single_alloc’:
> lib/librte_pmd_ixgbe/ixgbe_rxtx.c:1479: error: ‘next_rsc_entry’ may be used uninitialized in this function
> lib/librte_pmd_ixgbe/ixgbe_rxtx.c:1480: error: ‘next_rxe’ may be used uninitialized in this function

:D Looks like a gcc bug ;) Both are set and only after that (!!!) used 
under "!eop" condition.

>
> Fixes: 8eecb3295aed ("ixgbe: add LRO support")
>
> Signed-off-by: Thomas Monjalon <thomas.monjalon@6wind.com>
> ---
>   lib/librte_pmd_ixgbe/ixgbe_rxtx.c | 8 ++++----
>   1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/lib/librte_pmd_ixgbe/ixgbe_rxtx.c b/lib/librte_pmd_ixgbe/ixgbe_rxtx.c
> index f1da9ec..a2b8631 100644
> --- a/lib/librte_pmd_ixgbe/ixgbe_rxtx.c
> +++ b/lib/librte_pmd_ixgbe/ixgbe_rxtx.c
> @@ -1476,8 +1476,8 @@ ixgbe_recv_pkts_lro(void *rx_queue, struct rte_mbuf **rx_pkts, uint16_t nb_pkts,
>   		bool eop;
>   		struct ixgbe_rx_entry *rxe;
>   		struct ixgbe_rsc_entry *rsc_entry;
> -		struct ixgbe_rsc_entry *next_rsc_entry;
> -		struct ixgbe_rx_entry *next_rxe;
> +		struct ixgbe_rsc_entry *next_rsc_entry = NULL;
> +		struct ixgbe_rx_entry *next_rxe = NULL;
>   		struct rte_mbuf *first_seg;
>   		struct rte_mbuf *rxm;
>   		struct rte_mbuf *nmb;
> @@ -2506,7 +2506,7 @@ ixgbe_dev_rx_queue_setup(struct rte_eth_dev *dev,
>   	struct ixgbe_rx_queue *rxq;
>   	struct ixgbe_hw     *hw;
>   	uint16_t len;
> -	struct rte_eth_dev_info dev_info = { 0 };
> +	struct rte_eth_dev_info dev_info = { .max_rx_queues = 0 };
>   	struct rte_eth_rxmode *dev_rx_mode = &dev->data->dev_conf.rxmode;
>   	bool rsc_requested = false;
>   
> @@ -4069,7 +4069,7 @@ ixgbe_set_rsc(struct rte_eth_dev *dev)
>   {
>   	struct rte_eth_rxmode *rx_conf = &dev->data->dev_conf.rxmode;
>   	struct ixgbe_hw *hw = IXGBE_DEV_PRIVATE_TO_HW(dev->data->dev_private);
> -	struct rte_eth_dev_info dev_info = { 0 };
> +	struct rte_eth_dev_info dev_info = { .max_rx_queues = 0 };
>   	bool rsc_capable = false;
>   	uint16_t i;
>   	uint32_t rdrxctl;

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [dpdk-dev] [PATCH] ixgbe: fix build with gcc 4.4
  2015-04-14 12:48 ` Vlad Zolotarov
@ 2015-04-14 13:06   ` Ananyev, Konstantin
  2015-04-14 13:38     ` Vlad Zolotarov
  0 siblings, 1 reply; 33+ messages in thread
From: Ananyev, Konstantin @ 2015-04-14 13:06 UTC (permalink / raw)
  To: Vlad Zolotarov, Thomas Monjalon, Zhang, Helin; +Cc: dev



> -----Original Message-----
> From: Vlad Zolotarov [mailto:vladz@cloudius-systems.com]
> Sent: Tuesday, April 14, 2015 1:49 PM
> To: Thomas Monjalon; Ananyev, Konstantin; Zhang, Helin
> Cc: dev@dpdk.org
> Subject: Re: [PATCH] ixgbe: fix build with gcc 4.4
> 
> 
> 
> On 04/14/15 12:31, Thomas Monjalon wrote:
> > With GCC 4.4.7 from CentOS 6.5, the following errors arise:
> >
> > lib/librte_pmd_ixgbe/ixgbe_rxtx.c: In function ‘ixgbe_dev_rx_queue_setup’:
> > lib/librte_pmd_ixgbe/ixgbe_rxtx.c:2509: error: missing initializer
> > lib/librte_pmd_ixgbe/ixgbe_rxtx.c:2509: error: (near initialization for ‘dev_info.driver_name’)
> >
> > lib/librte_pmd_ixgbe/ixgbe_rxtx.c: In function ‘ixgbe_set_rsc’:
> > lib/librte_pmd_ixgbe/ixgbe_rxtx.c:4072: error: missing initializer
> > lib/librte_pmd_ixgbe/ixgbe_rxtx.c:4072: error: (near initialization for ‘dev_info.driver_name’)
> >
> > lib/librte_pmd_ixgbe/ixgbe_rxtx.c: In function ‘ixgbe_recv_pkts_lro_single_alloc’:
> > lib/librte_pmd_ixgbe/ixgbe_rxtx.c:1479: error: ‘next_rsc_entry’ may be used uninitialized in this function
> > lib/librte_pmd_ixgbe/ixgbe_rxtx.c:1480: error: ‘next_rxe’ may be used uninitialized in this function
> >
> > Fixes: 8eecb3295aed ("ixgbe: add LRO support")
> >
> > Signed-off-by: Thomas Monjalon <thomas.monjalon@6wind.com>
> > ---
> >   lib/librte_pmd_ixgbe/ixgbe_rxtx.c | 8 ++++----
> >   1 file changed, 4 insertions(+), 4 deletions(-)
> >
> > diff --git a/lib/librte_pmd_ixgbe/ixgbe_rxtx.c b/lib/librte_pmd_ixgbe/ixgbe_rxtx.c
> > index f1da9ec..a2b8631 100644
> > --- a/lib/librte_pmd_ixgbe/ixgbe_rxtx.c
> > +++ b/lib/librte_pmd_ixgbe/ixgbe_rxtx.c
> > @@ -1476,8 +1476,8 @@ ixgbe_recv_pkts_lro(void *rx_queue, struct rte_mbuf **rx_pkts, uint16_t nb_pkts,
> >   		bool eop;
> >   		struct ixgbe_rx_entry *rxe;
> >   		struct ixgbe_rsc_entry *rsc_entry;
> > -		struct ixgbe_rsc_entry *next_rsc_entry;
> > -		struct ixgbe_rx_entry *next_rxe;
> > +		struct ixgbe_rsc_entry *next_rsc_entry = NULL;
> > +		struct ixgbe_rx_entry *next_rxe = NULL;
> >   		struct rte_mbuf *first_seg;
> >   		struct rte_mbuf *rxm;
> >   		struct rte_mbuf *nmb;
> > @@ -2506,7 +2506,7 @@ ixgbe_dev_rx_queue_setup(struct rte_eth_dev *dev,
> >   	struct ixgbe_rx_queue *rxq;
> >   	struct ixgbe_hw     *hw;
> >   	uint16_t len;
> > -	struct rte_eth_dev_info dev_info = { 0 };
> > +	struct rte_eth_dev_info dev_info = { .max_rx_queues = 0 };
> >   	struct rte_eth_rxmode *dev_rx_mode = &dev->data->dev_conf.rxmode;
> >   	bool rsc_requested = false;
> >
> > @@ -4069,7 +4069,7 @@ ixgbe_set_rsc(struct rte_eth_dev *dev)
> >   {
> >   	struct rte_eth_rxmode *rx_conf = &dev->data->dev_conf.rxmode;
> >   	struct ixgbe_hw *hw = IXGBE_DEV_PRIVATE_TO_HW(dev->data->dev_private);
> > -	struct rte_eth_dev_info dev_info = { 0 };
> > +	struct rte_eth_dev_info dev_info = { .max_rx_queues = 0 };
> 
> Hmmm... Unless I miss something this and one above would zero only a
> single field - "max_rx_queues"; and would leave the rest uninitialized.
> The original code intend to zero the whole struct. The alternative to
> the original lines could be usage of memset().

As I understand, in that case compiler had to set all non-explicitly initialised members to 0.
So I think we are ok here.
 
> 
> >   	bool rsc_capable = false;
> >   	uint16_t i;
> >   	uint32_t rdrxctl;


^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [dpdk-dev] [PATCH] ixgbe: fix build with gcc 4.4
  2015-04-14 12:51 ` [dpdk-dev] [PATCH] ixgbe: fix build with gcc 4.4 Vlad Zolotarov
@ 2015-04-14 13:23   ` Ananyev, Konstantin
  2015-04-14 13:41     ` Vlad Zolotarov
  0 siblings, 1 reply; 33+ messages in thread
From: Ananyev, Konstantin @ 2015-04-14 13:23 UTC (permalink / raw)
  To: Vlad Zolotarov, Thomas Monjalon, Zhang, Helin; +Cc: dev



> -----Original Message-----
> From: Vlad Zolotarov [mailto:vladz@cloudius-systems.com]
> Sent: Tuesday, April 14, 2015 1:52 PM
> To: Thomas Monjalon; Ananyev, Konstantin; Zhang, Helin
> Cc: dev@dpdk.org
> Subject: Re: [PATCH] ixgbe: fix build with gcc 4.4
> 
> 
> 
> On 04/14/15 12:31, Thomas Monjalon wrote:
> > With GCC 4.4.7 from CentOS 6.5, the following errors arise:
> >
> > lib/librte_pmd_ixgbe/ixgbe_rxtx.c: In function ‘ixgbe_dev_rx_queue_setup’:
> > lib/librte_pmd_ixgbe/ixgbe_rxtx.c:2509: error: missing initializer
> > lib/librte_pmd_ixgbe/ixgbe_rxtx.c:2509: error: (near initialization for ‘dev_info.driver_name’)
> >
> > lib/librte_pmd_ixgbe/ixgbe_rxtx.c: In function ‘ixgbe_set_rsc’:
> > lib/librte_pmd_ixgbe/ixgbe_rxtx.c:4072: error: missing initializer
> > lib/librte_pmd_ixgbe/ixgbe_rxtx.c:4072: error: (near initialization for ‘dev_info.driver_name’)
> >
> > lib/librte_pmd_ixgbe/ixgbe_rxtx.c: In function ‘ixgbe_recv_pkts_lro_single_alloc’:
> > lib/librte_pmd_ixgbe/ixgbe_rxtx.c:1479: error: ‘next_rsc_entry’ may be used uninitialized in this function
> > lib/librte_pmd_ixgbe/ixgbe_rxtx.c:1480: error: ‘next_rxe’ may be used uninitialized in this function
> 
> :D Looks like a gcc bug ;) Both are set and only after that (!!!) used
> under "!eop" condition.

Possibly, but we still need to make it build cleanly.
Konstantin

> 
> >
> > Fixes: 8eecb3295aed ("ixgbe: add LRO support")
> >
> > Signed-off-by: Thomas Monjalon <thomas.monjalon@6wind.com>
> > ---
> >   lib/librte_pmd_ixgbe/ixgbe_rxtx.c | 8 ++++----
> >   1 file changed, 4 insertions(+), 4 deletions(-)
> >
> > diff --git a/lib/librte_pmd_ixgbe/ixgbe_rxtx.c b/lib/librte_pmd_ixgbe/ixgbe_rxtx.c
> > index f1da9ec..a2b8631 100644
> > --- a/lib/librte_pmd_ixgbe/ixgbe_rxtx.c
> > +++ b/lib/librte_pmd_ixgbe/ixgbe_rxtx.c
> > @@ -1476,8 +1476,8 @@ ixgbe_recv_pkts_lro(void *rx_queue, struct rte_mbuf **rx_pkts, uint16_t nb_pkts,
> >   		bool eop;
> >   		struct ixgbe_rx_entry *rxe;
> >   		struct ixgbe_rsc_entry *rsc_entry;
> > -		struct ixgbe_rsc_entry *next_rsc_entry;
> > -		struct ixgbe_rx_entry *next_rxe;
> > +		struct ixgbe_rsc_entry *next_rsc_entry = NULL;
> > +		struct ixgbe_rx_entry *next_rxe = NULL;
> >   		struct rte_mbuf *first_seg;
> >   		struct rte_mbuf *rxm;
> >   		struct rte_mbuf *nmb;
> > @@ -2506,7 +2506,7 @@ ixgbe_dev_rx_queue_setup(struct rte_eth_dev *dev,
> >   	struct ixgbe_rx_queue *rxq;
> >   	struct ixgbe_hw     *hw;
> >   	uint16_t len;
> > -	struct rte_eth_dev_info dev_info = { 0 };
> > +	struct rte_eth_dev_info dev_info = { .max_rx_queues = 0 };
> >   	struct rte_eth_rxmode *dev_rx_mode = &dev->data->dev_conf.rxmode;
> >   	bool rsc_requested = false;
> >
> > @@ -4069,7 +4069,7 @@ ixgbe_set_rsc(struct rte_eth_dev *dev)
> >   {
> >   	struct rte_eth_rxmode *rx_conf = &dev->data->dev_conf.rxmode;
> >   	struct ixgbe_hw *hw = IXGBE_DEV_PRIVATE_TO_HW(dev->data->dev_private);
> > -	struct rte_eth_dev_info dev_info = { 0 };
> > +	struct rte_eth_dev_info dev_info = { .max_rx_queues = 0 };
> >   	bool rsc_capable = false;
> >   	uint16_t i;
> >   	uint32_t rdrxctl;


^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [dpdk-dev] [PATCH] ixgbe: fix build with gcc 4.4
  2015-04-14 13:06   ` Ananyev, Konstantin
@ 2015-04-14 13:38     ` Vlad Zolotarov
  2015-04-14 14:17       ` Thomas Monjalon
  0 siblings, 1 reply; 33+ messages in thread
From: Vlad Zolotarov @ 2015-04-14 13:38 UTC (permalink / raw)
  To: Ananyev, Konstantin, Thomas Monjalon, Zhang, Helin; +Cc: dev



On 04/14/15 16:06, Ananyev, Konstantin wrote:
>
>> -----Original Message-----
>> From: Vlad Zolotarov [mailto:vladz@cloudius-systems.com]
>> Sent: Tuesday, April 14, 2015 1:49 PM
>> To: Thomas Monjalon; Ananyev, Konstantin; Zhang, Helin
>> Cc: dev@dpdk.org
>> Subject: Re: [PATCH] ixgbe: fix build with gcc 4.4
>>
>>
>>
>> On 04/14/15 12:31, Thomas Monjalon wrote:
>>> With GCC 4.4.7 from CentOS 6.5, the following errors arise:
>>>
>>> lib/librte_pmd_ixgbe/ixgbe_rxtx.c: In function ‘ixgbe_dev_rx_queue_setup’:
>>> lib/librte_pmd_ixgbe/ixgbe_rxtx.c:2509: error: missing initializer
>>> lib/librte_pmd_ixgbe/ixgbe_rxtx.c:2509: error: (near initialization for ‘dev_info.driver_name’)
>>>
>>> lib/librte_pmd_ixgbe/ixgbe_rxtx.c: In function ‘ixgbe_set_rsc’:
>>> lib/librte_pmd_ixgbe/ixgbe_rxtx.c:4072: error: missing initializer
>>> lib/librte_pmd_ixgbe/ixgbe_rxtx.c:4072: error: (near initialization for ‘dev_info.driver_name’)
>>>
>>> lib/librte_pmd_ixgbe/ixgbe_rxtx.c: In function ‘ixgbe_recv_pkts_lro_single_alloc’:
>>> lib/librte_pmd_ixgbe/ixgbe_rxtx.c:1479: error: ‘next_rsc_entry’ may be used uninitialized in this function
>>> lib/librte_pmd_ixgbe/ixgbe_rxtx.c:1480: error: ‘next_rxe’ may be used uninitialized in this function
>>>
>>> Fixes: 8eecb3295aed ("ixgbe: add LRO support")
>>>
>>> Signed-off-by: Thomas Monjalon <thomas.monjalon@6wind.com>
>>> ---
>>>    lib/librte_pmd_ixgbe/ixgbe_rxtx.c | 8 ++++----
>>>    1 file changed, 4 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/lib/librte_pmd_ixgbe/ixgbe_rxtx.c b/lib/librte_pmd_ixgbe/ixgbe_rxtx.c
>>> index f1da9ec..a2b8631 100644
>>> --- a/lib/librte_pmd_ixgbe/ixgbe_rxtx.c
>>> +++ b/lib/librte_pmd_ixgbe/ixgbe_rxtx.c
>>> @@ -1476,8 +1476,8 @@ ixgbe_recv_pkts_lro(void *rx_queue, struct rte_mbuf **rx_pkts, uint16_t nb_pkts,
>>>    		bool eop;
>>>    		struct ixgbe_rx_entry *rxe;
>>>    		struct ixgbe_rsc_entry *rsc_entry;
>>> -		struct ixgbe_rsc_entry *next_rsc_entry;
>>> -		struct ixgbe_rx_entry *next_rxe;
>>> +		struct ixgbe_rsc_entry *next_rsc_entry = NULL;
>>> +		struct ixgbe_rx_entry *next_rxe = NULL;
>>>    		struct rte_mbuf *first_seg;
>>>    		struct rte_mbuf *rxm;
>>>    		struct rte_mbuf *nmb;
>>> @@ -2506,7 +2506,7 @@ ixgbe_dev_rx_queue_setup(struct rte_eth_dev *dev,
>>>    	struct ixgbe_rx_queue *rxq;
>>>    	struct ixgbe_hw     *hw;
>>>    	uint16_t len;
>>> -	struct rte_eth_dev_info dev_info = { 0 };
>>> +	struct rte_eth_dev_info dev_info = { .max_rx_queues = 0 };
>>>    	struct rte_eth_rxmode *dev_rx_mode = &dev->data->dev_conf.rxmode;
>>>    	bool rsc_requested = false;
>>>
>>> @@ -4069,7 +4069,7 @@ ixgbe_set_rsc(struct rte_eth_dev *dev)
>>>    {
>>>    	struct rte_eth_rxmode *rx_conf = &dev->data->dev_conf.rxmode;
>>>    	struct ixgbe_hw *hw = IXGBE_DEV_PRIVATE_TO_HW(dev->data->dev_private);
>>> -	struct rte_eth_dev_info dev_info = { 0 };
>>> +	struct rte_eth_dev_info dev_info = { .max_rx_queues = 0 };
>> Hmmm... Unless I miss something this and one above would zero only a
>> single field - "max_rx_queues"; and would leave the rest uninitialized.
>> The original code intend to zero the whole struct. The alternative to
>> the original lines could be usage of memset().
> As I understand, in that case compiler had to set all non-explicitly initialised members to 0.
> So I think we are ok here.

Yeah, I guess it does zero-initializes the rest 
(https://gcc.gnu.org/onlinedocs/gcc/Designated-Inits.html) however I 
don't understand how the above change fixes the error if it complains 
about the dev_info.driver_name?

What I'm trying to say - the proposed fix is completely unclear and 
confusing. Think of somebody reading this line in a month from today - 
he wouldn't get a clue why is it there, why to explicitly set 
max_rx_queues to zero and leave the rest be zeroed automatically... Why 
to add such artifacts to the code instead of just zeroing the struct 
with a memset() and putting a good clear comment above it explaining why 
we use a memset() and not and initializer?

>   
>>>    	bool rsc_capable = false;
>>>    	uint16_t i;
>>>    	uint32_t rdrxctl;

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [dpdk-dev] [PATCH] ixgbe: fix build with gcc 4.4
  2015-04-14 13:23   ` Ananyev, Konstantin
@ 2015-04-14 13:41     ` Vlad Zolotarov
  0 siblings, 0 replies; 33+ messages in thread
From: Vlad Zolotarov @ 2015-04-14 13:41 UTC (permalink / raw)
  To: Ananyev, Konstantin, Thomas Monjalon, Zhang, Helin; +Cc: dev



On 04/14/15 16:23, Ananyev, Konstantin wrote:
>
>> -----Original Message-----
>> From: Vlad Zolotarov [mailto:vladz@cloudius-systems.com]
>> Sent: Tuesday, April 14, 2015 1:52 PM
>> To: Thomas Monjalon; Ananyev, Konstantin; Zhang, Helin
>> Cc: dev@dpdk.org
>> Subject: Re: [PATCH] ixgbe: fix build with gcc 4.4
>>
>>
>>
>> On 04/14/15 12:31, Thomas Monjalon wrote:
>>> With GCC 4.4.7 from CentOS 6.5, the following errors arise:
>>>
>>> lib/librte_pmd_ixgbe/ixgbe_rxtx.c: In function ‘ixgbe_dev_rx_queue_setup’:
>>> lib/librte_pmd_ixgbe/ixgbe_rxtx.c:2509: error: missing initializer
>>> lib/librte_pmd_ixgbe/ixgbe_rxtx.c:2509: error: (near initialization for ‘dev_info.driver_name’)
>>>
>>> lib/librte_pmd_ixgbe/ixgbe_rxtx.c: In function ‘ixgbe_set_rsc’:
>>> lib/librte_pmd_ixgbe/ixgbe_rxtx.c:4072: error: missing initializer
>>> lib/librte_pmd_ixgbe/ixgbe_rxtx.c:4072: error: (near initialization for ‘dev_info.driver_name’)
>>>
>>> lib/librte_pmd_ixgbe/ixgbe_rxtx.c: In function ‘ixgbe_recv_pkts_lro_single_alloc’:
>>> lib/librte_pmd_ixgbe/ixgbe_rxtx.c:1479: error: ‘next_rsc_entry’ may be used uninitialized in this function
>>> lib/librte_pmd_ixgbe/ixgbe_rxtx.c:1480: error: ‘next_rxe’ may be used uninitialized in this function
>> :D Looks like a gcc bug ;) Both are set and only after that (!!!) used
>> under "!eop" condition.
> Possibly, but we still need to make it build cleanly.

It's clearly - I was just trying to be polite here... ;)
Please, add the comment explaining this initialization so that nobody 
removes these workarounds by mistake...

> Konstantin
>
>>> Fixes: 8eecb3295aed ("ixgbe: add LRO support")
>>>
>>> Signed-off-by: Thomas Monjalon <thomas.monjalon@6wind.com>
>>> ---
>>>    lib/librte_pmd_ixgbe/ixgbe_rxtx.c | 8 ++++----
>>>    1 file changed, 4 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/lib/librte_pmd_ixgbe/ixgbe_rxtx.c b/lib/librte_pmd_ixgbe/ixgbe_rxtx.c
>>> index f1da9ec..a2b8631 100644
>>> --- a/lib/librte_pmd_ixgbe/ixgbe_rxtx.c
>>> +++ b/lib/librte_pmd_ixgbe/ixgbe_rxtx.c
>>> @@ -1476,8 +1476,8 @@ ixgbe_recv_pkts_lro(void *rx_queue, struct rte_mbuf **rx_pkts, uint16_t nb_pkts,
>>>    		bool eop;
>>>    		struct ixgbe_rx_entry *rxe;
>>>    		struct ixgbe_rsc_entry *rsc_entry;
>>> -		struct ixgbe_rsc_entry *next_rsc_entry;
>>> -		struct ixgbe_rx_entry *next_rxe;
>>> +		struct ixgbe_rsc_entry *next_rsc_entry = NULL;
>>> +		struct ixgbe_rx_entry *next_rxe = NULL;
>>>    		struct rte_mbuf *first_seg;
>>>    		struct rte_mbuf *rxm;
>>>    		struct rte_mbuf *nmb;
>>> @@ -2506,7 +2506,7 @@ ixgbe_dev_rx_queue_setup(struct rte_eth_dev *dev,
>>>    	struct ixgbe_rx_queue *rxq;
>>>    	struct ixgbe_hw     *hw;
>>>    	uint16_t len;
>>> -	struct rte_eth_dev_info dev_info = { 0 };
>>> +	struct rte_eth_dev_info dev_info = { .max_rx_queues = 0 };
>>>    	struct rte_eth_rxmode *dev_rx_mode = &dev->data->dev_conf.rxmode;
>>>    	bool rsc_requested = false;
>>>
>>> @@ -4069,7 +4069,7 @@ ixgbe_set_rsc(struct rte_eth_dev *dev)
>>>    {
>>>    	struct rte_eth_rxmode *rx_conf = &dev->data->dev_conf.rxmode;
>>>    	struct ixgbe_hw *hw = IXGBE_DEV_PRIVATE_TO_HW(dev->data->dev_private);
>>> -	struct rte_eth_dev_info dev_info = { 0 };
>>> +	struct rte_eth_dev_info dev_info = { .max_rx_queues = 0 };
>>>    	bool rsc_capable = false;
>>>    	uint16_t i;
>>>    	uint32_t rdrxctl;

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [dpdk-dev] [PATCH] ixgbe: fix build with gcc 4.4
  2015-04-14 13:38     ` Vlad Zolotarov
@ 2015-04-14 14:17       ` Thomas Monjalon
  2015-04-14 14:30         ` Vlad Zolotarov
  2015-04-14 14:59         ` Vlad Zolotarov
  0 siblings, 2 replies; 33+ messages in thread
From: Thomas Monjalon @ 2015-04-14 14:17 UTC (permalink / raw)
  To: Vlad Zolotarov; +Cc: dev

2015-04-14 16:38, Vlad Zolotarov:
> On 04/14/15 16:06, Ananyev, Konstantin wrote:
> > From: Vlad Zolotarov [mailto:vladz@cloudius-systems.com]
> >> On 04/14/15 12:31, Thomas Monjalon wrote:
> >>> -	struct rte_eth_dev_info dev_info = { 0 };
> >>> +	struct rte_eth_dev_info dev_info = { .max_rx_queues = 0 };
> >> 
> >> Hmmm... Unless I miss something this and one above would zero only a
> >> single field - "max_rx_queues"; and would leave the rest uninitialized.
> >> The original code intend to zero the whole struct. The alternative to
> >> the original lines could be usage of memset().
> > 
> > As I understand, in that case compiler had to set all non-explicitly initialised members to 0.
> > So I think we are ok here.
> 
> Yeah, I guess it does zero-initializes the rest 
> (https://gcc.gnu.org/onlinedocs/gcc/Designated-Inits.html) however I 
> don't understand how the above change fixes the error if it complains 
> about the dev_info.driver_name?

As only 1 field is required, I chose the one which should not be removed
from this structure in the future.

> What I'm trying to say - the proposed fix is completely unclear and 
> confusing. Think of somebody reading this line in a month from today - 
> he wouldn't get a clue why is it there, why to explicitly set 
> max_rx_queues to zero and leave the rest be zeroed automatically... Why 
> to add such artifacts to the code instead of just zeroing the struct 
> with a memset() and putting a good clear comment above it explaining why 
> we use a memset() and not and initializer?

We can make it longer yes.
I think you agree we should avoid extra lines if not needed.
In this case, when reading "= { .field = 0 }", it seems clear our goal
is to zero the structure (it is to me).
I thought it is a basic C practice.

You should try "git grep '\.[^ ]\+ *= *0 *}'" to be convinced that we are
not going to comment each occurence of this coding style.
But it must be explained in the coding style document. Agree?

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [dpdk-dev] [PATCH] ixgbe: fix build with gcc 4.4
  2015-04-14 14:17       ` Thomas Monjalon
@ 2015-04-14 14:30         ` Vlad Zolotarov
  2015-04-14 14:53           ` Thomas Monjalon
  2015-04-14 14:59         ` Vlad Zolotarov
  1 sibling, 1 reply; 33+ messages in thread
From: Vlad Zolotarov @ 2015-04-14 14:30 UTC (permalink / raw)
  To: Thomas Monjalon; +Cc: dev



On 04/14/15 17:17, Thomas Monjalon wrote:
> 2015-04-14 16:38, Vlad Zolotarov:
>> On 04/14/15 16:06, Ananyev, Konstantin wrote:
>>> From: Vlad Zolotarov [mailto:vladz@cloudius-systems.com]
>>>> On 04/14/15 12:31, Thomas Monjalon wrote:
>>>>> -	struct rte_eth_dev_info dev_info = { 0 };
>>>>> +	struct rte_eth_dev_info dev_info = { .max_rx_queues = 0 };
>>>> Hmmm... Unless I miss something this and one above would zero only a
>>>> single field - "max_rx_queues"; and would leave the rest uninitialized.
>>>> The original code intend to zero the whole struct. The alternative to
>>>> the original lines could be usage of memset().
>>> As I understand, in that case compiler had to set all non-explicitly initialised members to 0.
>>> So I think we are ok here.
>> Yeah, I guess it does zero-initializes the rest
>> (https://gcc.gnu.org/onlinedocs/gcc/Designated-Inits.html) however I
>> don't understand how the above change fixes the error if it complains
>> about the dev_info.driver_name?
> As only 1 field is required, I chose the one which should not be removed
> from this structure in the future.

I don't follow - where/why only one field is required? The function u 
are patching uses "rx_offload_capa" field. Or u mean this gcc version 
requires only one field? If so, could u, please, provide the errata u 
are referring, since standard doesn't require any field and {0} is an 
absolutely legal (and proper) initializer in this case...

>
>> What I'm trying to say - the proposed fix is completely unclear and
>> confusing. Think of somebody reading this line in a month from today -
>> he wouldn't get a clue why is it there, why to explicitly set
>> max_rx_queues to zero and leave the rest be zeroed automatically... Why
>> to add such artifacts to the code instead of just zeroing the struct
>> with a memset() and putting a good clear comment above it explaining why
>> we use a memset() and not and initializer?
> We can make it longer yes.
> I think you agree we should avoid extra lines if not needed.
> In this case, when reading "= { .field = 0 }", it seems clear our goal
> is to zero the structure (it is to me).
> I thought it is a basic C practice.
>
> You should try "git grep '\.[^ ]\+ *= *0 *}'" to be convinced that we are
> not going to comment each occurence of this coding style.
> But it must be explained in the coding style document. Agree?

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [dpdk-dev] [PATCH] ixgbe: fix build with gcc 4.4
  2015-04-14 14:30         ` Vlad Zolotarov
@ 2015-04-14 14:53           ` Thomas Monjalon
  2015-04-14 15:17             ` Vlad Zolotarov
  0 siblings, 1 reply; 33+ messages in thread
From: Thomas Monjalon @ 2015-04-14 14:53 UTC (permalink / raw)
  To: Vlad Zolotarov; +Cc: dev

2015-04-14 17:30, Vlad Zolotarov:
> On 04/14/15 17:17, Thomas Monjalon wrote:
> > 2015-04-14 16:38, Vlad Zolotarov:
> >> On 04/14/15 16:06, Ananyev, Konstantin wrote:
> >>> From: Vlad Zolotarov [mailto:vladz@cloudius-systems.com]
> >>>> On 04/14/15 12:31, Thomas Monjalon wrote:
> >>>>> -	struct rte_eth_dev_info dev_info = { 0 };
> >>>>> +	struct rte_eth_dev_info dev_info = { .max_rx_queues = 0 };
> >>>> Hmmm... Unless I miss something this and one above would zero only a
> >>>> single field - "max_rx_queues"; and would leave the rest uninitialized.
> >>>> The original code intend to zero the whole struct. The alternative to
> >>>> the original lines could be usage of memset().
> >>> As I understand, in that case compiler had to set all non-explicitly initialised members to 0.
> >>> So I think we are ok here.
> >> Yeah, I guess it does zero-initializes the rest
> >> (https://gcc.gnu.org/onlinedocs/gcc/Designated-Inits.html) however I
> >> don't understand how the above change fixes the error if it complains
> >> about the dev_info.driver_name?
> > As only 1 field is required, I chose the one which should not be removed
> > from this structure in the future.
> 
> I don't follow - where/why only one field is required? The function u 
> are patching uses "rx_offload_capa" field. Or u mean this gcc version 
> requires only one field? If so, could u, please, provide the errata u 
> are referring, since standard doesn't require any field and {0} is an 
> absolutely legal (and proper) initializer in this case...

Honestly I don't really care what is "legal". The most important is to make
it working with most C compilers with minimal overhead.
You're right about the variable choice: rx_offload_capa is more appropriate.
Are you OK for a v2 replacing max_rx_queues by rx_offload_capa?

> >> What I'm trying to say - the proposed fix is completely unclear and
> >> confusing. Think of somebody reading this line in a month from today -
> >> he wouldn't get a clue why is it there, why to explicitly set
> >> max_rx_queues to zero and leave the rest be zeroed automatically... Why
> >> to add such artifacts to the code instead of just zeroing the struct
> >> with a memset() and putting a good clear comment above it explaining why
> >> we use a memset() and not and initializer?
> > We can make it longer yes.
> > I think you agree we should avoid extra lines if not needed.
> > In this case, when reading "= { .field = 0 }", it seems clear our goal
> > is to zero the structure (it is to me).
> > I thought it is a basic C practice.
> >
> > You should try "git grep '\.[^ ]\+ *= *0 *}'" to be convinced that we are
> > not going to comment each occurence of this coding style.
> > But it must be explained in the coding style document. Agree?
> 

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [dpdk-dev] [PATCH] ixgbe: fix build with gcc 4.4
  2015-04-14 14:17       ` Thomas Monjalon
  2015-04-14 14:30         ` Vlad Zolotarov
@ 2015-04-14 14:59         ` Vlad Zolotarov
  2015-04-14 15:13           ` Thomas Monjalon
  1 sibling, 1 reply; 33+ messages in thread
From: Vlad Zolotarov @ 2015-04-14 14:59 UTC (permalink / raw)
  To: Thomas Monjalon; +Cc: dev



On 04/14/15 17:17, Thomas Monjalon wrote:
> 2015-04-14 16:38, Vlad Zolotarov:
>> On 04/14/15 16:06, Ananyev, Konstantin wrote:
>>> From: Vlad Zolotarov [mailto:vladz@cloudius-systems.com]
>>>> On 04/14/15 12:31, Thomas Monjalon wrote:
>>>>> -	struct rte_eth_dev_info dev_info = { 0 };
>>>>> +	struct rte_eth_dev_info dev_info = { .max_rx_queues = 0 };
>>>> Hmmm... Unless I miss something this and one above would zero only a
>>>> single field - "max_rx_queues"; and would leave the rest uninitialized.
>>>> The original code intend to zero the whole struct. The alternative to
>>>> the original lines could be usage of memset().
>>> As I understand, in that case compiler had to set all non-explicitly initialised members to 0.
>>> So I think we are ok here.
>> Yeah, I guess it does zero-initializes the rest
>> (https://gcc.gnu.org/onlinedocs/gcc/Designated-Inits.html) however I
>> don't understand how the above change fixes the error if it complains
>> about the dev_info.driver_name?
> As only 1 field is required, I chose the one which should not be removed
> from this structure in the future.
>
>> What I'm trying to say - the proposed fix is completely unclear and
>> confusing. Think of somebody reading this line in a month from today -
>> he wouldn't get a clue why is it there, why to explicitly set
>> max_rx_queues to zero and leave the rest be zeroed automatically... Why
>> to add such artifacts to the code instead of just zeroing the struct
>> with a memset() and putting a good clear comment above it explaining why
>> we use a memset() and not and initializer?
> We can make it longer yes.
> I think you agree we should avoid extra lines if not needed.
> In this case, when reading "= { .field = 0 }", it seems clear our goal
> is to zero the structure (it is to me).

I'm sorry but it's not clear to me at all since the common C practice 
for zeroing the struct would be

struct st a = {0};

Like in the lines u are changing. The lines as above are clearly should 
not be commented and are absolutely clear.
The lines u are adding on the other hand are absolutely unclear and 
confusing outside the gcc bug context. Therefore it should be clearly 
stated so in a form of comment. Otherwise somebody (like myself) may see 
this and immediately fix it back (as it should be).

> I thought it is a basic C practice.

I doubt that. ;) Explained above.

>
> You should try "git grep '\.[^ ]\+ *= *0 *}'" to be convinced that we are
> not going to comment each occurence of this coding style.
> But it must be explained in the coding style document. Agree?

OMG! This is awful! I think everybody agrees that this is a workaround 
and has nothing to do with a codding style (it's an opposite to a style 
actually). I don't know where this should be explained, frankly.

Getting back to the issue - I'm a bit surprised since I use this kind of 
initializer ({0}) in a C code for quite a long time - long before 2012. 
I'd like to understand what is a problem with this specific gcc version. 
This seems to trivial. I'm surprised CentOS has a gcc version with this 
kind of bugs.

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [dpdk-dev] [PATCH] ixgbe: fix build with gcc 4.4
  2015-04-14 14:59         ` Vlad Zolotarov
@ 2015-04-14 15:13           ` Thomas Monjalon
  2015-04-14 15:21             ` Vlad Zolotarov
  0 siblings, 1 reply; 33+ messages in thread
From: Thomas Monjalon @ 2015-04-14 15:13 UTC (permalink / raw)
  To: Vlad Zolotarov; +Cc: dev

2015-04-14 17:59, Vlad Zolotarov:
> On 04/14/15 17:17, Thomas Monjalon wrote:
> > 2015-04-14 16:38, Vlad Zolotarov:
> >> On 04/14/15 16:06, Ananyev, Konstantin wrote:
> >>> From: Vlad Zolotarov [mailto:vladz@cloudius-systems.com]
> >>>> On 04/14/15 12:31, Thomas Monjalon wrote:
> >>>>> -	struct rte_eth_dev_info dev_info = { 0 };
> >>>>> +	struct rte_eth_dev_info dev_info = { .max_rx_queues = 0 };
> >>>> Hmmm... Unless I miss something this and one above would zero only a
> >>>> single field - "max_rx_queues"; and would leave the rest uninitialized.
> >>>> The original code intend to zero the whole struct. The alternative to
> >>>> the original lines could be usage of memset().
> >>> As I understand, in that case compiler had to set all non-explicitly initialised members to 0.
> >>> So I think we are ok here.
> >> Yeah, I guess it does zero-initializes the rest
> >> (https://gcc.gnu.org/onlinedocs/gcc/Designated-Inits.html) however I
> >> don't understand how the above change fixes the error if it complains
> >> about the dev_info.driver_name?
> > As only 1 field is required, I chose the one which should not be removed
> > from this structure in the future.
> >
> >> What I'm trying to say - the proposed fix is completely unclear and
> >> confusing. Think of somebody reading this line in a month from today -
> >> he wouldn't get a clue why is it there, why to explicitly set
> >> max_rx_queues to zero and leave the rest be zeroed automatically... Why
> >> to add such artifacts to the code instead of just zeroing the struct
> >> with a memset() and putting a good clear comment above it explaining why
> >> we use a memset() and not and initializer?
> > We can make it longer yes.
> > I think you agree we should avoid extra lines if not needed.
> > In this case, when reading "= { .field = 0 }", it seems clear our goal
> > is to zero the structure (it is to me).
> 
> I'm sorry but it's not clear to me at all since the common C practice 
> for zeroing the struct would be
> 
> struct st a = {0};
> 
> Like in the lines u are changing. The lines as above are clearly should 
> not be commented and are absolutely clear.
> The lines u are adding on the other hand are absolutely unclear and 
> confusing outside the gcc bug context. Therefore it should be clearly 
> stated so in a form of comment. Otherwise somebody (like myself) may see 
> this and immediately fix it back (as it should be).
> 
> > I thought it is a basic C practice.
> 
> I doubt that. ;) Explained above.
> 
> > You should try "git grep '\.[^ ]\+ *= *0 *}'" to be convinced that we are
> > not going to comment each occurence of this coding style.
> > But it must be explained in the coding style document. Agree?
> 
> OMG! This is awful! I think everybody agrees that this is a workaround 
> and has nothing to do with a codding style (it's an opposite to a style 
> actually). I don't know where this should be explained, frankly.

Once we assert we want to support this buggy compiler, the workarounds
are automatically parts of the coding style.
I don't know how to deal differently with this constraint.

> Getting back to the issue - I'm a bit surprised since I use this kind of 
> initializer ({0}) in a C code for quite a long time - long before 2012. 
> I'd like to understand what is a problem with this specific gcc version. 
> This seems to trivial. I'm surprised CentOS has a gcc version with this 
> kind of bugs.

Each day brings its surprise :)

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [dpdk-dev] [PATCH] ixgbe: fix build with gcc 4.4
  2015-04-14 14:53           ` Thomas Monjalon
@ 2015-04-14 15:17             ` Vlad Zolotarov
  0 siblings, 0 replies; 33+ messages in thread
From: Vlad Zolotarov @ 2015-04-14 15:17 UTC (permalink / raw)
  To: Thomas Monjalon; +Cc: dev



On 04/14/15 17:53, Thomas Monjalon wrote:
> 2015-04-14 17:30, Vlad Zolotarov:
>> On 04/14/15 17:17, Thomas Monjalon wrote:
>>> 2015-04-14 16:38, Vlad Zolotarov:
>>>> On 04/14/15 16:06, Ananyev, Konstantin wrote:
>>>>> From: Vlad Zolotarov [mailto:vladz@cloudius-systems.com]
>>>>>> On 04/14/15 12:31, Thomas Monjalon wrote:
>>>>>>> -	struct rte_eth_dev_info dev_info = { 0 };
>>>>>>> +	struct rte_eth_dev_info dev_info = { .max_rx_queues = 0 };
>>>>>> Hmmm... Unless I miss something this and one above would zero only a
>>>>>> single field - "max_rx_queues"; and would leave the rest uninitialized.
>>>>>> The original code intend to zero the whole struct. The alternative to
>>>>>> the original lines could be usage of memset().
>>>>> As I understand, in that case compiler had to set all non-explicitly initialised members to 0.
>>>>> So I think we are ok here.
>>>> Yeah, I guess it does zero-initializes the rest
>>>> (https://gcc.gnu.org/onlinedocs/gcc/Designated-Inits.html) however I
>>>> don't understand how the above change fixes the error if it complains
>>>> about the dev_info.driver_name?
>>> As only 1 field is required, I chose the one which should not be removed
>>> from this structure in the future.
>> I don't follow - where/why only one field is required? The function u
>> are patching uses "rx_offload_capa" field. Or u mean this gcc version
>> requires only one field? If so, could u, please, provide the errata u
>> are referring, since standard doesn't require any field and {0} is an
>> absolutely legal (and proper) initializer in this case...
> Honestly I don't really care what is "legal". The most important is to make
> it working with most C compilers with minimal overhead.

It's not just a "legal" - it's the most correct and robust way of 
initializing the struct that is promised to always work correctly. See 
here 
http://stackoverflow.com/questions/11152160/initializing-a-struct-to-0. 
What u hit here is (as appears) a well known Bug #53119 in gcc (see here 
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=53119).

Have u considered adding the compilation options like 
-Wno-missing-braces that would silence this warning for say gcc versions 
below 4.7?

> You're right about the variable choice: rx_offload_capa is more appropriate.
> Are you OK for a v2 replacing max_rx_queues by rx_offload_capa?
>
>>>> What I'm trying to say - the proposed fix is completely unclear and
>>>> confusing. Think of somebody reading this line in a month from today -
>>>> he wouldn't get a clue why is it there, why to explicitly set
>>>> max_rx_queues to zero and leave the rest be zeroed automatically... Why
>>>> to add such artifacts to the code instead of just zeroing the struct
>>>> with a memset() and putting a good clear comment above it explaining why
>>>> we use a memset() and not and initializer?
>>> We can make it longer yes.
>>> I think you agree we should avoid extra lines if not needed.
>>> In this case, when reading "= { .field = 0 }", it seems clear our goal
>>> is to zero the structure (it is to me).
>>> I thought it is a basic C practice.
>>>
>>> You should try "git grep '\.[^ ]\+ *= *0 *}'" to be convinced that we are
>>> not going to comment each occurence of this coding style.
>>> But it must be explained in the coding style document. Agree?
>

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [dpdk-dev] [PATCH] ixgbe: fix build with gcc 4.4
  2015-04-14 15:13           ` Thomas Monjalon
@ 2015-04-14 15:21             ` Vlad Zolotarov
  2015-04-14 15:28               ` Thomas Monjalon
  0 siblings, 1 reply; 33+ messages in thread
From: Vlad Zolotarov @ 2015-04-14 15:21 UTC (permalink / raw)
  To: Thomas Monjalon; +Cc: dev



On 04/14/15 18:13, Thomas Monjalon wrote:
> 2015-04-14 17:59, Vlad Zolotarov:
>> On 04/14/15 17:17, Thomas Monjalon wrote:
>>> 2015-04-14 16:38, Vlad Zolotarov:
>>>> On 04/14/15 16:06, Ananyev, Konstantin wrote:
>>>>> From: Vlad Zolotarov [mailto:vladz@cloudius-systems.com]
>>>>>> On 04/14/15 12:31, Thomas Monjalon wrote:
>>>>>>> -	struct rte_eth_dev_info dev_info = { 0 };
>>>>>>> +	struct rte_eth_dev_info dev_info = { .max_rx_queues = 0 };
>>>>>> Hmmm... Unless I miss something this and one above would zero only a
>>>>>> single field - "max_rx_queues"; and would leave the rest uninitialized.
>>>>>> The original code intend to zero the whole struct. The alternative to
>>>>>> the original lines could be usage of memset().
>>>>> As I understand, in that case compiler had to set all non-explicitly initialised members to 0.
>>>>> So I think we are ok here.
>>>> Yeah, I guess it does zero-initializes the rest
>>>> (https://gcc.gnu.org/onlinedocs/gcc/Designated-Inits.html) however I
>>>> don't understand how the above change fixes the error if it complains
>>>> about the dev_info.driver_name?
>>> As only 1 field is required, I chose the one which should not be removed
>>> from this structure in the future.
>>>
>>>> What I'm trying to say - the proposed fix is completely unclear and
>>>> confusing. Think of somebody reading this line in a month from today -
>>>> he wouldn't get a clue why is it there, why to explicitly set
>>>> max_rx_queues to zero and leave the rest be zeroed automatically... Why
>>>> to add such artifacts to the code instead of just zeroing the struct
>>>> with a memset() and putting a good clear comment above it explaining why
>>>> we use a memset() and not and initializer?
>>> We can make it longer yes.
>>> I think you agree we should avoid extra lines if not needed.
>>> In this case, when reading "= { .field = 0 }", it seems clear our goal
>>> is to zero the structure (it is to me).
>> I'm sorry but it's not clear to me at all since the common C practice
>> for zeroing the struct would be
>>
>> struct st a = {0};
>>
>> Like in the lines u are changing. The lines as above are clearly should
>> not be commented and are absolutely clear.
>> The lines u are adding on the other hand are absolutely unclear and
>> confusing outside the gcc bug context. Therefore it should be clearly
>> stated so in a form of comment. Otherwise somebody (like myself) may see
>> this and immediately fix it back (as it should be).
>>
>>> I thought it is a basic C practice.
>> I doubt that. ;) Explained above.
>>
>>> You should try "git grep '\.[^ ]\+ *= *0 *}'" to be convinced that we are
>>> not going to comment each occurence of this coding style.
>>> But it must be explained in the coding style document. Agree?
>> OMG! This is awful! I think everybody agrees that this is a workaround
>> and has nothing to do with a codding style (it's an opposite to a style
>> actually). I don't know where this should be explained, frankly.
> Once we assert we want to support this buggy compiler, the workarounds
> are automatically parts of the coding style.

It'd rather not... ;)

> I don't know how to deal differently with this constraint.

Add -Wno-missing-braces compilation option for compiler versions below 
4.7. U (and me and I guess most other developers) compile DPDK code with 
a newer compiler thus the code would be properly inspected with these 
compilers and we may afford to be less restrictive with compilation 
warnings with legacy compiler versions...

>
>> Getting back to the issue - I'm a bit surprised since I use this kind of
>> initializer ({0}) in a C code for quite a long time - long before 2012.
>> I'd like to understand what is a problem with this specific gcc version.
>> This seems to trivial. I'm surprised CentOS has a gcc version with this
>> kind of bugs.
> Each day brings its surprise :)
>

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [dpdk-dev] [PATCH] ixgbe: fix build with gcc 4.4
  2015-04-14 15:21             ` Vlad Zolotarov
@ 2015-04-14 15:28               ` Thomas Monjalon
  2015-04-14 15:32                 ` Vlad Zolotarov
  2015-04-15 20:49                 ` [dpdk-dev] [PATCH v2 1/2] " Thomas Monjalon
  0 siblings, 2 replies; 33+ messages in thread
From: Thomas Monjalon @ 2015-04-14 15:28 UTC (permalink / raw)
  To: Vlad Zolotarov; +Cc: dev

2015-04-14 18:21, Vlad Zolotarov:
> 
> On 04/14/15 18:13, Thomas Monjalon wrote:
> > 2015-04-14 17:59, Vlad Zolotarov:
> >> On 04/14/15 17:17, Thomas Monjalon wrote:
> >>> 2015-04-14 16:38, Vlad Zolotarov:
> >>>> On 04/14/15 16:06, Ananyev, Konstantin wrote:
> >>>>> From: Vlad Zolotarov [mailto:vladz@cloudius-systems.com]
> >>>>>> On 04/14/15 12:31, Thomas Monjalon wrote:
> >>>>>>> -	struct rte_eth_dev_info dev_info = { 0 };
> >>>>>>> +	struct rte_eth_dev_info dev_info = { .max_rx_queues = 0 };
> >>>>>> Hmmm... Unless I miss something this and one above would zero only a
> >>>>>> single field - "max_rx_queues"; and would leave the rest uninitialized.
> >>>>>> The original code intend to zero the whole struct. The alternative to
> >>>>>> the original lines could be usage of memset().
> >>>>> As I understand, in that case compiler had to set all non-explicitly initialised members to 0.
> >>>>> So I think we are ok here.
> >>>> Yeah, I guess it does zero-initializes the rest
> >>>> (https://gcc.gnu.org/onlinedocs/gcc/Designated-Inits.html) however I
> >>>> don't understand how the above change fixes the error if it complains
> >>>> about the dev_info.driver_name?
> >>> As only 1 field is required, I chose the one which should not be removed
> >>> from this structure in the future.
> >>>
> >>>> What I'm trying to say - the proposed fix is completely unclear and
> >>>> confusing. Think of somebody reading this line in a month from today -
> >>>> he wouldn't get a clue why is it there, why to explicitly set
> >>>> max_rx_queues to zero and leave the rest be zeroed automatically... Why
> >>>> to add such artifacts to the code instead of just zeroing the struct
> >>>> with a memset() and putting a good clear comment above it explaining why
> >>>> we use a memset() and not and initializer?
> >>> We can make it longer yes.
> >>> I think you agree we should avoid extra lines if not needed.
> >>> In this case, when reading "= { .field = 0 }", it seems clear our goal
> >>> is to zero the structure (it is to me).
> >> I'm sorry but it's not clear to me at all since the common C practice
> >> for zeroing the struct would be
> >>
> >> struct st a = {0};
> >>
> >> Like in the lines u are changing. The lines as above are clearly should
> >> not be commented and are absolutely clear.
> >> The lines u are adding on the other hand are absolutely unclear and
> >> confusing outside the gcc bug context. Therefore it should be clearly
> >> stated so in a form of comment. Otherwise somebody (like myself) may see
> >> this and immediately fix it back (as it should be).
> >>
> >>> I thought it is a basic C practice.
> >> I doubt that. ;) Explained above.
> >>
> >>> You should try "git grep '\.[^ ]\+ *= *0 *}'" to be convinced that we are
> >>> not going to comment each occurence of this coding style.
> >>> But it must be explained in the coding style document. Agree?
> >> OMG! This is awful! I think everybody agrees that this is a workaround
> >> and has nothing to do with a codding style (it's an opposite to a style
> >> actually). I don't know where this should be explained, frankly.
> > Once we assert we want to support this buggy compiler, the workarounds
> > are automatically parts of the coding style.
> 
> It'd rather not... ;)
> 
> > I don't know how to deal differently with this constraint.
> 
> Add -Wno-missing-braces compilation option for compiler versions below 
> 4.7. U (and me and I guess most other developers) compile DPDK code with 
> a newer compiler thus the code would be properly inspected with these 
> compilers and we may afford to be less restrictive with compilation 
> warnings with legacy compiler versions...

You're right.
I will test it and submit a v2.
Then I could use the above grep command to replace other occurences of this
workaround.

> >> Getting back to the issue - I'm a bit surprised since I use this kind of
> >> initializer ({0}) in a C code for quite a long time - long before 2012.
> >> I'd like to understand what is a problem with this specific gcc version.
> >> This seems to trivial. I'm surprised CentOS has a gcc version with this
> >> kind of bugs.
> > Each day brings its surprise :)

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [dpdk-dev] [PATCH] ixgbe: fix build with gcc 4.4
  2015-04-14 15:28               ` Thomas Monjalon
@ 2015-04-14 15:32                 ` Vlad Zolotarov
  2015-04-15 20:49                 ` [dpdk-dev] [PATCH v2 1/2] " Thomas Monjalon
  1 sibling, 0 replies; 33+ messages in thread
From: Vlad Zolotarov @ 2015-04-14 15:32 UTC (permalink / raw)
  To: Thomas Monjalon; +Cc: dev



On 04/14/15 18:28, Thomas Monjalon wrote:
> 2015-04-14 18:21, Vlad Zolotarov:
>> On 04/14/15 18:13, Thomas Monjalon wrote:
>>> 2015-04-14 17:59, Vlad Zolotarov:
>>>> On 04/14/15 17:17, Thomas Monjalon wrote:
>>>>> 2015-04-14 16:38, Vlad Zolotarov:
>>>>>> On 04/14/15 16:06, Ananyev, Konstantin wrote:
>>>>>>> From: Vlad Zolotarov [mailto:vladz@cloudius-systems.com]
>>>>>>>> On 04/14/15 12:31, Thomas Monjalon wrote:
>>>>>>>>> -	struct rte_eth_dev_info dev_info = { 0 };
>>>>>>>>> +	struct rte_eth_dev_info dev_info = { .max_rx_queues = 0 };
>>>>>>>> Hmmm... Unless I miss something this and one above would zero only a
>>>>>>>> single field - "max_rx_queues"; and would leave the rest uninitialized.
>>>>>>>> The original code intend to zero the whole struct. The alternative to
>>>>>>>> the original lines could be usage of memset().
>>>>>>> As I understand, in that case compiler had to set all non-explicitly initialised members to 0.
>>>>>>> So I think we are ok here.
>>>>>> Yeah, I guess it does zero-initializes the rest
>>>>>> (https://gcc.gnu.org/onlinedocs/gcc/Designated-Inits.html) however I
>>>>>> don't understand how the above change fixes the error if it complains
>>>>>> about the dev_info.driver_name?
>>>>> As only 1 field is required, I chose the one which should not be removed
>>>>> from this structure in the future.
>>>>>
>>>>>> What I'm trying to say - the proposed fix is completely unclear and
>>>>>> confusing. Think of somebody reading this line in a month from today -
>>>>>> he wouldn't get a clue why is it there, why to explicitly set
>>>>>> max_rx_queues to zero and leave the rest be zeroed automatically... Why
>>>>>> to add such artifacts to the code instead of just zeroing the struct
>>>>>> with a memset() and putting a good clear comment above it explaining why
>>>>>> we use a memset() and not and initializer?
>>>>> We can make it longer yes.
>>>>> I think you agree we should avoid extra lines if not needed.
>>>>> In this case, when reading "= { .field = 0 }", it seems clear our goal
>>>>> is to zero the structure (it is to me).
>>>> I'm sorry but it's not clear to me at all since the common C practice
>>>> for zeroing the struct would be
>>>>
>>>> struct st a = {0};
>>>>
>>>> Like in the lines u are changing. The lines as above are clearly should
>>>> not be commented and are absolutely clear.
>>>> The lines u are adding on the other hand are absolutely unclear and
>>>> confusing outside the gcc bug context. Therefore it should be clearly
>>>> stated so in a form of comment. Otherwise somebody (like myself) may see
>>>> this and immediately fix it back (as it should be).
>>>>
>>>>> I thought it is a basic C practice.
>>>> I doubt that. ;) Explained above.
>>>>
>>>>> You should try "git grep '\.[^ ]\+ *= *0 *}'" to be convinced that we are
>>>>> not going to comment each occurence of this coding style.
>>>>> But it must be explained in the coding style document. Agree?
>>>> OMG! This is awful! I think everybody agrees that this is a workaround
>>>> and has nothing to do with a codding style (it's an opposite to a style
>>>> actually). I don't know where this should be explained, frankly.
>>> Once we assert we want to support this buggy compiler, the workarounds
>>> are automatically parts of the coding style.
>> It'd rather not... ;)
>>
>>> I don't know how to deal differently with this constraint.
>> Add -Wno-missing-braces compilation option for compiler versions below
>> 4.7. U (and me and I guess most other developers) compile DPDK code with
>> a newer compiler thus the code would be properly inspected with these
>> compilers and we may afford to be less restrictive with compilation
>> warnings with legacy compiler versions...
> You're right.
> I will test it and submit a v2.
> Then I could use the above grep command to replace other occurences of this
> workaround.

U read my mind!.. ;)

>
>>>> Getting back to the issue - I'm a bit surprised since I use this kind of
>>>> initializer ({0}) in a C code for quite a long time - long before 2012.
>>>> I'd like to understand what is a problem with this specific gcc version.
>>>> This seems to trivial. I'm surprised CentOS has a gcc version with this
>>>> kind of bugs.
>>> Each day brings its surprise :)
>

^ permalink raw reply	[flat|nested] 33+ messages in thread

* [dpdk-dev] [PATCH v2 1/2] ixgbe: fix build with gcc 4.4
  2015-04-14 15:28               ` Thomas Monjalon
  2015-04-14 15:32                 ` Vlad Zolotarov
@ 2015-04-15 20:49                 ` Thomas Monjalon
  2015-04-15 20:49                   ` [dpdk-dev] [PATCH v2 2/2] use simple zero initializers Thomas Monjalon
                                     ` (3 more replies)
  1 sibling, 4 replies; 33+ messages in thread
From: Thomas Monjalon @ 2015-04-15 20:49 UTC (permalink / raw)
  To: dev

With GCC 4.4.7 from CentOS 6.5, the following errors arise:

lib/librte_pmd_ixgbe/ixgbe_rxtx.c: In function ‘ixgbe_dev_rx_queue_setup’:
lib/librte_pmd_ixgbe/ixgbe_rxtx.c:2509: error: missing initializer
lib/librte_pmd_ixgbe/ixgbe_rxtx.c:2509: error: (near initialization for ‘dev_info.driver_name’)

lib/librte_pmd_ixgbe/ixgbe_rxtx.c: In function ‘ixgbe_set_rsc’:
lib/librte_pmd_ixgbe/ixgbe_rxtx.c:4072: error: missing initializer
lib/librte_pmd_ixgbe/ixgbe_rxtx.c:4072: error: (near initialization for ‘dev_info.driver_name’)

lib/librte_pmd_ixgbe/ixgbe_rxtx.c: In function ‘ixgbe_recv_pkts_lro_single_alloc’:
lib/librte_pmd_ixgbe/ixgbe_rxtx.c:1479: error: ‘next_rsc_entry’ may be used uninitialized in this function
lib/librte_pmd_ixgbe/ixgbe_rxtx.c:1480: error: ‘next_rxe’ may be used uninitialized in this function

The "missing initializer" warning is a GCC bug which seems fixed in 4.7.
The "may be used uninitialized" warning seems to be another GCC bug and is
workarounded with NULL initialization.

Fixes: 8eecb3295aed ("ixgbe: add LRO support")

Signed-off-by: Thomas Monjalon <thomas.monjalon@6wind.com>
---
changes in v2:
- option -Wno-missing-field-initializers for old GCC instead of code workaround

 lib/librte_pmd_ixgbe/ixgbe_rxtx.c | 4 ++--
 mk/toolchain/gcc/rte.vars.mk      | 5 +++++
 2 files changed, 7 insertions(+), 2 deletions(-)

diff --git a/lib/librte_pmd_ixgbe/ixgbe_rxtx.c b/lib/librte_pmd_ixgbe/ixgbe_rxtx.c
index f1da9ec..6475c44 100644
--- a/lib/librte_pmd_ixgbe/ixgbe_rxtx.c
+++ b/lib/librte_pmd_ixgbe/ixgbe_rxtx.c
@@ -1476,8 +1476,8 @@ ixgbe_recv_pkts_lro(void *rx_queue, struct rte_mbuf **rx_pkts, uint16_t nb_pkts,
 		bool eop;
 		struct ixgbe_rx_entry *rxe;
 		struct ixgbe_rsc_entry *rsc_entry;
-		struct ixgbe_rsc_entry *next_rsc_entry;
-		struct ixgbe_rx_entry *next_rxe;
+		struct ixgbe_rsc_entry *next_rsc_entry = NULL;
+		struct ixgbe_rx_entry *next_rxe = NULL;
 		struct rte_mbuf *first_seg;
 		struct rte_mbuf *rxm;
 		struct rte_mbuf *nmb;
diff --git a/mk/toolchain/gcc/rte.vars.mk b/mk/toolchain/gcc/rte.vars.mk
index 88f235c..208cddd 100644
--- a/mk/toolchain/gcc/rte.vars.mk
+++ b/mk/toolchain/gcc/rte.vars.mk
@@ -80,5 +80,10 @@ WERROR_FLAGS += -Wundef -Wwrite-strings
 # process cpu flags
 include $(RTE_SDK)/mk/toolchain/$(RTE_TOOLCHAIN)/rte.toolchain-compat.mk
 
+# workaround GCC bug with warning "missing initializer" for "= {0}"
+ifeq ($(shell test $(GCC_VERSION) -lt 47 && echo 1), 1)
+WERROR_FLAGS += -Wno-missing-field-initializers
+endif
+
 export CC AS AR LD OBJCOPY OBJDUMP STRIP READELF
 export TOOLCHAIN_CFLAGS TOOLCHAIN_LDFLAGS TOOLCHAIN_ASFLAGS
-- 
2.2.2

^ permalink raw reply	[flat|nested] 33+ messages in thread

* [dpdk-dev] [PATCH v2 2/2] use simple zero initializers
  2015-04-15 20:49                 ` [dpdk-dev] [PATCH v2 1/2] " Thomas Monjalon
@ 2015-04-15 20:49                   ` Thomas Monjalon
  2015-04-16 10:12                     ` Olivier MATZ
  2015-04-16  7:26                   ` [dpdk-dev] [PATCH v2 1/2] ixgbe: fix build with gcc 4.4 Zhang, Helin
                                     ` (2 subsequent siblings)
  3 siblings, 1 reply; 33+ messages in thread
From: Thomas Monjalon @ 2015-04-15 20:49 UTC (permalink / raw)
  To: dev

To initialize a structure with zeros, one field was explicitly set
to avoid "missing initializer" bug with old GCC (e.g. 4.4).
This warning is now disabled (commit <insertlater>) for old versions of GCC,
so the workarounds may be removed.

These initializers should not be needed for static variables but they
are still used to workaround an ICC bug (see commit b2595c4aa92d).

There is one remaining exception where {0} initializer doesn't work cleanly,
even with recent GCC:
lib/librte_pmd_ixgbe/ixgbe_rxtx_vec.c:735:9:
error: missing braces around initializer [-Werror=missing-braces]
  struct rte_mbuf mb_def = {0}; /* zeroed mbuf */

Tested with GCC 4.4.7 (CentOS), 4.7.2 (Debian) and 4.9.2 (Arch).

Signed-off-by: Thomas Monjalon <thomas.monjalon@6wind.com>
---
changes in v2:
- new patch

 app/test/test_ring_perf.c             | 2 +-
 lib/librte_pmd_e1000/em_ethdev.c      | 2 +-
 lib/librte_pmd_e1000/igb_ethdev.c     | 4 ++--
 lib/librte_pmd_e1000/igb_rxtx.c       | 6 ++----
 lib/librte_pmd_enic/enic_clsf.c       | 2 +-
 lib/librte_pmd_i40e/i40e_rxtx.c       | 2 +-
 lib/librte_pmd_ixgbe/ixgbe_rxtx.c     | 8 +++-----
 lib/librte_pmd_ixgbe/ixgbe_rxtx_vec.c | 3 +--
 lib/librte_pmd_mlx4/mlx4.c            | 2 +-
 9 files changed, 13 insertions(+), 18 deletions(-)

diff --git a/app/test/test_ring_perf.c b/app/test/test_ring_perf.c
index 44dda4d..8c47ccb 100644
--- a/app/test/test_ring_perf.c
+++ b/app/test/test_ring_perf.c
@@ -253,7 +253,7 @@ static void
 run_on_core_pair(struct lcore_pair *cores,
 		lcore_function_t f1, lcore_function_t f2)
 {
-	struct thread_params param1 = {.size = 0}, param2 = {.size = 0};
+	struct thread_params param1 = {0}, param2 = {0};
 	unsigned i;
 	for (i = 0; i < sizeof(bulk_sizes)/sizeof(bulk_sizes[0]); i++) {
 		lcore_count = 0;
diff --git a/lib/librte_pmd_e1000/em_ethdev.c b/lib/librte_pmd_e1000/em_ethdev.c
index 12ecf5f..82e0b7a 100644
--- a/lib/librte_pmd_e1000/em_ethdev.c
+++ b/lib/librte_pmd_e1000/em_ethdev.c
@@ -130,7 +130,7 @@ static struct rte_pci_id pci_id_em_map[] = {
 #define RTE_PCI_DEV_ID_DECL_EM(vend, dev) {RTE_PCI_DEVICE(vend, dev)},
 #include "rte_pci_dev_ids.h"
 
-{.device_id = 0},
+{0},
 };
 
 static const struct eth_dev_ops eth_em_ops = {
diff --git a/lib/librte_pmd_e1000/igb_ethdev.c b/lib/librte_pmd_e1000/igb_ethdev.c
index 1ea2d38..e2b7cf3 100644
--- a/lib/librte_pmd_e1000/igb_ethdev.c
+++ b/lib/librte_pmd_e1000/igb_ethdev.c
@@ -221,7 +221,7 @@ static struct rte_pci_id pci_id_igb_map[] = {
 #define RTE_PCI_DEV_ID_DECL_IGB(vend, dev) {RTE_PCI_DEVICE(vend, dev)},
 #include "rte_pci_dev_ids.h"
 
-{.device_id = 0},
+{0},
 };
 
 /*
@@ -232,7 +232,7 @@ static struct rte_pci_id pci_id_igbvf_map[] = {
 #define RTE_PCI_DEV_ID_DECL_IGBVF(vend, dev) {RTE_PCI_DEVICE(vend, dev)},
 #include "rte_pci_dev_ids.h"
 
-{.device_id = 0},
+{0},
 };
 
 static const struct eth_dev_ops eth_igb_ops = {
diff --git a/lib/librte_pmd_e1000/igb_rxtx.c b/lib/librte_pmd_e1000/igb_rxtx.c
index 946b39d..084e45a 100644
--- a/lib/librte_pmd_e1000/igb_rxtx.c
+++ b/lib/librte_pmd_e1000/igb_rxtx.c
@@ -1164,8 +1164,7 @@ igb_reset_tx_queue_stat(struct igb_tx_queue *txq)
 static void
 igb_reset_tx_queue(struct igb_tx_queue *txq, struct rte_eth_dev *dev)
 {
-	static const union e1000_adv_tx_desc zeroed_desc = { .read = {
-			.buffer_addr = 0}};
+	static const union e1000_adv_tx_desc zeroed_desc = {{0}};
 	struct igb_tx_entry *txe = txq->sw_ring;
 	uint16_t i, prev;
 	struct e1000_hw *hw;
@@ -1330,8 +1329,7 @@ eth_igb_rx_queue_release(void *rxq)
 static void
 igb_reset_rx_queue(struct igb_rx_queue *rxq)
 {
-	static const union e1000_adv_rx_desc zeroed_desc = { .read = {
-			.pkt_addr = 0}};
+	static const union e1000_adv_rx_desc zeroed_desc = {{0}};
 	unsigned i;
 
 	/* Zero out HW ring memory */
diff --git a/lib/librte_pmd_enic/enic_clsf.c b/lib/librte_pmd_enic/enic_clsf.c
index b61d625..a069194 100644
--- a/lib/librte_pmd_enic/enic_clsf.c
+++ b/lib/librte_pmd_enic/enic_clsf.c
@@ -96,7 +96,7 @@ int enic_fdir_add_fltr(struct enic *enic, struct rte_fdir_filter *params,
 	u16 queue, u8 drop)
 {
 	struct enic_fdir_node *key;
-	struct filter fltr = {.type = 0};
+	struct filter fltr = {0};
 	int32_t pos;
 	u8 do_free = 0;
 	u16 old_fltr_id = 0;
diff --git a/lib/librte_pmd_i40e/i40e_rxtx.c b/lib/librte_pmd_i40e/i40e_rxtx.c
index 9c7be6f..abe68f4 100644
--- a/lib/librte_pmd_i40e/i40e_rxtx.c
+++ b/lib/librte_pmd_i40e/i40e_rxtx.c
@@ -1228,7 +1228,7 @@ i40e_xmit_pkts(void *tx_queue, struct rte_mbuf **tx_pkts, uint16_t nb_pkts)
 	uint16_t tx_last;
 	uint16_t slen;
 	uint64_t buf_dma_addr;
-	union i40e_tx_offload tx_offload = { .data = 0 };
+	union i40e_tx_offload tx_offload = {0};
 
 	txq = tx_queue;
 	sw_ring = txq->sw_ring;
diff --git a/lib/librte_pmd_ixgbe/ixgbe_rxtx.c b/lib/librte_pmd_ixgbe/ixgbe_rxtx.c
index 6475c44..19286c9 100644
--- a/lib/librte_pmd_ixgbe/ixgbe_rxtx.c
+++ b/lib/librte_pmd_ixgbe/ixgbe_rxtx.c
@@ -579,7 +579,7 @@ ixgbe_xmit_pkts(void *tx_queue, struct rte_mbuf **tx_pkts,
 	uint64_t tx_ol_req;
 	uint32_t ctx = 0;
 	uint32_t new_ctx;
-	union ixgbe_tx_offload tx_offload = { .data = 0 };
+	union ixgbe_tx_offload tx_offload = {0};
 
 	txq = tx_queue;
 	sw_ring = txq->sw_ring;
@@ -2052,8 +2052,7 @@ ixgbe_dev_tx_queue_release(void *txq)
 static void
 ixgbe_reset_tx_queue(struct ixgbe_tx_queue *txq)
 {
-	static const union ixgbe_adv_tx_desc zeroed_desc = { .read = {
-			.buffer_addr = 0}};
+	static const union ixgbe_adv_tx_desc zeroed_desc = {{0}};
 	struct ixgbe_tx_entry *txe = txq->sw_ring;
 	uint16_t prev, i;
 
@@ -2445,8 +2444,7 @@ check_rx_burst_bulk_alloc_preconditions(__rte_unused struct ixgbe_rx_queue *rxq)
 static void
 ixgbe_reset_rx_queue(struct ixgbe_hw *hw, struct ixgbe_rx_queue *rxq)
 {
-	static const union ixgbe_adv_rx_desc zeroed_desc = { .read = {
-			.pkt_addr = 0}};
+	static const union ixgbe_adv_rx_desc zeroed_desc = {{0}};
 	unsigned i;
 	uint16_t len = rxq->nb_rx_desc;
 
diff --git a/lib/librte_pmd_ixgbe/ixgbe_rxtx_vec.c b/lib/librte_pmd_ixgbe/ixgbe_rxtx_vec.c
index 7ac6b61..abd10f6 100644
--- a/lib/librte_pmd_ixgbe/ixgbe_rxtx_vec.c
+++ b/lib/librte_pmd_ixgbe/ixgbe_rxtx_vec.c
@@ -691,8 +691,7 @@ ixgbe_tx_free_swring(struct ixgbe_tx_queue *txq)
 static void
 ixgbe_reset_tx_queue(struct ixgbe_tx_queue *txq)
 {
-	static const union ixgbe_adv_tx_desc zeroed_desc = { .read = {
-			.buffer_addr = 0} };
+	static const union ixgbe_adv_tx_desc zeroed_desc = {{0}};
 	struct ixgbe_tx_entry_v *txe = (struct ixgbe_tx_entry_v *)txq->sw_ring;
 	uint16_t i;
 
diff --git a/lib/librte_pmd_mlx4/mlx4.c b/lib/librte_pmd_mlx4/mlx4.c
index e096071..024282a 100644
--- a/lib/librte_pmd_mlx4/mlx4.c
+++ b/lib/librte_pmd_mlx4/mlx4.c
@@ -3497,7 +3497,7 @@ static void
 mlx4_stats_get(struct rte_eth_dev *dev, struct rte_eth_stats *stats)
 {
 	struct priv *priv = dev->data->dev_private;
-	struct rte_eth_stats tmp = { .ipackets = 0 };
+	struct rte_eth_stats tmp = {0};
 	unsigned int i;
 	unsigned int idx;
 
-- 
2.2.2

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [dpdk-dev] [PATCH v2 1/2] ixgbe: fix build with gcc 4.4
  2015-04-15 20:49                 ` [dpdk-dev] [PATCH v2 1/2] " Thomas Monjalon
  2015-04-15 20:49                   ` [dpdk-dev] [PATCH v2 2/2] use simple zero initializers Thomas Monjalon
@ 2015-04-16  7:26                   ` Zhang, Helin
  2015-04-16  9:14                   ` Vlad Zolotarov
  2015-04-16 22:10                   ` [dpdk-dev] [PATCH v3 1/2] mk: fix build with gcc 4.4 and clang Thomas Monjalon
  3 siblings, 0 replies; 33+ messages in thread
From: Zhang, Helin @ 2015-04-16  7:26 UTC (permalink / raw)
  To: Thomas Monjalon, dev



> -----Original Message-----
> From: Thomas Monjalon [mailto:thomas.monjalon@6wind.com]
> Sent: Thursday, April 16, 2015 4:49 AM
> To: dev@dpdk.org
> Cc: Vlad Zolotarov; Ananyev, Konstantin; Zhang, Helin
> Subject: [PATCH v2 1/2] ixgbe: fix build with gcc 4.4
> 
> With GCC 4.4.7 from CentOS 6.5, the following errors arise:
> 
> lib/librte_pmd_ixgbe/ixgbe_rxtx.c: In function ‘ixgbe_dev_rx_queue_setup’:
> lib/librte_pmd_ixgbe/ixgbe_rxtx.c:2509: error: missing initializer
> lib/librte_pmd_ixgbe/ixgbe_rxtx.c:2509: error: (near initialization for
> ‘dev_info.driver_name’)
> 
> lib/librte_pmd_ixgbe/ixgbe_rxtx.c: In function ‘ixgbe_set_rsc’:
> lib/librte_pmd_ixgbe/ixgbe_rxtx.c:4072: error: missing initializer
> lib/librte_pmd_ixgbe/ixgbe_rxtx.c:4072: error: (near initialization for
> ‘dev_info.driver_name’)
> 
> lib/librte_pmd_ixgbe/ixgbe_rxtx.c: In function
> ‘ixgbe_recv_pkts_lro_single_alloc’:
> lib/librte_pmd_ixgbe/ixgbe_rxtx.c:1479: error: ‘next_rsc_entry’ may be used
> uninitialized in this function
> lib/librte_pmd_ixgbe/ixgbe_rxtx.c:1480: error: ‘next_rxe’ may be used
> uninitialized in this function
> 
> The "missing initializer" warning is a GCC bug which seems fixed in 4.7.
> The "may be used uninitialized" warning seems to be another GCC bug and is
> workarounded with NULL initialization.
> 
> Fixes: 8eecb3295aed ("ixgbe: add LRO support")
> 
> Signed-off-by: Thomas Monjalon <thomas.monjalon@6wind.com>
Acked-by: Helin Zhang <helin.zhang@intel.com>

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [dpdk-dev] [PATCH v2 1/2] ixgbe: fix build with gcc 4.4
  2015-04-15 20:49                 ` [dpdk-dev] [PATCH v2 1/2] " Thomas Monjalon
  2015-04-15 20:49                   ` [dpdk-dev] [PATCH v2 2/2] use simple zero initializers Thomas Monjalon
  2015-04-16  7:26                   ` [dpdk-dev] [PATCH v2 1/2] ixgbe: fix build with gcc 4.4 Zhang, Helin
@ 2015-04-16  9:14                   ` Vlad Zolotarov
  2015-04-16  9:18                     ` Thomas Monjalon
  2015-04-16 22:10                   ` [dpdk-dev] [PATCH v3 1/2] mk: fix build with gcc 4.4 and clang Thomas Monjalon
  3 siblings, 1 reply; 33+ messages in thread
From: Vlad Zolotarov @ 2015-04-16  9:14 UTC (permalink / raw)
  To: Thomas Monjalon, dev



On 04/15/15 23:49, Thomas Monjalon wrote:
> With GCC 4.4.7 from CentOS 6.5, the following errors arise:
>
> lib/librte_pmd_ixgbe/ixgbe_rxtx.c: In function ‘ixgbe_dev_rx_queue_setup’:
> lib/librte_pmd_ixgbe/ixgbe_rxtx.c:2509: error: missing initializer
> lib/librte_pmd_ixgbe/ixgbe_rxtx.c:2509: error: (near initialization for ‘dev_info.driver_name’)
>
> lib/librte_pmd_ixgbe/ixgbe_rxtx.c: In function ‘ixgbe_set_rsc’:
> lib/librte_pmd_ixgbe/ixgbe_rxtx.c:4072: error: missing initializer
> lib/librte_pmd_ixgbe/ixgbe_rxtx.c:4072: error: (near initialization for ‘dev_info.driver_name’)
>
> lib/librte_pmd_ixgbe/ixgbe_rxtx.c: In function ‘ixgbe_recv_pkts_lro_single_alloc’:
> lib/librte_pmd_ixgbe/ixgbe_rxtx.c:1479: error: ‘next_rsc_entry’ may be used uninitialized in this function
> lib/librte_pmd_ixgbe/ixgbe_rxtx.c:1480: error: ‘next_rxe’ may be used uninitialized in this function
>
> The "missing initializer" warning is a GCC bug which seems fixed in 4.7.
> The "may be used uninitialized" warning seems to be another GCC bug and is
> workarounded with NULL initialization.
>
> Fixes: 8eecb3295aed ("ixgbe: add LRO support")
>
> Signed-off-by: Thomas Monjalon <thomas.monjalon@6wind.com>
> ---
> changes in v2:
> - option -Wno-missing-field-initializers for old GCC instead of code workaround
>
>   lib/librte_pmd_ixgbe/ixgbe_rxtx.c | 4 ++--
>   mk/toolchain/gcc/rte.vars.mk      | 5 +++++
>   2 files changed, 7 insertions(+), 2 deletions(-)
>
> diff --git a/lib/librte_pmd_ixgbe/ixgbe_rxtx.c b/lib/librte_pmd_ixgbe/ixgbe_rxtx.c
> index f1da9ec..6475c44 100644
> --- a/lib/librte_pmd_ixgbe/ixgbe_rxtx.c
> +++ b/lib/librte_pmd_ixgbe/ixgbe_rxtx.c
> @@ -1476,8 +1476,8 @@ ixgbe_recv_pkts_lro(void *rx_queue, struct rte_mbuf **rx_pkts, uint16_t nb_pkts,
>   		bool eop;
>   		struct ixgbe_rx_entry *rxe;
>   		struct ixgbe_rsc_entry *rsc_entry;
> -		struct ixgbe_rsc_entry *next_rsc_entry;
> -		struct ixgbe_rx_entry *next_rxe;
> +		struct ixgbe_rsc_entry *next_rsc_entry = NULL;
> +		struct ixgbe_rx_entry *next_rxe = NULL;

-Wno-maybe-uninitialized ?

>   		struct rte_mbuf *first_seg;
>   		struct rte_mbuf *rxm;
>   		struct rte_mbuf *nmb;
> diff --git a/mk/toolchain/gcc/rte.vars.mk b/mk/toolchain/gcc/rte.vars.mk
> index 88f235c..208cddd 100644
> --- a/mk/toolchain/gcc/rte.vars.mk
> +++ b/mk/toolchain/gcc/rte.vars.mk
> @@ -80,5 +80,10 @@ WERROR_FLAGS += -Wundef -Wwrite-strings
>   # process cpu flags
>   include $(RTE_SDK)/mk/toolchain/$(RTE_TOOLCHAIN)/rte.toolchain-compat.mk
>   
> +# workaround GCC bug with warning "missing initializer" for "= {0}"
> +ifeq ($(shell test $(GCC_VERSION) -lt 47 && echo 1), 1)
> +WERROR_FLAGS += -Wno-missing-field-initializers
> +endif
> +
>   export CC AS AR LD OBJCOPY OBJDUMP STRIP READELF
>   export TOOLCHAIN_CFLAGS TOOLCHAIN_LDFLAGS TOOLCHAIN_ASFLAGS

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [dpdk-dev] [PATCH v2 1/2] ixgbe: fix build with gcc 4.4
  2015-04-16  9:14                   ` Vlad Zolotarov
@ 2015-04-16  9:18                     ` Thomas Monjalon
  2015-04-16  9:35                       ` Vlad Zolotarov
  0 siblings, 1 reply; 33+ messages in thread
From: Thomas Monjalon @ 2015-04-16  9:18 UTC (permalink / raw)
  To: Vlad Zolotarov; +Cc: dev

2015-04-16 12:14, Vlad Zolotarov:
> On 04/15/15 23:49, Thomas Monjalon wrote:
> > The "may be used uninitialized" warning seems to be another GCC bug and is
> > workarounded with NULL initialization.
> > --- a/lib/librte_pmd_ixgbe/ixgbe_rxtx.c
> > +++ b/lib/librte_pmd_ixgbe/ixgbe_rxtx.c
> > @@ -1476,8 +1476,8 @@ ixgbe_recv_pkts_lro(void *rx_queue, struct rte_mbuf **rx_pkts, uint16_t nb_pkts,
> >   		bool eop;
> >   		struct ixgbe_rx_entry *rxe;
> >   		struct ixgbe_rsc_entry *rsc_entry;
> > -		struct ixgbe_rsc_entry *next_rsc_entry;
> > -		struct ixgbe_rx_entry *next_rxe;
> > +		struct ixgbe_rsc_entry *next_rsc_entry = NULL;
> > +		struct ixgbe_rx_entry *next_rxe = NULL;
> 
> -Wno-maybe-uninitialized ?

I prefer avoiding this flag for 2 reasons:
	- It's not supported in every GCC versions (need special handling)
	- NULL assigment doesn't hurt

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [dpdk-dev] [PATCH v2 1/2] ixgbe: fix build with gcc 4.4
  2015-04-16  9:18                     ` Thomas Monjalon
@ 2015-04-16  9:35                       ` Vlad Zolotarov
  0 siblings, 0 replies; 33+ messages in thread
From: Vlad Zolotarov @ 2015-04-16  9:35 UTC (permalink / raw)
  To: Thomas Monjalon; +Cc: dev



On 04/16/15 12:18, Thomas Monjalon wrote:
> 2015-04-16 12:14, Vlad Zolotarov:
>> On 04/15/15 23:49, Thomas Monjalon wrote:
>>> The "may be used uninitialized" warning seems to be another GCC bug and is
>>> workarounded with NULL initialization.
>>> --- a/lib/librte_pmd_ixgbe/ixgbe_rxtx.c
>>> +++ b/lib/librte_pmd_ixgbe/ixgbe_rxtx.c
>>> @@ -1476,8 +1476,8 @@ ixgbe_recv_pkts_lro(void *rx_queue, struct rte_mbuf **rx_pkts, uint16_t nb_pkts,
>>>    		bool eop;
>>>    		struct ixgbe_rx_entry *rxe;
>>>    		struct ixgbe_rsc_entry *rsc_entry;
>>> -		struct ixgbe_rsc_entry *next_rsc_entry;
>>> -		struct ixgbe_rx_entry *next_rxe;
>>> +		struct ixgbe_rsc_entry *next_rsc_entry = NULL;
>>> +		struct ixgbe_rx_entry *next_rxe = NULL;
>> -Wno-maybe-uninitialized ?
> I prefer avoiding this flag for 2 reasons:
> 	- It's not supported in every GCC versions (need special handling)

Is it supported for 4.4? U don't have to support all ever existed gcc 
versions. ;)

> 	- NULL assigment doesn't hurt

Right, but still it's ugly since it's clear that it's a workaround - 
right above the patched ones there are variables that are not 
initialized and this discloses the workaround... ;)

>

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [dpdk-dev] [PATCH v2 2/2] use simple zero initializers
  2015-04-15 20:49                   ` [dpdk-dev] [PATCH v2 2/2] use simple zero initializers Thomas Monjalon
@ 2015-04-16 10:12                     ` Olivier MATZ
  2015-04-16 12:55                       ` Thomas Monjalon
  0 siblings, 1 reply; 33+ messages in thread
From: Olivier MATZ @ 2015-04-16 10:12 UTC (permalink / raw)
  To: Thomas Monjalon, dev

Hi Thomas,

On 04/15/2015 10:49 PM, Thomas Monjalon wrote:
> To initialize a structure with zeros, one field was explicitly set
> to avoid "missing initializer" bug with old GCC (e.g. 4.4).
> This warning is now disabled (commit <insertlater>) for old versions of GCC,
> so the workarounds may be removed.
>
> These initializers should not be needed for static variables but they
> are still used to workaround an ICC bug (see commit b2595c4aa92d).
>
> There is one remaining exception where {0} initializer doesn't work cleanly,
> even with recent GCC:
> lib/librte_pmd_ixgbe/ixgbe_rxtx_vec.c:735:9:
> error: missing braces around initializer [-Werror=missing-braces]
>    struct rte_mbuf mb_def = {0}; /* zeroed mbuf */
>
> Tested with GCC 4.4.7 (CentOS), 4.7.2 (Debian) and 4.9.2 (Arch).
>
> Signed-off-by: Thomas Monjalon <thomas.monjalon@6wind.com>

I'm trying to compile the head of dpdk (without this patch applied),
and I have this error with clang:

     ixgbe_rxtx.c:2509:41: error: missing field 'driver_name' initializer
          [-Werror,-Wmissing-field-initializers]
             struct rte_eth_dev_info dev_info = { 0 };

I'm wondering if adding more {0} would compile on clang, at least with
the current clang flags.

$ clang --version
Debian clang version 3.5.0-10 (tags/RELEASE_350/final) (based on LLVM 3.5.0)
Target: x86_64-pc-linux-gnu
Thread model: posix


Regards,
Olivier

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [dpdk-dev] [PATCH v2 2/2] use simple zero initializers
  2015-04-16 10:12                     ` Olivier MATZ
@ 2015-04-16 12:55                       ` Thomas Monjalon
  2015-04-16 16:31                         ` Mcnamara, John
  0 siblings, 1 reply; 33+ messages in thread
From: Thomas Monjalon @ 2015-04-16 12:55 UTC (permalink / raw)
  To: Olivier MATZ; +Cc: dev

2015-04-16 12:12, Olivier MATZ:
> On 04/15/2015 10:49 PM, Thomas Monjalon wrote:
> > To initialize a structure with zeros, one field was explicitly set
> > to avoid "missing initializer" bug with old GCC (e.g. 4.4).
> > This warning is now disabled (commit <insertlater>) for old versions of GCC,
> > so the workarounds may be removed.
> >
> > These initializers should not be needed for static variables but they
> > are still used to workaround an ICC bug (see commit b2595c4aa92d).
> >
> > There is one remaining exception where {0} initializer doesn't work cleanly,
> > even with recent GCC:
> > lib/librte_pmd_ixgbe/ixgbe_rxtx_vec.c:735:9:
> > error: missing braces around initializer [-Werror=missing-braces]
> >    struct rte_mbuf mb_def = {0}; /* zeroed mbuf */
> >
> > Tested with GCC 4.4.7 (CentOS), 4.7.2 (Debian) and 4.9.2 (Arch).
> >
> > Signed-off-by: Thomas Monjalon <thomas.monjalon@6wind.com>
> 
> I'm trying to compile the head of dpdk (without this patch applied),
> and I have this error with clang:
> 
>      ixgbe_rxtx.c:2509:41: error: missing field 'driver_name' initializer
>           [-Werror,-Wmissing-field-initializers]
>              struct rte_eth_dev_info dev_info = { 0 };
> 
> I'm wondering if adding more {0} would compile on clang, at least with
> the current clang flags.

It's fixed by adding -Wno-missing-field-initializers to clang flags.

Someone to test with ICC?

Thanks

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [dpdk-dev] [PATCH v2 2/2] use simple zero initializers
  2015-04-16 12:55                       ` Thomas Monjalon
@ 2015-04-16 16:31                         ` Mcnamara, John
  0 siblings, 0 replies; 33+ messages in thread
From: Mcnamara, John @ 2015-04-16 16:31 UTC (permalink / raw)
  To: Thomas Monjalon, Olivier MATZ; +Cc: dev



> -----Original Message-----
> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Thomas Monjalon
> Sent: Thursday, April 16, 2015 1:55 PM
> To: Olivier MATZ
> Cc: dev@dpdk.org
> Subject: Re: [dpdk-dev] [PATCH v2 2/2] use simple zero initializers


> > I'm wondering if adding more {0} would compile on clang, at least with
> > the current clang flags.
> 
> It's fixed by adding -Wno-missing-field-initializers to clang flags.
> 
> Someone to test with ICC?
> 
> Thanks


Hi,

I applied patches 4318 and 4319 to the current head and they compile with icc 13.1.1.

I also see the same compilation issues as Oliver with clang 3.3.

I'm not clear on the first statement, are you going to add the -Wno-missing-field-initializers flag for clang?

John
-- 

^ permalink raw reply	[flat|nested] 33+ messages in thread

* [dpdk-dev] [PATCH v3 1/2] mk: fix build with gcc 4.4 and clang
  2015-04-15 20:49                 ` [dpdk-dev] [PATCH v2 1/2] " Thomas Monjalon
                                     ` (2 preceding siblings ...)
  2015-04-16  9:14                   ` Vlad Zolotarov
@ 2015-04-16 22:10                   ` Thomas Monjalon
  2015-04-16 22:10                     ` [dpdk-dev] [PATCH v3 2/2] use simple zero initializers Thomas Monjalon
                                       ` (2 more replies)
  3 siblings, 3 replies; 33+ messages in thread
From: Thomas Monjalon @ 2015-04-16 22:10 UTC (permalink / raw)
  To: dev

With GCC 4.4.7 from CentOS 6.5, the following errors arise:

lib/librte_pmd_ixgbe/ixgbe_rxtx.c: In function 'ixgbe_dev_rx_queue_setup':
lib/librte_pmd_ixgbe/ixgbe_rxtx.c:2509: error: missing initializer
lib/librte_pmd_ixgbe/ixgbe_rxtx.c:2509: error: (near initialization for 'dev_info.driver_name')

lib/librte_pmd_ixgbe/ixgbe_rxtx.c: In function 'ixgbe_set_rsc':
lib/librte_pmd_ixgbe/ixgbe_rxtx.c:4072: error: missing initializer
lib/librte_pmd_ixgbe/ixgbe_rxtx.c:4072: error: (near initialization for 'dev_info.driver_name')

lib/librte_pmd_ixgbe/ixgbe_rxtx.c: In function 'ixgbe_recv_pkts_lro_single_alloc':
lib/librte_pmd_ixgbe/ixgbe_rxtx.c:1479: error: 'next_rsc_entry' may be used uninitialized in this function
lib/librte_pmd_ixgbe/ixgbe_rxtx.c:1480: error: 'next_rxe' may be used uninitialized in this function

The "missing initializer" warning is a GCC bug which seems fixed in 4.7.
The same warning is thrown by clang.
The "may be used uninitialized" warning is another GCC bug which seems fixed in 4.7.

Fixes: 8eecb3295aed ("ixgbe: add LRO support")

Signed-off-by: Thomas Monjalon <thomas.monjalon@6wind.com>
---
changes in v2:
- option -Wno-missing-field-initializers for old GCC instead of code workaround
changes in v3:
- option -Wno-missing-field-initializers for clang
- option -Wno-uninitialized for old GCC instead of code workaround (=NULL)
- remove redundants -Wno-uninitialized from ixgbe Makefile

 lib/librte_pmd_ixgbe/Makefile  | 4 ----
 mk/toolchain/clang/rte.vars.mk | 3 +++
 mk/toolchain/gcc/rte.vars.mk   | 9 +++++++++
 3 files changed, 12 insertions(+), 4 deletions(-)

diff --git a/lib/librte_pmd_ixgbe/Makefile b/lib/librte_pmd_ixgbe/Makefile
index ae36202..fbf6966 100644
--- a/lib/librte_pmd_ixgbe/Makefile
+++ b/lib/librte_pmd_ixgbe/Makefile
@@ -76,10 +76,6 @@ ifeq ($(shell test $(GCC_VERSION) -ge 50 && echo 1), 1)
 CFLAGS_ixgbe_common.o += -Wno-logical-not-parentheses
 endif
 
-ifeq ($(shell test $(GCC_VERSION) -le 46 && echo 1), 1)
-CFLAGS_ixgbe_x550.o += -Wno-uninitialized
-CFLAGS_ixgbe_phy.o += -Wno-uninitialized
-endif
 endif
 
 #
diff --git a/mk/toolchain/clang/rte.vars.mk b/mk/toolchain/clang/rte.vars.mk
index 40cb389..245ea7e 100644
--- a/mk/toolchain/clang/rte.vars.mk
+++ b/mk/toolchain/clang/rte.vars.mk
@@ -72,5 +72,8 @@ WERROR_FLAGS += -Wundef -Wwrite-strings
 # process cpu flags
 include $(RTE_SDK)/mk/toolchain/$(RTE_TOOLCHAIN)/rte.toolchain-compat.mk
 
+# workaround clang bug with warning "missing field initializer" for "= {0}"
+WERROR_FLAGS += -Wno-missing-field-initializers
+
 export CC AS AR LD OBJCOPY OBJDUMP STRIP READELF
 export TOOLCHAIN_CFLAGS TOOLCHAIN_LDFLAGS TOOLCHAIN_ASFLAGS
diff --git a/mk/toolchain/gcc/rte.vars.mk b/mk/toolchain/gcc/rte.vars.mk
index 88f235c..0f51c66 100644
--- a/mk/toolchain/gcc/rte.vars.mk
+++ b/mk/toolchain/gcc/rte.vars.mk
@@ -80,5 +80,14 @@ WERROR_FLAGS += -Wundef -Wwrite-strings
 # process cpu flags
 include $(RTE_SDK)/mk/toolchain/$(RTE_TOOLCHAIN)/rte.toolchain-compat.mk
 
+# workaround GCC bug with warning "missing initializer" for "= {0}"
+ifeq ($(shell test $(GCC_VERSION) -lt 47 && echo 1), 1)
+WERROR_FLAGS += -Wno-missing-field-initializers
+endif
+# workaround GCC bug with warning "may be used uninitialized"
+ifeq ($(shell test $(GCC_VERSION) -lt 47 && echo 1), 1)
+WERROR_FLAGS += -Wno-uninitialized
+endif
+
 export CC AS AR LD OBJCOPY OBJDUMP STRIP READELF
 export TOOLCHAIN_CFLAGS TOOLCHAIN_LDFLAGS TOOLCHAIN_ASFLAGS
-- 
2.2.2

^ permalink raw reply	[flat|nested] 33+ messages in thread

* [dpdk-dev] [PATCH v3 2/2] use simple zero initializers
  2015-04-16 22:10                   ` [dpdk-dev] [PATCH v3 1/2] mk: fix build with gcc 4.4 and clang Thomas Monjalon
@ 2015-04-16 22:10                     ` Thomas Monjalon
  2015-04-17 11:17                       ` Mcnamara, John
  2015-04-19  8:22                       ` Vlad Zolotarov
  2015-04-17 11:15                     ` [dpdk-dev] [PATCH v3 1/2] mk: fix build with gcc 4.4 and clang Mcnamara, John
  2015-04-19  8:21                     ` Vlad Zolotarov
  2 siblings, 2 replies; 33+ messages in thread
From: Thomas Monjalon @ 2015-04-16 22:10 UTC (permalink / raw)
  To: dev

To initialize a structure with zeros, one field was explicitly set
to avoid "missing initializer" bug with old GCC (e.g. 4.4).
This warning is now disabled (commit <insertlater>) for old versions of GCC,
so the workarounds may be removed.

These initializers should not be needed for static variables but they
are still used to workaround an ICC bug (see commit b2595c4aa92d).

There is one remaining exception where {0} initializer doesn't work cleanly,
even with recent GCC:
lib/librte_pmd_ixgbe/ixgbe_rxtx_vec.c:735:9:
error: missing braces around initializer [-Werror=missing-braces]
  struct rte_mbuf mb_def = {0}; /* zeroed mbuf */

Tested with gcc-4.4.7 (CentOS), gcc-4.7.2 (Debian), gcc-4.9.2 (Arch),
clang-3.6.0 and icc-13.1.1.

Signed-off-by: Thomas Monjalon <thomas.monjalon@6wind.com>
Tested-by: Thomas Monjalon <thomas.monjalon@6wind.com>
Tested-by: John McNamara <john.mcnamara@intel.com>
---
changes in v2:
- new patch
changes in v3:
- tested with clang and icc

 app/test/test_ring_perf.c             | 2 +-
 lib/librte_pmd_e1000/em_ethdev.c      | 2 +-
 lib/librte_pmd_e1000/igb_ethdev.c     | 4 ++--
 lib/librte_pmd_e1000/igb_rxtx.c       | 6 ++----
 lib/librte_pmd_enic/enic_clsf.c       | 2 +-
 lib/librte_pmd_i40e/i40e_rxtx.c       | 2 +-
 lib/librte_pmd_ixgbe/ixgbe_rxtx.c     | 8 +++-----
 lib/librte_pmd_ixgbe/ixgbe_rxtx_vec.c | 3 +--
 lib/librte_pmd_mlx4/mlx4.c            | 2 +-
 9 files changed, 13 insertions(+), 18 deletions(-)

diff --git a/app/test/test_ring_perf.c b/app/test/test_ring_perf.c
index 44dda4d..8c47ccb 100644
--- a/app/test/test_ring_perf.c
+++ b/app/test/test_ring_perf.c
@@ -253,7 +253,7 @@ static void
 run_on_core_pair(struct lcore_pair *cores,
 		lcore_function_t f1, lcore_function_t f2)
 {
-	struct thread_params param1 = {.size = 0}, param2 = {.size = 0};
+	struct thread_params param1 = {0}, param2 = {0};
 	unsigned i;
 	for (i = 0; i < sizeof(bulk_sizes)/sizeof(bulk_sizes[0]); i++) {
 		lcore_count = 0;
diff --git a/lib/librte_pmd_e1000/em_ethdev.c b/lib/librte_pmd_e1000/em_ethdev.c
index 12ecf5f..82e0b7a 100644
--- a/lib/librte_pmd_e1000/em_ethdev.c
+++ b/lib/librte_pmd_e1000/em_ethdev.c
@@ -130,7 +130,7 @@ static struct rte_pci_id pci_id_em_map[] = {
 #define RTE_PCI_DEV_ID_DECL_EM(vend, dev) {RTE_PCI_DEVICE(vend, dev)},
 #include "rte_pci_dev_ids.h"
 
-{.device_id = 0},
+{0},
 };
 
 static const struct eth_dev_ops eth_em_ops = {
diff --git a/lib/librte_pmd_e1000/igb_ethdev.c b/lib/librte_pmd_e1000/igb_ethdev.c
index 1ea2d38..e2b7cf3 100644
--- a/lib/librte_pmd_e1000/igb_ethdev.c
+++ b/lib/librte_pmd_e1000/igb_ethdev.c
@@ -221,7 +221,7 @@ static struct rte_pci_id pci_id_igb_map[] = {
 #define RTE_PCI_DEV_ID_DECL_IGB(vend, dev) {RTE_PCI_DEVICE(vend, dev)},
 #include "rte_pci_dev_ids.h"
 
-{.device_id = 0},
+{0},
 };
 
 /*
@@ -232,7 +232,7 @@ static struct rte_pci_id pci_id_igbvf_map[] = {
 #define RTE_PCI_DEV_ID_DECL_IGBVF(vend, dev) {RTE_PCI_DEVICE(vend, dev)},
 #include "rte_pci_dev_ids.h"
 
-{.device_id = 0},
+{0},
 };
 
 static const struct eth_dev_ops eth_igb_ops = {
diff --git a/lib/librte_pmd_e1000/igb_rxtx.c b/lib/librte_pmd_e1000/igb_rxtx.c
index 946b39d..084e45a 100644
--- a/lib/librte_pmd_e1000/igb_rxtx.c
+++ b/lib/librte_pmd_e1000/igb_rxtx.c
@@ -1164,8 +1164,7 @@ igb_reset_tx_queue_stat(struct igb_tx_queue *txq)
 static void
 igb_reset_tx_queue(struct igb_tx_queue *txq, struct rte_eth_dev *dev)
 {
-	static const union e1000_adv_tx_desc zeroed_desc = { .read = {
-			.buffer_addr = 0}};
+	static const union e1000_adv_tx_desc zeroed_desc = {{0}};
 	struct igb_tx_entry *txe = txq->sw_ring;
 	uint16_t i, prev;
 	struct e1000_hw *hw;
@@ -1330,8 +1329,7 @@ eth_igb_rx_queue_release(void *rxq)
 static void
 igb_reset_rx_queue(struct igb_rx_queue *rxq)
 {
-	static const union e1000_adv_rx_desc zeroed_desc = { .read = {
-			.pkt_addr = 0}};
+	static const union e1000_adv_rx_desc zeroed_desc = {{0}};
 	unsigned i;
 
 	/* Zero out HW ring memory */
diff --git a/lib/librte_pmd_enic/enic_clsf.c b/lib/librte_pmd_enic/enic_clsf.c
index b61d625..a069194 100644
--- a/lib/librte_pmd_enic/enic_clsf.c
+++ b/lib/librte_pmd_enic/enic_clsf.c
@@ -96,7 +96,7 @@ int enic_fdir_add_fltr(struct enic *enic, struct rte_fdir_filter *params,
 	u16 queue, u8 drop)
 {
 	struct enic_fdir_node *key;
-	struct filter fltr = {.type = 0};
+	struct filter fltr = {0};
 	int32_t pos;
 	u8 do_free = 0;
 	u16 old_fltr_id = 0;
diff --git a/lib/librte_pmd_i40e/i40e_rxtx.c b/lib/librte_pmd_i40e/i40e_rxtx.c
index 9c7be6f..abe68f4 100644
--- a/lib/librte_pmd_i40e/i40e_rxtx.c
+++ b/lib/librte_pmd_i40e/i40e_rxtx.c
@@ -1228,7 +1228,7 @@ i40e_xmit_pkts(void *tx_queue, struct rte_mbuf **tx_pkts, uint16_t nb_pkts)
 	uint16_t tx_last;
 	uint16_t slen;
 	uint64_t buf_dma_addr;
-	union i40e_tx_offload tx_offload = { .data = 0 };
+	union i40e_tx_offload tx_offload = {0};
 
 	txq = tx_queue;
 	sw_ring = txq->sw_ring;
diff --git a/lib/librte_pmd_ixgbe/ixgbe_rxtx.c b/lib/librte_pmd_ixgbe/ixgbe_rxtx.c
index f1da9ec..3c61d1c 100644
--- a/lib/librte_pmd_ixgbe/ixgbe_rxtx.c
+++ b/lib/librte_pmd_ixgbe/ixgbe_rxtx.c
@@ -579,7 +579,7 @@ ixgbe_xmit_pkts(void *tx_queue, struct rte_mbuf **tx_pkts,
 	uint64_t tx_ol_req;
 	uint32_t ctx = 0;
 	uint32_t new_ctx;
-	union ixgbe_tx_offload tx_offload = { .data = 0 };
+	union ixgbe_tx_offload tx_offload = {0};
 
 	txq = tx_queue;
 	sw_ring = txq->sw_ring;
@@ -2052,8 +2052,7 @@ ixgbe_dev_tx_queue_release(void *txq)
 static void
 ixgbe_reset_tx_queue(struct ixgbe_tx_queue *txq)
 {
-	static const union ixgbe_adv_tx_desc zeroed_desc = { .read = {
-			.buffer_addr = 0}};
+	static const union ixgbe_adv_tx_desc zeroed_desc = {{0}};
 	struct ixgbe_tx_entry *txe = txq->sw_ring;
 	uint16_t prev, i;
 
@@ -2445,8 +2444,7 @@ check_rx_burst_bulk_alloc_preconditions(__rte_unused struct ixgbe_rx_queue *rxq)
 static void
 ixgbe_reset_rx_queue(struct ixgbe_hw *hw, struct ixgbe_rx_queue *rxq)
 {
-	static const union ixgbe_adv_rx_desc zeroed_desc = { .read = {
-			.pkt_addr = 0}};
+	static const union ixgbe_adv_rx_desc zeroed_desc = {{0}};
 	unsigned i;
 	uint16_t len = rxq->nb_rx_desc;
 
diff --git a/lib/librte_pmd_ixgbe/ixgbe_rxtx_vec.c b/lib/librte_pmd_ixgbe/ixgbe_rxtx_vec.c
index 7ac6b61..abd10f6 100644
--- a/lib/librte_pmd_ixgbe/ixgbe_rxtx_vec.c
+++ b/lib/librte_pmd_ixgbe/ixgbe_rxtx_vec.c
@@ -691,8 +691,7 @@ ixgbe_tx_free_swring(struct ixgbe_tx_queue *txq)
 static void
 ixgbe_reset_tx_queue(struct ixgbe_tx_queue *txq)
 {
-	static const union ixgbe_adv_tx_desc zeroed_desc = { .read = {
-			.buffer_addr = 0} };
+	static const union ixgbe_adv_tx_desc zeroed_desc = {{0}};
 	struct ixgbe_tx_entry_v *txe = (struct ixgbe_tx_entry_v *)txq->sw_ring;
 	uint16_t i;
 
diff --git a/lib/librte_pmd_mlx4/mlx4.c b/lib/librte_pmd_mlx4/mlx4.c
index e096071..024282a 100644
--- a/lib/librte_pmd_mlx4/mlx4.c
+++ b/lib/librte_pmd_mlx4/mlx4.c
@@ -3497,7 +3497,7 @@ static void
 mlx4_stats_get(struct rte_eth_dev *dev, struct rte_eth_stats *stats)
 {
 	struct priv *priv = dev->data->dev_private;
-	struct rte_eth_stats tmp = { .ipackets = 0 };
+	struct rte_eth_stats tmp = {0};
 	unsigned int i;
 	unsigned int idx;
 
-- 
2.2.2

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [dpdk-dev] [PATCH v3 1/2] mk: fix build with gcc 4.4 and clang
  2015-04-16 22:10                   ` [dpdk-dev] [PATCH v3 1/2] mk: fix build with gcc 4.4 and clang Thomas Monjalon
  2015-04-16 22:10                     ` [dpdk-dev] [PATCH v3 2/2] use simple zero initializers Thomas Monjalon
@ 2015-04-17 11:15                     ` Mcnamara, John
  2015-04-19  8:21                     ` Vlad Zolotarov
  2 siblings, 0 replies; 33+ messages in thread
From: Mcnamara, John @ 2015-04-17 11:15 UTC (permalink / raw)
  To: Thomas Monjalon, dev



> -----Original Message-----
> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Thomas Monjalon
> Sent: Thursday, April 16, 2015 11:11 PM
> To: dev@dpdk.org
> Subject: [dpdk-dev] [PATCH v3 1/2] mk: fix build with gcc 4.4 and clang

Acked-by: John McNamara <john.mcnamara@intel.com>

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [dpdk-dev] [PATCH v3 2/2] use simple zero initializers
  2015-04-16 22:10                     ` [dpdk-dev] [PATCH v3 2/2] use simple zero initializers Thomas Monjalon
@ 2015-04-17 11:17                       ` Mcnamara, John
  2015-04-19  8:22                       ` Vlad Zolotarov
  1 sibling, 0 replies; 33+ messages in thread
From: Mcnamara, John @ 2015-04-17 11:17 UTC (permalink / raw)
  To: Thomas Monjalon, dev



> -----Original Message-----
> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Thomas Monjalon
> Sent: Thursday, April 16, 2015 11:11 PM
> To: dev@dpdk.org
> Subject: [dpdk-dev] [PATCH v3 2/2] use simple zero initializers

Acked-by: John McNamara <john.mcnamara@intel.com>

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [dpdk-dev] [PATCH v3 1/2] mk: fix build with gcc 4.4 and clang
  2015-04-16 22:10                   ` [dpdk-dev] [PATCH v3 1/2] mk: fix build with gcc 4.4 and clang Thomas Monjalon
  2015-04-16 22:10                     ` [dpdk-dev] [PATCH v3 2/2] use simple zero initializers Thomas Monjalon
  2015-04-17 11:15                     ` [dpdk-dev] [PATCH v3 1/2] mk: fix build with gcc 4.4 and clang Mcnamara, John
@ 2015-04-19  8:21                     ` Vlad Zolotarov
  2015-04-20 12:44                       ` Thomas Monjalon
  2 siblings, 1 reply; 33+ messages in thread
From: Vlad Zolotarov @ 2015-04-19  8:21 UTC (permalink / raw)
  To: Thomas Monjalon, dev



On 04/17/15 01:10, Thomas Monjalon wrote:
> With GCC 4.4.7 from CentOS 6.5, the following errors arise:
>
> lib/librte_pmd_ixgbe/ixgbe_rxtx.c: In function 'ixgbe_dev_rx_queue_setup':
> lib/librte_pmd_ixgbe/ixgbe_rxtx.c:2509: error: missing initializer
> lib/librte_pmd_ixgbe/ixgbe_rxtx.c:2509: error: (near initialization for 'dev_info.driver_name')
>
> lib/librte_pmd_ixgbe/ixgbe_rxtx.c: In function 'ixgbe_set_rsc':
> lib/librte_pmd_ixgbe/ixgbe_rxtx.c:4072: error: missing initializer
> lib/librte_pmd_ixgbe/ixgbe_rxtx.c:4072: error: (near initialization for 'dev_info.driver_name')
>
> lib/librte_pmd_ixgbe/ixgbe_rxtx.c: In function 'ixgbe_recv_pkts_lro_single_alloc':
> lib/librte_pmd_ixgbe/ixgbe_rxtx.c:1479: error: 'next_rsc_entry' may be used uninitialized in this function
> lib/librte_pmd_ixgbe/ixgbe_rxtx.c:1480: error: 'next_rxe' may be used uninitialized in this function
>
> The "missing initializer" warning is a GCC bug which seems fixed in 4.7.
> The same warning is thrown by clang.
> The "may be used uninitialized" warning is another GCC bug which seems fixed in 4.7.
>
> Fixes: 8eecb3295aed ("ixgbe: add LRO support")
>
> Signed-off-by: Thomas Monjalon <thomas.monjalon@6wind.com>

Acked-by: Vlad Zolotarov <vladz@cloudius-systems.com>

> ---
> changes in v2:
> - option -Wno-missing-field-initializers for old GCC instead of code workaround
> changes in v3:
> - option -Wno-missing-field-initializers for clang
> - option -Wno-uninitialized for old GCC instead of code workaround (=NULL)
> - remove redundants -Wno-uninitialized from ixgbe Makefile
>
>   lib/librte_pmd_ixgbe/Makefile  | 4 ----
>   mk/toolchain/clang/rte.vars.mk | 3 +++
>   mk/toolchain/gcc/rte.vars.mk   | 9 +++++++++
>   3 files changed, 12 insertions(+), 4 deletions(-)
>
> diff --git a/lib/librte_pmd_ixgbe/Makefile b/lib/librte_pmd_ixgbe/Makefile
> index ae36202..fbf6966 100644
> --- a/lib/librte_pmd_ixgbe/Makefile
> +++ b/lib/librte_pmd_ixgbe/Makefile
> @@ -76,10 +76,6 @@ ifeq ($(shell test $(GCC_VERSION) -ge 50 && echo 1), 1)
>   CFLAGS_ixgbe_common.o += -Wno-logical-not-parentheses
>   endif
>   
> -ifeq ($(shell test $(GCC_VERSION) -le 46 && echo 1), 1)
> -CFLAGS_ixgbe_x550.o += -Wno-uninitialized
> -CFLAGS_ixgbe_phy.o += -Wno-uninitialized
> -endif
>   endif
>   
>   #
> diff --git a/mk/toolchain/clang/rte.vars.mk b/mk/toolchain/clang/rte.vars.mk
> index 40cb389..245ea7e 100644
> --- a/mk/toolchain/clang/rte.vars.mk
> +++ b/mk/toolchain/clang/rte.vars.mk
> @@ -72,5 +72,8 @@ WERROR_FLAGS += -Wundef -Wwrite-strings
>   # process cpu flags
>   include $(RTE_SDK)/mk/toolchain/$(RTE_TOOLCHAIN)/rte.toolchain-compat.mk
>   
> +# workaround clang bug with warning "missing field initializer" for "= {0}"
> +WERROR_FLAGS += -Wno-missing-field-initializers
> +
>   export CC AS AR LD OBJCOPY OBJDUMP STRIP READELF
>   export TOOLCHAIN_CFLAGS TOOLCHAIN_LDFLAGS TOOLCHAIN_ASFLAGS
> diff --git a/mk/toolchain/gcc/rte.vars.mk b/mk/toolchain/gcc/rte.vars.mk
> index 88f235c..0f51c66 100644
> --- a/mk/toolchain/gcc/rte.vars.mk
> +++ b/mk/toolchain/gcc/rte.vars.mk
> @@ -80,5 +80,14 @@ WERROR_FLAGS += -Wundef -Wwrite-strings
>   # process cpu flags
>   include $(RTE_SDK)/mk/toolchain/$(RTE_TOOLCHAIN)/rte.toolchain-compat.mk
>   
> +# workaround GCC bug with warning "missing initializer" for "= {0}"
> +ifeq ($(shell test $(GCC_VERSION) -lt 47 && echo 1), 1)
> +WERROR_FLAGS += -Wno-missing-field-initializers
> +endif
> +# workaround GCC bug with warning "may be used uninitialized"
> +ifeq ($(shell test $(GCC_VERSION) -lt 47 && echo 1), 1)
> +WERROR_FLAGS += -Wno-uninitialized
> +endif
> +
>   export CC AS AR LD OBJCOPY OBJDUMP STRIP READELF
>   export TOOLCHAIN_CFLAGS TOOLCHAIN_LDFLAGS TOOLCHAIN_ASFLAGS

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [dpdk-dev] [PATCH v3 2/2] use simple zero initializers
  2015-04-16 22:10                     ` [dpdk-dev] [PATCH v3 2/2] use simple zero initializers Thomas Monjalon
  2015-04-17 11:17                       ` Mcnamara, John
@ 2015-04-19  8:22                       ` Vlad Zolotarov
  2015-04-20 12:45                         ` Thomas Monjalon
  1 sibling, 1 reply; 33+ messages in thread
From: Vlad Zolotarov @ 2015-04-19  8:22 UTC (permalink / raw)
  To: Thomas Monjalon, dev



On 04/17/15 01:10, Thomas Monjalon wrote:
> To initialize a structure with zeros, one field was explicitly set
> to avoid "missing initializer" bug with old GCC (e.g. 4.4).
> This warning is now disabled (commit <insertlater>) for old versions of GCC,
> so the workarounds may be removed.
>
> These initializers should not be needed for static variables but they
> are still used to workaround an ICC bug (see commit b2595c4aa92d).
>
> There is one remaining exception where {0} initializer doesn't work cleanly,
> even with recent GCC:
> lib/librte_pmd_ixgbe/ixgbe_rxtx_vec.c:735:9:
> error: missing braces around initializer [-Werror=missing-braces]
>    struct rte_mbuf mb_def = {0}; /* zeroed mbuf */
>
> Tested with gcc-4.4.7 (CentOS), gcc-4.7.2 (Debian), gcc-4.9.2 (Arch),
> clang-3.6.0 and icc-13.1.1.
>
> Signed-off-by: Thomas Monjalon <thomas.monjalon@6wind.com>
> Tested-by: Thomas Monjalon <thomas.monjalon@6wind.com>
> Tested-by: John McNamara <john.mcnamara@intel.com>

Acked-by: Vlad Zolotarov <vladz@cloudius-systems.com>

> ---
> changes in v2:
> - new patch
> changes in v3:
> - tested with clang and icc
>
>   app/test/test_ring_perf.c             | 2 +-
>   lib/librte_pmd_e1000/em_ethdev.c      | 2 +-
>   lib/librte_pmd_e1000/igb_ethdev.c     | 4 ++--
>   lib/librte_pmd_e1000/igb_rxtx.c       | 6 ++----
>   lib/librte_pmd_enic/enic_clsf.c       | 2 +-
>   lib/librte_pmd_i40e/i40e_rxtx.c       | 2 +-
>   lib/librte_pmd_ixgbe/ixgbe_rxtx.c     | 8 +++-----
>   lib/librte_pmd_ixgbe/ixgbe_rxtx_vec.c | 3 +--
>   lib/librte_pmd_mlx4/mlx4.c            | 2 +-
>   9 files changed, 13 insertions(+), 18 deletions(-)
>
> diff --git a/app/test/test_ring_perf.c b/app/test/test_ring_perf.c
> index 44dda4d..8c47ccb 100644
> --- a/app/test/test_ring_perf.c
> +++ b/app/test/test_ring_perf.c
> @@ -253,7 +253,7 @@ static void
>   run_on_core_pair(struct lcore_pair *cores,
>   		lcore_function_t f1, lcore_function_t f2)
>   {
> -	struct thread_params param1 = {.size = 0}, param2 = {.size = 0};
> +	struct thread_params param1 = {0}, param2 = {0};
>   	unsigned i;
>   	for (i = 0; i < sizeof(bulk_sizes)/sizeof(bulk_sizes[0]); i++) {
>   		lcore_count = 0;
> diff --git a/lib/librte_pmd_e1000/em_ethdev.c b/lib/librte_pmd_e1000/em_ethdev.c
> index 12ecf5f..82e0b7a 100644
> --- a/lib/librte_pmd_e1000/em_ethdev.c
> +++ b/lib/librte_pmd_e1000/em_ethdev.c
> @@ -130,7 +130,7 @@ static struct rte_pci_id pci_id_em_map[] = {
>   #define RTE_PCI_DEV_ID_DECL_EM(vend, dev) {RTE_PCI_DEVICE(vend, dev)},
>   #include "rte_pci_dev_ids.h"
>   
> -{.device_id = 0},
> +{0},
>   };
>   
>   static const struct eth_dev_ops eth_em_ops = {
> diff --git a/lib/librte_pmd_e1000/igb_ethdev.c b/lib/librte_pmd_e1000/igb_ethdev.c
> index 1ea2d38..e2b7cf3 100644
> --- a/lib/librte_pmd_e1000/igb_ethdev.c
> +++ b/lib/librte_pmd_e1000/igb_ethdev.c
> @@ -221,7 +221,7 @@ static struct rte_pci_id pci_id_igb_map[] = {
>   #define RTE_PCI_DEV_ID_DECL_IGB(vend, dev) {RTE_PCI_DEVICE(vend, dev)},
>   #include "rte_pci_dev_ids.h"
>   
> -{.device_id = 0},
> +{0},
>   };
>   
>   /*
> @@ -232,7 +232,7 @@ static struct rte_pci_id pci_id_igbvf_map[] = {
>   #define RTE_PCI_DEV_ID_DECL_IGBVF(vend, dev) {RTE_PCI_DEVICE(vend, dev)},
>   #include "rte_pci_dev_ids.h"
>   
> -{.device_id = 0},
> +{0},
>   };
>   
>   static const struct eth_dev_ops eth_igb_ops = {
> diff --git a/lib/librte_pmd_e1000/igb_rxtx.c b/lib/librte_pmd_e1000/igb_rxtx.c
> index 946b39d..084e45a 100644
> --- a/lib/librte_pmd_e1000/igb_rxtx.c
> +++ b/lib/librte_pmd_e1000/igb_rxtx.c
> @@ -1164,8 +1164,7 @@ igb_reset_tx_queue_stat(struct igb_tx_queue *txq)
>   static void
>   igb_reset_tx_queue(struct igb_tx_queue *txq, struct rte_eth_dev *dev)
>   {
> -	static const union e1000_adv_tx_desc zeroed_desc = { .read = {
> -			.buffer_addr = 0}};
> +	static const union e1000_adv_tx_desc zeroed_desc = {{0}};
>   	struct igb_tx_entry *txe = txq->sw_ring;
>   	uint16_t i, prev;
>   	struct e1000_hw *hw;
> @@ -1330,8 +1329,7 @@ eth_igb_rx_queue_release(void *rxq)
>   static void
>   igb_reset_rx_queue(struct igb_rx_queue *rxq)
>   {
> -	static const union e1000_adv_rx_desc zeroed_desc = { .read = {
> -			.pkt_addr = 0}};
> +	static const union e1000_adv_rx_desc zeroed_desc = {{0}};
>   	unsigned i;
>   
>   	/* Zero out HW ring memory */
> diff --git a/lib/librte_pmd_enic/enic_clsf.c b/lib/librte_pmd_enic/enic_clsf.c
> index b61d625..a069194 100644
> --- a/lib/librte_pmd_enic/enic_clsf.c
> +++ b/lib/librte_pmd_enic/enic_clsf.c
> @@ -96,7 +96,7 @@ int enic_fdir_add_fltr(struct enic *enic, struct rte_fdir_filter *params,
>   	u16 queue, u8 drop)
>   {
>   	struct enic_fdir_node *key;
> -	struct filter fltr = {.type = 0};
> +	struct filter fltr = {0};
>   	int32_t pos;
>   	u8 do_free = 0;
>   	u16 old_fltr_id = 0;
> diff --git a/lib/librte_pmd_i40e/i40e_rxtx.c b/lib/librte_pmd_i40e/i40e_rxtx.c
> index 9c7be6f..abe68f4 100644
> --- a/lib/librte_pmd_i40e/i40e_rxtx.c
> +++ b/lib/librte_pmd_i40e/i40e_rxtx.c
> @@ -1228,7 +1228,7 @@ i40e_xmit_pkts(void *tx_queue, struct rte_mbuf **tx_pkts, uint16_t nb_pkts)
>   	uint16_t tx_last;
>   	uint16_t slen;
>   	uint64_t buf_dma_addr;
> -	union i40e_tx_offload tx_offload = { .data = 0 };
> +	union i40e_tx_offload tx_offload = {0};
>   
>   	txq = tx_queue;
>   	sw_ring = txq->sw_ring;
> diff --git a/lib/librte_pmd_ixgbe/ixgbe_rxtx.c b/lib/librte_pmd_ixgbe/ixgbe_rxtx.c
> index f1da9ec..3c61d1c 100644
> --- a/lib/librte_pmd_ixgbe/ixgbe_rxtx.c
> +++ b/lib/librte_pmd_ixgbe/ixgbe_rxtx.c
> @@ -579,7 +579,7 @@ ixgbe_xmit_pkts(void *tx_queue, struct rte_mbuf **tx_pkts,
>   	uint64_t tx_ol_req;
>   	uint32_t ctx = 0;
>   	uint32_t new_ctx;
> -	union ixgbe_tx_offload tx_offload = { .data = 0 };
> +	union ixgbe_tx_offload tx_offload = {0};
>   
>   	txq = tx_queue;
>   	sw_ring = txq->sw_ring;
> @@ -2052,8 +2052,7 @@ ixgbe_dev_tx_queue_release(void *txq)
>   static void
>   ixgbe_reset_tx_queue(struct ixgbe_tx_queue *txq)
>   {
> -	static const union ixgbe_adv_tx_desc zeroed_desc = { .read = {
> -			.buffer_addr = 0}};
> +	static const union ixgbe_adv_tx_desc zeroed_desc = {{0}};
>   	struct ixgbe_tx_entry *txe = txq->sw_ring;
>   	uint16_t prev, i;
>   
> @@ -2445,8 +2444,7 @@ check_rx_burst_bulk_alloc_preconditions(__rte_unused struct ixgbe_rx_queue *rxq)
>   static void
>   ixgbe_reset_rx_queue(struct ixgbe_hw *hw, struct ixgbe_rx_queue *rxq)
>   {
> -	static const union ixgbe_adv_rx_desc zeroed_desc = { .read = {
> -			.pkt_addr = 0}};
> +	static const union ixgbe_adv_rx_desc zeroed_desc = {{0}};
>   	unsigned i;
>   	uint16_t len = rxq->nb_rx_desc;
>   
> diff --git a/lib/librte_pmd_ixgbe/ixgbe_rxtx_vec.c b/lib/librte_pmd_ixgbe/ixgbe_rxtx_vec.c
> index 7ac6b61..abd10f6 100644
> --- a/lib/librte_pmd_ixgbe/ixgbe_rxtx_vec.c
> +++ b/lib/librte_pmd_ixgbe/ixgbe_rxtx_vec.c
> @@ -691,8 +691,7 @@ ixgbe_tx_free_swring(struct ixgbe_tx_queue *txq)
>   static void
>   ixgbe_reset_tx_queue(struct ixgbe_tx_queue *txq)
>   {
> -	static const union ixgbe_adv_tx_desc zeroed_desc = { .read = {
> -			.buffer_addr = 0} };
> +	static const union ixgbe_adv_tx_desc zeroed_desc = {{0}};
>   	struct ixgbe_tx_entry_v *txe = (struct ixgbe_tx_entry_v *)txq->sw_ring;
>   	uint16_t i;
>   
> diff --git a/lib/librte_pmd_mlx4/mlx4.c b/lib/librte_pmd_mlx4/mlx4.c
> index e096071..024282a 100644
> --- a/lib/librte_pmd_mlx4/mlx4.c
> +++ b/lib/librte_pmd_mlx4/mlx4.c
> @@ -3497,7 +3497,7 @@ static void
>   mlx4_stats_get(struct rte_eth_dev *dev, struct rte_eth_stats *stats)
>   {
>   	struct priv *priv = dev->data->dev_private;
> -	struct rte_eth_stats tmp = { .ipackets = 0 };
> +	struct rte_eth_stats tmp = {0};
>   	unsigned int i;
>   	unsigned int idx;
>   

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [dpdk-dev] [PATCH v3 1/2] mk: fix build with gcc 4.4 and clang
  2015-04-19  8:21                     ` Vlad Zolotarov
@ 2015-04-20 12:44                       ` Thomas Monjalon
  0 siblings, 0 replies; 33+ messages in thread
From: Thomas Monjalon @ 2015-04-20 12:44 UTC (permalink / raw)
  To: dev

> > With GCC 4.4.7 from CentOS 6.5, the following errors arise:
> >
> > lib/librte_pmd_ixgbe/ixgbe_rxtx.c: In function 'ixgbe_dev_rx_queue_setup':
> > lib/librte_pmd_ixgbe/ixgbe_rxtx.c:2509: error: missing initializer
> > lib/librte_pmd_ixgbe/ixgbe_rxtx.c:2509: error: (near initialization for 'dev_info.driver_name')
> >
> > lib/librte_pmd_ixgbe/ixgbe_rxtx.c: In function 'ixgbe_set_rsc':
> > lib/librte_pmd_ixgbe/ixgbe_rxtx.c:4072: error: missing initializer
> > lib/librte_pmd_ixgbe/ixgbe_rxtx.c:4072: error: (near initialization for 'dev_info.driver_name')
> >
> > lib/librte_pmd_ixgbe/ixgbe_rxtx.c: In function 'ixgbe_recv_pkts_lro_single_alloc':
> > lib/librte_pmd_ixgbe/ixgbe_rxtx.c:1479: error: 'next_rsc_entry' may be used uninitialized in this function
> > lib/librte_pmd_ixgbe/ixgbe_rxtx.c:1480: error: 'next_rxe' may be used uninitialized in this function
> >
> > The "missing initializer" warning is a GCC bug which seems fixed in 4.7.
> > The same warning is thrown by clang.
> > The "may be used uninitialized" warning is another GCC bug which seems fixed in 4.7.
> >
> > Fixes: 8eecb3295aed ("ixgbe: add LRO support")
> >
> > Signed-off-by: Thomas Monjalon <thomas.monjalon@6wind.com>
> 
> Acked-by: Vlad Zolotarov <vladz@cloudius-systems.com>

Applied, thanks

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [dpdk-dev] [PATCH v3 2/2] use simple zero initializers
  2015-04-19  8:22                       ` Vlad Zolotarov
@ 2015-04-20 12:45                         ` Thomas Monjalon
  0 siblings, 0 replies; 33+ messages in thread
From: Thomas Monjalon @ 2015-04-20 12:45 UTC (permalink / raw)
  To: dev

> > To initialize a structure with zeros, one field was explicitly set
> > to avoid "missing initializer" bug with old GCC (e.g. 4.4).
> > This warning is now disabled (commit <insertlater>) for old versions of GCC,
> > so the workarounds may be removed.
> >
> > These initializers should not be needed for static variables but they
> > are still used to workaround an ICC bug (see commit b2595c4aa92d).
> >
> > There is one remaining exception where {0} initializer doesn't work cleanly,
> > even with recent GCC:
> > lib/librte_pmd_ixgbe/ixgbe_rxtx_vec.c:735:9:
> > error: missing braces around initializer [-Werror=missing-braces]
> >    struct rte_mbuf mb_def = {0}; /* zeroed mbuf */
> >
> > Tested with gcc-4.4.7 (CentOS), gcc-4.7.2 (Debian), gcc-4.9.2 (Arch),
> > clang-3.6.0 and icc-13.1.1.
> >
> > Signed-off-by: Thomas Monjalon <thomas.monjalon@6wind.com>
> > Tested-by: Thomas Monjalon <thomas.monjalon@6wind.com>
> > Tested-by: John McNamara <john.mcnamara@intel.com>
> 
> Acked-by: Vlad Zolotarov <vladz@cloudius-systems.com>

Applied, thanks

^ permalink raw reply	[flat|nested] 33+ messages in thread

end of thread, other threads:[~2015-04-20 12:45 UTC | newest]

Thread overview: 33+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-04-14  9:31 [dpdk-dev] [PATCH] ixgbe: fix build with gcc 4.4 Thomas Monjalon
2015-04-14 12:48 ` Vlad Zolotarov
2015-04-14 13:06   ` Ananyev, Konstantin
2015-04-14 13:38     ` Vlad Zolotarov
2015-04-14 14:17       ` Thomas Monjalon
2015-04-14 14:30         ` Vlad Zolotarov
2015-04-14 14:53           ` Thomas Monjalon
2015-04-14 15:17             ` Vlad Zolotarov
2015-04-14 14:59         ` Vlad Zolotarov
2015-04-14 15:13           ` Thomas Monjalon
2015-04-14 15:21             ` Vlad Zolotarov
2015-04-14 15:28               ` Thomas Monjalon
2015-04-14 15:32                 ` Vlad Zolotarov
2015-04-15 20:49                 ` [dpdk-dev] [PATCH v2 1/2] " Thomas Monjalon
2015-04-15 20:49                   ` [dpdk-dev] [PATCH v2 2/2] use simple zero initializers Thomas Monjalon
2015-04-16 10:12                     ` Olivier MATZ
2015-04-16 12:55                       ` Thomas Monjalon
2015-04-16 16:31                         ` Mcnamara, John
2015-04-16  7:26                   ` [dpdk-dev] [PATCH v2 1/2] ixgbe: fix build with gcc 4.4 Zhang, Helin
2015-04-16  9:14                   ` Vlad Zolotarov
2015-04-16  9:18                     ` Thomas Monjalon
2015-04-16  9:35                       ` Vlad Zolotarov
2015-04-16 22:10                   ` [dpdk-dev] [PATCH v3 1/2] mk: fix build with gcc 4.4 and clang Thomas Monjalon
2015-04-16 22:10                     ` [dpdk-dev] [PATCH v3 2/2] use simple zero initializers Thomas Monjalon
2015-04-17 11:17                       ` Mcnamara, John
2015-04-19  8:22                       ` Vlad Zolotarov
2015-04-20 12:45                         ` Thomas Monjalon
2015-04-17 11:15                     ` [dpdk-dev] [PATCH v3 1/2] mk: fix build with gcc 4.4 and clang Mcnamara, John
2015-04-19  8:21                     ` Vlad Zolotarov
2015-04-20 12:44                       ` Thomas Monjalon
2015-04-14 12:51 ` [dpdk-dev] [PATCH] ixgbe: fix build with gcc 4.4 Vlad Zolotarov
2015-04-14 13:23   ` Ananyev, Konstantin
2015-04-14 13:41     ` Vlad Zolotarov

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).