[igt-dev] [PATCH i-g-t 2/5] lib/tests: add internal_assert_wexited/wsignaled

[Chris Wilson]

Quoting Daniel Vetter (2019-02-19 07:57:13)
> And convert everything over.
> 
> igt_segfault needed a bit of care to differentiate between a real
> death-by-signal and igt_exit mapping a child process signal death to
> an exit code.
> 
> Signed-off-by: Daniel Vetter <daniel.vetter at ffwll.ch>
> ---
>  lib/tests/igt_assert.c       |  4 ++--
>  lib/tests/igt_exit_handler.c |  6 +++---
>  lib/tests/igt_fork.c         | 10 +++++-----
>  lib/tests/igt_segfault.c     | 14 +++++---------
>  lib/tests/igt_tests_common.h | 11 +++++++++++
>  5 files changed, 26 insertions(+), 19 deletions(-)
> 
> diff --git a/lib/tests/igt_assert.c b/lib/tests/igt_assert.c
> index e3c1ec49daf2..632e15978978 100644
> --- a/lib/tests/igt_assert.c
> +++ b/lib/tests/igt_assert.c
> @@ -151,7 +151,7 @@ igt_main
>         test_to_run = test_cmpint_negative;
>         ret = do_fork();
>         igt_subtest("igt_cmpint_negative")
> -               internal_assert(WEXITSTATUS(ret) == IGT_EXIT_FAILURE);
> +               internal_assert_wexited(ret, IGT_EXIT_FAILURE);
>  
>         igt_subtest("igt_assert_fd")
>                 test_fd();
> @@ -159,5 +159,5 @@ igt_main
>         test_to_run = test_fd_negative;
>         ret = do_fork();
>         igt_subtest("igt_assert_fd_negative")
> -               internal_assert(WEXITSTATUS(ret) == IGT_EXIT_FAILURE);
> +               internal_assert_wexited(ret, IGT_EXIT_FAILURE);

Ok, exit -> exit.

>  }
> diff --git a/lib/tests/igt_exit_handler.c b/lib/tests/igt_exit_handler.c
> index 7546fde6d6f8..f8a747862c01 100644
> --- a/lib/tests/igt_exit_handler.c
> +++ b/lib/tests/igt_exit_handler.c
> @@ -118,11 +118,11 @@ int main(int argc, char **argv)
>         internal_assert(testfunc(NORMAL) == 0);
>  
>         status = testfunc(FAIL);
> -       assert(WIFEXITED(status) && WEXITSTATUS(status) == 1);
> +       internal_assert_wexited(status, 1);
>  
>         status = testfunc(SKIP);
> -       assert(WIFEXITED(status) && WEXITSTATUS(status) == IGT_EXIT_SKIP);
> +       internal_assert_wexited(status, IGT_EXIT_SKIP);
>  
>         status = testfunc(SIG);
> -       assert(WIFSIGNALED(status) && WTERMSIG(status) == SIGTERM);
> +       internal_assert_wsignaled(status, SIGTERM);
>  }

Ok.

> diff --git a/lib/tests/igt_fork.c b/lib/tests/igt_fork.c
> index 38c55d11f7ec..100031207461 100644
> --- a/lib/tests/igt_fork.c
> +++ b/lib/tests/igt_fork.c
> @@ -113,21 +113,21 @@ int main(int argc, char **argv)
>  
>         /* check that igt_assert is forwarded */
>         ret = do_fork(igt_fork_vs_assert);
> -       internal_assert(WEXITSTATUS(ret) == IGT_EXIT_FAILURE);
> +       internal_assert_wexited(ret, IGT_EXIT_FAILURE);
>  
>         /* check that igt_skip within a fork blows up */
>         ret = do_fork(igt_fork_vs_skip);
> -       internal_assert(WEXITSTATUS(ret) == SIGABRT + 128);
> +       internal_assert_wexited(ret, SIGABRT + 128);
>  
>         /* check that failure to clean up fails */
>         ret = do_fork(igt_fork_leak);
> -       internal_assert(WTERMSIG(ret) == SIGABRT);
> +       internal_assert_wsignaled(ret, SIGABRT);
>  
>         /* check that igt_waitchildren_timeout cleans up*/
>         ret = do_fork(igt_fork_timeout_leak);
> -       internal_assert(WEXITSTATUS(ret) == SIGKILL + 128);
> +       internal_assert_wexited(ret, SIGKILL + 128);
>  
>         /* check that any other process leaks are caught*/
>         ret = do_fork(plain_fork_leak);
> -       internal_assert(WTERMSIG(ret) == SIGABRT);
> +       internal_assert_wsignaled(ret, SIGABRT);
>  }

Ok.

> diff --git a/lib/tests/igt_segfault.c b/lib/tests/igt_segfault.c
> index bfbbff564fac..2a24531aaa8c 100644
> --- a/lib/tests/igt_segfault.c
> +++ b/lib/tests/igt_segfault.c
> @@ -94,10 +94,7 @@ static int do_fork(void)
>                        errno == EINTR)
>                         ;
>  
> -               if(WIFSIGNALED(status))
> -                       return WTERMSIG(status) + 128;
> -
> -               return WEXITSTATUS(status);
> +               return status;
>         }
>  }
>  
> @@ -109,20 +106,20 @@ int main(int argc, char **argv)
>         runc=false;
>         igt_info("Simple test.\n");
>         fflush(stdout);
> -       internal_assert(WTERMSIG(do_fork()) == SIGSEGV);
> +       internal_assert_wsignaled(do_fork(), SIGSEGV);
>  
>         /* Test crash in a single subtest is reported */
>         simple = false;
>         igt_info("Single subtest.\n");
>         fflush(stdout);
> -       internal_assert(WTERMSIG(do_fork()) == SIGSEGV);
> +       internal_assert_wexited(do_fork(), SIGSEGV + 128);
>  
>         /* Test crash in a subtest following a pass is reported */
>         simple = false;
>         runa=true;
>         igt_info("Passing then crashing subtest.\n");
>         fflush(stdout);
> -       internal_assert(WTERMSIG(do_fork()) == SIGSEGV);
> +       internal_assert_wexited(do_fork(), SIGSEGV + 128);
>  
>         /* Test crash in a subtest preceeding a pass is reported */
>         simple = false;
> @@ -130,8 +127,7 @@ int main(int argc, char **argv)
>         runc=true;
>         igt_info("Crashing then passing subtest.\n");
>         fflush(stdout);
> -       internal_assert(WTERMSIG(do_fork()) == SIGSEGV);
> +       internal_assert_wexited(do_fork(), SIGSEGV + 128);

Ok.

>         return 0;
>  }
> -
> diff --git a/lib/tests/igt_tests_common.h b/lib/tests/igt_tests_common.h
> index 9b347a4565d9..e66ee37c0331 100644
> --- a/lib/tests/igt_tests_common.h
> +++ b/lib/tests/igt_tests_common.h
> @@ -35,4 +35,15 @@
>   */
>  #define internal_assert assert
>  
> +static inline void internal_assert_wexited(int wstatus, int exitcode)
> +{
> +       internal_assert(WIFEXITED(wstatus) &&
> +                       WEXITSTATUS(wstatus) == exitcode);
> +}
> +
> +static inline void internal_assert_wsignaled(int wstatus, int signal)
> +{
> +       internal_assert(WIFSIGNALED(wstatus) &&
> +                       WTERMSIG(wstatus) == signal);
> +}
>  #endif

Ok. internal_assert_f() ? ;)

Reviewed-by: Chris Wilson <chris at chris-wilson.co.uk>
-Chris