[v2,1/3] mremap02: Convert to new API

Signed-off-by: Yang Xu <xuyang2018.jy@fujitsu.com>
---
 testcases/kernel/syscalls/mremap/mremap02.c | 195 ++++----------------
 1 file changed, 40 insertions(+), 155 deletions(-)

On Tuesday, February 27, 2024 9:42:42 AM CET Yang Xu via ltp wrote:
> Signed-off-by: Yang Xu <xuyang2018.jy@fujitsu.com>
> ---
>  testcases/kernel/syscalls/mremap/mremap02.c | 195 ++++----------------
>  1 file changed, 40 insertions(+), 155 deletions(-)
> 
> diff --git a/testcases/kernel/syscalls/mremap/mremap02.c
> b/testcases/kernel/syscalls/mremap/mremap02.c index 2dabc6847..9e4a67c79
> 100644
> --- a/testcases/kernel/syscalls/mremap/mremap02.c
> +++ b/testcases/kernel/syscalls/mremap/mremap02.c
> @@ -1,178 +1,63 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
>  /*
> - *
> - *   Copyright (c) International Business Machines  Corp., 2001
> - *
> - *   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) International Business Machines  Corp.,
> 2001
> + * Copyright (c) Linux Test Project, 2001-2024
> + * 07/2001 Ported by Wayne Boyer
>   */
> 
> -/*
> - * Test Name: mremap02
> - *
> - * Test Description:
> - *  Verify that,
> - *   mremap() fails when used to expand the existing virtual memory mapped
> - *   region to the requested size, if the virtual memory area previously
> - *   mapped was not page aligned or invalid argument specified.
> - *
> - * Expected Result:
> - *  mremap() should return -1 and set errno to EINVAL.
> - *
> - * Algorithm:
> - *  Setup:
> - *   Setup signal handling.
> - *   Pause for SIGUSR1 if option specified.
> - *
> - *  Test:
> - *   Loop if the proper options are given.
> - *   Execute system call
> - *   Check return code, if system call failed (return=-1)
> - *	if errno set == expected errno
> - *		Issue sys call fails with expected return value and errno.
> - *	Otherwise,
> - *		Issue sys call fails with unexpected errno.
> - *   Otherwise,
> - *	Issue sys call returns unexpected value.
> - *
> - *  Cleanup:
> - *   Print errno log and/or timing stats if options given
> +/*\
> + * [Description]
>   *
> - * Usage:  <for command-line>
> - *  mremap02 [-c n] [-e] [-i n] [-I x] [-P x] [-t]
> - *     where,  -c n : Run n copies concurrently.
> - *	       -e   : Turn on errno logging.
> - *	       -i n : Execute test n times.
> - *	       -I x : Execute test for x seconds.
> - *	       -p x : Pause for x seconds between iterations.
> - *	       -t   : Turn on syscall timing.
> + * Test for EINVAL error.
>   *
> - * HISTORY
> - *	07/2001 Ported by Wayne Boyer
> - *
> - *      11/09/2001 Manoj Iyer (manjo@austin.ibm.com)
> - *      Modified.
> - *      - #include <linux/mman.h> should not be included as per man page
> for - *        mremap, #include <sys/mman.h> alone should do the job. But
> inorder - *        to include definition of MREMAP_MAYMOVE defined in
> bits/mman.h - *        (included by sys/mman.h) __USE_GNU needs to be
> defined. - *        There may be a more elegant way of doing this...
> - *
> - *
> - * RESTRICTIONS:
> - *  None.
> + * - mremap fail with virtual memory area previously mapped was not page
> aligned. */
> -#define _GNU_SOURCE
> -#include <errno.h>
> -#include <unistd.h>
> -#include <fcntl.h>
> -#include <sys/mman.h>
> -
> -#include "test.h"
> 
> -char *TCID = "mremap02";
> -int TST_TOTAL = 1;
> -char *addr;			/* addr of memory mapped region */
> -int memsize;			/* memory mapped size */
> -int newsize;			/* new size of virtual memory block */
> +#define _GNU_SOURCE
> +#include "tst_test.h"
> 
> -void setup();			/* Main setup function of test */
> -void cleanup();			/* cleanup function for the test */
> +static char *addr;
> +static int memsize;
> +static int newsize;
> 
> -int main(int ac, char **av)
> +static void verify_mremap(void)
>  {
> -	int lc;
> -
> -	tst_parse_opts(ac, av, NULL, NULL);
> -
> -	setup();
> -
> -	for (lc = 0; TEST_LOOPING(lc); lc++) {
> +	errno = 0;
> +	addr = mremap(addr, memsize, newsize, MREMAP_MAYMOVE);
> +	TST_ERR = errno;
> 
> -		tst_count = 0;
> -
> -		/*
> -		 * Attempt to expand the existing mapped
> -		 * memory region (memsize) by newsize limits using
> -		 * mremap() should fail as old virtual address is not
> -		 * page aligned.
> -		 */
> -		errno = 0;
> -		addr = mremap(addr, memsize, newsize, MREMAP_MAYMOVE);
> -		TEST_ERRNO = errno;
> -
> -		/* Check for the return value of mremap() */
> -		if (addr != MAP_FAILED) {
> -			tst_resm(TFAIL,
> -				 "mremap returned invalid value, expected: -1");
should be - "expected: (void *)-1"
> -
> -			/* Unmap the mapped memory region */
> -			if (munmap(addr, newsize) != 0) {
> -				tst_brkm(TBROK, cleanup, "munmap fails to "
> -					 "unmap the expanded memory region, "
> -					 "error=%d", errno);
> -			}
> -			continue;
> -		}
> -
> -		if (errno == EINVAL) {
> -			tst_resm(TPASS, "mremap() Failed, 'invalid argument "
> -				 "specified' - errno %d", TEST_ERRNO);
> -		} else {
> -			tst_resm(TFAIL, "mremap() Failed, "
> -				 "'Unexpected errno %d", TEST_ERRNO);
> -		}
> +	if (addr != MAP_FAILED) {
> +		tst_res(TFAIL | TTERRNO,
> +			 "mremap returned invalid value, expected: -1");
>  	}
> 
> -	cleanup();
> -	tst_exit();
> -
> +	if (errno == EINVAL) {
> +		tst_res(TPASS | TTERRNO, "mremap() Failed, 'invalid argument "
> +			 "specified' - errno %d", TST_ERR);
I think, 'errno %d", TST_ERR)' is not required if we are using TTERRNO here.

> +	} else {
> +		tst_res(TFAIL | TTERRNO, "mremap() Failed, "
> +			 "'Unexpected errno %d", TST_ERR);
> +	}
>  }
> 
> -/*
> - * setup() - performs all ONE TIME setup for this test.
> - *
> - * Get system page size, Set the size of virtual memory area and the
> - * new size after resize, Set the virtual memory address such that it
> - * is not aligned.
> - */
> -void setup(void)
> -{
> 
> -	tst_sig(FORK, DEF_HANDLER, cleanup);
> -
> -	TEST_PAUSE;
> -
> -	/* Get the system page size */
> -	if ((memsize = getpagesize()) < 0) {
> -		tst_brkm(TFAIL, NULL,
> -			 "getpagesize() fails to get system page size");
> -	}
> -
> -	/* Get the New size of virtual memory block after resize */
> +static void setup(void)
> +{
> +	memsize = SAFE_SYSCONF(_SC_PAGESIZE);
> +	addr = SAFE_MMAP(NULL, memsize, PROT_WRITE,
> +			MAP_PRIVATE | MAP_ANONYMOUS, 0, 0);
>  	newsize = (memsize * 2);
> -
> -	/* Set the old virtual memory address */
>  	addr = (char *)(addr + (memsize - 1));
We should use a different variable and not overwrite addr so we can
successfully munmap in cleanup().
>  }
> 
> -/*
> - * cleanup() - performs all ONE TIME cleanup for this test at
> - *             completion or premature exit.
> - */
> -void cleanup(void)
> +static void cleanup(void)
>  {
> -
> -	/* Exit the program */
> +	if (addr != MAP_FAILED)
> +		SAFE_MUNMAP(addr, newsize);
we need to unmap the original mapping which we did in setup() and if the
testcase is failing (remap() somehow succeeds) then we need to munmap the
new mapping inside the main test function.

> 
>  }
> +static struct tst_test test = {
> +	.test_all = verify_mremap,
> +	.setup = setup,
> +	.cleanup = cleanup,
> +};


Regards,
Avinesh

Hi,
some suggestions below.

On 27. 02. 24 9:42, Yang Xu via ltp wrote:
> Signed-off-by: Yang Xu <xuyang2018.jy@fujitsu.com>
> ---
>   testcases/kernel/syscalls/mremap/mremap02.c | 195 ++++----------------
>   1 file changed, 40 insertions(+), 155 deletions(-)
> 
> diff --git a/testcases/kernel/syscalls/mremap/mremap02.c b/testcases/kernel/syscalls/mremap/mremap02.c
> index 2dabc6847..9e4a67c79 100644
> --- a/testcases/kernel/syscalls/mremap/mremap02.c
> +++ b/testcases/kernel/syscalls/mremap/mremap02.c
> @@ -1,178 +1,63 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
>   /*
> - *
> - *   Copyright (c) International Business Machines  Corp., 2001
> - *
> - *   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) International Business Machines  Corp., 2001
> + * Copyright (c) Linux Test Project, 2001-2024
> + * 07/2001 Ported by Wayne Boyer
>    */
>   
> -/*
> - * Test Name: mremap02
> - *
> - * Test Description:
> - *  Verify that,
> - *   mremap() fails when used to expand the existing virtual memory mapped
> - *   region to the requested size, if the virtual memory area previously
> - *   mapped was not page aligned or invalid argument specified.
> - *
> - * Expected Result:
> - *  mremap() should return -1 and set errno to EINVAL.
> - *
> - * Algorithm:
> - *  Setup:
> - *   Setup signal handling.
> - *   Pause for SIGUSR1 if option specified.
> - *
> - *  Test:
> - *   Loop if the proper options are given.
> - *   Execute system call
> - *   Check return code, if system call failed (return=-1)
> - *	if errno set == expected errno
> - *		Issue sys call fails with expected return value and errno.
> - *	Otherwise,
> - *		Issue sys call fails with unexpected errno.
> - *   Otherwise,
> - *	Issue sys call returns unexpected value.
> - *
> - *  Cleanup:
> - *   Print errno log and/or timing stats if options given
> +/*\
> + * [Description]
>    *
> - * Usage:  <for command-line>
> - *  mremap02 [-c n] [-e] [-i n] [-I x] [-P x] [-t]
> - *     where,  -c n : Run n copies concurrently.
> - *	       -e   : Turn on errno logging.
> - *	       -i n : Execute test n times.
> - *	       -I x : Execute test for x seconds.
> - *	       -p x : Pause for x seconds between iterations.
> - *	       -t   : Turn on syscall timing.
> + * Test for EINVAL error.
>    *
> - * HISTORY
> - *	07/2001 Ported by Wayne Boyer
> - *
> - *      11/09/2001 Manoj Iyer (manjo@austin.ibm.com)
> - *      Modified.
> - *      - #include <linux/mman.h> should not be included as per man page for
> - *        mremap, #include <sys/mman.h> alone should do the job. But inorder
> - *        to include definition of MREMAP_MAYMOVE defined in bits/mman.h
> - *        (included by sys/mman.h) __USE_GNU needs to be defined.
> - *        There may be a more elegant way of doing this...
> - *
> - *
> - * RESTRICTIONS:
> - *  None.
> + * - mremap fail with virtual memory area previously mapped was not page aligned.
>    */
> -#define _GNU_SOURCE
> -#include <errno.h>
> -#include <unistd.h>
> -#include <fcntl.h>
> -#include <sys/mman.h>
> -
> -#include "test.h"
>   
> -char *TCID = "mremap02";
> -int TST_TOTAL = 1;
> -char *addr;			/* addr of memory mapped region */
> -int memsize;			/* memory mapped size */
> -int newsize;			/* new size of virtual memory block */
> +#define _GNU_SOURCE
> +#include "tst_test.h"
>   
> -void setup();			/* Main setup function of test */
> -void cleanup();			/* cleanup function for the test */
> +static char *addr;
> +static int memsize;
> +static int newsize;
>   
> -int main(int ac, char **av)
> +static void verify_mremap(void)
>   {
> -	int lc;
> -
> -	tst_parse_opts(ac, av, NULL, NULL);
> -
> -	setup();
> -
> -	for (lc = 0; TEST_LOOPING(lc); lc++) {
> +	errno = 0;
> +	addr = mremap(addr, memsize, newsize, MREMAP_MAYMOVE);
> +	TST_ERR = errno;

You can simply use TESTPTR(mremap(...)); here and then check TST_RET_PTR 
value. That macro will handle errno for you.

>   
> -		tst_count = 0;
> -
> -		/*
> -		 * Attempt to expand the existing mapped
> -		 * memory region (memsize) by newsize limits using
> -		 * mremap() should fail as old virtual address is not
> -		 * page aligned.
> -		 */
> -		errno = 0;
> -		addr = mremap(addr, memsize, newsize, MREMAP_MAYMOVE);
> -		TEST_ERRNO = errno;
> -
> -		/* Check for the return value of mremap() */
> -		if (addr != MAP_FAILED) {
> -			tst_resm(TFAIL,
> -				 "mremap returned invalid value, expected: -1");
> -
> -			/* Unmap the mapped memory region */
> -			if (munmap(addr, newsize) != 0) {
> -				tst_brkm(TBROK, cleanup, "munmap fails to "
> -					 "unmap the expanded memory region, "
> -					 "error=%d", errno);
> -			}
> -			continue;
> -		}
> -
> -		if (errno == EINVAL) {
> -			tst_resm(TPASS, "mremap() Failed, 'invalid argument "
> -				 "specified' - errno %d", TEST_ERRNO);
> -		} else {
> -			tst_resm(TFAIL, "mremap() Failed, "
> -				 "'Unexpected errno %d", TEST_ERRNO);
> -		}
> +	if (addr != MAP_FAILED) {
> +		tst_res(TFAIL | TTERRNO,
> +			 "mremap returned invalid value, expected: -1");

The expected value is MAP_FAILED. It's value might theoretically be 
different from -1.

Also, you should unmap the new mapping here, set addr = MAP_FAILED; and 
return from the function. The errno checks are pointless if the call 
succeeded.

You should also consider either using tst_brk(TBROK) here or moving the 
initial SAFE_MMAP() from setup() to the start of this function. 
Otherwise when the test runs in multiple loops (-i/-I parameters), a 
successful mremap() will break the test mapping for all further loops.

>   	}
>   
> -	cleanup();
> -	tst_exit();
> -
> +	if (errno == EINVAL) {

You want to check TST_ERR here.

> +		tst_res(TPASS | TTERRNO, "mremap() Failed, 'invalid argument "
> +			 "specified' - errno %d", TST_ERR);

tst_res() will print the error code and name automatically when you pass 
the TTERRNO flag.

> +	} else {
> +		tst_res(TFAIL | TTERRNO, "mremap() Failed, "
> +			 "'Unexpected errno %d", TST_ERR);
> +	}
>   }
>   
> -/*
> - * setup() - performs all ONE TIME setup for this test.
> - *
> - * Get system page size, Set the size of virtual memory area and the
> - * new size after resize, Set the virtual memory address such that it
> - * is not aligned.
> - */
> -void setup(void)
> -{
>   
> -	tst_sig(FORK, DEF_HANDLER, cleanup);
> -
> -	TEST_PAUSE;
> -
> -	/* Get the system page size */
> -	if ((memsize = getpagesize()) < 0) {
> -		tst_brkm(TFAIL, NULL,
> -			 "getpagesize() fails to get system page size");
> -	}
> -
> -	/* Get the New size of virtual memory block after resize */
> +static void setup(void)
> +{
> +	memsize = SAFE_SYSCONF(_SC_PAGESIZE);
> +	addr = SAFE_MMAP(NULL, memsize, PROT_WRITE,
> +			MAP_PRIVATE | MAP_ANONYMOUS, 0, 0);
>   	newsize = (memsize * 2);
> -
> -	/* Set the old virtual memory address */
>   	addr = (char *)(addr + (memsize - 1));

You should keep the original pointer for cleanup(). It'd be much better 
to explicitly call mremap(addr + offset, ...) in the main test function.

>   }
>   
> -/*
> - * cleanup() - performs all ONE TIME cleanup for this test at
> - *             completion or premature exit.
> - */
> -void cleanup(void)
> +static void cleanup(void)
>   {
> -
> -	/* Exit the program */
> +	if (addr != MAP_FAILED)
> +		SAFE_MUNMAP(addr, newsize);

This should be SAFE_MUNMAP(addr, memsize); The mapping size is not 
supposed to change during the test and the next page may also be mapped 
e.g. by libc.

>   
>   }
> +static struct tst_test test = {
> +	.test_all = verify_mremap,
> +	.setup = setup,
> +	.cleanup = cleanup,
> +};