diff mbox series

[v3,2/2] test: fix missing check for thread creation

Message ID 1618487412-26678-3-git-send-email-humin29@huawei.com (mailing list archive)
State Superseded, archived
Delegated to: David Marchand
Headers
Series fix missing check for thread creation |

Checks

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

Commit Message

humin (Q) April 15, 2021, 11:50 a.m. UTC 
  From: Chengwen Feng <fengchengwen@huawei.com>

There was a call for thread create function without result check.
Add result check and message print out after failure.

Fixes: 086eb64db39e ("test/pdump: add unit test for pdump library")
Cc: stable@dpdk.org

Signed-off-by: Chengwen Feng <fengchengwen@huawei.com>
Signed-off-by: Min Hu (Connor) <humin29@huawei.com>
---
 app/test/process.h | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

Comments

Pattan, Reshma April 15, 2021, 5:05 p.m. UTC | #1 
> -----Original Message-----
> From: Min Hu (Connor) <humin29@huawei.com>
> +	if ((strcmp(env_value, "run_pdump_server_tests") == 0)) {
> +		rc = pthread_create(&thread, NULL, &send_pkts, NULL);
> +		if (rc != 0)
> +			rte_panic("Cannot start send pkts thread\n");
> +	}


I think you still have not addressed the David comment on previous version of the patch. 
So , can you change this to  something like below

Option1)
rc = pthread_create(&thread, NULL, &send_pkts, NULL);
If ( rc ! = 0 )
{
	rte_panic("Cannot start send pkts thread : %s\n", strerror(rc));
}

Or 

Option2)
If ( pthread_create(&thread, NULL, &send_pkts, NULL) !=0 )
	rte_panic("Cannot start send pkts thread\n");


Also, 

>> test: fix missing check for thread creation
Change this subject line to  "test/pdump: fix missing check for thread creation"

Thanks,
Reshma
humin (Q) April 16, 2021, 8:21 a.m. UTC | #2 
在 2021/4/16 1:05, Pattan, Reshma 写道:
> 
> 
>> -----Original Message-----
>> From: Min Hu (Connor) <humin29@huawei.com>
>> +	if ((strcmp(env_value, "run_pdump_server_tests") == 0)) {
>> +		rc = pthread_create(&thread, NULL, &send_pkts, NULL);
>> +		if (rc != 0)
>> +			rte_panic("Cannot start send pkts thread\n");
>> +	}
> 
> 
> I think you still have not addressed the David comment on previous version of the patch.
> So , can you change this to  something like below
> 
> Option1)
> rc = pthread_create(&thread, NULL, &send_pkts, NULL);
> If ( rc ! = 0 )
> {
> 	rte_panic("Cannot start send pkts thread : %s\n", strerror(rc));
> }
> 
> Or
> 
> Option2)
> If ( pthread_create(&thread, NULL, &send_pkts, NULL) !=0 )
> 	rte_panic("Cannot start send pkts thread\n");
> 
> 
> Also,
> 
>>> test: fix missing check for thread creation
> Change this subject line to  "test/pdump: fix missing check for thread creation"
> 
Hi, Pattan, fixed in v4, except for "test/pdump", if v4 is OK, please
modify it for me, @Ferruh, thanks.
> Thanks,
> Reshma
> 
> .
>
Ferruh Yigit April 16, 2021, 8:34 a.m. UTC | #3 
On 4/16/2021 9:21 AM, Min Hu (Connor) wrote:
> 
> 
> 在 2021/4/16 1:05, Pattan, Reshma 写道:
>>
>>
>>> -----Original Message-----
>>> From: Min Hu (Connor) <humin29@huawei.com>
>>> +    if ((strcmp(env_value, "run_pdump_server_tests") == 0)) {
>>> +        rc = pthread_create(&thread, NULL, &send_pkts, NULL);
>>> +        if (rc != 0)
>>> +            rte_panic("Cannot start send pkts thread\n");
>>> +    }
>>
>>
>> I think you still have not addressed the David comment on previous version of 
>> the patch.
>> So , can you change this to  something like below
>>
>> Option1)
>> rc = pthread_create(&thread, NULL, &send_pkts, NULL);
>> If ( rc ! = 0 )
>> {
>>     rte_panic("Cannot start send pkts thread : %s\n", strerror(rc));
>> }
>>
>> Or
>>
>> Option2)
>> If ( pthread_create(&thread, NULL, &send_pkts, NULL) !=0 )
>>     rte_panic("Cannot start send pkts thread\n");
>>
>>
>> Also,
>>
>>>> test: fix missing check for thread creation
>> Change this subject line to  "test/pdump: fix missing check for thread creation"
>>
> Hi, Pattan, fixed in v4, except for "test/pdump", if v4 is OK, please
> modify it for me, @Ferruh, thanks.

