[dpdk-dev] [PATCH v1] examples/ipsec-secgw: resolve coverity issue

[dpdk-dev] [PATCH v1] examples/ipsec-secgw: resolve coverity issue

[Praveen Shetty]

Function create_ipsec_esp_flow returns a negative number in case of any
failure. But passing negative number to strerror is causing the coverity
issue.
In case of failure, displaying exact error message to console is handled
in create_ipsec_esp_flow function.So it is not required to print the
error message again using strerror.
This patch will remove the unnecessary calling of strerror function
to fix the coverity issue.

Coverity issue: 357691
Fixes: 6738c0a95695 ("examples/ipsec-secgw: support flow director")
Cc: praveen.shetty@intel.com

Signed-off-by: Praveen Shetty <praveen.shetty@intel.com>
---
 examples/ipsec-secgw/sa.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/examples/ipsec-secgw/sa.c b/examples/ipsec-secgw/sa.c
index e3a1a5aff..632482176 100644
--- a/examples/ipsec-secgw/sa.c
+++ b/examples/ipsec-secgw/sa.c
@@ -1223,8 +1223,7 @@ sa_add_rules(struct sa_ctx *sa_ctx, const struct ipsec_sa entries[],
 			rc = create_ipsec_esp_flow(sa);
 			if (rc != 0)
 				RTE_LOG(ERR, IPSEC_ESP,
-					"create_ipsec_esp_flow() failed %s\n",
-					strerror(rc));
+					"create_ipsec_esp_flow() failed\n");
 		}
 		print_one_sa_rule(sa, inbound);
 	}
-- 
2.17.1

Re: [dpdk-dev] [PATCH v1] examples/ipsec-secgw: resolve coverity issue

[Lukasz Wojciechowski]

W dniu 06.05.2020 o 11:02, Praveen Shetty pisze:
> Function create_ipsec_esp_flow returns a negative number in case of any
> failure. But passing negative number to strerror is causing the coverity
> issue.
> In case of failure, displaying exact error message to console is handled
> in create_ipsec_esp_flow function.So it is not required to print the
> error message again using strerror.
> This patch will remove the unnecessary calling of strerror function
> to fix the coverity issue.
>
> Coverity issue: 357691
> Fixes: 6738c0a95695 ("examples/ipsec-secgw: support flow director")
> Cc: praveen.shetty@intel.com
>
> Signed-off-by: Praveen Shetty <praveen.shetty@intel.com>
> ---
>   examples/ipsec-secgw/sa.c | 3 +--
>   1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/examples/ipsec-secgw/sa.c b/examples/ipsec-secgw/sa.c
> index e3a1a5aff..632482176 100644
> --- a/examples/ipsec-secgw/sa.c
> +++ b/examples/ipsec-secgw/sa.c
> @@ -1223,8 +1223,7 @@ sa_add_rules(struct sa_ctx *sa_ctx, const struct ipsec_sa entries[],
>   			rc = create_ipsec_esp_flow(sa);
>   			if (rc != 0)
>   				RTE_LOG(ERR, IPSEC_ESP,
> -					"create_ipsec_esp_flow() failed %s\n",
> -					strerror(rc));
> +					"create_ipsec_esp_flow() failed\n");
>   		}
>   		print_one_sa_rule(sa, inbound);
>   	}

great!, especially that create_ipsec_esp_flow returns mostly -1 in case 
of errors and that would be -EPERM ;)

Acked-by: Lukasz Wojciechowski <l.wojciechow@partner.samsung.com>


-- 

Lukasz Wojciechowski
Principal Software Engineer

Samsung R&D Institute Poland
Samsung Electronics
Office +48 22 377 88 25
l.wojciechow@partner.samsung.com

Re: [dpdk-dev] [EXT] [PATCH v1] examples/ipsec-secgw: resolve coverity issue

[Anoob Joseph]

Hi Praveen,

Please see inline.

Thanks,
Anoob

> -----Original Message-----
> From: Praveen Shetty <praveen.shetty@intel.com>
> Sent: Wednesday, May 6, 2020 2:33 PM
> To: dev@dpdk.org; declan.doherty@intel.com; akhil.goyal@nxp.com; Anoob
> Joseph <anoobj@marvell.com>
> Cc: bernard.iremonger@intel.com; konstantin.ananyev@intel.com;
> praveen.shetty@intel.com
> Subject: [EXT] [PATCH v1] examples/ipsec-secgw: resolve coverity issue

[Anoob] I believe Thomas insists on explaining the issue that you are trying to fix rather than stating "resolve coverity issue" (which is too generic). So I guess you can remove all such references to coverity and retain rest of your description.
 
