[dpdk-dev] [PATCH 1/2] net/avp: fix gcc 10 maybe-uninitialized warning

[dpdk-dev] [PATCH 1/2] net/avp: fix gcc 10 maybe-uninitialized warning

[Kevin Traynor]

gcc 10.0.1 reports:

../drivers/net/avp/avp_ethdev.c: In function ‘avp_xmit_scattered_pkts’:
../drivers/net/avp/avp_ethdev.c:1791:24:
warning: ‘avp_bufs[count]’ may be used uninitialized in this function [-Wmaybe-uninitialized]
 1791 |   tx_bufs[i] = avp_bufs[count];
      |                ~~~~~~~~^~~~~~~
../drivers/net/avp/avp_ethdev.c:1791:24:
warning: ‘avp_bufs[count]’ may be used uninitialized in this function [-Wmaybe-uninitialized]

Fix by intializing the array.

Fixes: 295abce2d25b ("net/avp: add packet transmit functions")
Cc: stable@dpdk.org

Signed-off-by: Kevin Traynor <ktraynor@redhat.com>
---
note, commit log violates line length but I didn't want to split warning msg.

Cc: allain.legacy@windriver.com
Cc: Steven Webster <steven.webster@windriver.com>
Cc: Matt Peters <matt.peters@windriver.com>
---
 drivers/net/avp/avp_ethdev.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/avp/avp_ethdev.c b/drivers/net/avp/avp_ethdev.c
index cd747b6be..1abe96ce5 100644
--- a/drivers/net/avp/avp_ethdev.c
+++ b/drivers/net/avp/avp_ethdev.c
@@ -1695,5 +1695,5 @@ avp_xmit_scattered_pkts(void *tx_queue,
 {
 	struct rte_avp_desc *avp_bufs[(AVP_MAX_TX_BURST *
-				       RTE_AVP_MAX_MBUF_SEGMENTS)];
+				       RTE_AVP_MAX_MBUF_SEGMENTS)] = {};
 	struct avp_queue *txq = (struct avp_queue *)tx_queue;
 	struct rte_avp_desc *tx_bufs[AVP_MAX_TX_BURST];
-- 
2.21.1

[dpdk-dev] [PATCH 2/2] examples/ipsec-gw: fix gcc 10 maybe-uninitialized warning

[Kevin Traynor]

gcc 10.0.1 reports:

../examples/ipsec-secgw/ipsec_process.c: In function ‘ipsec_process’:
../examples/ipsec-secgw/ipsec_process.c:132:34:
error: ‘grp.m’ may be used uninitialized in this function [-Werror=maybe-uninitialized]
  132 |    grp[n].cnt = pkts + i - grp[n].m;
      |                            ~~~~~~^~

Fix by initializing the array.

Fixes: 3e5f4625dc17 ("examples/ipsec-secgw: make data-path to use IPsec library")
Cc: stable@dpdk.org

Signed-off-by: Kevin Traynor <ktraynor@redhat.com>
---
note, commit log violates line length but I didn't want to split warning msg.

Cc: konstantin.ananyev@intel.com
Cc: Radu Nicolau <radu.nicolau@intel.com>
Cc: Akhil Goyal <akhil.goyal@nxp.com>
---
 examples/ipsec-secgw/ipsec_process.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/examples/ipsec-secgw/ipsec_process.c b/examples/ipsec-secgw/ipsec_process.c
index bb2f2b82d..0032c5c08 100644
--- a/examples/ipsec-secgw/ipsec_process.c
+++ b/examples/ipsec-secgw/ipsec_process.c
@@ -287,5 +287,5 @@ ipsec_process(struct ipsec_ctx *ctx, struct ipsec_traffic *trf)
 	struct rte_ipsec_group *pg;
 	struct rte_ipsec_session *ips;
-	struct rte_ipsec_group grp[RTE_DIM(trf->ipsec.pkts)];
+	struct rte_ipsec_group grp[RTE_DIM(trf->ipsec.pkts)] = {};
 
 	n = sa_group(trf->ipsec.saptr, trf->ipsec.pkts, grp, trf->ipsec.num);
-- 
2.21.1

Re: [dpdk-dev] [PATCH 2/2] examples/ipsec-gw: fix gcc 10 maybe-uninitialized warning

[Akhil Goyal]

