[LTP] [PATCH 0/7] openposix: Fix 'no return in nonvoid function' warnings

[LTP] [PATCH 0/7] openposix: Fix 'no return in nonvoid function' warnings

[Cyril Hrubis]

Fixes LTP compilation in build systems where the 'no return in nonvoid
function' warning is treated as error. More than half of the cases were
actually a buggy tests, which only shows that maybe treating this
warning as an error makes sense.

Cyril Hrubis (7):
  openposix: pthread_rwlockattr_getpshared/2-1: Fix
  openposix: pthread_spin_init/{2-1,2-2}: Fix
  openposix: sem_destroy/3-1: Fix
  openposix: sem_timedwait/11-1: Fix
  openposix: aio_h/2-1: Add return at the end of main()
  openposix: mq_timedreceive: Silence warning
  opeposix: pthread_barrierattr_getpshared/2-1: Simplify codeflow

 .../conformance/definitions/aio_h/2-1.c       |  2 ++
 .../interfaces/mq_timedreceive/5-3.c          |  1 +
 .../pthread_barrierattr_getpshared/2-1.c      | 11 +++----
 .../pthread_rwlockattr_getpshared/2-1.c       | 20 ++++++++++--
 .../interfaces/pthread_spin_init/2-1.c        | 20 ++++++++++--
 .../interfaces/pthread_spin_init/2-2.c        | 20 ++++++++++--
 .../conformance/interfaces/sem_destroy/3-1.c  | 31 +++++++++++++------
 .../interfaces/sem_timedwait/11-1.c           |  9 +++---
 8 files changed, 89 insertions(+), 25 deletions(-)

-- 
2.35.1


-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

[LTP] [PATCH 1/7] openposix: pthread_rwlockattr_getpshared/2-1: Fix

[Cyril Hrubis]

Propagate a failure in child to the parent properly.

Signed-off-by: Cyril Hrubis <chrubis@suse.cz>
---
 .../pthread_rwlockattr_getpshared/2-1.c       | 20 +++++++++++++++++--
 1 file changed, 18 insertions(+), 2 deletions(-)

diff --git a/testcases/open_posix_testsuite/conformance/interfaces/pthread_rwlockattr_getpshared/2-1.c b/testcases/open_posix_testsuite/conformance/interfaces/pthread_rwlockattr_getpshared/2-1.c
index 3ffdc0cea..72c40f117 100644
--- a/testcases/open_posix_testsuite/conformance/interfaces/pthread_rwlockattr_getpshared/2-1.c
+++ b/testcases/open_posix_testsuite/conformance/interfaces/pthread_rwlockattr_getpshared/2-1.c
@@ -126,7 +126,11 @@ int main(void)
 	if (pid == -1) {
 		perror("Error at fork()");
 		return PTS_UNRESOLVED;
-	} else if (pid > 0) {
+	}
+
+	if (pid > 0) {
+		int status;
+
 		/* Parent */
 		/* wait until child do wrlock */
 		while (rwlock_data->data == 0) {
@@ -141,7 +145,7 @@ int main(void)
 		printf("Parent unlocked.\n");
 
 		/* Wait for child to end */
-		wait(NULL);
+		wait(&status);
 
 		if ((shm_unlink(shm_name)) != 0) {
 			perror("Error at shm_unlink()");
@@ -154,6 +158,16 @@ int main(void)
 			return PTS_FAIL;
 		}
 
+		if (!WIFEXITED(status)) {
+			printf("Parent: did not exit properly!\n");
+			return PTS_FAIL;
+		}
+
+		if (WEXITSTATUS(status)) {
+			printf("Parent: failure in child\n");
+			return WEXITSTATUS(status);
+		}
+
 		printf("Test PASSED\n");
 		return PTS_PASS;
 	} else {
@@ -195,5 +209,7 @@ int main(void)
 			rwlock_data->data = -1;
 			return PTS_FAIL;
 		}
+
+		return PTS_PASS;
 	}
 }
-- 
2.35.1


-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

[LTP] [PATCH 2/7] openposix: pthread_spin_init/{2-1,2-2}: Fix

[Cyril Hrubis]

Propagate a failure in child to the parent properly.

Signed-off-by: Cyril Hrubis <chrubis@suse.cz>
---
 .../interfaces/pthread_spin_init/2-1.c        | 20 +++++++++++++++++--
 .../interfaces/pthread_spin_init/2-2.c        | 20 +++++++++++++++++--
 2 files changed, 36 insertions(+), 4 deletions(-)

