Re: [LTP] [PATCH v2 2/2] setitimer01: rewrite using new API

[Richard Palethorpe <rpalethorpe@suse.de>]

Hello,

Li Wang <liwang@redhat.com> writes:

>  
> -#include "test.h"
> -
>  #include <errno.h>
>  #include <sys/time.h>
> +#include <stdlib.h>
> +#include "tst_test.h"
> +#include "lapi/syscalls.h"
> +
> +#define USEC1	10000
> +#define USEC2	20000
> +
> +static struct itimerval *value, *ovalue;
> +
> +static struct tcase {
> +	int which;
> +	char *des;
> +	int signo;
> +} tcases[] = {
> +	{ITIMER_REAL,    "ITIMER_REAL",    SIGALRM},
> +	{ITIMER_VIRTUAL, "ITIMER_VIRTUAL", SIGVTALRM},
> +	{ITIMER_PROF,    "ITIMER_PROF",    SIGPROF},
> +};
> +
> +static int sys_setitimer(int which, void *new_value, void *old_value)
> +{
> +	return tst_syscall(__NR_setitimer, which, new_value, old_value);
> +}
> +
> +static void set_setitimer_value(int usec, int o_usec)
> +{
> +	value->it_value.tv_sec = 0;
> +	value->it_value.tv_usec = usec;
> +	value->it_interval.tv_sec = 0;
> +	value->it_interval.tv_usec = 0;
> +
> +	ovalue->it_value.tv_sec = o_usec;
> +	ovalue->it_value.tv_usec = o_usec;
> +	ovalue->it_interval.tv_sec = 0;
> +	ovalue->it_interval.tv_usec = 0;
> +}
>  
> -void cleanup(void);
> -void setup(void);
> +static void verify_setitimer(unsigned int i)
> +{
> +	pid_t pid;
> +	int status;
> +	struct tcase *tc = &tcases[i];
>  
> -char *TCID = "setitimer01";
> -int TST_TOTAL = 1;
> +	pid = SAFE_FORK();
>  
> -#define SEC0	0
> -#define SEC1	20
> -#define SEC2	40
> +	if (pid == 0) {
> +		tst_res(TINFO, "tc->which = %s", tc->des);
>  
> -int main(int ac, char **av)
> -{
> -	int lc;
> -	struct itimerval *value, *ovalue;
> -
> -	tst_parse_opts(ac, av, NULL, NULL);
> -
> -	setup();		/* global setup */
> -
> -	/* The following loop checks looping state if -i option given */
> -
> -	for (lc = 0; TEST_LOOPING(lc); lc++) {
> -		/* reset tst_count in case we are looping */
> -		tst_count = 0;
> -
> -		/* allocate some space for the timer structures */
> -
> -		if ((value = malloc((size_t)sizeof(struct itimerval))) ==
> -		    NULL) {
> -			tst_brkm(TBROK, cleanup, "value malloc failed");
> -		}
> -
> -		if ((ovalue = malloc((size_t)sizeof(struct itimerval))) ==
> -		    NULL) {
> -			tst_brkm(TBROK, cleanup, "ovalue malloc failed");
> -		}
> -
> -		/* set up some reasonable values */
> -
> -		value->it_value.tv_sec = SEC1;
> -		value->it_value.tv_usec = SEC0;
> -		value->it_interval.tv_sec = 0;
> -		value->it_interval.tv_usec = 0;
> -		/*
> -		 * issue the system call with the TEST() macro
> -		 * ITIMER_REAL = 0, ITIMER_VIRTUAL = 1 and ITIMER_PROF = 2
> -		 */
> -
> -		TEST(setitimer(ITIMER_REAL, value, ovalue));
> -
> -		if (TEST_RETURN != 0) {
> -			tst_resm(TFAIL, "call failed - errno = %d - %s",
> -				 TEST_ERRNO, strerror(TEST_ERRNO));
> -			continue;
> -		}
> -
> -		/*
> -		 * call setitimer again with new values.
> -		 * the old values should be stored in ovalue
> -		 */
> -		value->it_value.tv_sec = SEC2;
> -		value->it_value.tv_usec = SEC0;
> -
> -		if ((setitimer(ITIMER_REAL, value, ovalue)) == -1) {
> -			tst_brkm(TBROK, cleanup, "second setitimer "
> -				 "call failed");
> -		}
> -
> -		if (ovalue->it_value.tv_sec <= SEC1) {
> -			tst_resm(TPASS, "functionality is correct");
> -		} else {
> -			tst_brkm(TFAIL, cleanup, "old timer value is "
> -				 "not equal to expected value");
> -		}
> -	}
> +		tst_no_corefile(0);
>  
> -	cleanup();
> -	tst_exit();
> -}
> +		set_setitimer_value(USEC1, 0);
> +		TST_EXP_PASS(sys_setitimer(tc->which, value, NULL));
>  
> -/*
> - * setup() - performs all the ONE TIME setup for this test.
> - */
> -void setup(void)
> -{
> +		set_setitimer_value(USEC2, USEC2);
> +		TST_EXP_PASS(sys_setitimer(tc->which, value, ovalue));
>  
> -	tst_sig(NOFORK, DEF_HANDLER, cleanup);
> +		if (ovalue->it_value.tv_sec != 0 || ovalue->it_value.tv_usec >= USEC2)
> +			tst_brk(TFAIL, "old timer value is not within
> the expected range");

Maybe we could split testing the return value and ovalue from testing
the signal is delivered?

When testing the return value and ovalue; a very long timeout can be
used (which is never hit). When testing the signal a very short one can
be used.

This way, the test is not racing with the signal and the loop below
won't be executed much.

In any case LGTM

Reviewed-by: Richard Palethorpe <rpalethorpe@suse.com>

>  
> -	TEST_PAUSE;
> -}
> +		for (;;)
> +			;
> +	}
>  
> -/*
> - * cleanup() - performs all the ONE TIME cleanup for this test at completion
> - * 	       or premature exit.
> - */
> -void cleanup(void)
> -{
> +	SAFE_WAITPID(pid, &status, 0);
>  
> +	if (WIFSIGNALED(status) && WTERMSIG(status) == tc->signo)
> +		tst_res(TPASS, "Child received signal: %s", tst_strsig(tc->signo));
> +	else
> +		tst_res(TFAIL, "Child: %s", tst_strstatus(status));
>  }
> +
> +static struct tst_test test = {
> +	.tcnt = ARRAY_SIZE(tcases),
> +	.forks_child = 1,
> +	.test = verify_setitimer,
> +	.bufs = (struct tst_buffers[]) {
> +		{&value,  .size = sizeof(struct itimerval)},
> +		{&ovalue, .size = sizeof(struct itimerval)},
> +		{}
> +	}
> +};
> -- 
> 2.35.3


-- 
Thank you,
Richard.

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