diff mbox series

[v2] test/timer: fix memzone reserve failure check

Message ID 1620090469-30484-1-git-send-email-humin29@huawei.com (mailing list archive)
State Superseded, archived
Delegated to: Thomas Monjalon
Headers
Series [v2] test/timer: fix memzone reserve failure check |

Checks

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

Commit Message

humin (Q) May 4, 2021, 1:07 a.m. UTC 
  Segmentation fault may occur without checking if memzone
reserves succeed or not.

This patch fixed it.

Fixes: 50247fe03fe0 ("test/timer: exercise new APIs in secondary process")
Cc: stable@dpdk.org

Signed-off-by: Min Hu (Connor) <humin29@huawei.com>
---
v2:
* use TEST_ASSERT_NOT_NULL check "mz" instead of checking
  the test_info pointer.
---
 app/test/test_timer_secondary.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

Comments

Carrillo, Erik G May 4, 2021, 1:18 a.m. UTC | #1 
> -----Original Message-----
> From: Min Hu (Connor) <humin29@huawei.com>
> Sent: Monday, May 3, 2021 8:08 PM
> To: dev@dpdk.org
> Cc: Yigit, Ferruh <ferruh.yigit@intel.com>; Carrillo, Erik G
> <erik.g.carrillo@intel.com>; rsanford@akamai.com
> Subject: [PATCH v2] test/timer: fix memzone reserve failure check
> 
> Segmentation fault may occur without checking if memzone reserves
> succeed or not.
> 
> This patch fixed it.
> 
> Fixes: 50247fe03fe0 ("test/timer: exercise new APIs in secondary process")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Min Hu (Connor) <humin29@huawei.com>
> ---
> v2:
> * use TEST_ASSERT_NOT_NULL check "mz" instead of checking
>   the test_info pointer.
> ---

Thanks,

Acked-by: Erik Gabriel Carrillo <erik.g.carrillo@intel.com>
Thomas Monjalon May 10, 2021, 2:13 p.m. UTC | #2 
04/05/2021 03:07, Min Hu (Connor):
> Segmentation fault may occur without checking if memzone
> reserves succeed or not.

The sentence is confusing. Please try to make it more logical.
Something like "It was potentially dereferencing a null pointer.
It is fixed by checking the pointer before dereferencing."

> 
> This patch fixed it.
> 
> Fixes: 50247fe03fe0 ("test/timer: exercise new APIs in secondary process")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Min Hu (Connor) <humin29@huawei.com>
[...]
> -		test_info = mz->addr;
> -		TEST_ASSERT_NOT_NULL(test_info, "Couldn't allocate memory for "
> +		TEST_ASSERT_NOT_NULL(mz, "Couldn't allocate memory for "
>  				     "test data");

Error messages should not be split. I makes search difficult.
Please fix it in this patch while touching this line.
humin (Q) May 11, 2021, 12:53 a.m. UTC | #3 
在 2021/5/10 22:13, Thomas Monjalon 写道:
> 04/05/2021 03:07, Min Hu (Connor):
>> Segmentation fault may occur without checking if memzone
>> reserves succeed or not.
> 
> The sentence is confusing. Please try to make it more logical.
> Something like "It was potentially dereferencing a null pointer.
> It is fixed by checking the pointer before dereferencing."
> 
>>
>> This patch fixed it.
>>
>> Fixes: 50247fe03fe0 ("test/timer: exercise new APIs in secondary process")
>> Cc: stable@dpdk.org
>>
>> Signed-off-by: Min Hu (Connor) <humin29@huawei.com>
> [...]
>> -		test_info = mz->addr;
>> -		TEST_ASSERT_NOT_NULL(test_info, "Couldn't allocate memory for "
>> +		TEST_ASSERT_NOT_NULL(mz, "Couldn't allocate memory for "
>>   				     "test data");
> 
> Error messages should not be split. I makes search difficult.
> Please fix it in this patch while touching this line.
> 
Fixed in v3, thanks.
> 
> 
> .
>
diff mbox series

Patch

diff --git a/app/test/test_timer_secondary.c b/app/test/test_timer_secondary.c
index 1e8f1d4..16a9f18 100644
--- a/app/test/test_timer_secondary.c
+++ b/app/test/test_timer_secondary.c
@@ -125,9 +125,9 @@  test_timer_secondary(void)
 
 		mz = rte_memzone_reserve(TEST_INFO_MZ_NAME, sizeof(*test_info),
 					 SOCKET_ID_ANY, 0);
-		test_info = mz->addr;
-		TEST_ASSERT_NOT_NULL(test_info, "Couldn't allocate memory for "
+		TEST_ASSERT_NOT_NULL(mz, "Couldn't allocate memory for "
 				     "test data");
+		test_info = mz->addr;
 
 		test_info->tim_mempool = rte_mempool_create("test_timer_mp",
 				NUM_TIMERS, sizeof(struct rte_timer), 0, 0,
@@ -171,9 +171,9 @@  test_timer_secondary(void)
 		int i;
 
 		mz = rte_memzone_lookup(TEST_INFO_MZ_NAME);
-		test_info = mz->addr;
-		TEST_ASSERT_NOT_NULL(test_info, "Couldn't lookup memzone for "
+		TEST_ASSERT_NOT_NULL(mz, "Couldn't lookup memzone for "
 				     "test info");
+		test_info = mz->addr;
 
 		for (i = 0; i < NUM_TIMERS; i++) {
 			rte_mempool_get(test_info->tim_mempool, (void **)&tim);