diff --git a/testcases/open_posix_testsuite/conformance/interfaces/pthread_spin_init/2-1.c b/testcases/open_posix_testsuite/conformance/interfaces/pthread_spin_init/2-1.c
index b7dd9e05e..f20822c50 100644
--- a/testcases/open_posix_testsuite/conformance/interfaces/pthread_spin_init/2-1.c
+++ b/testcases/open_posix_testsuite/conformance/interfaces/pthread_spin_init/2-1.c
@@ -100,7 +100,11 @@ int main(void)
 	if (pid == -1) {
 		perror("Error at fork()");
 		return PTS_UNRESOLVED;
-	} else if (pid > 0) {
+	}
+
+	if (pid > 0) {
+		int status;
+
 		/* Parent */
 		/* wait until child writes to spinlock data */
 		while (spinlock_data->data != 1)
@@ -116,13 +120,23 @@ int main(void)
 		spinlock_data->data = 2;
 
 		/* Wait until child ends */
-		wait(NULL);
+		wait(&status);
 
 		if ((shm_unlink(shm_name)) != 0) {
 			perror("Error at shm_unlink()");
 			return PTS_UNRESOLVED;
 		}
 
+		if (!WIFEXITED(status)) {
+			printf("Parent: did not exit properly!\n");
+			return PTS_FAIL;
+		}
+
+		if (WEXITSTATUS(status)) {
+			printf("Parent: failure in child\n");
+			return WEXITSTATUS(status);
+		}
+
 		printf("Test PASSED\n");
 		return PTS_PASS;
 	} else {
@@ -170,5 +184,7 @@ int main(void)
 			printf("Child: error at pthread_spin_destroy()\n");
 			return PTS_UNRESOLVED;
 		}
+
+		return PTS_PASS;
 	}
 }
diff --git a/testcases/open_posix_testsuite/conformance/interfaces/pthread_spin_init/2-2.c b/testcases/open_posix_testsuite/conformance/interfaces/pthread_spin_init/2-2.c
index f3cb9b2a3..df0d4df87 100644
--- a/testcases/open_posix_testsuite/conformance/interfaces/pthread_spin_init/2-2.c
+++ b/testcases/open_posix_testsuite/conformance/interfaces/pthread_spin_init/2-2.c
@@ -106,7 +106,11 @@ int main(void)
 	if (pid == -1) {
 		perror("Error at fork()");
 		return PTS_UNRESOLVED;
-	} else if (pid > 0) {
+	}
+
+	if (pid > 0) {
+		int status;
+
 		/* Parent */
 		/* wait until child writes to spinlock data */
 		while (spinlock_data->data != 1)
@@ -122,13 +126,23 @@ int main(void)
 		spinlock_data->data = 2;
 
 		/* Wait until child ends */
-		wait(NULL);
+		wait(&status);
 
 		if ((shm_unlink(shm_name)) != 0) {
 			perror("Error at shm_unlink()");
 			return PTS_UNRESOLVED;
 		}
 
+		if (!WIFEXITED(status)) {
+			printf("Parent: did not exit properly!\n");
+			return PTS_FAIL;
+		}
+
+		if (WEXITSTATUS(status)) {
+			printf("Parent: failure in child\n");
+			return WEXITSTATUS(status);
+		}
+
 		printf("Test PASSED\n");
 		return PTS_PASS;
 	} else {
@@ -175,5 +189,7 @@ int main(void)
 			printf("Child: error at pthread_spin_destroy()\n");
 			return PTS_UNRESOLVED;
 		}
+
+		return PTS_PASS;
 	}
 }
-- 
2.35.1


-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

[LTP] [PATCH 3/7] openposix: sem_destroy/3-1: Fix

[Cyril Hrubis]

There were several problems in the test and it actually didn't even call
the sem_destroy() function at all.

- The main thread did call thread_exit() before actually attempting to
  destroy the semaphores

- The program exit value was undefined (random garbage) in the unlikely
  case that pthread_join() failed for one of the threads

Signed-off-by: Cyril Hrubis <chrubis@suse.cz>
---
 .../conformance/interfaces/sem_destroy/3-1.c  | 31 +++++++++++++------
 1 file changed, 22 insertions(+), 9 deletions(-)

diff --git a/testcases/open_posix_testsuite/conformance/interfaces/sem_destroy/3-1.c b/testcases/open_posix_testsuite/conformance/interfaces/sem_destroy/3-1.c
index 2a02a7532..bca21371f 100644
--- a/testcases/open_posix_testsuite/conformance/interfaces/sem_destroy/3-1.c
+++ b/testcases/open_posix_testsuite/conformance/interfaces/sem_destroy/3-1.c
@@ -19,11 +19,11 @@
 #include <sys/stat.h>
 #include <fcntl.h>
 #include <pthread.h>
+#include <string.h>
 #include "posixtest.h"
 
 #define TEST "3-1"
 #define FUNCTION "sem_destroy"
-#define ERROR_PREFIX "unexpected error: " FUNCTION " " TEST ": "
 
 static sem_t psem, csem;
 static int n;
@@ -33,6 +33,7 @@ static void *consumer(void *);
 int main(void)
 {
 	pthread_t prod, cons;
+	int err;
 	long cnt = 3;
 
 	n = 0;
@@ -40,28 +41,40 @@ int main(void)
 		perror("sem_init");
 		return PTS_UNRESOLVED;
 	}
+
 	if (sem_init(&psem, 0, 1) < 0) {
 		perror("sem_init");
 		return PTS_UNRESOLVED;
 	}
+
 	if (pthread_create(&prod, NULL, producer, (void *)cnt) != 0) {
 		perror("pthread_create");
 		return PTS_UNRESOLVED;
 	}
+
 	if (pthread_create(&cons, NULL, consumer, (void *)cnt) != 0) {
 		perror("pthread_create");
 		return PTS_UNRESOLVED;
 	}
 
