| Message ID | 20220127221115.3731388-1-axelrasmussen@google.com (mailing list archive) |
|---|---|
| State | Accepted |
| Headers | show |
| Series | selftests: fixup build warnings in pidfd / clone3 tests | expand |
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 >
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
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),
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(-)