Re: [dpdk-stable] [PATCH] app/testpmd: fix uninitialized members of mpls

[Ferruh Yigit <ferruh.yigit@intel.com>]

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;
>>>
>>>
>