[LTP] [PATCH v2 1/3] mremap02: Convert to new API
Martin Doucha
mdoucha@suse.cz
Fri Mar 1 14:06:48 CET 2024
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,
> +};
--
Martin Doucha mdoucha@suse.cz
SW Quality Engineer
SUSE LINUX, s.r.o.
CORSO IIa
Krizikova 148/34
186 00 Prague 8
Czech Republic
More information about the ltp
mailing list