Hi Connor, this patch is on David's domain, he will commit the patch, not me.
humin (Q) April 16, 2021, 9:16 a.m. UTC | #4 
在 2021/4/16 16:34, Ferruh Yigit 写道:
> On 4/16/2021 9:21 AM, Min Hu (Connor) wrote:
>>
>>
>> 在 2021/4/16 1:05, Pattan, Reshma 写道:
>>>
>>>
>>>> -----Original Message-----
>>>> From: Min Hu (Connor) <humin29@huawei.com>
>>>> +    if ((strcmp(env_value, "run_pdump_server_tests") == 0)) {
>>>> +        rc = pthread_create(&thread, NULL, &send_pkts, NULL);
>>>> +        if (rc != 0)
>>>> +            rte_panic("Cannot start send pkts thread\n");
>>>> +    }
>>>
>>>
>>> I think you still have not addressed the David comment on previous 
>>> version of the patch.
>>> So , can you change this to  something like below
>>>
>>> Option1)
>>> rc = pthread_create(&thread, NULL, &send_pkts, NULL);
>>> If ( rc ! = 0 )
>>> {
>>>     rte_panic("Cannot start send pkts thread : %s\n", strerror(rc));
>>> }
>>>
>>> Or
>>>
>>> Option2)
>>> If ( pthread_create(&thread, NULL, &send_pkts, NULL) !=0 )
>>>     rte_panic("Cannot start send pkts thread\n");
>>>
>>>
>>> Also,
>>>
>>>>> test: fix missing check for thread creation
>>> Change this subject line to  "test/pdump: fix missing check for 
>>> thread creation"
>>>
>> Hi, Pattan, fixed in v4, except for "test/pdump", if v4 is OK, please
>> modify it for me, @Ferruh, thanks.
> 
> Hi Connor, this patch is on David's domain, he will commit the patch, 
> not me.
Sorry for that, Ferruh, @David, thanks.
> .
diff mbox series

Patch

diff --git a/app/test/process.h b/app/test/process.h
index 27f1b1c..2a0a8da 100644
--- a/app/test/process.h
+++ b/app/test/process.h
@@ -48,6 +48,7 @@  process_dup(const char *const argv[], int numargs, const char *env_value)
 #ifdef RTE_LIB_PDUMP
 #ifdef RTE_NET_RING
 	pthread_t thread;
+	int rc;
 #endif
 #endif
 
@@ -126,8 +127,11 @@  process_dup(const char *const argv[], int numargs, const char *env_value)
 	/* parent process does a wait */
 #ifdef RTE_LIB_PDUMP
 #ifdef RTE_NET_RING
-	if ((strcmp(env_value, "run_pdump_server_tests") == 0))
-		pthread_create(&thread, NULL, &send_pkts, NULL);
+	if ((strcmp(env_value, "run_pdump_server_tests") == 0)) {
+		rc = pthread_create(&thread, NULL, &send_pkts, NULL);
+		if (rc != 0)
+			rte_panic("Cannot start send pkts thread\n");
+	}
 #endif
 #endif