> 
> gcc 10.0.1 reports:
> 
> ../examples/ipsec-secgw/ipsec_process.c: In function ‘ipsec_process’:
> ../examples/ipsec-secgw/ipsec_process.c:132:34:
> error: ‘grp.m’ may be used uninitialized in this function [-Werror=maybe-
> uninitialized]
>   132 |    grp[n].cnt = pkts + i - grp[n].m;
>       |                            ~~~~~~^~
> 
> Fix by initializing the array.
> 
> Fixes: 3e5f4625dc17 ("examples/ipsec-secgw: make data-path to use IPsec
> library")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Kevin Traynor <ktraynor@redhat.com>
> ---
> note, commit log violates line length but I didn't want to split warning msg.
> 
> Cc: konstantin.ananyev@intel.com
> Cc: Radu Nicolau <radu.nicolau@intel.com>
> Cc: Akhil Goyal <akhil.goyal@nxp.com>
> ---
Acked-by: Akhil Goyal <akhil.goyal@nxp.com>

Re: [dpdk-dev] [PATCH 2/2] examples/ipsec-gw: fix gcc 10 maybe-uninitialized warning

[Ananyev, Konstantin]

Hi Kevin,

> gcc 10.0.1 reports:
> 
> ../examples/ipsec-secgw/ipsec_process.c: In function ‘ipsec_process’:
> ../examples/ipsec-secgw/ipsec_process.c:132:34:
> error: ‘grp.m’ may be used uninitialized in this function [-Werror=maybe-uninitialized]
>   132 |    grp[n].cnt = pkts + i - grp[n].m;
>       |                            ~~~~~~^~
> 
> Fix by initializing the array.
> 
> Fixes: 3e5f4625dc17 ("examples/ipsec-secgw: make data-path to use IPsec library")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Kevin Traynor <ktraynor@redhat.com>
> ---
> note, commit log violates line length but I didn't want to split warning msg.
> 
> Cc: konstantin.ananyev@intel.com
> Cc: Radu Nicolau <radu.nicolau@intel.com>
> Cc: Akhil Goyal <akhil.goyal@nxp.com>
> ---
>  examples/ipsec-secgw/ipsec_process.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/examples/ipsec-secgw/ipsec_process.c b/examples/ipsec-secgw/ipsec_process.c
> index bb2f2b82d..0032c5c08 100644
> --- a/examples/ipsec-secgw/ipsec_process.c
> +++ b/examples/ipsec-secgw/ipsec_process.c
> @@ -287,5 +287,5 @@ ipsec_process(struct ipsec_ctx *ctx, struct ipsec_traffic *trf)
>  	struct rte_ipsec_group *pg;
>  	struct rte_ipsec_session *ips;
> -	struct rte_ipsec_group grp[RTE_DIM(trf->ipsec.pkts)];
> +	struct rte_ipsec_group grp[RTE_DIM(trf->ipsec.pkts)] = {};

Wouldn't that force to generate an extra instructions to zero-out a chunk of memory,
grp pointitg to?
Considering that this is perf critical pass, that's probably not a best thing.
If disabling compiler warning, is not an option, then probably something like code 
below would help?
Konstantin 

diff --git a/examples/ipsec-secgw/ipsec_process.c b/examples/ipsec-secgw/ipsec_process.c
index bb2f2b82d..6d3a3c9a1 100644
--- a/examples/ipsec-secgw/ipsec_process.c
+++ b/examples/ipsec-secgw/ipsec_process.c
@@ -126,6 +126,7 @@ sa_group(void *sa_ptr[], struct rte_mbuf *pkts[],
        void * const nosa = &spi;

        sa = nosa;
+       grp[0].m = pkts;
        for (i = 0, n = 0; i != num; i++) {

                if (sa != sa_ptr[i]) {

Re: [dpdk-dev] [PATCH 2/2] examples/ipsec-gw: fix gcc 10 maybe-uninitialized warning

[Kevin Traynor]

On 10/03/2020 13:08, Ananyev, Konstantin wrote:
> Hi Kevin,
> 

Hi Konstantin,

>> gcc 10.0.1 reports:
>>
>> ../examples/ipsec-secgw/ipsec_process.c: In function ‘ipsec_process’:
>> ../examples/ipsec-secgw/ipsec_process.c:132:34:
>> error: ‘grp.m’ may be used uninitialized in this function [-Werror=maybe-uninitialized]
>>   132 |    grp[n].cnt = pkts + i - grp[n].m;
>>       |                            ~~~~~~^~
>>
>> Fix by initializing the array.
>>
>> Fixes: 3e5f4625dc17 ("examples/ipsec-secgw: make data-path to use IPsec library")
>> Cc: stable@dpdk.org
>>
>> Signed-off-by: Kevin Traynor <ktraynor@redhat.com>
>> ---
>> note, commit log violates line length but I didn't want to split warning msg.
>>
>> Cc: konstantin.ananyev@intel.com
>> Cc: Radu Nicolau <radu.nicolau@intel.com>
>> Cc: Akhil Goyal <akhil.goyal@nxp.com>
>> ---
>>  examples/ipsec-secgw/ipsec_process.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/examples/ipsec-secgw/ipsec_process.c b/examples/ipsec-secgw/ipsec_process.c
>> index bb2f2b82d..0032c5c08 100644
>> --- a/examples/ipsec-secgw/ipsec_process.c
>> +++ b/examples/ipsec-secgw/ipsec_process.c
>> @@ -287,5 +287,5 @@ ipsec_process(struct ipsec_ctx *ctx, struct ipsec_traffic *trf)
>>  	struct rte_ipsec_group *pg;
>>  	struct rte_ipsec_session *ips;
>> -	struct rte_ipsec_group grp[RTE_DIM(trf->ipsec.pkts)];
>> +	struct rte_ipsec_group grp[RTE_DIM(trf->ipsec.pkts)] = {};
> 
> Wouldn't that force to generate an extra instructions to zero-out a chunk of memory,
> grp pointitg to?

Yes

> Considering that this is perf critical pass, that's probably not a best thing.
> If disabling compiler warning, is not an option, then probably something like code 
> below would help?

Yes, that is a nice suggestion - this will remove the warning with less
instructions and LGTM.

In this case we can see that the code is safe because the grp[0].cnt is
written with a valid value in the second instance but I suppose
disabling the warning could mask something else later.

If you're ok with the approach below, I can prepare a v2 with your
"Suggested-by". WDYT?

Kevin.

> Konstantin 
> 
> diff --git a/examples/ipsec-secgw/ipsec_process.c b/examples/ipsec-secgw/ipsec_process.c
> index bb2f2b82d..6d3a3c9a1 100644
> --- a/examples/ipsec-secgw/ipsec_process.c
> +++ b/examples/ipsec-secgw/ipsec_process.c
> @@ -126,6 +126,7 @@ sa_group(void *sa_ptr[], struct rte_mbuf *pkts[],
>         void * const nosa = &spi;
> 
>         sa = nosa;
> +       grp[0].m = pkts;
>         for (i = 0, n = 0; i != num; i++) {
> 
>                 if (sa != sa_ptr[i]) {
> 

Re: [dpdk-dev] [PATCH 2/2] examples/ipsec-gw: fix gcc 10 maybe-uninitialized warning

[Ananyev, Konstantin]

> >> gcc 10.0.1 reports:
> >>
> >> ../examples/ipsec-secgw/ipsec_process.c: In function ‘ipsec_process’:
> >> ../examples/ipsec-secgw/ipsec_process.c:132:34:
> >> error: ‘grp.m’ may be used uninitialized in this function [-Werror=maybe-uninitialized]
> >>   132 |    grp[n].cnt = pkts + i - grp[n].m;
> >>       |                            ~~~~~~^~
> >>
> >> Fix by initializing the array.
> >>
> >> Fixes: 3e5f4625dc17 ("examples/ipsec-secgw: make data-path to use IPsec library")
> >> Cc: stable@dpdk.org
> >>
> >> Signed-off-by: Kevin Traynor <ktraynor@redhat.com>
> >> ---
> >> note, commit log violates line length but I didn't want to split warning msg.
> >>
> >> Cc: konstantin.ananyev@intel.com
> >> Cc: Radu Nicolau <radu.nicolau@intel.com>
> >> Cc: Akhil Goyal <akhil.goyal@nxp.com>
> >> ---
> >>  examples/ipsec-secgw/ipsec_process.c | 2 +-
> >>  1 file changed, 1 insertion(+), 1 deletion(-)
> >>
> >> diff --git a/examples/ipsec-secgw/ipsec_process.c b/examples/ipsec-secgw/ipsec_process.c
> >> index bb2f2b82d..0032c5c08 100644
> >> --- a/examples/ipsec-secgw/ipsec_process.c
> >> +++ b/examples/ipsec-secgw/ipsec_process.c
> >> @@ -287,5 +287,5 @@ ipsec_process(struct ipsec_ctx *ctx, struct ipsec_traffic *trf)
> >>  	struct rte_ipsec_group *pg;
> >>  	struct rte_ipsec_session *ips;
> >> -	struct rte_ipsec_group grp[RTE_DIM(trf->ipsec.pkts)];
> >> +	struct rte_ipsec_group grp[RTE_DIM(trf->ipsec.pkts)] = {};
> >
> > Wouldn't that force to generate an extra instructions to zero-out a chunk of memory,
> > grp pointitg to?
> 
> Yes
> 
> > Considering that this is perf critical pass, that's probably not a best thing.
> > If disabling compiler warning, is not an option, then probably something like code
> > below would help?
> 
> Yes, that is a nice suggestion - this will remove the warning with less
> instructions and LGTM.
> 
> In this case we can see that the code is safe because the grp[0].cnt is
> written with a valid value in the second instance but I suppose
> disabling the warning could mask something else later.
> 
> If you're ok with the approach below, I can prepare a v2 with your
> "Suggested-by". WDYT?

Sounds like a good plan to me.
Thanks
Konstantin

> 
> Kevin.
> 
> > Konstantin
> >
> > diff --git a/examples/ipsec-secgw/ipsec_process.c b/examples/ipsec-secgw/ipsec_process.c
> > index bb2f2b82d..6d3a3c9a1 100644
> > --- a/examples/ipsec-secgw/ipsec_process.c
> > +++ b/examples/ipsec-secgw/ipsec_process.c
> > @@ -126,6 +126,7 @@ sa_group(void *sa_ptr[], struct rte_mbuf *pkts[],
> >         void * const nosa = &spi;
> >
> >         sa = nosa;
> > +       grp[0].m = pkts;
> >         for (i = 0, n = 0; i != num; i++) {
> >
> >                 if (sa != sa_ptr[i]) {
> >

[dpdk-dev] [PATCH v2 1/2] net/avp: fix gcc 10 maybe-uninitialized warning

[Kevin Traynor]

gcc 10.0.1 reports:

../drivers/net/avp/avp_ethdev.c: In function ‘avp_xmit_scattered_pkts’:
../drivers/net/avp/avp_ethdev.c:1791:24:
warning: ‘avp_bufs[count]’ may be used uninitialized in this function [-Wmaybe-uninitialized]
 1791 |   tx_bufs[i] = avp_bufs[count];
      |                ~~~~~~~~^~~~~~~
../drivers/net/avp/avp_ethdev.c:1791:24:
warning: ‘avp_bufs[count]’ may be used uninitialized in this function [-Wmaybe-uninitialized]

Fix by intializing the array.

Fixes: 295abce2d25b ("net/avp: add packet transmit functions")
Cc: stable@dpdk.org

Signed-off-by: Kevin Traynor <ktraynor@redhat.com>
---
v2: no change

note, commit log violates line length but I didn't want to split warning msg.

Cc: allain.legacy@windriver.com
Cc: Steven Webster <steven.webster@windriver.com>
Cc: Matt Peters <matt.peters@windriver.com>
---
 drivers/net/avp/avp_ethdev.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/avp/avp_ethdev.c b/drivers/net/avp/avp_ethdev.c
index cd747b6be..1abe96ce5 100644
--- a/drivers/net/avp/avp_ethdev.c
+++ b/drivers/net/avp/avp_ethdev.c
@@ -1695,5 +1695,5 @@ avp_xmit_scattered_pkts(void *tx_queue,
 {
 	struct rte_avp_desc *avp_bufs[(AVP_MAX_TX_BURST *
-				       RTE_AVP_MAX_MBUF_SEGMENTS)];
+				       RTE_AVP_MAX_MBUF_SEGMENTS)] = {};
 	struct avp_queue *txq = (struct avp_queue *)tx_queue;
 	struct rte_avp_desc *tx_bufs[AVP_MAX_TX_BURST];
-- 
2.21.1

[dpdk-dev] [PATCH v2 2/2] examples/ipsec-gw: fix gcc 10 maybe-uninitialized warning

[Kevin Traynor]

gcc 10.0.1 reports:

../examples/ipsec-secgw/ipsec_process.c: In function ‘ipsec_process’:
../examples/ipsec-secgw/ipsec_process.c:132:34:
error: ‘grp.m’ may be used uninitialized in this function [-Werror=maybe-uninitialized]
  132 |    grp[n].cnt = pkts + i - grp[n].m;
      |                            ~~~~~~^~

This is a correct warning for the initial execution of the statement.
However, it is the design of the loop that grp[0].cnt will later be
written with the correct value using an initialized grp[0].m before it
is used.

In order to remove the warning, initialize grp[0].m for the initial and
unused calculation of grp[0].cnt.

Fixes: 3e5f4625dc17 ("examples/ipsec-secgw: make data-path to use IPsec library")
Cc: stable@dpdk.org

Suggested-by: Konstantin Ananyev <konstantin.ananyev@intel.com>
Signed-off-by: Kevin Traynor <ktraynor@redhat.com>
---
v2: just initialize grp[0].m instead of the full array.

note, commit log violates line length but I didn't want to split warning msg.

Cc: konstantin.ananyev@intel.com
Cc: Radu Nicolau <radu.nicolau@intel.com>
Cc: Akhil Goyal <akhil.goyal@nxp.com>
---
 examples/ipsec-secgw/ipsec_process.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/examples/ipsec-secgw/ipsec_process.c b/examples/ipsec-secgw/ipsec_process.c
index bb2f2b82d..6d3a3c9a1 100644
--- a/examples/ipsec-secgw/ipsec_process.c
+++ b/examples/ipsec-secgw/ipsec_process.c
@@ -127,4 +127,5 @@ sa_group(void *sa_ptr[], struct rte_mbuf *pkts[],
 
 	sa = nosa;
+	grp[0].m = pkts;
 	for (i = 0, n = 0; i != num; i++) {
 
-- 
2.21.1

Re: [dpdk-dev] [PATCH v2 2/2] examples/ipsec-gw: fix gcc 10 maybe-uninitialized warning

[Ananyev, Konstantin]

> -----Original Message-----
> From: Kevin Traynor <ktraynor@redhat.com>
> Sent: Wednesday, March 11, 2020 11:33 AM
> To: dev@dpdk.org
> Cc: Kevin Traynor <ktraynor@redhat.com>; stable@dpdk.org; Ananyev, Konstantin <konstantin.ananyev@intel.com>; Nicolau, Radu
> <radu.nicolau@intel.com>; Akhil Goyal <akhil.goyal@nxp.com>
> Subject: [PATCH v2 2/2] examples/ipsec-gw: fix gcc 10 maybe-uninitialized warning
> 
> gcc 10.0.1 reports:
> 
> ../examples/ipsec-secgw/ipsec_process.c: In function ‘ipsec_process’:
> ../examples/ipsec-secgw/ipsec_process.c:132:34:
> error: ‘grp.m’ may be used uninitialized in this function [-Werror=maybe-uninitialized]
>   132 |    grp[n].cnt = pkts + i - grp[n].m;
>       |                            ~~~~~~^~
> 
> This is a correct warning for the initial execution of the statement.
> However, it is the design of the loop that grp[0].cnt will later be
> written with the correct value using an initialized grp[0].m before it
> is used.
> 
> In order to remove the warning, initialize grp[0].m for the initial and
> unused calculation of grp[0].cnt.
> 
> Fixes: 3e5f4625dc17 ("examples/ipsec-secgw: make data-path to use IPsec library")
> Cc: stable@dpdk.org
> 
> Suggested-by: Konstantin Ananyev <konstantin.ananyev@intel.com>
> Signed-off-by: Kevin Traynor <ktraynor@redhat.com>
> ---
> v2: just initialize grp[0].m instead of the full array.
> 
> note, commit log violates line length but I didn't want to split warning msg.
> 
> Cc: konstantin.ananyev@intel.com
> Cc: Radu Nicolau <radu.nicolau@intel.com>
> Cc: Akhil Goyal <akhil.goyal@nxp.com>
> ---
>  examples/ipsec-secgw/ipsec_process.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/examples/ipsec-secgw/ipsec_process.c b/examples/ipsec-secgw/ipsec_process.c
> index bb2f2b82d..6d3a3c9a1 100644
> --- a/examples/ipsec-secgw/ipsec_process.c
> +++ b/examples/ipsec-secgw/ipsec_process.c
> @@ -127,4 +127,5 @@ sa_group(void *sa_ptr[], struct rte_mbuf *pkts[],
> 
>  	sa = nosa;
> +	grp[0].m = pkts;
>  	for (i = 0, n = 0; i != num; i++) {
> 
> --

Acked-by: Konstantin Ananyev <konstantin.ananyev@intel.com>

> 2.21.1

Re: [dpdk-dev] [PATCH v2 2/2] examples/ipsec-gw: fix gcc 10 maybe-uninitialized warning

[David Marchand]

On Wed, Mar 11, 2020 at 1:05 PM Ananyev, Konstantin
<konstantin.ananyev@intel.com> wrote:
> > From: Kevin Traynor <ktraynor@redhat.com>
> > Sent: Wednesday, March 11, 2020 11:33 AM
> > To: dev@dpdk.org
> > Cc: Kevin Traynor <ktraynor@redhat.com>; stable@dpdk.org; Ananyev, Konstantin <konstantin.ananyev@intel.com>; Nicolau, Radu
> > <radu.nicolau@intel.com>; Akhil Goyal <akhil.goyal@nxp.com>
> > Subject: [PATCH v2 2/2] examples/ipsec-gw: fix gcc 10 maybe-uninitialized warning
> >
> > gcc 10.0.1 reports:
> >
> > ../examples/ipsec-secgw/ipsec_process.c: In function ‘ipsec_process’:
> > ../examples/ipsec-secgw/ipsec_process.c:132:34:
> > error: ‘grp.m’ may be used uninitialized in this function [-Werror=maybe-uninitialized]
> >   132 |    grp[n].cnt = pkts + i - grp[n].m;
> >       |                            ~~~~~~^~
> >
> > This is a correct warning for the initial execution of the statement.
> > However, it is the design of the loop that grp[0].cnt will later be
> > written with the correct value using an initialized grp[0].m before it
> > is used.
> >
> > In order to remove the warning, initialize grp[0].m for the initial and
> > unused calculation of grp[0].cnt.
> >
> > Fixes: 3e5f4625dc17 ("examples/ipsec-secgw: make data-path to use IPsec library")
> > Cc: stable@dpdk.org
> >
> > Suggested-by: Konstantin Ananyev <konstantin.ananyev@intel.com>
> > Signed-off-by: Kevin Traynor <ktraynor@redhat.com>
> Acked-by: Konstantin Ananyev <konstantin.ananyev@intel.com>

Applied, thanks.

-- 
David Marchand

Re: [dpdk-dev] [PATCH v2 1/2] net/avp: fix gcc 10 maybe-uninitialized warning

[Webster, Steven]

> gcc 10.0.1 reports:
> 
> ../drivers/net/avp/avp_ethdev.c: In function ‘avp_xmit_scattered_pkts’:
> ../drivers/net/avp/avp_ethdev.c:1791:24:
> warning: ‘avp_bufs[count]’ may be used uninitialized in this function [-
> Wmaybe-uninitialized]
>  1791 |   tx_bufs[i] = avp_bufs[count];
>       |                ~~~~~~~~^~~~~~~
> ../drivers/net/avp/avp_ethdev.c:1791:24:
> warning: ‘avp_bufs[count]’ may be used uninitialized in this function [-
> Wmaybe-uninitialized]
> 
> Fix by intializing the array.
> 
> Fixes: 295abce2d25b ("net/avp: add packet transmit functions")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Kevin Traynor <ktraynor@redhat.com>
> ---
Acked-by: Steven Webster <steven.webster@windriver.com>

Re: [dpdk-dev] [dpdk-stable] [PATCH v2 1/2] net/avp: fix gcc 10 maybe-uninitialized warning

[David Marchand]

On Thu, Mar 12, 2020 at 12:56 AM Webster, Steven
<Steven.Webster@windriver.com> wrote:
>
> > gcc 10.0.1 reports:
> >
> > ../drivers/net/avp/avp_ethdev.c: In function ‘avp_xmit_scattered_pkts’:
> > ../drivers/net/avp/avp_ethdev.c:1791:24:
> > warning: ‘avp_bufs[count]’ may be used uninitialized in this function [-
> > Wmaybe-uninitialized]
> >  1791 |   tx_bufs[i] = avp_bufs[count];
> >       |                ~~~~~~~~^~~~~~~
> > ../drivers/net/avp/avp_ethdev.c:1791:24:
> > warning: ‘avp_bufs[count]’ may be used uninitialized in this function [-
> > Wmaybe-uninitialized]
> >
> > Fix by intializing the array.
> >
> > Fixes: 295abce2d25b ("net/avp: add packet transmit functions")
> > Cc: stable@dpdk.org
> >
> > Signed-off-by: Kevin Traynor <ktraynor@redhat.com>
> > ---
> Acked-by: Steven Webster <steven.webster@windriver.com>

Applied, thanks.


-- 
David Marchand

Re: [dpdk-dev] [PATCH v2 1/2] net/avp: fix gcc 10 maybe-uninitialized warning

[Ferruh Yigit]

On 3/11/2020 11:32 AM, Kevin Traynor wrote:
> gcc 10.0.1 reports:
> 
> ../drivers/net/avp/avp_ethdev.c: In function ‘avp_xmit_scattered_pkts’:
> ../drivers/net/avp/avp_ethdev.c:1791:24:
> warning: ‘avp_bufs[count]’ may be used uninitialized in this function [-Wmaybe-uninitialized]
>  1791 |   tx_bufs[i] = avp_bufs[count];
>       |                ~~~~~~~~^~~~~~~
> ../drivers/net/avp/avp_ethdev.c:1791:24:
> warning: ‘avp_bufs[count]’ may be used uninitialized in this function [-Wmaybe-uninitialized]
> 
> Fix by intializing the array.
> 
> Fixes: 295abce2d25b ("net/avp: add packet transmit functions")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Kevin Traynor <ktraynor@redhat.com>
> ---
> v2: no change
> 
> note, commit log violates line length but I didn't want to split warning msg.
> 
> Cc: allain.legacy@windriver.com
> Cc: Steven Webster <steven.webster@windriver.com>
> Cc: Matt Peters <matt.peters@windriver.com>
> ---
>  drivers/net/avp/avp_ethdev.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/net/avp/avp_ethdev.c b/drivers/net/avp/avp_ethdev.c
> index cd747b6be..1abe96ce5 100644
> --- a/drivers/net/avp/avp_ethdev.c
> +++ b/drivers/net/avp/avp_ethdev.c
> @@ -1695,5 +1695,5 @@ avp_xmit_scattered_pkts(void *tx_queue,
>  {
>  	struct rte_avp_desc *avp_bufs[(AVP_MAX_TX_BURST *
> -				       RTE_AVP_MAX_MBUF_SEGMENTS)];
> +				       RTE_AVP_MAX_MBUF_SEGMENTS)] = {};
>  	struct avp_queue *txq = (struct avp_queue *)tx_queue;
>  	struct rte_avp_desc *tx_bufs[AVP_MAX_TX_BURST];
> 

Isn't this a false positive, can there be any case 'avp_bufs[]' used
uninitialized? Or am I missing something.

If this is false positive, does it worth to report to issue to gcc?

Re: [dpdk-dev] [PATCH v2 1/2] net/avp: fix gcc 10 maybe-uninitialized warning

[Kevin Traynor]

On 12/03/2020 13:25, Ferruh Yigit wrote:
> On 3/11/2020 11:32 AM, Kevin Traynor wrote:
>> gcc 10.0.1 reports:
>>
>> ../drivers/net/avp/avp_ethdev.c: In function ‘avp_xmit_scattered_pkts’:
>> ../drivers/net/avp/avp_ethdev.c:1791:24:
>> warning: ‘avp_bufs[count]’ may be used uninitialized in this function [-Wmaybe-uninitialized]
>>  1791 |   tx_bufs[i] = avp_bufs[count];
>>       |                ~~~~~~~~^~~~~~~
>> ../drivers/net/avp/avp_ethdev.c:1791:24:
>> warning: ‘avp_bufs[count]’ may be used uninitialized in this function [-Wmaybe-uninitialized]
>>
>> Fix by intializing the array.
>>
>> Fixes: 295abce2d25b ("net/avp: add packet transmit functions")
>> Cc: stable@dpdk.org
>>
>> Signed-off-by: Kevin Traynor <ktraynor@redhat.com>
>> ---
>> v2: no change
>>
>> note, commit log violates line length but I didn't want to split warning msg.
>>
>> Cc: allain.legacy@windriver.com
>> Cc: Steven Webster <steven.webster@windriver.com>
>> Cc: Matt Peters <matt.peters@windriver.com>
>> ---
>>  drivers/net/avp/avp_ethdev.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/net/avp/avp_ethdev.c b/drivers/net/avp/avp_ethdev.c
>> index cd747b6be..1abe96ce5 100644
>> --- a/drivers/net/avp/avp_ethdev.c
>> +++ b/drivers/net/avp/avp_ethdev.c
>> @@ -1695,5 +1695,5 @@ avp_xmit_scattered_pkts(void *tx_queue,
>>  {
>>  	struct rte_avp_desc *avp_bufs[(AVP_MAX_TX_BURST *
>> -				       RTE_AVP_MAX_MBUF_SEGMENTS)];
>> +				       RTE_AVP_MAX_MBUF_SEGMENTS)] = {};
>>  	struct avp_queue *txq = (struct avp_queue *)tx_queue;
>>  	struct rte_avp_desc *tx_bufs[AVP_MAX_TX_BURST];
>>
> 
> Isn't this a false positive, can there be any case 'avp_bufs[]' used
> uninitialized? Or am I missing something.
> 

I presume it's because it's not being initialized in the fn and there is
some paths in fn's it's passed to that don't initialize it. Of course in
practice with "normal" values this might not happen.

> If this is false positive, does it worth to report to issue to gcc?
> 

Re: [dpdk-dev] [PATCH v2 1/2] net/avp: fix gcc 10 maybe-uninitialized warning

[Ferruh Yigit]

On 3/12/2020 2:18 PM, Kevin Traynor wrote:
> On 12/03/2020 13:25, Ferruh Yigit wrote:
>> On 3/11/2020 11:32 AM, Kevin Traynor wrote:
>>> gcc 10.0.1 reports:
>>>
>>> ../drivers/net/avp/avp_ethdev.c: In function ‘avp_xmit_scattered_pkts’:
>>> ../drivers/net/avp/avp_ethdev.c:1791:24:
>>> warning: ‘avp_bufs[count]’ may be used uninitialized in this function [-Wmaybe-uninitialized]
>>>  1791 |   tx_bufs[i] = avp_bufs[count];
>>>       |                ~~~~~~~~^~~~~~~
>>> ../drivers/net/avp/avp_ethdev.c:1791:24:
>>> warning: ‘avp_bufs[count]’ may be used uninitialized in this function [-Wmaybe-uninitialized]
>>>
>>> Fix by intializing the array.
>>>
>>> Fixes: 295abce2d25b ("net/avp: add packet transmit functions")
>>> Cc: stable@dpdk.org
>>>
>>> Signed-off-by: Kevin Traynor <ktraynor@redhat.com>
>>> ---
>>> v2: no change
>>>
>>> note, commit log violates line length but I didn't want to split warning msg.
>>>
>>> Cc: allain.legacy@windriver.com
>>> Cc: Steven Webster <steven.webster@windriver.com>
>>> Cc: Matt Peters <matt.peters@windriver.com>
>>> ---
>>>  drivers/net/avp/avp_ethdev.c | 2 +-
>>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/net/avp/avp_ethdev.c b/drivers/net/avp/avp_ethdev.c
>>> index cd747b6be..1abe96ce5 100644
>>> --- a/drivers/net/avp/avp_ethdev.c
>>> +++ b/drivers/net/avp/avp_ethdev.c
>>> @@ -1695,5 +1695,5 @@ avp_xmit_scattered_pkts(void *tx_queue,
>>>  {
>>>  	struct rte_avp_desc *avp_bufs[(AVP_MAX_TX_BURST *
>>> -				       RTE_AVP_MAX_MBUF_SEGMENTS)];
>>> +				       RTE_AVP_MAX_MBUF_SEGMENTS)] = {};
>>>  	struct avp_queue *txq = (struct avp_queue *)tx_queue;
>>>  	struct rte_avp_desc *tx_bufs[AVP_MAX_TX_BURST];
>>>
>>
>> Isn't this a false positive, can there be any case 'avp_bufs[]' used
>> uninitialized? Or am I missing something.
>>
> 
> I presume it's because it's not being initialized in the fn and there is
> some paths in fn's it's passed to that don't initialize it. Of course in
> practice with "normal" values this might not happen.

'avp_fifo_get(alloc_q, (void **)&avp_bufs, segments);' initializes it, and I am
not just talking about 'normal' case, I don't see any case that 'avp_bufs[]'
used uninitialized, can you see any?

> 
>> If this is false positive, does it worth to report to issue to gcc?
>>
> 

Re: [dpdk-dev] [PATCH v2 1/2] net/avp: fix gcc 10 maybe-uninitialized warning

[Kevin Traynor]

On 12/03/2020 14:31, Ferruh Yigit wrote:
> On 3/12/2020 2:18 PM, Kevin Traynor wrote:
>> On 12/03/2020 13:25, Ferruh Yigit wrote:
>>> On 3/11/2020 11:32 AM, Kevin Traynor wrote:
>>>> gcc 10.0.1 reports:
>>>>
>>>> ../drivers/net/avp/avp_ethdev.c: In function ‘avp_xmit_scattered_pkts’:
>>>> ../drivers/net/avp/avp_ethdev.c:1791:24:
>>>> warning: ‘avp_bufs[count]’ may be used uninitialized in this function [-Wmaybe-uninitialized]
>>>>  1791 |   tx_bufs[i] = avp_bufs[count];
>>>>       |                ~~~~~~~~^~~~~~~
>>>> ../drivers/net/avp/avp_ethdev.c:1791:24:
>>>> warning: ‘avp_bufs[count]’ may be used uninitialized in this function [-Wmaybe-uninitialized]
>>>>
>>>> Fix by intializing the array.
>>>>
>>>> Fixes: 295abce2d25b ("net/avp: add packet transmit functions")
>>>> Cc: stable@dpdk.org
>>>>
>>>> Signed-off-by: Kevin Traynor <ktraynor@redhat.com>
>>>> ---
>>>> v2: no change
>>>>
>>>> note, commit log violates line length but I didn't want to split warning msg.
>>>>
>>>> Cc: allain.legacy@windriver.com
>>>> Cc: Steven Webster <steven.webster@windriver.com>
>>>> Cc: Matt Peters <matt.peters@windriver.com>
>>>> ---
>>>>  drivers/net/avp/avp_ethdev.c | 2 +-
>>>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>>>
>>>> diff --git a/drivers/net/avp/avp_ethdev.c b/drivers/net/avp/avp_ethdev.c
>>>> index cd747b6be..1abe96ce5 100644
>>>> --- a/drivers/net/avp/avp_ethdev.c
>>>> +++ b/drivers/net/avp/avp_ethdev.c
>>>> @@ -1695,5 +1695,5 @@ avp_xmit_scattered_pkts(void *tx_queue,
>>>>  {
>>>>  	struct rte_avp_desc *avp_bufs[(AVP_MAX_TX_BURST *
>>>> -				       RTE_AVP_MAX_MBUF_SEGMENTS)];
>>>> +				       RTE_AVP_MAX_MBUF_SEGMENTS)] = {};
>>>>  	struct avp_queue *txq = (struct avp_queue *)tx_queue;
>>>>  	struct rte_avp_desc *tx_bufs[AVP_MAX_TX_BURST];
>>>>
>>>
>>> Isn't this a false positive, can there be any case 'avp_bufs[]' used
>>> uninitialized? Or am I missing something.
>>>
>>
>> I presume it's because it's not being initialized in the fn and there is
>> some paths in fn's it's passed to that don't initialize it. Of course in
>> practice with "normal" values this might not happen.
> 
> 'avp_fifo_get(alloc_q, (void **)&avp_bufs, segments);' initializes it, and I am
> not just talking about 'normal' case, I don't see any case that 'avp_bufs[]'
> used uninitialized, can you see any?
> 

Well, it's initialization there is dependent on not hitting the first
return and the loop executing.

>>
>>> If this is false positive, does it worth to report to issue to gcc?
>>>
>>
> 

Re: [dpdk-dev] [PATCH v2 1/2] net/avp: fix gcc 10 maybe-uninitialized warning

[Ferruh Yigit]

On 3/12/2020 3:15 PM, Kevin Traynor wrote:
> On 12/03/2020 14:31, Ferruh Yigit wrote:
>> On 3/12/2020 2:18 PM, Kevin Traynor wrote:
>>> On 12/03/2020 13:25, Ferruh Yigit wrote:
>>>> On 3/11/2020 11:32 AM, Kevin Traynor wrote:
>>>>> gcc 10.0.1 reports:
>>>>>
>>>>> ../drivers/net/avp/avp_ethdev.c: In function ‘avp_xmit_scattered_pkts’:
>>>>> ../drivers/net/avp/avp_ethdev.c:1791:24:
>>>>> warning: ‘avp_bufs[count]’ may be used uninitialized in this function [-Wmaybe-uninitialized]
>>>>>  1791 |   tx_bufs[i] = avp_bufs[count];
>>>>>       |                ~~~~~~~~^~~~~~~
>>>>> ../drivers/net/avp/avp_ethdev.c:1791:24:
>>>>> warning: ‘avp_bufs[count]’ may be used uninitialized in this function [-Wmaybe-uninitialized]
>>>>>
>>>>> Fix by intializing the array.
>>>>>
>>>>> Fixes: 295abce2d25b ("net/avp: add packet transmit functions")
>>>>> Cc: stable@dpdk.org
>>>>>
>>>>> Signed-off-by: Kevin Traynor <ktraynor@redhat.com>
>>>>> ---
>>>>> v2: no change
>>>>>
>>>>> note, commit log violates line length but I didn't want to split warning msg.
>>>>>
>>>>> Cc: allain.legacy@windriver.com
>>>>> Cc: Steven Webster <steven.webster@windriver.com>
>>>>> Cc: Matt Peters <matt.peters@windriver.com>
>>>>> ---
>>>>>  drivers/net/avp/avp_ethdev.c | 2 +-
>>>>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>>>>
>>>>> diff --git a/drivers/net/avp/avp_ethdev.c b/drivers/net/avp/avp_ethdev.c
>>>>> index cd747b6be..1abe96ce5 100644
>>>>> --- a/drivers/net/avp/avp_ethdev.c
>>>>> +++ b/drivers/net/avp/avp_ethdev.c
>>>>> @@ -1695,5 +1695,5 @@ avp_xmit_scattered_pkts(void *tx_queue,
>>>>>  {
>>>>>  	struct rte_avp_desc *avp_bufs[(AVP_MAX_TX_BURST *
>>>>> -				       RTE_AVP_MAX_MBUF_SEGMENTS)];
>>>>> +				       RTE_AVP_MAX_MBUF_SEGMENTS)] = {};
>>>>>  	struct avp_queue *txq = (struct avp_queue *)tx_queue;
>>>>>  	struct rte_avp_desc *tx_bufs[AVP_MAX_TX_BURST];
>>>>>
>>>>
>>>> Isn't this a false positive, can there be any case 'avp_bufs[]' used
>>>> uninitialized? Or am I missing something.
>>>>
>>>
>>> I presume it's because it's not being initialized in the fn and there is
>>> some paths in fn's it's passed to that don't initialize it. Of course in
>>> practice with "normal" values this might not happen.
>>
>> 'avp_fifo_get(alloc_q, (void **)&avp_bufs, segments);' initializes it, and I am
>> not just talking about 'normal' case, I don't see any case that 'avp_bufs[]'
>> used uninitialized, can you see any?
>>
> 
> Well, it's initialization there is dependent on not hitting the first
> return and the loop executing.

If whole array not initialized, the next line, 'if (unlikely(n != segments))',
will catch it and function return without using 'avp_bufs[]' at all.

Anyway, as I said I can't see a case that 'avp_bufs[]' used uninitialized,
and not sure about additional zeroing out in datapath function if this is a
false positive, but if windriver guys are OK I won't object.

> 
>>>
>>>> If this is false positive, does it worth to report to issue to gcc?
>>>>
>>>
>>
>