Re: [LTP] [PATCH v3 1/8] Refactor mmap03 test

[Petr Vorel <pvorel@suse.cz>]

Hi Andrea, Cyril,

...
> +++ b/testcases/kernel/syscalls/mmap/mmap03.c
> -		/*
> -		 * Check whether the mapped memory region
> -		 * has the file contents.
> -		 *
> -		 * with ia64 and PARISC/hppa, this should
> -		 * generate a SIGSEGV which will be caught below.
> -		 *
> -		 */
> -
> -		if (sigsetjmp(env, 1) == 0) {
> -			if (memcmp(dummy, addr, page_sz)) {
> -				tst_resm(TFAIL,
> -					 "mapped memory region "
> -					 "contains invalid data");
> -			} else {
> -				tst_resm(TPASS,
> -					 "mmap() functionality is "
> -					 "correct");
> -			}
> -		}
> -#if defined(__ia64__) || defined(__hppa__) || defined(__mips__)
> -		if (pass) {
> -			tst_resm(TPASS, "Got SIGSEGV as expected");
> -		} else {
> -			tst_resm(TFAIL, "Mapped memory region with NO "
> -				 "access is accessible");
> -		}
...

> +#if defined(__ia64__) || defined(__hppa__) || defined(__mips__)
> +	if (WIFSIGNALED(status) && WTERMSIG(status) == SIGSEGV) {
> +		tst_res(TPASS, "SIGSEGV has been received");

In the previous discussion [1] Cyril noted:

	The fact that memory mapped with PROT_EXEC is readable is an
	implementation detail for an architecture and not guaranteed. So the
	best result is either to expect that the child returns with 0 or dies
	with SIGSEGV, either one is correct behavior.

Therefore we don't need to check this for *any* specific architecture (e.g.
remove #if defined preprocessor wrap. And we should check either for 0 or
SIGSEGV, many tests do this, e.g. fstatfs/fstatfs02.c, clone/clone07.c.

> +	} else {
> +		tst_res(TFAIL, "Mapped memory region with NO "
> +			"access is accessible");
nit: Please no split strings (when they are reasonable long).

Kind regards,
Petr

[1] https://lore.kernel.org/ltp/Z7ykr8it7KmHjWsY@yuki.lan/

...
> +static struct tst_test test = {
> +	.test_all = run,
> +	.setup = setup,
> +	.cleanup = cleanup,
> +	.needs_tmpdir = 1,
> +	.forks_child = 1,
> +};

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