> 
> External Email
> 
> ----------------------------------------------------------------------
> Function create_ipsec_esp_flow returns a negative number in case of any
> failure. But passing negative number to strerror is causing the coverity issue.
> In case of failure, displaying exact error message to console is handled in
> create_ipsec_esp_flow function.So it is not required to print the error message
> again using strerror.
> This patch will remove the unnecessary calling of strerror function to fix the
> coverity issue.
> 
> Coverity issue: 357691
> Fixes: 6738c0a95695 ("examples/ipsec-secgw: support flow director")
> Cc: praveen.shetty@intel.com
> 
> Signed-off-by: Praveen Shetty <praveen.shetty@intel.com>
> ---
>  examples/ipsec-secgw/sa.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/examples/ipsec-secgw/sa.c b/examples/ipsec-secgw/sa.c index
> e3a1a5aff..632482176 100644
> --- a/examples/ipsec-secgw/sa.c
> +++ b/examples/ipsec-secgw/sa.c
> @@ -1223,8 +1223,7 @@ sa_add_rules(struct sa_ctx *sa_ctx, const struct
> ipsec_sa entries[],
>  			rc = create_ipsec_esp_flow(sa);
>  			if (rc != 0)
>  				RTE_LOG(ERR, IPSEC_ESP,
> -					"create_ipsec_esp_flow() failed %s\n",
> -					strerror(rc));
> +					"create_ipsec_esp_flow() failed\n");
>  		}
>  		print_one_sa_rule(sa, inbound);
>  	}
> --
> 2.17.1

[dpdk-dev] [PATCH v2] examples/ipsec-secgw: fix negative argument passing

[Praveen Shetty]

Function create_ipsec_esp_flow returns a negative number in case of any
failure and we are passing this to strerror to dsiplay the error message.
But strerror()'s argument cannot be negative.
In case of failure, displaying exact error message to console is handled
in create_ipsec_esp_flow function.So it is not required to print the
error message again using strerror.
This patch will remove the unnecessary calling of strerror function
to fix the negative argument passing to strerror issue.

Coverity issue: 357691
Fixes: 6738c0a95695 ("examples/ipsec-secgw: support flow director")
Cc: praveen.shetty@intel.com

Signed-off-by: Praveen Shetty <praveen.shetty@intel.com>
Acked-by: Lukasz Wojciechowski <l.wojciechow@partner.samsung.com>
---
v2 changes:
changed commit headline and rephrased commit message.

 examples/ipsec-secgw/sa.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/examples/ipsec-secgw/sa.c b/examples/ipsec-secgw/sa.c
index e3a1a5aff..632482176 100644
--- a/examples/ipsec-secgw/sa.c
+++ b/examples/ipsec-secgw/sa.c
@@ -1223,8 +1223,7 @@ sa_add_rules(struct sa_ctx *sa_ctx, const struct ipsec_sa entries[],
 			rc = create_ipsec_esp_flow(sa);
 			if (rc != 0)
 				RTE_LOG(ERR, IPSEC_ESP,
-					"create_ipsec_esp_flow() failed %s\n",
-					strerror(rc));
+					"create_ipsec_esp_flow() failed\n");
 		}
 		print_one_sa_rule(sa, inbound);
 	}
-- 
2.17.1

Re: [dpdk-dev] [EXT] [PATCH v1] examples/ipsec-secgw: resolve coverity issue

[Shetty, Praveen]

Hi Anoob,

Thank you.

I have modified the commit message and headline accordingly and sent v2.

Regards,
Praveen

-----Original Message-----
From: Anoob Joseph <anoobj@marvell.com> 
Sent: Wednesday, May 6, 2020 3:13 PM
To: Shetty, Praveen <praveen.shetty@intel.com>; dev@dpdk.org; Doherty, Declan <declan.doherty@intel.com>; akhil.goyal@nxp.com
Cc: Iremonger, Bernard <bernard.iremonger@intel.com>; Ananyev, Konstantin <konstantin.ananyev@intel.com>
Subject: RE: [EXT] [PATCH v1] examples/ipsec-secgw: resolve coverity issue

Hi Praveen,

Please see inline.

Thanks,
Anoob

> -----Original Message-----
> From: Praveen Shetty <praveen.shetty@intel.com>
> Sent: Wednesday, May 6, 2020 2:33 PM
> To: dev@dpdk.org; declan.doherty@intel.com; akhil.goyal@nxp.com; Anoob 
> Joseph <anoobj@marvell.com>
> Cc: bernard.iremonger@intel.com; konstantin.ananyev@intel.com; 
> praveen.shetty@intel.com
> Subject: [EXT] [PATCH v1] examples/ipsec-secgw: resolve coverity issue