-	if (pthread_join(prod, NULL) == 0 && pthread_join(cons, NULL) == 0) {
+	err = pthread_join(prod, NULL);
+	if (err) {
+		printf("Failed to join thread: %s", strerror(err));
+		return PTS_UNRESOLVED;
+	}
+
+	err = pthread_join(cons, NULL);
+	if (err) {
+		printf("Failed to join thread: %s", strerror(err));
+		return PTS_UNRESOLVED;
+	}
+
+	if (sem_destroy(&psem) == 0 && sem_destroy(&csem) == 0) {
 		puts("TEST PASS");
-		pthread_exit(NULL);
-		if ((sem_destroy(&psem) == 0) && sem_destroy(&csem) == 0) {
-			return PTS_PASS;
-		} else {
-			puts("TEST FAILED");
-			return PTS_FAIL;
-		}
+		return PTS_PASS;
+	} else {
+		puts("TEST FAILED");
+		return PTS_FAIL;
 	}
 }
 
-- 
2.35.1


-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

[LTP] [PATCH 4/7] openposix: sem_timedwait/11-1: Fix

[Cyril Hrubis]

Actually run both of the cases (valid timeout and invalid timeout).

The timeout is not actually invalid, but rather in the past, which is
important to test as the system has to try to lock the semaphore first
and only if that fails it should check the timeout.

Signed-off-by: Cyril Hrubis <chrubis@suse.cz>
---
 .../conformance/interfaces/sem_timedwait/11-1.c          | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/testcases/open_posix_testsuite/conformance/interfaces/sem_timedwait/11-1.c b/testcases/open_posix_testsuite/conformance/interfaces/sem_timedwait/11-1.c
index f87afaa43..663edd836 100644
--- a/testcases/open_posix_testsuite/conformance/interfaces/sem_timedwait/11-1.c
+++ b/testcases/open_posix_testsuite/conformance/interfaces/sem_timedwait/11-1.c
@@ -24,7 +24,7 @@
 #include "posixtest.h"
 
 #define TIMEOUT 2
-#define INVALIDTIMEOUT -2
+#define NEGATIVETIMEOUT -2
 #define TEST "11-1"
 #define FUNCTION "sem_timedwait"
 #define ERROR_PREFIX "unexpected error: " FUNCTION " " TEST ": "
@@ -45,7 +45,7 @@ int main(void)
 			ts[i].tv_sec = time(NULL) + TIMEOUT;
 			ts[i].tv_nsec = 0;
 		} else if (i == 1) {
-			ts[i].tv_sec = time(NULL) + INVALIDTIMEOUT;
+			ts[i].tv_sec = time(NULL) + NEGATIVETIMEOUT;
 			ts[i].tv_nsec = 0;
 		}
 		/* Lock Semaphore */
@@ -63,13 +63,14 @@ int main(void)
 
 		/* Checking if the value of the Semaphore decremented by one */
 		if ((val[i] == 0) && (sts[i] == 0)) {
-			puts("TEST PASSED");
 			sem_destroy(&mysemp[i]);
-			return PTS_PASS;
 		} else {
 			puts("TEST FAILED");
 			sem_destroy(&mysemp[i]);
 			return PTS_FAIL;
 		}
 	}
+
+	puts("TEST PASSED");
+	return PTS_PASS;
 }
-- 
2.35.1


-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

[LTP] [PATCH 5/7] openposix: aio_h/2-1: Add return at the end of main()

[Cyril Hrubis]

This is compile-only test so the return value is not actually used but
that's not a reason not to fix warnings.

Signed-off-by: Cyril Hrubis <chrubis@suse.cz>
---
 .../open_posix_testsuite/conformance/definitions/aio_h/2-1.c    | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/testcases/open_posix_testsuite/conformance/definitions/aio_h/2-1.c b/testcases/open_posix_testsuite/conformance/definitions/aio_h/2-1.c
index 39fb41d22..133b3a516 100644
--- a/testcases/open_posix_testsuite/conformance/definitions/aio_h/2-1.c
+++ b/testcases/open_posix_testsuite/conformance/definitions/aio_h/2-1.c
@@ -25,4 +25,6 @@ int main(void)
 	aiocb.aio_nbytes = 0;
 	aiocb.aio_sigevent = sigevent;
 	aiocb.aio_reqprio = -1;
+
+	return 0;
 }
-- 
2.35.1


-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

[LTP] [PATCH 6/7] openposix: mq_timedreceive: Silence warning

[Cyril Hrubis]

We do not use the exit value from the child at all, but that's not a
reason to produce warnings.

Signed-off-by: Cyril Hrubis <chrubis@suse.cz>
---
 .../conformance/interfaces/mq_timedreceive/5-3.c                 | 1 +
 1 file changed, 1 insertion(+)

