Re: [LTP] [PATCH v2 1/3] syscalls/pause01: Clean up

[Cyril Hrubis <chrubis@suse.cz>]

Hi!
> Signed-off-by: Ricardo B. Marlière <rbm@suse.com>
> ---
>  testcases/kernel/syscalls/pause/pause01.c | 34 +++++++++++++------------------
>  1 file changed, 14 insertions(+), 20 deletions(-)
> 
> diff --git a/testcases/kernel/syscalls/pause/pause01.c b/testcases/kernel/syscalls/pause/pause01.c
> index c88248da0d9961c5414a694a91cf1aef40ff263a..adce0ddcf2e0639407b4e21de0a1be7b30efaa01 100644
> --- a/testcases/kernel/syscalls/pause/pause01.c
> +++ b/testcases/kernel/syscalls/pause/pause01.c
> @@ -1,10 +1,14 @@
>  // SPDX-License-Identifier: GPL-2.0-or-later
>  /*
>   * Copyright (c) 2016 Linux Test Project
> + * Copyright (c) 2025 Ricardo B. Marlière <rbm@suse.com>
>   */
> -#include <errno.h>
> -#include <signal.h>
> -#include <stdlib.h>
> +
> +/*\
> + * Verify that, pause() returns -1 and sets errno to EINTR after receipt of a
> + * signal which is caught by the calling process.
> + */
> +
>  #include "tst_test.h"
>  
>  static void sig_handler(int sig LTP_ATTRIBUTE_UNUSED)
> @@ -14,32 +18,22 @@ static void sig_handler(int sig LTP_ATTRIBUTE_UNUSED)
>  static void do_child(void)
>  {
>  	SAFE_SIGNAL(SIGINT, sig_handler);
> -
> +	TST_EXP_FAIL(pause(), EINTR);
>  	TST_CHECKPOINT_WAKE(0);
> -
> -	TEST(pause());
> -	if (TST_RET != -1)
> -		tst_res(TFAIL, "pause() succeeded unexpectedly");
> -	else if (TST_ERR == EINTR)
> -		tst_res(TPASS, "pause() interrupted with EINTR");
> -	else
> -		tst_res(TFAIL | TTERRNO, "pause() unexpected errno");
> -
> -	TST_CHECKPOINT_WAKE(0);
> -	exit(0);

This reorders how things are called which is wrong. The checkpoint wake
call before the pause() is there for a reason. The process may sleep in
kernel for any other reason not related to the test, so we need to
ensure that we are just about to call the pause() syscall before we wait
for the process state to change in the run() function.

If anything should be removed it's the second pair of checkpoints after
the pause() because the new test library has build-in timeout and there
is no reason to depend on checkpoint timeouts at the end of the test.

And there is no reason to wait() the child in the new test library
either. So (on the top of your patch) we can do:

diff --git a/testcases/kernel/syscalls/pause/pause01.c b/testcases/kernel/syscalls/pause/pause01.c
index adce0ddcf..680fa2c05 100644
--- a/testcases/kernel/syscalls/pause/pause01.c
+++ b/testcases/kernel/syscalls/pause/pause01.c
@@ -18,8 +18,8 @@ static void sig_handler(int sig LTP_ATTRIBUTE_UNUSED)
 static void do_child(void)
 {
        SAFE_SIGNAL(SIGINT, sig_handler);
-       TST_EXP_FAIL(pause(), EINTR);
        TST_CHECKPOINT_WAKE(0);
+       TST_EXP_FAIL(pause(), EINTR);
 }

 static void run(void)
@@ -32,15 +32,9 @@ static void run(void)
                exit(0);
        }

+       TST_CHECKPOINT_WAIT(0);
        TST_PROCESS_STATE_WAIT(pid, 'S', 0);
        SAFE_KILL(pid, SIGINT);
-
-       /*
-        * TST_CHECKPOINT_WAIT has built-in timeout, if pause() doesn't return,
-        * this checkpoint call will reliably end the test.
-        */
-       TST_CHECKPOINT_WAIT(0);
-       SAFE_WAIT(&status);
 }

 static struct tst_test test = {


-- 
Cyril Hrubis
chrubis@suse.cz

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