* [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).