diff --git a/testcases/open_posix_testsuite/conformance/interfaces/mq_timedreceive/5-3.c b/testcases/open_posix_testsuite/conformance/interfaces/mq_timedreceive/5-3.c
index a843c13ee..d79d9720d 100644
--- a/testcases/open_posix_testsuite/conformance/interfaces/mq_timedreceive/5-3.c
+++ b/testcases/open_posix_testsuite/conformance/interfaces/mq_timedreceive/5-3.c
@@ -110,5 +110,6 @@ int main(void)
 		sleep(1);	/* give time to parent to set up handler */
 		/* send signal to parent */
 		kill(getppid(), SIGABRT);
+		return 0;
 	}
 }
-- 
2.35.1


-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

[LTP] [PATCH 7/7] opeposix: pthread_barrierattr_getpshared/2-1: Simplify codeflow

[Cyril Hrubis]

This actually fixes a 'no return in nonvoid function' warning since gcc
may get confused during the codeflow analysis.

Signed-off-by: Cyril Hrubis <chrubis@suse.cz>
---
 .../interfaces/pthread_barrierattr_getpshared/2-1.c   | 11 +++++------
 1 file changed, 5 insertions(+), 6 deletions(-)

diff --git a/testcases/open_posix_testsuite/conformance/interfaces/pthread_barrierattr_getpshared/2-1.c b/testcases/open_posix_testsuite/conformance/interfaces/pthread_barrierattr_getpshared/2-1.c
index a21a5a507..52c074173 100644
--- a/testcases/open_posix_testsuite/conformance/interfaces/pthread_barrierattr_getpshared/2-1.c
+++ b/testcases/open_posix_testsuite/conformance/interfaces/pthread_barrierattr_getpshared/2-1.c
@@ -141,7 +141,9 @@ int main(void)
 	if (pid == -1) {
 		perror("Error at fork()");
 		return PTS_UNRESOLVED;
-	} else if (pid == 0) {
+	}
+
+	if (pid == 0) {
 		/* Child */
 		/* Map the shared object to child's memory */
 		barrier =
@@ -209,10 +211,7 @@ int main(void)
 
 		printf("Test PASSED\n");
 		return PTS_PASS;
+	} else {
+		return serial;
 	}
-
-	if (pid == 0) {
-		exit(serial);
-	}
-
 }
-- 
2.35.1


-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH 0/7] openposix: Fix 'no return in nonvoid function' warnings

[Li Wang]

[-- Attachment #1.1: Type: text/plain, Size: 176 bytes --]

Hi Cyril,

Generally looks good, thanks for the fix and cleanup.
Tests work well from my side.

For the series:
Reviewed-by: Li Wang <liwang@redhat.com>


-- 
Regards,
Li Wang

[-- Attachment #1.2: Type: text/html, Size: 854 bytes --]

[-- Attachment #2: Type: text/plain, Size: 60 bytes --]


-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH 4/7] openposix: sem_timedwait/11-1: Fix

[Martin Doucha]

Hi,
small suggestion below, otherwise it looks good.

Reviewed-by: Martin Doucha <mdoucha@suse.cz>

On 20. 06. 22 11:21, Cyril Hrubis wrote:
> Actually run both of the cases (valid timeout and invalid timeout).
> 
> The timeout is not actually invalid, but rather in the past, which is
> important to test as the system has to try to lock the semaphore first
> and only if that fails it should check the timeout.
> 
> Signed-off-by: Cyril Hrubis <chrubis@suse.cz>
> ---
>  .../conformance/interfaces/sem_timedwait/11-1.c          | 9 +++++----
>  1 file changed, 5 insertions(+), 4 deletions(-)
> 
> diff --git a/testcases/open_posix_testsuite/conformance/interfaces/sem_timedwait/11-1.c b/testcases/open_posix_testsuite/conformance/interfaces/sem_timedwait/11-1.c
> index f87afaa43..663edd836 100644
> --- a/testcases/open_posix_testsuite/conformance/interfaces/sem_timedwait/11-1.c
> +++ b/testcases/open_posix_testsuite/conformance/interfaces/sem_timedwait/11-1.c
> @@ -24,7 +24,7 @@
>  #include "posixtest.h"
>  
>  #define TIMEOUT 2
> -#define INVALIDTIMEOUT -2
> +#define NEGATIVETIMEOUT -2
>  #define TEST "11-1"
>  #define FUNCTION "sem_timedwait"
>  #define ERROR_PREFIX "unexpected error: " FUNCTION " " TEST ": "
> @@ -45,7 +45,7 @@ int main(void)
>  			ts[i].tv_sec = time(NULL) + TIMEOUT;
>  			ts[i].tv_nsec = 0;
>  		} else if (i == 1) {
> -			ts[i].tv_sec = time(NULL) + INVALIDTIMEOUT;
> +			ts[i].tv_sec = time(NULL) + NEGATIVETIMEOUT;
>  			ts[i].tv_nsec = 0;
>  		}
>  		/* Lock Semaphore */
> @@ -63,13 +63,14 @@ int main(void)
>  
>  		/* Checking if the value of the Semaphore decremented by one */
>  		if ((val[i] == 0) && (sts[i] == 0)) {
> -			puts("TEST PASSED");
>  			sem_destroy(&mysemp[i]);

It'd be better to move sem_destroy() above the condition. See code
example at the end.

> -			return PTS_PASS;
>  		} else {
>  			puts("TEST FAILED");
>  			sem_destroy(&mysemp[i]);
>  			return PTS_FAIL;
>  		}
>  	}
> +
> +	puts("TEST PASSED");
> +	return PTS_PASS;
>  }

