DPDK patches and discussions
 help / color / mirror / Atom feed
* [dpdk-dev] [PATCH] app/testpmd: fix uninitialized members of mpls
@ 2020-01-30 16:59 Ori Kam
  2020-01-31 11:07 ` Ferruh Yigit
  2020-02-04 11:49 ` [dpdk-dev] " Ferruh Yigit
  0 siblings, 2 replies; 7+ messages in thread
From: Ori Kam @ 2020-01-30 16:59 UTC (permalink / raw)
  To: Wenzhuo Lu, Jingjing Wu, Bernard Iremonger
  Cc: dev, orika, ferruh.yigit, viacheslavo, stable

Some of the memebers of the mpls struct are not initialized.
this commit init the uninitialized members.

Coverity issue: 325735
Fixes: 3e77031be855 ("app/testpmd: add MPLSoGRE encapsulation")
Cc: stable@dpdk.org

Signed-off-by: Ori Kam <orika@mellanox.com>
---
 app/test-pmd/cmdline_flow.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/app/test-pmd/cmdline_flow.c b/app/test-pmd/cmdline_flow.c
index e99e24c..c2cc4c5 100644
--- a/app/test-pmd/cmdline_flow.c
+++ b/app/test-pmd/cmdline_flow.c
@@ -4576,7 +4576,9 @@ static int comp_set_raw_index(struct context *, const struct token *,
 	struct rte_flow_item_gre gre = {
 		.protocol = rte_cpu_to_be_16(ETHER_TYPE_MPLS_UNICAST),
 	};
-	struct rte_flow_item_mpls mpls;
+	struct rte_flow_item_mpls mpls = {
+		.ttl = 0,
+	};
 	uint8_t *header;
 	int ret;
 
-- 
1.8.3.1


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

* Re: [dpdk-dev] [PATCH] app/testpmd: fix uninitialized members of mpls
  2020-01-30 16:59 [dpdk-dev] [PATCH] app/testpmd: fix uninitialized members of mpls Ori Kam
@ 2020-01-31 11:07 ` Ferruh Yigit
  2020-02-02  8:23   ` Ori Kam
  2020-02-04 11:49 ` [dpdk-dev] " Ferruh Yigit
  1 sibling, 1 reply; 7+ messages in thread
From: Ferruh Yigit @ 2020-01-31 11:07 UTC (permalink / raw)
  To: Ori Kam, Wenzhuo Lu, Jingjing Wu, Bernard Iremonger
  Cc: dev, viacheslavo, stable

On 1/30/2020 4:59 PM, Ori Kam wrote:
> Some of the memebers of the mpls struct are not initialized.
> this commit init the uninitialized members.
> 
> Coverity issue: 325735
> Fixes: 3e77031be855 ("app/testpmd: add MPLSoGRE encapsulation")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Ori Kam <orika@mellanox.com>
> ---
>  app/test-pmd/cmdline_flow.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/app/test-pmd/cmdline_flow.c b/app/test-pmd/cmdline_flow.c
> index e99e24c..c2cc4c5 100644
> --- a/app/test-pmd/cmdline_flow.c
> +++ b/app/test-pmd/cmdline_flow.c
> @@ -4576,7 +4576,9 @@ static int comp_set_raw_index(struct context *, const struct token *,
>  	struct rte_flow_item_gre gre = {
>  		.protocol = rte_cpu_to_be_16(ETHER_TYPE_MPLS_UNICAST),
>  	};
> -	struct rte_flow_item_mpls mpls;
> +	struct rte_flow_item_mpls mpls = {
> +		.ttl = 0,
> +	};

why not use "= {0};", the end result will be same, struct will be all zeroed out
in both case, "= {0};" makes the intention more obvious I think.

>  	uint8_t *header;
>  	int ret;
>  
> 


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

* Re: [dpdk-dev] [PATCH] app/testpmd: fix uninitialized members of mpls
  2020-01-31 11:07 ` Ferruh Yigit
@ 2020-02-02  8:23   ` Ori Kam
  2020-02-03 10:21     ` Ferruh Yigit
  0 siblings, 1 reply; 7+ messages in thread
From: Ori Kam @ 2020-02-02  8:23 UTC (permalink / raw)
  To: Ferruh Yigit, Wenzhuo Lu, Jingjing Wu, Bernard Iremonger
  Cc: dev, Slava Ovsiienko, stable

Hi Ferruh,

PSB,
Thanks,
Ori

> -----Original Message-----
> From: Ferruh Yigit <ferruh.yigit@intel.com>
> Sent: Friday, January 31, 2020 1:08 PM
> To: Ori Kam <orika@mellanox.com>; Wenzhuo Lu <wenzhuo.lu@intel.com>;
> Jingjing Wu <jingjing.wu@intel.com>; Bernard Iremonger
> <bernard.iremonger@intel.com>
> Cc: dev@dpdk.org; Slava Ovsiienko <viacheslavo@mellanox.com>;
> stable@dpdk.org
> Subject: Re: [PATCH] app/testpmd: fix uninitialized members of mpls
> 
> On 1/30/2020 4:59 PM, Ori Kam wrote:
> > Some of the memebers of the mpls struct are not initialized.
> > this commit init the uninitialized members.
> >
> > Coverity issue: 325735
> > Fixes: 3e77031be855 ("app/testpmd: add MPLSoGRE encapsulation")
> > Cc: stable@dpdk.org
> >
> > Signed-off-by: Ori Kam <orika@mellanox.com>
> > ---
> >  app/test-pmd/cmdline_flow.c | 4 +++-
> >  1 file changed, 3 insertions(+), 1 deletion(-)
> >
> > diff --git a/app/test-pmd/cmdline_flow.c b/app/test-pmd/cmdline_flow.c
> > index e99e24c..c2cc4c5 100644
> > --- a/app/test-pmd/cmdline_flow.c
> > +++ b/app/test-pmd/cmdline_flow.c
> > @@ -4576,7 +4576,9 @@ static int comp_set_raw_index(struct context *,
> const struct token *,
> >  	struct rte_flow_item_gre gre = {
> >  		.protocol = rte_cpu_to_be_16(ETHER_TYPE_MPLS_UNICAST),
> >  	};
> > -	struct rte_flow_item_mpls mpls;
> > +	struct rte_flow_item_mpls mpls = {
> > +		.ttl = 0,
> > +	};
> 
> why not use "= {0};", the end result will be same, struct will be all zeroed out
> in both case, "= {0};" makes the intention more obvious I think.
> 

On some compilers this kind of  initialization result in an error:
error: missing braces around initializer
it looks like a known issue of GCC
So I just prefer to init the relevant fields.

> >  	uint8_t *header;
> >  	int ret;
> >
> >


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

* Re: [dpdk-dev] [PATCH] app/testpmd: fix uninitialized members of mpls
  2020-02-02  8:23   ` Ori Kam
@ 2020-02-03 10:21     ` Ferruh Yigit
  2020-02-03 17:46       ` [dpdk-dev] [dpdk-stable] " Ferruh Yigit
  0 siblings, 1 reply; 7+ messages in thread
From: Ferruh Yigit @ 2020-02-03 10:21 UTC (permalink / raw)
  To: Ori Kam, Wenzhuo Lu, Jingjing Wu, Bernard Iremonger
  Cc: dev, Slava Ovsiienko, stable

On 2/2/2020 8:23 AM, Ori Kam wrote:
> Hi Ferruh,
> 
> PSB,
> Thanks,
> Ori
> 
>> -----Original Message-----
>> From: Ferruh Yigit <ferruh.yigit@intel.com>
>> Sent: Friday, January 31, 2020 1:08 PM
>> To: Ori Kam <orika@mellanox.com>; Wenzhuo Lu <wenzhuo.lu@intel.com>;
>> Jingjing Wu <jingjing.wu@intel.com>; Bernard Iremonger
>> <bernard.iremonger@intel.com>
>> Cc: dev@dpdk.org; Slava Ovsiienko <viacheslavo@mellanox.com>;
>> stable@dpdk.org
>> Subject: Re: [PATCH] app/testpmd: fix uninitialized members of mpls
>>
>> On 1/30/2020 4:59 PM, Ori Kam wrote:
>>> Some of the memebers of the mpls struct are not initialized.
>>> this commit init the uninitialized members.
>>>
>>> Coverity issue: 325735
>>> Fixes: 3e77031be855 ("app/testpmd: add MPLSoGRE encapsulation")
>>> Cc: stable@dpdk.org
>>>
>>> Signed-off-by: Ori Kam <orika@mellanox.com>
>>> ---
>>>  app/test-pmd/cmdline_flow.c | 4 +++-
>>>  1 file changed, 3 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/app/test-pmd/cmdline_flow.c b/app/test-pmd/cmdline_flow.c
>>> index e99e24c..c2cc4c5 100644
>>> --- a/app/test-pmd/cmdline_flow.c
>>> +++ b/app/test-pmd/cmdline_flow.c
>>> @@ -4576,7 +4576,9 @@ static int comp_set_raw_index(struct context *,
>> const struct token *,
>>>  	struct rte_flow_item_gre gre = {
>>>  		.protocol = rte_cpu_to_be_16(ETHER_TYPE_MPLS_UNICAST),
>>>  	};
>>> -	struct rte_flow_item_mpls mpls;
>>> +	struct rte_flow_item_mpls mpls = {
>>> +		.ttl = 0,
>>> +	};
>>
>> why not use "= {0};", the end result will be same, struct will be all zeroed out
>> in both case, "= {0};" makes the intention more obvious I think.
>>
> 
> On some compilers this kind of  initialization result in an error:
> error: missing braces around initializer

As far as I remember, that is triggered with old compiler, when you are
initializing array of structs, or first element of the struct is an array (which
is the case for mpls struct), in that case ={{0}} was solving the issue.

Anyway, I got the concern, the patch looks good to me.

> it looks like a known issue of GCC
> So I just prefer to init the relevant fields.
> 
>>>  	uint8_t *header;
>>>  	int ret;
>>>
>>>
> 


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

* Re: [dpdk-dev] [dpdk-stable] [PATCH] app/testpmd: fix uninitialized members of mpls
  2020-02-03 10:21     ` Ferruh Yigit
@ 2020-02-03 17:46       ` Ferruh Yigit
  2020-02-04  7:50         ` Ori Kam
  0 siblings, 1 reply; 7+ messages in thread
From: Ferruh Yigit @ 2020-02-03 17:46 UTC (permalink / raw)
  To: Ori Kam, Wenzhuo Lu, Jingjing Wu, Bernard Iremonger
  Cc: dev, Slava Ovsiienko, stable

On 2/3/2020 10:21 AM, Ferruh Yigit wrote:
> On 2/2/2020 8:23 AM, Ori Kam wrote:
>> Hi Ferruh,
>>
>> PSB,
>> Thanks,
>> Ori
>>
>>> -----Original Message-----
>>> From: Ferruh Yigit <ferruh.yigit@intel.com>
>>> Sent: Friday, January 31, 2020 1:08 PM
>>> To: Ori Kam <orika@mellanox.com>; Wenzhuo Lu <wenzhuo.lu@intel.com>;
>>> Jingjing Wu <jingjing.wu@intel.com>; Bernard Iremonger
>>> <bernard.iremonger@intel.com>
>>> Cc: dev@dpdk.org; Slava Ovsiienko <viacheslavo@mellanox.com>;
>>> stable@dpdk.org
>>> Subject: Re: [PATCH] app/testpmd: fix uninitialized members of mpls
>>>
>>> On 1/30/2020 4:59 PM, Ori Kam wrote:
>>>> Some of the memebers of the mpls struct are not initialized.
>>>> this commit init the uninitialized members.
>>>>
>>>> Coverity issue: 325735
>>>> Fixes: 3e77031be855 ("app/testpmd: add MPLSoGRE encapsulation")
>>>> Cc: stable@dpdk.org
>>>>
>>>> Signed-off-by: Ori Kam <orika@mellanox.com>
>>>> ---
>>>>  app/test-pmd/cmdline_flow.c | 4 +++-
>>>>  1 file changed, 3 insertions(+), 1 deletion(-)
>>>>
>>>> diff --git a/app/test-pmd/cmdline_flow.c b/app/test-pmd/cmdline_flow.c
>>>> index e99e24c..c2cc4c5 100644
>>>> --- a/app/test-pmd/cmdline_flow.c
>>>> +++ b/app/test-pmd/cmdline_flow.c
>>>> @@ -4576,7 +4576,9 @@ static int comp_set_raw_index(struct context *,
>>> const struct token *,
>>>>  	struct rte_flow_item_gre gre = {
>>>>  		.protocol = rte_cpu_to_be_16(ETHER_TYPE_MPLS_UNICAST),
>>>>  	};
>>>> -	struct rte_flow_item_mpls mpls;
>>>> +	struct rte_flow_item_mpls mpls = {
>>>> +		.ttl = 0,
>>>> +	};
>>>
>>> why not use "= {0};", the end result will be same, struct will be all zeroed out
>>> in both case, "= {0};" makes the intention more obvious I think.
>>>
>>
>> On some compilers this kind of  initialization result in an error:
>> error: missing braces around initializer
> 
> As far as I remember, that is triggered with old compiler, when you are
> initializing array of structs, or first element of the struct is an array (which
> is the case for mpls struct), in that case ={{0}} was solving the issue.
> 
> Anyway, I got the concern, the patch looks good to me.

Also seems "={}" is working in that case, thanks to Stephen, it is cleaner than
"={{0}}", what do you think about using "={}" ?

> 
>> it looks like a known issue of GCC
>> So I just prefer to init the relevant fields.
>>
>>>>  	uint8_t *header;
>>>>  	int ret;
>>>>
>>>>
>>
> 


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

* Re: [dpdk-dev] [dpdk-stable] [PATCH] app/testpmd: fix uninitialized members of mpls
  2020-02-03 17:46       ` [dpdk-dev] [dpdk-stable] " Ferruh Yigit
@ 2020-02-04  7:50         ` Ori Kam
  0 siblings, 0 replies; 7+ messages in thread
From: Ori Kam @ 2020-02-04  7:50 UTC (permalink / raw)
  To: Ferruh Yigit, Wenzhuo Lu, Jingjing Wu, Bernard Iremonger
  Cc: dev, Slava Ovsiienko, stable



> -----Original Message-----
> From: Ferruh Yigit <ferruh.yigit@intel.com>
> Sent: Monday, February 3, 2020 7:46 PM
> To: Ori Kam <orika@mellanox.com>; Wenzhuo Lu <wenzhuo.lu@intel.com>;
> Jingjing Wu <jingjing.wu@intel.com>; Bernard Iremonger
> <bernard.iremonger@intel.com>
> Cc: dev@dpdk.org; Slava Ovsiienko <viacheslavo@mellanox.com>;
> stable@dpdk.org
> Subject: Re: [dpdk-stable] [PATCH] app/testpmd: fix uninitialized members of
> mpls
> 
> On 2/3/2020 10:21 AM, Ferruh Yigit wrote:
> > On 2/2/2020 8:23 AM, Ori Kam wrote:
> >> Hi Ferruh,
> >>
> >> PSB,
> >> Thanks,
> >> Ori
> >>
> >>> -----Original Message-----
> >>> From: Ferruh Yigit <ferruh.yigit@intel.com>
> >>> Sent: Friday, January 31, 2020 1:08 PM
> >>> To: Ori Kam <orika@mellanox.com>; Wenzhuo Lu
> <wenzhuo.lu@intel.com>;
> >>> Jingjing Wu <jingjing.wu@intel.com>; Bernard Iremonger
> >>> <bernard.iremonger@intel.com>
> >>> Cc: dev@dpdk.org; Slava Ovsiienko <viacheslavo@mellanox.com>;
> >>> stable@dpdk.org
> >>> Subject: Re: [PATCH] app/testpmd: fix uninitialized members of mpls
> >>>
> >>> On 1/30/2020 4:59 PM, Ori Kam wrote:
> >>>> Some of the memebers of the mpls struct are not initialized.
> >>>> this commit init the uninitialized members.
> >>>>
> >>>> Coverity issue: 325735
> >>>> Fixes: 3e77031be855 ("app/testpmd: add MPLSoGRE encapsulation")
> >>>> Cc: stable@dpdk.org
> >>>>
> >>>> Signed-off-by: Ori Kam <orika@mellanox.com>
> >>>> ---
> >>>>  app/test-pmd/cmdline_flow.c | 4 +++-
> >>>>  1 file changed, 3 insertions(+), 1 deletion(-)
> >>>>
> >>>> diff --git a/app/test-pmd/cmdline_flow.c b/app/test-pmd/cmdline_flow.c
> >>>> index e99e24c..c2cc4c5 100644
> >>>> --- a/app/test-pmd/cmdline_flow.c
> >>>> +++ b/app/test-pmd/cmdline_flow.c
> >>>> @@ -4576,7 +4576,9 @@ static int comp_set_raw_index(struct context *,
> >>> const struct token *,
> >>>>  	struct rte_flow_item_gre gre = {
> >>>>  		.protocol = rte_cpu_to_be_16(ETHER_TYPE_MPLS_UNICAST),
> >>>>  	};
> >>>> -	struct rte_flow_item_mpls mpls;
> >>>> +	struct rte_flow_item_mpls mpls = {
> >>>> +		.ttl = 0,
> >>>> +	};
> >>>
> >>> why not use "= {0};", the end result will be same, struct will be all zeroed
> out
> >>> in both case, "= {0};" makes the intention more obvious I think.
> >>>
> >>
> >> On some compilers this kind of  initialization result in an error:
> >> error: missing braces around initializer
> >
> > As far as I remember, that is triggered with old compiler, when you are
> > initializing array of structs, or first element of the struct is an array (which
> > is the case for mpls struct), in that case ={{0}} was solving the issue.
> >
> > Anyway, I got the concern, the patch looks good to me.
> 
> Also seems "={}" is working in that case, thanks to Stephen, it is cleaner than
> "={{0}}", what do you think about using "={}" ?
> 
Thanks for the idea, but it still causes issue,
"error: missing initializer for field 'label_tc_s' of 'struct rte_flow_item_mpls' [-Werror=missing-field-initializers]
 struct rte_flow_item_mpls mpls = {};"
Just for reference  I'm using gcc 4.8.5 (Red Hat 4.8.5-4)
I know it is very old, but some of our customers are using it.
So if you don't mind I will keep my code as is.

> >
> >> it looks like a known issue of GCC
> >> So I just prefer to init the relevant fields.
> >>
> >>>>  	uint8_t *header;
> >>>>  	int ret;
> >>>>
> >>>>
> >>
> >


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

* Re: [dpdk-dev] [PATCH] app/testpmd: fix uninitialized members of mpls
  2020-01-30 16:59 [dpdk-dev] [PATCH] app/testpmd: fix uninitialized members of mpls Ori Kam
  2020-01-31 11:07 ` Ferruh Yigit
@ 2020-02-04 11:49 ` Ferruh Yigit
  1 sibling, 0 replies; 7+ messages in thread
From: Ferruh Yigit @ 2020-02-04 11:49 UTC (permalink / raw)
  To: Ori Kam, Wenzhuo Lu, Jingjing Wu, Bernard Iremonger
  Cc: dev, viacheslavo, stable

On 1/30/2020 4:59 PM, Ori Kam wrote:
> Some of the memebers of the mpls struct are not initialized.
> this commit init the uninitialized members.
> 
> Coverity issue: 325735
> Fixes: 3e77031be855 ("app/testpmd: add MPLSoGRE encapsulation")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Ori Kam <orika@mellanox.com>

Reviewed-by: Ferruh Yigit <ferruh.yigit@intel.com>

Applied to dpdk-next-net/master, thanks.

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

end of thread, other threads:[~2020-02-04 11:49 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-01-30 16:59 [dpdk-dev] [PATCH] app/testpmd: fix uninitialized members of mpls Ori Kam
2020-01-31 11:07 ` Ferruh Yigit
2020-02-02  8:23   ` Ori Kam
2020-02-03 10:21     ` Ferruh Yigit
2020-02-03 17:46       ` [dpdk-dev] [dpdk-stable] " Ferruh Yigit
2020-02-04  7:50         ` Ori Kam
2020-02-04 11:49 ` [dpdk-dev] " Ferruh Yigit

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