[PATCH] selftests: fixup build warnings in pidfd / clone3 tests

[PATCH] selftests: fixup build warnings in pidfd / clone3 tests

[Axel Rasmussen]

These are some trivial fixups, which were needed to build the tests with
clang and -Werror. The following issues are fixed:

- Remove various unused variables.
- In child_poll_leader_exit_test, clang isn't smart enough to realize
  syscall(SYS_exit, 0) won't return, so it complains we never return
  from a non-void function. Add an extra exit(0) to appease it.
- In test_pidfd_poll_leader_exit, ret may be branched on despite being
  uninitialized, if we have !use_waitpid. Initialize it to zero to get
  the right behavior in that case.

Signed-off-by: Axel Rasmussen <axelrasmussen@google.com>
---
 tools/testing/selftests/clone3/clone3.c    | 2 --
 tools/testing/selftests/pidfd/pidfd_test.c | 6 +++---
 tools/testing/selftests/pidfd/pidfd_wait.c | 5 ++---
 3 files changed, 5 insertions(+), 8 deletions(-)

diff --git a/tools/testing/selftests/clone3/clone3.c b/tools/testing/selftests/clone3/clone3.c
index 076cf4325f78..cd4582129c7d 100644
--- a/tools/testing/selftests/clone3/clone3.c
+++ b/tools/testing/selftests/clone3/clone3.c
@@ -126,8 +126,6 @@ static void test_clone3(uint64_t flags, size_t size, int expected,
 
 int main(int argc, char *argv[])
 {
-	pid_t pid;
-
 	uid_t uid = getuid();
 
 	ksft_print_header();
diff --git a/tools/testing/selftests/pidfd/pidfd_test.c b/tools/testing/selftests/pidfd/pidfd_test.c
index 529eb700ac26..9a2d64901d59 100644
--- a/tools/testing/selftests/pidfd/pidfd_test.c
+++ b/tools/testing/selftests/pidfd/pidfd_test.c
@@ -441,7 +441,6 @@ static void test_pidfd_poll_exec(int use_waitpid)
 {
 	int pid, pidfd = 0;
 	int status, ret;
-	pthread_t t1;
 	time_t prog_start = time(NULL);
 	const char *test_name = "pidfd_poll check for premature notification on child thread exec";
 
@@ -500,13 +499,14 @@ static int child_poll_leader_exit_test(void *args)
 	 */
 	*child_exit_secs = time(NULL);
 	syscall(SYS_exit, 0);
+	/* Never reached, but appeases compiler thinking we should return. */
+	exit(0);
 }
 
 static void test_pidfd_poll_leader_exit(int use_waitpid)
 {
 	int pid, pidfd = 0;
-	int status, ret;
-	time_t prog_start = time(NULL);
+	int status, ret = 0;
 	const char *test_name = "pidfd_poll check for premature notification on non-empty"
 				"group leader exit";
 
diff --git a/tools/testing/selftests/pidfd/pidfd_wait.c b/tools/testing/selftests/pidfd/pidfd_wait.c
index be2943f072f6..17999e082aa7 100644
--- a/tools/testing/selftests/pidfd/pidfd_wait.c
+++ b/tools/testing/selftests/pidfd/pidfd_wait.c
@@ -39,7 +39,7 @@ static int sys_waitid(int which, pid_t pid, siginfo_t *info, int options,
 
 TEST(wait_simple)
 {
-	int pidfd = -1, status = 0;
+	int pidfd = -1;
 	pid_t parent_tid = -1;
 	struct clone_args args = {
 		.parent_tid = ptr_to_u64(&parent_tid),
@@ -47,7 +47,6 @@ TEST(wait_simple)
 		.flags = CLONE_PIDFD | CLONE_PARENT_SETTID,
 		.exit_signal = SIGCHLD,
 	};
-	int ret;
 	pid_t pid;
 	siginfo_t info = {
 		.si_signo = 0,
@@ -88,7 +87,7 @@ TEST(wait_simple)
 
 TEST(wait_states)
 {
-	int pidfd = -1, status = 0;
+	int pidfd = -1;
 	pid_t parent_tid = -1;
 	struct clone_args args = {
 		.parent_tid = ptr_to_u64(&parent_tid),
-- 
2.35.0.rc2.247.g8bbb082509-goog

Re: [PATCH] selftests: fixup build warnings in pidfd / clone3 tests

[Christian Brauner]

On Thu, Jan 27, 2022 at 02:11:15PM -0800, Axel Rasmussen wrote:
> These are some trivial fixups, which were needed to build the tests with
> clang and -Werror. The following issues are fixed:
> 
> - Remove various unused variables.
> - In child_poll_leader_exit_test, clang isn't smart enough to realize
>   syscall(SYS_exit, 0) won't return, so it complains we never return
>   from a non-void function. Add an extra exit(0) to appease it.
> - In test_pidfd_poll_leader_exit, ret may be branched on despite being
>   uninitialized, if we have !use_waitpid. Initialize it to zero to get
>   the right behavior in that case.
> 
> Signed-off-by: Axel Rasmussen <axelrasmussen@google.com>
> ---

Thanks!
(Fwiw, all those tests should also be ported to use the TEST_*() harness
infra. Currently it's an annoying mix.)
Acked-by: Christian Brauner <brauner@kernel.org>

>  tools/testing/selftests/clone3/clone3.c    | 2 --
>  tools/testing/selftests/pidfd/pidfd_test.c | 6 +++---
>  tools/testing/selftests/pidfd/pidfd_wait.c | 5 ++---
>  3 files changed, 5 insertions(+), 8 deletions(-)
> 
> diff --git a/tools/testing/selftests/clone3/clone3.c b/tools/testing/selftests/clone3/clone3.c
> index 076cf4325f78..cd4582129c7d 100644
> --- a/tools/testing/selftests/clone3/clone3.c
> +++ b/tools/testing/selftests/clone3/clone3.c
> @@ -126,8 +126,6 @@ static void test_clone3(uint64_t flags, size_t size, int expected,
>  
>  int main(int argc, char *argv[])
>  {
> -	pid_t pid;
> -
>  	uid_t uid = getuid();
>  
>  	ksft_print_header();
> diff --git a/tools/testing/selftests/pidfd/pidfd_test.c b/tools/testing/selftests/pidfd/pidfd_test.c
> index 529eb700ac26..9a2d64901d59 100644
> --- a/tools/testing/selftests/pidfd/pidfd_test.c
> +++ b/tools/testing/selftests/pidfd/pidfd_test.c
> @@ -441,7 +441,6 @@ static void test_pidfd_poll_exec(int use_waitpid)
>  {
>  	int pid, pidfd = 0;
>  	int status, ret;
> -	pthread_t t1;
>  	time_t prog_start = time(NULL);
>  	const char *test_name = "pidfd_poll check for premature notification on child thread exec";
>  
> @@ -500,13 +499,14 @@ static int child_poll_leader_exit_test(void *args)
>  	 */
>  	*child_exit_secs = time(NULL);
>  	syscall(SYS_exit, 0);
> +	/* Never reached, but appeases compiler thinking we should return. */
> +	exit(0);
>  }
>  
>  static void test_pidfd_poll_leader_exit(int use_waitpid)
>  {
>  	int pid, pidfd = 0;
> -	int status, ret;
> -	time_t prog_start = time(NULL);
> +	int status, ret = 0;
>  	const char *test_name = "pidfd_poll check for premature notification on non-empty"
>  				"group leader exit";
>  
> diff --git a/tools/testing/selftests/pidfd/pidfd_wait.c b/tools/testing/selftests/pidfd/pidfd_wait.c
> index be2943f072f6..17999e082aa7 100644
> --- a/tools/testing/selftests/pidfd/pidfd_wait.c
> +++ b/tools/testing/selftests/pidfd/pidfd_wait.c
> @@ -39,7 +39,7 @@ static int sys_waitid(int which, pid_t pid, siginfo_t *info, int options,
>  
>  TEST(wait_simple)
>  {
> -	int pidfd = -1, status = 0;
> +	int pidfd = -1;
>  	pid_t parent_tid = -1;
>  	struct clone_args args = {
>  		.parent_tid = ptr_to_u64(&parent_tid),
> @@ -47,7 +47,6 @@ TEST(wait_simple)
>  		.flags = CLONE_PIDFD | CLONE_PARENT_SETTID,
>  		.exit_signal = SIGCHLD,
>  	};
> -	int ret;
>  	pid_t pid;
>  	siginfo_t info = {
>  		.si_signo = 0,
> @@ -88,7 +87,7 @@ TEST(wait_simple)
>  
>  TEST(wait_states)
>  {
> -	int pidfd = -1, status = 0;
> +	int pidfd = -1;
>  	pid_t parent_tid = -1;
>  	struct clone_args args = {
>  		.parent_tid = ptr_to_u64(&parent_tid),
> -- 
> 2.35.0.rc2.247.g8bbb082509-goog
> 

Re: [PATCH] selftests: fixup build warnings in pidfd / clone3 tests

[Shuah Khan]

On 1/28/22 1:57 AM, Christian Brauner wrote:
> On Thu, Jan 27, 2022 at 02:11:15PM -0800, Axel Rasmussen wrote:
>> These are some trivial fixups, which were needed to build the tests with
>> clang and -Werror. The following issues are fixed:
>>
>> - Remove various unused variables.
>> - In child_poll_leader_exit_test, clang isn't smart enough to realize
>>    syscall(SYS_exit, 0) won't return, so it complains we never return
>>    from a non-void function. Add an extra exit(0) to appease it.
>> - In test_pidfd_poll_leader_exit, ret may be branched on despite being
>>    uninitialized, if we have !use_waitpid. Initialize it to zero to get
>>    the right behavior in that case.
>>
>> Signed-off-by: Axel Rasmussen <axelrasmussen@google.com>
>> ---
> 
> Thanks!
> (Fwiw, all those tests should also be ported to use the TEST_*() harness
> infra. Currently it's an annoying mix.)
> Acked-by: Christian Brauner <brauner@kernel.org>
> 

Yes. Porting would be great. I will take this for now for 5.17-rc4

thanks,
-- Shuah