Re: [LTP] [PATCH v2 1/3] mremap02: Convert to new API

[Avinesh Kumar <akumar@suse.de>]

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



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