...

	sem_destroy(&mysemp[i]);

	/* Checking if the value of the Semaphore decremented by one */
	if ((val[i] != 0) || (sts[i] != 0)) {
		puts("TEST FAILED");
		return PTS_FAIL;
	}

	puts("TEST PASSED");
	return PTS_PASS;
}

-- 
Martin Doucha   mdoucha@suse.cz
QA Engineer for Software Maintenance
SUSE LINUX, s.r.o.
CORSO IIa
Krizikova 148/34
186 00 Prague 8
Czech Republic

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH 7/7] opeposix: pthread_barrierattr_getpshared/2-1: Simplify codeflow

[Martin Doucha]

Hi,
one small suggestion below, otherwise for the whole patchset:

Reviewed-by: Martin Doucha <mdoucha@suse.cz>

On 20. 06. 22 11:21, Cyril Hrubis wrote:
> This actually fixes a 'no return in nonvoid function' warning since gcc
> may get confused during the codeflow analysis.
> 
> Signed-off-by: Cyril Hrubis <chrubis@suse.cz>
> ---
>  .../interfaces/pthread_barrierattr_getpshared/2-1.c   | 11 +++++------
>  1 file changed, 5 insertions(+), 6 deletions(-)
> 
> diff --git a/testcases/open_posix_testsuite/conformance/interfaces/pthread_barrierattr_getpshared/2-1.c b/testcases/open_posix_testsuite/conformance/interfaces/pthread_barrierattr_getpshared/2-1.c
> index a21a5a507..52c074173 100644
> --- a/testcases/open_posix_testsuite/conformance/interfaces/pthread_barrierattr_getpshared/2-1.c
> +++ b/testcases/open_posix_testsuite/conformance/interfaces/pthread_barrierattr_getpshared/2-1.c
> @@ -141,7 +141,9 @@ int main(void)
>  	if (pid == -1) {
>  		perror("Error at fork()");
>  		return PTS_UNRESOLVED;
> -	} else if (pid == 0) {
> +	}
> +
> +	if (pid == 0) {
>  		/* Child */
>  		/* Map the shared object to child's memory */
>  		barrier =
> @@ -209,10 +211,7 @@ int main(void)
>  
>  		printf("Test PASSED\n");
>  		return PTS_PASS;
> +	} else {
> +		return serial;

The return can be unconditional.

>  	}
> -
> -	if (pid == 0) {
> -		exit(serial);
> -	}
> -
>  }


-- 
Martin Doucha   mdoucha@suse.cz
QA Engineer for Software Maintenance
SUSE LINUX, s.r.o.
CORSO IIa
Krizikova 148/34
186 00 Prague 8
Czech Republic

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH 1/7] openposix: pthread_rwlockattr_getpshared/2-1: Fix

[Richard Palethorpe]

Hello,

Reviewed-by: Richard Palethorpe <rpalethorpe@suse.com>

Cyril Hrubis <chrubis@suse.cz> writes:

> Propagate a failure in child to the parent properly.
>
> Signed-off-by: Cyril Hrubis <chrubis@suse.cz>
> ---
>  .../pthread_rwlockattr_getpshared/2-1.c       | 20 +++++++++++++++++--
>  1 file changed, 18 insertions(+), 2 deletions(-)
>
> diff --git a/testcases/open_posix_testsuite/conformance/interfaces/pthread_rwlockattr_getpshared/2-1.c b/testcases/open_posix_testsuite/conformance/interfaces/pthread_rwlockattr_getpshared/2-1.c
> index 3ffdc0cea..72c40f117 100644
> --- a/testcases/open_posix_testsuite/conformance/interfaces/pthread_rwlockattr_getpshared/2-1.c
> +++ b/testcases/open_posix_testsuite/conformance/interfaces/pthread_rwlockattr_getpshared/2-1.c
> @@ -126,7 +126,11 @@ int main(void)
>  	if (pid == -1) {
>  		perror("Error at fork()");
>  		return PTS_UNRESOLVED;
> -	} else if (pid > 0) {
> +	}
> +
> +	if (pid > 0) {
> +		int status;
> +
>  		/* Parent */
>  		/* wait until child do wrlock */
>  		while (rwlock_data->data == 0) {
> @@ -141,7 +145,7 @@ int main(void)
>  		printf("Parent unlocked.\n");
>  
>  		/* Wait for child to end */
> -		wait(NULL);
> +		wait(&status);
>  
>  		if ((shm_unlink(shm_name)) != 0) {
>  			perror("Error at shm_unlink()");
> @@ -154,6 +158,16 @@ int main(void)
>  			return PTS_FAIL;
>  		}
>  
> +		if (!WIFEXITED(status)) {
> +			printf("Parent: did not exit properly!\n");
> +			return PTS_FAIL;
> +		}
> +
> +		if (WEXITSTATUS(status)) {
> +			printf("Parent: failure in child\n");
> +			return WEXITSTATUS(status);
> +		}
> +
>  		printf("Test PASSED\n");
>  		return PTS_PASS;
>  	} else {
> @@ -195,5 +209,7 @@ int main(void)
>  			rwlock_data->data = -1;
>  			return PTS_FAIL;
>  		}
> +
> +		return PTS_PASS;
>  	}
>  }
> -- 
> 2.35.1


