diff mbox series

[02/12] posix/mq_(timed)send/5-1: Fix error reporting

Message ID 20211119074602.857595-3-lkml@jv-coder.de
State Accepted
Headers show
Series Fix or suppress compiler warnings in posix/conformance/interfaces | expand

Commit Message

Joerg Vehlow Nov. 19, 2021, 7:45 a.m. UTC 
From: Joerg Vehlow <joerg.vehlow@aox-tech.de>

In case mq_receive failed, the whole test could still return PASS/FAIL

Signed-off-by: Joerg Vehlow <joerg.vehlow@aox-tech.de>
---
 .../conformance/interfaces/mq_send/5-1.c                    | 3 +--
 .../conformance/interfaces/mq_timedsend/5-1.c               | 6 ++++--
 2 files changed, 5 insertions(+), 4 deletions(-)

Comments

Cyril Hrubis Nov. 19, 2021, 2:44 p.m. UTC | #1 
Hi!
Applied, thanks.

> --- a/testcases/open_posix_testsuite/conformance/interfaces/mq_timedsend/5-1.c
> +++ b/testcases/open_posix_testsuite/conformance/interfaces/mq_timedsend/5-1.c
> @@ -58,7 +58,6 @@ int main(void)
>  	char msgrcd[BUFFER];
>  	const char *msgptr = MSGSTR;
>  	struct mq_attr attr;
> -	int unresolved = 0;
>  	unsigned pri;
>  
>  	sprintf(gqname, "/mq_timedsend_5-1_%d", getpid());
> @@ -119,7 +118,10 @@ int main(void)
>  		/* receive message and allow blocked send to complete */
>  		if (mq_receive(gqueue, msgrcd, BUFFER, &pri) == -1) {
>  			perror("mq_receive() did not return success");
> -			unresolved = 1;
> +			kill(pid, SIGKILL);	//kill child
> +			mq_close(gqueue);
> +			mq_unlink(gqname);
> +			return PTS_UNRESOLVED;
>  		}

There is another place where it does not clean up the queue in the code
under the if (j == MAXMSG+1) condition, that should be fixed as well.

Also I guess the cleanest way how to write this test would be a kernel
style goto cleanup. E.g.:

	int rval = PTS_UNRESOLVED;


	if (foo) {
		rval = PTS_FAIL;
		goto ret;
	}

	if (bar)
		goto ret;

	...

	pid = 0;
ret:
	if (pid)
		kill(pid, SIGKILL);
	mq_close(queue);
	mq_unlink(queue);
	return rval;
Joerg Vehlow Nov. 22, 2021, 6:45 a.m. UTC | #2 
Hi Cyril,

On 11/19/2021 3:44 PM, Cyril Hrubis wrote:
> Hi!
> Applied, thanks.
>
>> --- a/testcases/open_posix_testsuite/conformance/interfaces/mq_timedsend/5-1.c
>> +++ b/testcases/open_posix_testsuite/conformance/interfaces/mq_timedsend/5-1.c
>> @@ -58,7 +58,6 @@ int main(void)
>>   	char msgrcd[BUFFER];
>>   	const char *msgptr = MSGSTR;
>>   	struct mq_attr attr;
>> -	int unresolved = 0;
>>   	unsigned pri;
>>   
>>   	sprintf(gqname, "/mq_timedsend_5-1_%d", getpid());
>> @@ -119,7 +118,10 @@ int main(void)
>>   		/* receive message and allow blocked send to complete */
>>   		if (mq_receive(gqueue, msgrcd, BUFFER, &pri) == -1) {
>>   			perror("mq_receive() did not return success");
>> -			unresolved = 1;
>> +			kill(pid, SIGKILL);	//kill child
>> +			mq_close(gqueue);
>> +			mq_unlink(gqname);
>> +			return PTS_UNRESOLVED;
>>   		}
> There is another place where it does not clean up the queue in the code
> under the if (j == MAXMSG+1) condition, that should be fixed as well.
>
> Also I guess the cleanest way how to write this test would be a kernel
> style goto cleanup. E.g.:
>
> 	int rval = PTS_UNRESOLVED;
>
>
> 	if (foo) {
> 		rval = PTS_FAIL;
> 		goto ret;
> 	}
>
> 	if (bar)
> 		goto ret;
>
> 	...
>
> 	pid = 0;
> ret:
> 	if (pid)
> 		kill(pid, SIGKILL);
> 	mq_close(queue);
> 	mq_unlink(queue);
> 	return rval;
You are right, but I wanted to keep changes minimal. I think there are 
lots of small bug in the posix tests,
but fixing all of them while trying to just get rid of warnings, would 
have made this patchset even bigger and
I would have never finished. So I had to stop somewhere.

Joerg
Cyril Hrubis Nov. 22, 2021, 9:10 a.m. UTC | #3 
Hi!
> You are right, but I wanted to keep changes minimal. I think there are 
> lots of small bug in the posix tests,
> but fixing all of them while trying to just get rid of warnings, would 
> have made this patchset even bigger and
> I would have never finished. So I had to stop somewhere.

Understood, looking forward to a next patchset then :-).
diff mbox series

Patch

diff --git a/testcases/open_posix_testsuite/conformance/interfaces/mq_send/5-1.c b/testcases/open_posix_testsuite/conformance/interfaces/mq_send/5-1.c
index a2b3025d4..174e4f69e 100644
--- a/testcases/open_posix_testsuite/conformance/interfaces/mq_send/5-1.c
+++ b/testcases/open_posix_testsuite/conformance/interfaces/mq_send/5-1.c
@@ -68,7 +68,6 @@  int main(void)
 	char msgrcd[BUFFER];
 	const char *msgptr = MSGSTR;
 	struct mq_attr attr;
-	int unresolved = 0;
 	unsigned pri;
 
 	sprintf(gqname, "/mq_send_5-1_%d", getpid());
@@ -128,7 +127,7 @@  int main(void)
 		/* receive one message and allow child's mq_send to complete */
 		if (mq_receive(gqueue, msgrcd, BUFFER, &pri) == -1) {
 			perror("mq_receive() did not return success");
-			unresolved = 1;
+			return cleanup_for_exit(gqueue, gqname, PTS_UNRESOLVED);
 		}
 
 		/* child has 5 seconds to call mq_send() again and notify us */
diff --git a/testcases/open_posix_testsuite/conformance/interfaces/mq_timedsend/5-1.c b/testcases/open_posix_testsuite/conformance/interfaces/mq_timedsend/5-1.c
index fb4a81fef..371cdbcd4 100644
--- a/testcases/open_posix_testsuite/conformance/interfaces/mq_timedsend/5-1.c
+++ b/testcases/open_posix_testsuite/conformance/interfaces/mq_timedsend/5-1.c
@@ -58,7 +58,6 @@  int main(void)
 	char msgrcd[BUFFER];
 	const char *msgptr = MSGSTR;
 	struct mq_attr attr;
-	int unresolved = 0;
 	unsigned pri;
 
 	sprintf(gqname, "/mq_timedsend_5-1_%d", getpid());
@@ -119,7 +118,10 @@  int main(void)
 		/* receive message and allow blocked send to complete */
 		if (mq_receive(gqueue, msgrcd, BUFFER, &pri) == -1) {
 			perror("mq_receive() did not return success");
-			unresolved = 1;
+			kill(pid, SIGKILL);	//kill child
+			mq_close(gqueue);
+			mq_unlink(gqname);
+			return PTS_UNRESOLVED;
 		}
 
 		if (sleep(3) == 0) {