[Anoob] I believe Thomas insists on explaining the issue that you are trying to fix rather than stating "resolve coverity issue" (which is too generic). So I guess you can remove all such references to coverity and retain rest of your description.
 
> 
> External Email
> 
> ----------------------------------------------------------------------
> Function create_ipsec_esp_flow returns a negative number in case of 
> any failure. But passing negative number to strerror is causing the coverity issue.
> In case of failure, displaying exact error message to console is 
> handled in create_ipsec_esp_flow function.So it is not required to 
> print the error message again using strerror.
> This patch will remove the unnecessary calling of strerror function to 
> fix the coverity issue.
> 
> Coverity issue: 357691
> Fixes: 6738c0a95695 ("examples/ipsec-secgw: support flow director")
> Cc: praveen.shetty@intel.com
> 
> Signed-off-by: Praveen Shetty <praveen.shetty@intel.com>
> ---
>  examples/ipsec-secgw/sa.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/examples/ipsec-secgw/sa.c b/examples/ipsec-secgw/sa.c 
> index
> e3a1a5aff..632482176 100644
> --- a/examples/ipsec-secgw/sa.c
> +++ b/examples/ipsec-secgw/sa.c
> @@ -1223,8 +1223,7 @@ sa_add_rules(struct sa_ctx *sa_ctx, const struct 
> ipsec_sa entries[],
>  			rc = create_ipsec_esp_flow(sa);
>  			if (rc != 0)
>  				RTE_LOG(ERR, IPSEC_ESP,
> -					"create_ipsec_esp_flow() failed %s\n",
> -					strerror(rc));
> +					"create_ipsec_esp_flow() failed\n");
>  		}
>  		print_one_sa_rule(sa, inbound);
>  	}
> --
> 2.17.1

Re: [dpdk-dev] [PATCH v2] examples/ipsec-secgw: fix negative argument passing

[Lukasz Wojciechowski]

W dniu 06.05.2020 o 12:37, Praveen Shetty pisze:
> Function create_ipsec_esp_flow returns a negative number in case of any
> failure and we are passing this to strerror to dsiplay the error message.
typo dsiplay -> display
> But strerror()'s argument cannot be negative.
> In case of failure, displaying exact error message to console is handled
> in create_ipsec_esp_flow function.So it is not required to print the
> error message again using strerror.
> This patch will remove the unnecessary calling of strerror function
> to fix the negative argument passing to strerror issue.
>
> Coverity issue: 357691
> Fixes: 6738c0a95695 ("examples/ipsec-secgw: support flow director")
> Cc: praveen.shetty@intel.com
>
> Signed-off-by: Praveen Shetty <praveen.shetty@intel.com>
> Acked-by: Lukasz Wojciechowski <l.wojciechow@partner.samsung.com>
> ---
> v2 changes:
> changed commit headline and rephrased commit message.
>
>   examples/ipsec-secgw/sa.c | 3 +--
>   1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/examples/ipsec-secgw/sa.c b/examples/ipsec-secgw/sa.c
> index e3a1a5aff..632482176 100644
> --- a/examples/ipsec-secgw/sa.c
> +++ b/examples/ipsec-secgw/sa.c
> @@ -1223,8 +1223,7 @@ sa_add_rules(struct sa_ctx *sa_ctx, const struct ipsec_sa entries[],
>   			rc = create_ipsec_esp_flow(sa);
>   			if (rc != 0)
>   				RTE_LOG(ERR, IPSEC_ESP,
> -					"create_ipsec_esp_flow() failed %s\n",
> -					strerror(rc));
> +					"create_ipsec_esp_flow() failed\n");
>   		}
>   		print_one_sa_rule(sa, inbound);
>   	}

-- 

Lukasz Wojciechowski
Principal Software Engineer

Samsung R&D Institute Poland
Samsung Electronics
Office +48 22 377 88 25
l.wojciechow@partner.samsung.com

[dpdk-dev] [PATCH v3] examples/ipsec-secgw: fix negative argument passing

[Praveen Shetty]

Function create_ipsec_esp_flow returns a negative number in case of any
failure and we are passing this to strerror to display the error message.
But strerror()'s argument cannot be negative.
In case of failure, displaying exact error message to console is handled
in create_ipsec_esp_flow function.So it is not required to print the
error message again using strerror.
This patch will remove the unnecessary calling of strerror function
to fix the negative argument passing to strerror issue.

Coverity issue: 357691
Fixes: 6738c0a95695 ("examples/ipsec-secgw: support flow director")
Cc: praveen.shetty@intel.com

Signed-off-by: Praveen Shetty <praveen.shetty@intel.com>
Acked-by: Lukasz Wojciechowski <l.wojciechow@partner.samsung.com>
---
v3 changes:
fixed typo in commit message.

 examples/ipsec-secgw/sa.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/examples/ipsec-secgw/sa.c b/examples/ipsec-secgw/sa.c
index e3a1a5aff..632482176 100644
--- a/examples/ipsec-secgw/sa.c
+++ b/examples/ipsec-secgw/sa.c
@@ -1223,8 +1223,7 @@ sa_add_rules(struct sa_ctx *sa_ctx, const struct ipsec_sa entries[],
 			rc = create_ipsec_esp_flow(sa);
 			if (rc != 0)
 				RTE_LOG(ERR, IPSEC_ESP,
-					"create_ipsec_esp_flow() failed %s\n",
-					strerror(rc));
+					"create_ipsec_esp_flow() failed\n");
 		}
 		print_one_sa_rule(sa, inbound);
 	}
-- 
2.17.1

Re: [dpdk-dev] [PATCH v2] examples/ipsec-secgw: fix negative argument passing

[Shetty, Praveen]

-----Original Message-----
From: Lukasz Wojciechowski <l.wojciechow@partner.samsung.com> 
Sent: Wednesday, May 6, 2020 4:20 PM
To: Shetty, Praveen <praveen.shetty@intel.com>; dev@dpdk.org; Doherty, Declan <declan.doherty@intel.com>; akhil.goyal@nxp.com; anoobj@marvell.com
Cc: Iremonger, Bernard <bernard.iremonger@intel.com>; Ananyev, Konstantin <konstantin.ananyev@intel.com>
Subject: Re: [dpdk-dev] [PATCH v2] examples/ipsec-secgw: fix negative argument passing


W dniu 06.05.2020 o 12:37, Praveen Shetty pisze:
> Function create_ipsec_esp_flow returns a negative number in case of 
> any failure and we are passing this to strerror to dsiplay the error message.
typo dsiplay -> display

[Praveen] Thanks, fixed in v3.

> But strerror()'s argument cannot be negative.
> In case of failure, displaying exact error message to console is 
> handled in create_ipsec_esp_flow function.So it is not required to 
> print the error message again using strerror.
> This patch will remove the unnecessary calling of strerror function to 
> fix the negative argument passing to strerror issue.
>
> Coverity issue: 357691
> Fixes: 6738c0a95695 ("examples/ipsec-secgw: support flow director")
> Cc: praveen.shetty@intel.com
>
> Signed-off-by: Praveen Shetty <praveen.shetty@intel.com>
> Acked-by: Lukasz Wojciechowski <l.wojciechow@partner.samsung.com>
> ---
> v2 changes:
> changed commit headline and rephrased commit message.
>
>   examples/ipsec-secgw/sa.c | 3 +--
>   1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/examples/ipsec-secgw/sa.c b/examples/ipsec-secgw/sa.c 
> index e3a1a5aff..632482176 100644
> --- a/examples/ipsec-secgw/sa.c
> +++ b/examples/ipsec-secgw/sa.c
> @@ -1223,8 +1223,7 @@ sa_add_rules(struct sa_ctx *sa_ctx, const struct ipsec_sa entries[],
>   			rc = create_ipsec_esp_flow(sa);
>   			if (rc != 0)
>   				RTE_LOG(ERR, IPSEC_ESP,
> -					"create_ipsec_esp_flow() failed %s\n",
> -					strerror(rc));
> +					"create_ipsec_esp_flow() failed\n");
>   		}
>   		print_one_sa_rule(sa, inbound);
>   	}

-- 

Lukasz Wojciechowski
Principal Software Engineer

Samsung R&D Institute Poland
Samsung Electronics
Office +48 22 377 88 25
l.wojciechow@partner.samsung.com

Re: [dpdk-dev] [PATCH v2] examples/ipsec-secgw: fix negative argument passing

[Lukasz Wojciechowski]