-- 
Thank you,
Richard.

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH 2/7] openposix: pthread_spin_init/{2-1,2-2}: Fix

[Richard Palethorpe]

Hello,

Reviewed-by: Richard Palethorpe <rpalethorpe@suse.com>

Cyril Hrubis <chrubis@suse.cz> writes:

> Propagate a failure in child to the parent properly.
>
> Signed-off-by: Cyril Hrubis <chrubis@suse.cz>
> ---
>  .../interfaces/pthread_spin_init/2-1.c        | 20 +++++++++++++++++--
>  .../interfaces/pthread_spin_init/2-2.c        | 20 +++++++++++++++++--
>  2 files changed, 36 insertions(+), 4 deletions(-)
>
> diff --git a/testcases/open_posix_testsuite/conformance/interfaces/pthread_spin_init/2-1.c b/testcases/open_posix_testsuite/conformance/interfaces/pthread_spin_init/2-1.c
> index b7dd9e05e..f20822c50 100644
> --- a/testcases/open_posix_testsuite/conformance/interfaces/pthread_spin_init/2-1.c
> +++ b/testcases/open_posix_testsuite/conformance/interfaces/pthread_spin_init/2-1.c
> @@ -100,7 +100,11 @@ int main(void)
>  	if (pid == -1) {
>  		perror("Error at fork()");
>  		return PTS_UNRESOLVED;
> -	} else if (pid > 0) {
> +	}
> +
> +	if (pid > 0) {
> +		int status;
> +
>  		/* Parent */
>  		/* wait until child writes to spinlock data */
>  		while (spinlock_data->data != 1)
> @@ -116,13 +120,23 @@ int main(void)
>  		spinlock_data->data = 2;
>  
>  		/* Wait until child ends */
> -		wait(NULL);
> +		wait(&status);
>  
>  		if ((shm_unlink(shm_name)) != 0) {
>  			perror("Error at shm_unlink()");
>  			return PTS_UNRESOLVED;
>  		}
>  
> +		if (!WIFEXITED(status)) {
> +			printf("Parent: did not exit properly!\n");
> +			return PTS_FAIL;
> +		}
> +
> +		if (WEXITSTATUS(status)) {
> +			printf("Parent: failure in child\n");
> +			return WEXITSTATUS(status);
> +		}
> +
>  		printf("Test PASSED\n");
>  		return PTS_PASS;
>  	} else {
> @@ -170,5 +184,7 @@ int main(void)
>  			printf("Child: error at pthread_spin_destroy()\n");
>  			return PTS_UNRESOLVED;
>  		}
> +
> +		return PTS_PASS;
>  	}
>  }
> diff --git a/testcases/open_posix_testsuite/conformance/interfaces/pthread_spin_init/2-2.c b/testcases/open_posix_testsuite/conformance/interfaces/pthread_spin_init/2-2.c
> index f3cb9b2a3..df0d4df87 100644
> --- a/testcases/open_posix_testsuite/conformance/interfaces/pthread_spin_init/2-2.c
> +++ b/testcases/open_posix_testsuite/conformance/interfaces/pthread_spin_init/2-2.c
> @@ -106,7 +106,11 @@ int main(void)
>  	if (pid == -1) {
>  		perror("Error at fork()");
>  		return PTS_UNRESOLVED;
> -	} else if (pid > 0) {
> +	}
> +
> +	if (pid > 0) {
> +		int status;
> +
>  		/* Parent */
>  		/* wait until child writes to spinlock data */
>  		while (spinlock_data->data != 1)
> @@ -122,13 +126,23 @@ int main(void)
>  		spinlock_data->data = 2;
>  
>  		/* Wait until child ends */
> -		wait(NULL);
> +		wait(&status);
>  
>  		if ((shm_unlink(shm_name)) != 0) {
>  			perror("Error at shm_unlink()");
>  			return PTS_UNRESOLVED;
>  		}
>  
> +		if (!WIFEXITED(status)) {
> +			printf("Parent: did not exit properly!\n");
> +			return PTS_FAIL;
> +		}
> +
> +		if (WEXITSTATUS(status)) {
> +			printf("Parent: failure in child\n");
> +			return WEXITSTATUS(status);
> +		}
> +
>  		printf("Test PASSED\n");
>  		return PTS_PASS;
>  	} else {
> @@ -175,5 +189,7 @@ int main(void)
>  			printf("Child: error at pthread_spin_destroy()\n");
>  			return PTS_UNRESOLVED;
>  		}
> +
> +		return PTS_PASS;
>  	}
>  }
> -- 
> 2.35.1


-- 
Thank you,
Richard.

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH 3/7] openposix: sem_destroy/3-1: Fix

[Richard Palethorpe]

Hello,

Cyril Hrubis <chrubis@suse.cz> writes:

