diff mbox series

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

Message ID 20200220093744.13925-2-ktraynor@redhat.com (mailing list archive)
State Superseded, archived
Delegated to: Thomas Monjalon
Headers
Series [1/2] net/avp: fix gcc 10 maybe-uninitialized warning |

Checks

Context Check Description
ci/checkpatch warning coding style issues
ci/Intel-compilation success Compilation OK
ci/travis-robot success Travis build: passed

Commit Message

Kevin Traynor Feb. 20, 2020, 9:37 a.m. UTC 
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(-)

Comments

Akhil Goyal Feb. 26, 2020, 6:12 a.m. UTC | #1 
> 
> 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>
Ananyev, Konstantin March 10, 2020, 1:08 p.m. UTC | #2 
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]) {
Kevin Traynor March 10, 2020, 6:52 p.m. UTC | #3 
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]) {
>
Ananyev, Konstantin March 10, 2020, 7:03 p.m. UTC | #4 
> >> 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]) {
> >
diff mbox series

Patch

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