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