> +	if (sem_destroy(&psem) == 0 && sem_destroy(&csem) == 0) {
>  		puts("TEST PASS");
> -		pthread_exit(NULL);

lol

> -		if ((sem_destroy(&psem) == 0) && sem_destroy(&csem) == 0) {
> -			return PTS_PASS;
> -		} else {
> -			puts("TEST FAILED");
> -			return PTS_FAIL;
> -		}
> +		return PTS_PASS;
> +	} else {
> +		puts("TEST FAILED");
> +		return PTS_FAIL;
>  	}
>  }
>  
> -- 
> 2.35.1

Reviewed-by: Richard Palethorpe <rpalethorpe@suse.com>

-- 
Thank you,
Richard.

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH 5/7] openposix: aio_h/2-1: Add return at the end of main()

[Richard Palethorpe]

Reviewed-by: Richard Palethorpe <rpalethorpe@suse.com>

Cyril Hrubis <chrubis@suse.cz> writes:

> This is compile-only test so the return value is not actually used but
> that's not a reason not to fix warnings.
>
> Signed-off-by: Cyril Hrubis <chrubis@suse.cz>
> ---
>  .../open_posix_testsuite/conformance/definitions/aio_h/2-1.c    | 2 ++
>  1 file changed, 2 insertions(+)
>
> diff --git a/testcases/open_posix_testsuite/conformance/definitions/aio_h/2-1.c b/testcases/open_posix_testsuite/conformance/definitions/aio_h/2-1.c
> index 39fb41d22..133b3a516 100644
> --- a/testcases/open_posix_testsuite/conformance/definitions/aio_h/2-1.c
> +++ b/testcases/open_posix_testsuite/conformance/definitions/aio_h/2-1.c
> @@ -25,4 +25,6 @@ int main(void)
>  	aiocb.aio_nbytes = 0;
>  	aiocb.aio_sigevent = sigevent;
>  	aiocb.aio_reqprio = -1;
> +
> +	return 0;
>  }
> -- 
> 2.35.1


-- 
Thank you,
Richard.

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH 6/7] openposix: mq_timedreceive: Silence warning

[Richard Palethorpe]

Reviewed-by: Richard Palethorpe <rpalethorpe@suse.com>

Cyril Hrubis <chrubis@suse.cz> writes:

> We do not use the exit value from the child at all, but that's not a
> reason to produce warnings.
>
> Signed-off-by: Cyril Hrubis <chrubis@suse.cz>
> ---
>  .../conformance/interfaces/mq_timedreceive/5-3.c                 | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/testcases/open_posix_testsuite/conformance/interfaces/mq_timedreceive/5-3.c b/testcases/open_posix_testsuite/conformance/interfaces/mq_timedreceive/5-3.c
> index a843c13ee..d79d9720d 100644
> --- a/testcases/open_posix_testsuite/conformance/interfaces/mq_timedreceive/5-3.c
> +++ b/testcases/open_posix_testsuite/conformance/interfaces/mq_timedreceive/5-3.c
> @@ -110,5 +110,6 @@ int main(void)
>  		sleep(1);	/* give time to parent to set up handler */
>  		/* send signal to parent */
>  		kill(getppid(), SIGABRT);
> +		return 0;
>  	}
>  }
> -- 
> 2.35.1


-- 
Thank you,
Richard.

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH 4/7] openposix: sem_timedwait/11-1: Fix

[Richard Palethorpe]

Hello,

Martin Doucha <mdoucha@suse.cz> writes:

> Hi,
> small suggestion below, otherwise it looks good.
>
> Reviewed-by: Martin Doucha <mdoucha@suse.cz>
>
> On 20. 06. 22 11:21, Cyril Hrubis wrote:
>> Actually run both of the cases (valid timeout and invalid timeout).
>> 
>> The timeout is not actually invalid, but rather in the past, which is
>> important to test as the system has to try to lock the semaphore first
>> and only if that fails it should check the timeout.
>> 
>> Signed-off-by: Cyril Hrubis <chrubis@suse.cz>
>> ---
>>  .../conformance/interfaces/sem_timedwait/11-1.c          | 9 +++++----
>>  1 file changed, 5 insertions(+), 4 deletions(-)
>> 
>> diff --git a/testcases/open_posix_testsuite/conformance/interfaces/sem_timedwait/11-1.c b/testcases/open_posix_testsuite/conformance/interfaces/sem_timedwait/11-1.c
>> index f87afaa43..663edd836 100644
>> --- a/testcases/open_posix_testsuite/conformance/interfaces/sem_timedwait/11-1.c
>> +++ b/testcases/open_posix_testsuite/conformance/interfaces/sem_timedwait/11-1.c
>> @@ -24,7 +24,7 @@
>>  #include "posixtest.h"
>>  
>>  #define TIMEOUT 2
>> -#define INVALIDTIMEOUT -2
>> +#define NEGATIVETIMEOUT -2
>>  #define TEST "11-1"
>>  #define FUNCTION "sem_timedwait"
>>  #define ERROR_PREFIX "unexpected error: " FUNCTION " " TEST ": "
>> @@ -45,7 +45,7 @@ int main(void)
>>  			ts[i].tv_sec = time(NULL) + TIMEOUT;
>>  			ts[i].tv_nsec = 0;
>>  		} else if (i == 1) {
>> -			ts[i].tv_sec = time(NULL) + INVALIDTIMEOUT;
>> +			ts[i].tv_sec = time(NULL) + NEGATIVETIMEOUT;
>>  			ts[i].tv_nsec = 0;
>>  		}
>>  		/* Lock Semaphore */
>> @@ -63,13 +63,14 @@ int main(void)
>>  
>>  		/* Checking if the value of the Semaphore decremented by one */
>>  		if ((val[i] == 0) && (sts[i] == 0)) {
>> -			puts("TEST PASSED");
>>  			sem_destroy(&mysemp[i]);
>
> It'd be better to move sem_destroy() above the condition. See code
> example at the end.
>
>> -			return PTS_PASS;
>>  		} else {
>>  			puts("TEST FAILED");
>>  			sem_destroy(&mysemp[i]);
>>  			return PTS_FAIL;
>>  		}
>>  	}
>> +
>> +	puts("TEST PASSED");
>> +	return PTS_PASS;
>>  }
>
> ...
>
> 	sem_destroy(&mysemp[i]);