W dniu 06.05.2020 o 13:03, Shetty, Praveen pisze:
>
> -----Original Message-----
> From: Lukasz Wojciechowski <l.wojciechow@partner.samsung.com>
> Sent: Wednesday, May 6, 2020 4:20 PM
> To: Shetty, Praveen <praveen.shetty@intel.com>; dev@dpdk.org; Doherty, Declan <declan.doherty@intel.com>; akhil.goyal@nxp.com; anoobj@marvell.com
> Cc: Iremonger, Bernard <bernard.iremonger@intel.com>; Ananyev, Konstantin <konstantin.ananyev@intel.com>
> Subject: Re: [dpdk-dev] [PATCH v2] examples/ipsec-secgw: fix negative argument passing
>
>
> W dniu 06.05.2020 o 12:37, Praveen Shetty pisze:
>> Function create_ipsec_esp_flow returns a negative number in case of
>> any failure and we are passing this to strerror to dsiplay the error message.
> typo dsiplay -> display
>
> [Praveen] Thanks, fixed in v3.
>
>> But strerror()'s argument cannot be negative.
>> In case of failure, displaying exact error message to console is
>> handled in create_ipsec_esp_flow function.So it is not required to
>> print the error message again using strerror.
>> This patch will remove the unnecessary calling of strerror function to
>> fix the negative argument passing to strerror issue.
>>
>> Coverity issue: 357691
>> Fixes: 6738c0a95695 ("examples/ipsec-secgw: support flow director")
>> Cc: praveen.shetty@intel.com
>>
>> Signed-off-by: Praveen Shetty <praveen.shetty@intel.com>
>> Acked-by: Lukasz Wojciechowski <l.wojciechow@partner.samsung.com>
>> ---
>> v2 changes:
>> changed commit headline and rephrased commit message.
>>
>>    examples/ipsec-secgw/sa.c | 3 +--
>>    1 file changed, 1 insertion(+), 2 deletions(-)
>>
>> diff --git a/examples/ipsec-secgw/sa.c b/examples/ipsec-secgw/sa.c
>> index e3a1a5aff..632482176 100644
>> --- a/examples/ipsec-secgw/sa.c
>> +++ b/examples/ipsec-secgw/sa.c
>> @@ -1223,8 +1223,7 @@ sa_add_rules(struct sa_ctx *sa_ctx, const struct ipsec_sa entries[],
>>    			rc = create_ipsec_esp_flow(sa);
>>    			if (rc != 0)
>>    				RTE_LOG(ERR, IPSEC_ESP,
>> -					"create_ipsec_esp_flow() failed %s\n",
>> -					strerror(rc));
>> +					"create_ipsec_esp_flow() failed\n");
>>    		}
>>    		print_one_sa_rule(sa, inbound);
>>    	}
Acked-by: Lukasz Wojciechowski <l.wojciechow@partner.samsung.com>
>
-- 

Lukasz Wojciechowski
Principal Software Engineer

Samsung R&D Institute Poland
Samsung Electronics
Office +48 22 377 88 25
l.wojciechow@partner.samsung.com

Re: [dpdk-dev] [EXT] [PATCH v3] examples/ipsec-secgw: fix negative argument passing

[Anoob Joseph]

> Function create_ipsec_esp_flow returns a negative number in case of any failure
> and we are passing this to strerror to display the error message.
> But strerror()'s argument cannot be negative.
> In case of failure, displaying exact error message to console is handled in
> create_ipsec_esp_flow function.So it is not required to print the error message
> again using strerror.
> This patch will remove the unnecessary calling of strerror function to fix the
> negative argument passing to strerror issue.
> 
> Coverity issue: 357691
> Fixes: 6738c0a95695 ("examples/ipsec-secgw: support flow director")
> Cc: praveen.shetty@intel.com
> 
> Signed-off-by: Praveen Shetty <praveen.shetty@intel.com>
> Acked-by: Lukasz Wojciechowski <l.wojciechow@partner.samsung.com>

Acked-by: Anoob Joseph <anoobj@marvell.com>

Re: [dpdk-dev] [EXT] [PATCH v3] examples/ipsec-secgw: fix negative argument passing

[Akhil Goyal]

> > Function create_ipsec_esp_flow returns a negative number in case of any
> failure
> > and we are passing this to strerror to display the error message.
> > But strerror()'s argument cannot be negative.
> > In case of failure, displaying exact error message to console is handled in
> > create_ipsec_esp_flow function.So it is not required to print the error message
> > again using strerror.
> > This patch will remove the unnecessary calling of strerror function to fix the
> > negative argument passing to strerror issue.
> >
> > Coverity issue: 357691
> > Fixes: 6738c0a95695 ("examples/ipsec-secgw: support flow director")
> > Cc: praveen.shetty@intel.com
> >
> > Signed-off-by: Praveen Shetty <praveen.shetty@intel.com>
> > Acked-by: Lukasz Wojciechowski <l.wojciechow@partner.samsung.com>
> 
> Acked-by: Anoob Joseph <anoobj@marvell.com>

Applied to dpdk-next-crypto

Thanks.