[LTP] [PATCH v1] Refactor pidns20 test using new LTP API

[Cyril Hrubis]

Hi!
> -static void child_signal_handler(int sig, siginfo_t * si, void *unused)
> +static void child_signal_handler(LTP_ATTRIBUTE_UNUSED int sig, siginfo_t *si, LTP_ATTRIBUTE_UNUSED void *unused)
>  {
>  	if (si->si_signo != SIGUSR1)
> -		tst_resm(TBROK, "cinit: received %s unexpectedly!",
> -			 strsignal(si->si_signo));
> -	else
> -		tst_resm(TPASS, "cinit: user function is called as expected");
> +		tst_brk(TBROK, "Received %s unexpectedly!", tst_strsig(si->si_signo));

It's not safe to call tst_brk() from a signal handler.

The most safe way is to store the signo here and do the check in the
child func.

So here I would do:

	signals++;
	last_sig = si->si_signo;


And then in the main loop:

	if (singals != 1) {
		tst_res(TFAIL, "Received %i signals", signals);
		return 0;
	}

	if (last_sig != SIGUSR1) {
		...
	}

	...

> -	/* Disable broken flag */
> -	broken = 0;
> +	passed++;
>  }
>  
> -/*
> - * child_fn() - Inside container
> - */
> -int child_fn(void *arg)
> +static int child_func(LTP_ATTRIBUTE_UNUSED void *arg)
>  {
> -	pid_t pid, ppid;
> -	sigset_t newset;
>  	struct sigaction sa;
> -	char buf[5];
> +	sigset_t newset;
> +	pid_t cpid, ppid;
>  
> -	/* Setup pipe read and write ends */
> -	pid = getpid();
> +	cpid = getpid();
>  	ppid = getppid();
>  
> -	if (pid != CHILD_PID || ppid != PARENT_PID) {
> -		printf("cinit: pidns was not created properly\n");
> -		exit(1);
> +	if (cpid != 1 || ppid != 0) {
> +		tst_res(TFAIL, "Got unexpected result of cpid=%d ppid=%d", cpid, ppid);
> +		return 1;

No return 1 here, we already reported TFAIL and the non-zero return
value will cause TBROK in the test library when the process is waited
for.

>  	}
>  
> -	/* Setup pipes to communicate with parent */
> -	close(cinit_parent[0]);
> -	close(parent_cinit[1]);
> -
> -	/* Block SIGUSR1 signal */
> -	sigemptyset(&newset);
> -	sigaddset(&newset, SIGUSR1);
> -	if (sigprocmask(SIG_BLOCK, &newset, 0) == -1) {
> -		perror("cinit: sigprocmask() failed");
> -		exit(1);
> -	}
> -	tst_resm(TINFO, "cinit: blocked SIGUSR1");
> -
> -	/* Let parent to queue SIGUSR1 in pending */
> -	if (write(cinit_parent[1], "c:go", 5) != 5) {
> -		perror("cinit: pipe is broken to write");
> -		exit(1);
> -	}
> +	SAFE_SIGEMPTYSET(&newset);
> +	SAFE_SIGADDSET(&newset, SIGUSR1);
> +	SAFE_SIGPROCMASK(SIG_BLOCK, &newset, 0);
>  
> -	/* Check if parent has queued up SIGUSR1 */
> -	read(parent_cinit[0], buf, 5);
> -	if (strcmp(buf, "p:go") != 0) {
> -		printf("cinit: parent did not respond!\n");
> -		exit(1);
> -	}
> +	TST_CHECKPOINT_WAKE_AND_WAIT(0);
>  
> -	/* Now redefine handler for SIGUSR1 */
>  	sa.sa_flags = SA_SIGINFO;
> -	sigfillset(&sa.sa_mask);
> +	SAFE_SIGFILLSET(&sa.sa_mask);
>  	sa.sa_sigaction = child_signal_handler;
> -	if (sigaction(SIGUSR1, &sa, NULL) == -1) {
> -		perror("cinit: sigaction failed");
> -		exit(1);
> -	}
>  
> -	/* Unblock traffic on SIGUSR1 queue */
> -	tst_resm(TINFO, "cinit: unblocking SIGUSR1");
> -	sigprocmask(SIG_UNBLOCK, &newset, 0);
> +	SAFE_SIGACTION(SIGUSR1, &sa, NULL);
>  
> -	/* Check if new handler is called */
> -	if (broken == 1) {
> -		printf("cinit: broken flag didn't change\n");
> -		exit(1);
> +	SAFE_SIGPROCMASK(SIG_UNBLOCK, &newset, 0);
> +
> +	if (!passed){
                    ^
		    Missing space
> +		tst_res(TFAIL, "User function has not been called after unblocking signal");
> +		return 1;
>  	}
>  
> -	/* Cleanup and exit */
> -	close(cinit_parent[1]);
> -	close(parent_cinit[0]);
> -	exit(0);
> -}
> +	tst_res(TPASS, "User function is called as expected after unblocking signal");
>  
> -static void setup(void)
> -{
> -	tst_require_root();
> -	check_newpid();
> +	return 0;
>  }
>  
> -int main(void)
> +static void run(void)
>  {
> -	int status;
> -	char buf[5];
> -	pid_t cpid;
> -
> -	setup();
> -
> -	/* Create pipes for intercommunication */
> -	if (pipe(parent_cinit) == -1 || pipe(cinit_parent) == -1) {
> -		tst_brkm(TBROK | TERRNO, NULL, "pipe failed");
> -	}
> +	int ret;
>  
> -	cpid = ltp_clone_quick(CLONE_NEWPID | SIGCHLD, child_fn, NULL);
> -	if (cpid == -1) {
> -		tst_brkm(TBROK | TERRNO, NULL, "clone failed");
> -	}
> -
> -	/* Setup pipe read and write ends */
> -	close(cinit_parent[1]);
> -	close(parent_cinit[0]);
> -
> -	/* Is container ready */
> -	read(cinit_parent[0], buf, 5);
> -	if (strcmp(buf, "c:go") != 0) {
> -		tst_brkm(TBROK, NULL, "parent: container did not respond!");
> -	}
> +	ret = ltp_clone_quick(CLONE_NEWPID | SIGCHLD, child_func, NULL);
> +	if (ret < 0)
> +		tst_brk(TBROK | TERRNO, "clone failed");
>  
> -	/* Enqueue SIGUSR1 in pending signal queue of container */
> -	SAFE_KILL(NULL, cpid, SIGUSR1);
> +	TST_CHECKPOINT_WAIT(0);
>  
> -	tst_resm(TINFO, "parent: signalled SIGUSR1 to container");
> -	if (write(parent_cinit[1], "p:go", 5) != 5) {
> -		tst_brkm(TBROK | TERRNO, NULL, "write failed");
> -	}
> -
> -	/* collect exit status of child */
> -	SAFE_WAIT(NULL, &status);
> -
> -	if (WIFSIGNALED(status)) {
> -		if (WTERMSIG(status) == SIGUSR1)
> -			tst_resm(TFAIL,
> -				 "user function was not called inside cinit");
> -		else
> -			tst_resm(TBROK,
> -				 "cinit was terminated by %d",
> -				 WTERMSIG(status));
> -	}
> +	SAFE_KILL(ret, SIGUSR1);
>  
> -	/* Cleanup and exit */
> -	close(parent_cinit[1]);
> -	close(cinit_parent[0]);
> -	tst_exit();
> +	TST_CHECKPOINT_WAKE(0);
>  }
> +
> +static struct tst_test test = {
> +	.test_all = run,
> +	.needs_root = 1,
> +	.needs_checkpoints = 1,
> +};
> -- 
> 2.35.3
> 
> 
> -- 
> Mailing list info: https://lists.linux.it/listinfo/ltp

-- 
Cyril Hrubis
chrubis@suse.cz