And with that also:

Reviewed-by: Richard Palethorpe <rpalethorpe@suse.com>

>
> 	/* Checking if the value of the Semaphore decremented by one */
> 	if ((val[i] != 0) || (sts[i] != 0)) {
> 		puts("TEST FAILED");
> 		return PTS_FAIL;
> 	}
>
> 	puts("TEST PASSED");
> 	return PTS_PASS;
> }
>
> -- 
> Martin Doucha   mdoucha@suse.cz
> QA Engineer for Software Maintenance
> SUSE LINUX, s.r.o.
> CORSO IIa
> Krizikova 148/34
> 186 00 Prague 8
> Czech Republic


-- 
Thank you,
Richard.

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH 7/7] opeposix: pthread_barrierattr_getpshared/2-1: Simplify codeflow

[Richard Palethorpe]

Hello,

Martin Doucha <mdoucha@suse.cz> writes:

> Hi,
> one small suggestion below, otherwise for the whole patchset:
>
> Reviewed-by: Martin Doucha <mdoucha@suse.cz>
>
> On 20. 06. 22 11:21, Cyril Hrubis wrote:
>> This actually fixes a 'no return in nonvoid function' warning since gcc
>> may get confused during the codeflow analysis.
>> 
>> Signed-off-by: Cyril Hrubis <chrubis@suse.cz>
>> ---
>>  .../interfaces/pthread_barrierattr_getpshared/2-1.c   | 11 +++++------
>>  1 file changed, 5 insertions(+), 6 deletions(-)
>> 
>> diff --git a/testcases/open_posix_testsuite/conformance/interfaces/pthread_barrierattr_getpshared/2-1.c b/testcases/open_posix_testsuite/conformance/interfaces/pthread_barrierattr_getpshared/2-1.c
>> index a21a5a507..52c074173 100644
>> --- a/testcases/open_posix_testsuite/conformance/interfaces/pthread_barrierattr_getpshared/2-1.c
>> +++ b/testcases/open_posix_testsuite/conformance/interfaces/pthread_barrierattr_getpshared/2-1.c
>> @@ -141,7 +141,9 @@ int main(void)
>>  	if (pid == -1) {
>>  		perror("Error at fork()");
>>  		return PTS_UNRESOLVED;
>> -	} else if (pid == 0) {
>> +	}
>> +
>> +	if (pid == 0) {
>>  		/* Child */
>>  		/* Map the shared object to child's memory */
>>  		barrier =
>> @@ -209,10 +211,7 @@ int main(void)
>>  
>>  		printf("Test PASSED\n");
>>  		return PTS_PASS;
>> +	} else {
>> +		return serial;
>
> The return can be unconditional.

And with that also:

Reviewed-by: Richard Palethorpe <rpalethorpe@suse.com>

>
>>  	}
>> -
>> -	if (pid == 0) {
>> -		exit(serial);
>> -	}
>> -
>>  }
>
>
> -- 
> Martin Doucha   mdoucha@suse.cz
> QA Engineer for Software Maintenance
> SUSE LINUX, s.r.o.
> CORSO IIa
> Krizikova 148/34
> 186 00 Prague 8
> Czech Republic


-- 
Thank you,
Richard.

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH 7/7] opeposix: pthread_barrierattr_getpshared/2-1: Simplify codeflow

[Cyril Hrubis]

Hi!
> > +	} else {
> > +		return serial;
> 
> The return can be unconditional.

I changed the code a bit more in the end to make it even more readable,
now it's structured as:

	if (pid == 0)
		return serial;

	/* parent */
	if (wait(&status) != pid) {
		...
	}
	...

	printf("Test PASSED\n");
	return PTS_PASS;

-- 
Cyril Hrubis
chrubis@suse.cz

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH 0/7] openposix: Fix 'no return in nonvoid function' warnings

[Cyril Hrubis]

Hi!
I've adjusted the patches accordingly to suggestions from Martin and
pushed the patchset. Thanks everyone for the review.

-- 
Cyril Hrubis
chrubis@suse.cz

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp