[LTP] [PATCH 1/2] syscalls/pause02: Refactor into new API

[Andrea Cervesato]

Hi!

On 2/17/25 23:01, Ricardo B. Marlière wrote:
> Signed-off-by: Ricardo B. Marlière <ricardo@marliere.net>
> ---
>   testcases/kernel/syscalls/pause/pause02.c | 159 ++++++++----------------------
>   1 file changed, 41 insertions(+), 118 deletions(-)
>
> diff --git a/testcases/kernel/syscalls/pause/pause02.c b/testcases/kernel/syscalls/pause/pause02.c
> index 0853232fd07b2c6278049dd4472a1ee8e7ab7646..05706ab6ad07119763e29dd878f80a3de8f7bbb9 100644
> --- a/testcases/kernel/syscalls/pause/pause02.c
> +++ b/testcases/kernel/syscalls/pause/pause02.c
> @@ -1,20 +1,8 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
>   /*
>    * Copyright (c) International Business Machines  Corp., 2001
>    * Copyright (c) 2012 Cyril Hrubis <chrubis@suse.cz>
> - *
> - * This program is free software;  you can redistribute it and/or modify
> - * it under the terms of the GNU General Public License as published by
> - * the Free Software Foundation; either version 2 of the License, or
> - * (at your option) any later version.
> - *
> - * This program is distributed in the hope that it will be useful,
> - * but WITHOUT ANY WARRANTY;  without even the implied warranty of
> - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See
> - * the GNU General Public License for more details.
> - *
> - * You should have received a copy of the GNU General Public License
> - * along with this program;  if not, write to the Free Software
> - * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
> + * Copyright (c) Linux Test Project, 2025
You can add your e-mail here. Please take a look at other refactoring tests.
>    */
>   
>   /*
Missing backlash in the description: /*\
> @@ -23,125 +11,60 @@
>    * process will resume execution from the point of suspension.
>    */
>   
> -#include <unistd.h>
> -#include <errno.h>
> -#include <fcntl.h>
> -#include <sys/wait.h>
> +#include "tst_test.h"
>   
> -#include "test.h"
> -
> -char *TCID = "pause02";
> -int TST_TOTAL = 1;
> -static pid_t cpid;
> -
> -static void do_child(void);
> -static void setup(void);
> -static void cleanup(void);
> -
> -int main(int ac, char **av)
> -{
> -	int lc;
> -	int status;
> -
> -	tst_parse_opts(ac, av, NULL, NULL);
> -
> -	setup();
> -
> -	for (lc = 0; TEST_LOOPING(lc); lc++) {
> -		tst_count = 0;
> -
> -		cpid = tst_fork();
> -		switch (cpid) {
> -		case -1:
> -			tst_brkm(TBROK, cleanup, "fork() failed");
> -		break;
> -		case 0:
> -			do_child();
> -		break;
> -		default:
> -		break;
> -		}
> -
> -		/*
> -		 * Wait for child to enter pause().
> -		 */
> -		TST_PROCESS_STATE_WAIT(cleanup, cpid, 'S');
> -
> -		/*
> -		 * Send the SIGINT signal now, so that child
> -		 * returns from pause and resumes execution.
> -		 */
> -		kill(cpid, SIGINT);
> -
> -		wait(&status);
> -
> -		if (WIFEXITED(status)) {
> -			if (WEXITSTATUS(status) == 0)
> -				tst_resm(TPASS, "pause was interrupted correctly");
> -			else
> -				tst_resm(TFAIL, "pause was interrupted but the "
> -				                "retval and/or errno was wrong");
> -			continue;
> -		}
> -
> -		if (WIFSIGNALED(status)) {
> -			switch (WTERMSIG(status)) {
> -			case SIGALRM:
> -				tst_resm(TFAIL, "Timeout: SIGINT wasn't received by child");
> -			break;
> -			default:
> -				tst_resm(TFAIL, "Child killed by signal");
> -			}
> -
> -			continue;
> -		}
> -
> -		tst_resm(TFAIL, "Pause was not interrupted");
> -	}
> -
> -	cleanup();
> -	tst_exit();
> -}
> -
> -static void sig_handle(int sig)
> +static void sig_handler(int sig)
We can use LTP_ATTRIBUTE_UNUSED in the sig definition in order to avoid 
compile warnings.
>   {
>   }
>   
>   static void do_child(void)
>   {
> -	/* avoid LTP framework to do whatever it likes */
> -	signal(SIGALRM, SIG_DFL);
> -
> -	if (signal(SIGINT, sig_handle) == SIG_ERR) {
> -		fprintf(stderr, "Child: Failed to setup signal handler\n");
> -		exit(1);
> -	}
> +	SAFE_SIGNAL(SIGALRM, SIG_DFL);
> +	SAFE_SIGNAL(SIGINT, sig_handler);
>   
>   	/* Commit suicide after 10 seconds */
no need for comment here
>   	alarm(10);
>   
>   	TEST(pause());
> +	if (TST_RET == -1 && TST_ERR == EINTR)
> +		exit(0);
> +	else
> +		tst_res(TFAIL | TTERRNO, "pause() unexpected errno");
TST_EXP_FAIL(pause(), EINTR)
>   
> -	if (TEST_RETURN == -1) {
> -		if (TEST_ERRNO == EINTR)
> -			exit(0);
> -
> -		fprintf(stderr, "Child: Pause returned -1 but errno is %d (%s)\n",
> -		        TEST_ERRNO, strerror(TEST_ERRNO));
> -		exit(1);
> -	}
> -
> -	fprintf(stderr, "Child: Pause returned %ld\n", TEST_RETURN);
>   	exit(1);
no need to use exit() since...
>   }
>   
> -static void setup(void)
> +void run(void)
>   {
> -	tst_sig(FORK, DEF_HANDLER, cleanup);
> -
> -	TEST_PAUSE;
> +	int status;
> +	pid_t pid;
> +
> +	pid = SAFE_FORK();
> +	if (!pid)
> +		do_child();
...we use exit(0) here
> +
> +	TST_PROCESS_STATE_WAIT(pid, 'S', 10000);
> +	kill(pid, SIGINT);
SAFE_KILL()
> +	SAFE_WAITPID(pid, &status, 0);
> +
> +	if (WIFEXITED(status)) {
> +		if (WEXITSTATUS(status) == 0)
> +			tst_res(TPASS, "pause() was interrupted correctly");
> +		else
> +			tst_res(TFAIL, "Child exited with %i",
> +				WEXITSTATUS(status));

We can sum it up like this:

     TST_EXP_EXPR(WIFEXITED(status) && !WEXITSTATUS(status),
            "pause() was interrupted correctly");
> +	} else if (WIFSIGNALED(status)) {
> +		if (WTERMSIG(status) == SIGALRM)
> +			tst_res(TFAIL,
> +				"Timeout: SIGINT wasn't received by child");
> +		else
> +			tst_res(TFAIL, "Child killed by signal");
> +	} else {
> +		tst_res(TFAIL, "pause() was not interrupted");
> +	}
We can keep this statement for debugging reasons.
>   }
>   
> -static void cleanup(void)
> -{
> -}
> +static struct tst_test test = {
> +	.test_all = run,
> +	.forks_child = 1,
> +};
>
Andrea