diff mbox series

app/testpmd: remove useless pointer checks

Message ID 20220324161503.13047-1-david.marchand@redhat.com (mailing list archive)
State Accepted, archived
Delegated to: Ferruh Yigit
Headers
Series app/testpmd: remove useless pointer checks |

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/iol-mellanox-Performance success Performance Testing PASS
ci/iol-intel-Performance success Performance Testing PASS
ci/iol-intel-Functional success Functional Testing PASS
ci/iol-aarch64-unit-testing success Testing PASS
ci/github-robot: build success github build: passed
ci/iol-x86_64-compile-testing success Testing PASS
ci/iol-aarch64-compile-testing success Testing PASS
ci/iol-x86_64-unit-testing success Testing PASS
ci/iol-abi-testing success Testing PASS
ci/iol-broadcom-Functional fail Functional Testing issues
ci/Intel-compilation success Compilation OK
ci/intel-Testing success Testing PASS

Commit Message

David Marchand March 24, 2022, 4:15 p.m. UTC 
  Parameters to this static helper can't be NULL.
str has already been dereferenced in caller.
dst and size point to variable in stack.

Fixes: 169a9fed1f4c ("app/testpmd: fix hex string parser support for flow API")
Cc: stable@dpdk.org

Signed-off-by: David Marchand <david.marchand@redhat.com>
---
 app/test-pmd/cmdline_flow.c | 6 +-----
 1 file changed, 1 insertion(+), 5 deletions(-)

Comments

Thomas Monjalon March 29, 2022, 9:26 a.m. UTC | #1 
24/03/2022 17:15, David Marchand:
> Parameters to this static helper can't be NULL.
> str has already been dereferenced in caller.
> dst and size point to variable in stack.

The same function is copy/pasted in several places.
Why simplifying only this one? because of its static nature?

Shouldn't we make it a common function as other string helpers?
David Marchand May 12, 2022, 7:19 a.m. UTC | #2 
On Tue, Mar 29, 2022 at 11:26 AM Thomas Monjalon <thomas@monjalon.net> wrote:
>
> 24/03/2022 17:15, David Marchand:
> > Parameters to this static helper can't be NULL.
> > str has already been dereferenced in caller.
> > dst and size point to variable in stack.
>
> The same function is copy/pasted in several places.
> Why simplifying only this one? because of its static nature?
>
> Shouldn't we make it a common function as other string helpers?

Sorry, this thread fell through the cracks.

The issue was raised by covscan:

Error: REVERSE_INULL (CWE-476):
dpdk-21.11/app/test-pmd/cmdline_flow.c:7705: deref_ptr: Directly
dereferencing pointer "size".
dpdk-21.11/app/test-pmd/cmdline_flow.c:7711: check_after_deref:
Null-checking "size" suggests that it may be null, but it has already
been dereferenced on all paths leading to the check.
# 7709|       if ((src == NULL) ||
# 7710|           (dst == NULL) ||
# 7711|->         (size == NULL) ||
# 7712|           (*size == 0))
# 7713|           return -1;


As for the rest of the code, there might be more cleanups to do, as followups.
Ferruh Yigit May 20, 2022, 3:04 p.m. UTC | #3 
On 5/12/2022 8:19 AM, David Marchand wrote:
> On Tue, Mar 29, 2022 at 11:26 AM Thomas Monjalon <thomas@monjalon.net> wrote:
>>
>> 24/03/2022 17:15, David Marchand:
>>> Parameters to this static helper can't be NULL.
>>> str has already been dereferenced in caller.
>>> dst and size point to variable in stack.
>>
>> The same function is copy/pasted in several places.
>> Why simplifying only this one? because of its static nature?
>>
>> Shouldn't we make it a common function as other string helpers?
> 
> Sorry, this thread fell through the cracks.
> 
> The issue was raised by covscan:
> 
> Error: REVERSE_INULL (CWE-476):
> dpdk-21.11/app/test-pmd/cmdline_flow.c:7705: deref_ptr: Directly
> dereferencing pointer "size".
> dpdk-21.11/app/test-pmd/cmdline_flow.c:7711: check_after_deref:
> Null-checking "size" suggests that it may be null, but it has already
> been dereferenced on all paths leading to the check.
> # 7709|       if ((src == NULL) ||
> # 7710|           (dst == NULL) ||
> # 7711|->         (size == NULL) ||
> # 7712|           (*size == 0))
> # 7713|           return -1;
> 
> 
> As for the rest of the code, there might be more cleanups to do, as followups.
> 

Proceeding with this one as it solves coverity issue,

Applied to dpdk-next-net/main, thanks.
diff mbox series

Patch

diff --git a/app/test-pmd/cmdline_flow.c b/app/test-pmd/cmdline_flow.c
index fc4a6d9cca..e3269e278d 100644
--- a/app/test-pmd/cmdline_flow.c
+++ b/app/test-pmd/cmdline_flow.c
@@ -9338,11 +9338,7 @@  parse_hex_string(const char *src, uint8_t *dst, uint32_t *size)
 	const uint8_t *head = dst;
 	uint32_t left;
 
-	/* Check input parameters */
-	if ((src == NULL) ||
-		(dst == NULL) ||
-		(size == NULL) ||
-		(*size == 0))
+	if (*size == 0)
 		return -1;
 
 	left = *size;