| Message ID | 20241118-convert_mmap01-v3-1-b275c90035f5@suse.com |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | [v3] mmap01: Convert to new API | expand |
On Mon, Nov 18, 2024 at 11:16 AM Ricardo B. Marliere via ltp <ltp@lists.linux.it> wrote: > > From: Ricardo B. Marliere <rbm@suse.com> > > Refactor the old mmap01 code into the new LTP API. > > Signed-off-by: Ricardo B. Marliere <rbm@suse.com> Acked-by: Jan Stancek <jstancek@redhat.com> > --- > Changes in v3: > - Use SAFE_MMAP and SAFE_MUNMAP as suggested by Jan Stancek > - Re-aligned a few comments > - Link to v2: https://lore.kernel.org/r/20241114-convert_mmap01-v2-1-a8a1379dec89@suse.com > > Changes in v2: > - Fixed metadata alignment > - Link to v1: https://lore.kernel.org/r/20241113-convert_mmap01-v1-1-e7d60e7404c4@suse.com > --- > testcases/kernel/syscalls/mmap/mmap01.c | 231 +++++++++++--------------------- > 1 file changed, 76 insertions(+), 155 deletions(-) > > diff --git a/testcases/kernel/syscalls/mmap/mmap01.c b/testcases/kernel/syscalls/mmap/mmap01.c > index 99266b57ffc16e6c3a0e31a2c87d0f8106f429e5..cb60fed024af7ad7a3114ad2838a5a52d5d3a1dc 100644 > --- a/testcases/kernel/syscalls/mmap/mmap01.c > +++ b/testcases/kernel/syscalls/mmap/mmap01.c > @@ -1,57 +1,30 @@ > +// SPDX-License-Identifier: GPL-2.0-or-later > /* > * Copyright (c) International Business Machines Corp., 2001 > + * Copyright (c) 2024 Ricardo B. Marliere <rbm@suse.com> > * > - * 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 > + * HISTORY > + * 07/2001 Ported by Wayne Boyer > */ > > -/* > - * Test Description: > - * Verify that, mmap() succeeds when used to map a file where size of the > - * file is not a multiple of the page size, the memory area beyond the end > - * of the file to the end of the page is accessible. Also, verify that > - * this area is all zeroed and the modifications done to this area are > - * not written to the file. > +/*\ > + * [Description] > * > - * Expected Result: > - * mmap() should succeed returning the address of the mapped region. > - * The memory area beyond the end of file to the end of page should be > - * filled with zero. > - * The changes beyond the end of file should not get written to the file. > + * Verify that, mmap() succeeds when used to map a file where size of the > + * file is not a multiple of the page size, the memory area beyond the end > + * of the file to the end of the page is accessible. Also, verify that > + * this area is all zeroed and the modifications done to this area are > + * not written to the file. > * > - * HISTORY > - * 07/2001 Ported by Wayne Boyer > + * mmap() should succeed returning the address of the mapped region. > + * The memory area beyond the end of file to the end of page should be > + * filled with zero. > + * The changes beyond the end of file should not get written to the file. > */ > -#include <stdio.h> > -#include <stdlib.h> > -#include <sys/types.h> > -#include <errno.h> > -#include <unistd.h> > -#include <fcntl.h> > -#include <string.h> > -#include <signal.h> > -#include <stdint.h> > -#include <sys/stat.h> > -#include <sys/mman.h> > -#include <sys/shm.h> > - > -#include "test.h" > - > -#define TEMPFILE "mmapfile" > - > -char *TCID = "mmap01"; > -int TST_TOTAL = 1; > + > +#include "tst_test.h" > + > +#define TEMPFILE "mmapfile" > > static char *addr; > static char *dummy; > @@ -60,135 +33,83 @@ static size_t file_sz; > static int fildes; > static char cmd_buffer[BUFSIZ]; > > -static void setup(void); > -static void cleanup(void); > - > -int main(int ac, char **av) > +static void run(void) > { > - int lc; > - > - tst_parse_opts(ac, av, NULL, NULL); > - > - setup(); > - > - for (lc = 0; TEST_LOOPING(lc); lc++) { > - > - tst_count = 0; > - > - /* > - * Call mmap to map the temporary file beyond EOF > - * with write access. > - */ > - errno = 0; > - addr = mmap(NULL, page_sz, PROT_READ | PROT_WRITE, > - MAP_FILE | MAP_SHARED, fildes, 0); > - > - /* Check for the return value of mmap() */ > - if (addr == MAP_FAILED) { > - tst_resm(TFAIL | TERRNO, "mmap of %s failed", TEMPFILE); > - continue; > - } > - > - /* > - * Check if mapped memory area beyond EOF are > - * zeros and changes beyond EOF are not written > - * to file. > - */ > - if (memcmp(&addr[file_sz], dummy, page_sz - file_sz)) { > - tst_brkm(TFAIL, cleanup, > - "mapped memory area contains invalid " > - "data"); > - } > - > - /* > - * Initialize memory beyond file size > - */ > - addr[file_sz] = 'X'; > - addr[file_sz + 1] = 'Y'; > - addr[file_sz + 2] = 'Z'; > - > - /* > - * Synchronize the mapped memory region > - * with the file. > - */ > - if (msync(addr, page_sz, MS_SYNC) != 0) { > - tst_brkm(TFAIL | TERRNO, cleanup, > - "failed to synchronize mapped file"); > - } > - > - /* > - * Now, Search for the pattern 'XYZ' in the > - * temporary file. The pattern should not be > - * found and the return value should be 1. > - */ > - if (system(cmd_buffer) != 0) { > - tst_resm(TPASS, > - "Functionality of mmap() successful"); > - } else { > - tst_resm(TFAIL, > - "Specified pattern found in file"); > - } > - > - /* Clean up things in case we are looping */ > - /* Unmap the mapped memory */ > - if (munmap(addr, page_sz) != 0) { > - tst_brkm(TFAIL | TERRNO, NULL, "munmap failed"); > - } > + addr = SAFE_MMAP(NULL, page_sz, PROT_READ | PROT_WRITE, > + MAP_FILE | MAP_SHARED, fildes, 0); > + > + /* > + * Check if mapped memory area beyond EOF are zeros and changes beyond > + * EOF are not written to file. > + */ > + if (memcmp(&addr[file_sz], dummy, page_sz - file_sz)) > + tst_brk(TFAIL, "mapped memory area contains invalid data"); > + > + /* > + * Initialize memory beyond file size > + */ > + addr[file_sz] = 'X'; > + addr[file_sz + 1] = 'Y'; > + addr[file_sz + 2] = 'Z'; > + > + /* > + * Synchronize the mapped memory region with the file. > + */ > + if (msync(addr, page_sz, MS_SYNC) != 0) { > + tst_res(TFAIL | TERRNO, "failed to synchronize mapped file"); > + return; > } > > - cleanup(); > - tst_exit(); > + /* > + * Now, search for the pattern 'XYZ' in the temporary file. > + * The pattern should not be found and the return value should be 1. > + */ > + if (system(cmd_buffer) != 0) { > + tst_res(TPASS, "Functionality of mmap() successful"); > + } else { > + tst_res(TFAIL, "Specified pattern found in file"); > + return; > + } > + > + SAFE_MUNMAP(addr, page_sz); > +} > + > +static void cleanup(void) > +{ > + if (dummy) > + free(dummy); > + if (fildes > 0) > + SAFE_CLOSE(fildes); > } > > static void setup(void) > { > struct stat stat_buf; > - char Path_name[PATH_MAX]; > char write_buf[] = "hello world\n"; > > - tst_sig(FORK, DEF_HANDLER, cleanup); > - > - TEST_PAUSE; > - > - tst_tmpdir(); > - > - /* Get the path of temporary file to be created */ > - if (getcwd(Path_name, sizeof(Path_name)) == NULL) { > - tst_brkm(TFAIL | TERRNO, cleanup, > - "getcwd failed to get current working directory"); > - } > - > - /* Creat a temporary file used for mapping */ > - if ((fildes = open(TEMPFILE, O_RDWR | O_CREAT, 0666)) < 0) { > - tst_brkm(TFAIL, cleanup, "opening %s failed", TEMPFILE); > - } > + /* Create a temporary file used for mapping */ > + fildes = SAFE_OPEN(TEMPFILE, O_RDWR | O_CREAT, 0666); > > /* Write some data into temporary file */ > - if (write(fildes, write_buf, strlen(write_buf)) != (long)strlen(write_buf)) { > - tst_brkm(TFAIL, cleanup, "writing to %s", TEMPFILE); > - } > + SAFE_WRITE(SAFE_WRITE_ALL, fildes, write_buf, strlen(write_buf)); > > /* Get the size of temporary file */ > - if (stat(TEMPFILE, &stat_buf) < 0) { > - tst_brkm(TFAIL | TERRNO, cleanup, "stat of %s failed", > - TEMPFILE); > - } > + SAFE_STAT(TEMPFILE, &stat_buf); > file_sz = stat_buf.st_size; > > page_sz = getpagesize(); > > /* Allocate and initialize dummy string of system page size bytes */ > - if ((dummy = calloc(page_sz, sizeof(char))) == NULL) { > - tst_brkm(TFAIL, cleanup, "calloc failed (dummy)"); > - } > + dummy = SAFE_CALLOC(page_sz, sizeof(char)); > > /* Create the command which will be executed in the test */ > - sprintf(cmd_buffer, "grep XYZ %s/%s > /dev/null", Path_name, TEMPFILE); > + snprintf(cmd_buffer, sizeof(cmd_buffer), > + "grep XYZ %s > /dev/null", TEMPFILE); > } > > -static void cleanup(void) > -{ > - close(fildes); > - free(dummy); > - tst_rmdir(); > -} > +static struct tst_test test = { > + .setup = setup, > + .cleanup = cleanup, > + .needs_tmpdir = 1, > + .test_all = run, > +}; > > --- > base-commit: 865be36ffce81e795854d0bdd1cda17b4d5fd37d > change-id: 20241113-convert_mmap01-db208e002801 > > Best regards, > -- > Ricardo B. Marliere <rbm@suse.com> > > > -- > Mailing list info: https://lists.linux.it/listinfo/ltp >
Hi! > + if (system(cmd_buffer) != 0) { > + tst_res(TPASS, "Functionality of mmap() successful"); > + } else { > + tst_res(TFAIL, "Specified pattern found in file"); > + return; > + } Can we please get rid of this ugly hack as well? We can easily fork a child that would read the file and look for the pattern instead. Also we do not even have to look for the patern, just occurence of one these letters would be a failure. So we can write even easier loop that would just read the file and check that it contains only valid data.
diff --git a/testcases/kernel/syscalls/mmap/mmap01.c b/testcases/kernel/syscalls/mmap/mmap01.c index 99266b57ffc16e6c3a0e31a2c87d0f8106f429e5..cb60fed024af7ad7a3114ad2838a5a52d5d3a1dc 100644 --- a/testcases/kernel/syscalls/mmap/mmap01.c +++ b/testcases/kernel/syscalls/mmap/mmap01.c @@ -1,57 +1,30 @@ +// SPDX-License-Identifier: GPL-2.0-or-later /* * Copyright (c) International Business Machines Corp., 2001 + * Copyright (c) 2024 Ricardo B. Marliere <rbm@suse.com> * - * 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 + * HISTORY + * 07/2001 Ported by Wayne Boyer */ -/* - * Test Description: - * Verify that, mmap() succeeds when used to map a file where size of the - * file is not a multiple of the page size, the memory area beyond the end - * of the file to the end of the page is accessible. Also, verify that - * this area is all zeroed and the modifications done to this area are - * not written to the file. +/*\ + * [Description] * - * Expected Result: - * mmap() should succeed returning the address of the mapped region. - * The memory area beyond the end of file to the end of page should be - * filled with zero. - * The changes beyond the end of file should not get written to the file. + * Verify that, mmap() succeeds when used to map a file where size of the + * file is not a multiple of the page size, the memory area beyond the end + * of the file to the end of the page is accessible. Also, verify that + * this area is all zeroed and the modifications done to this area are + * not written to the file. * - * HISTORY - * 07/2001 Ported by Wayne Boyer + * mmap() should succeed returning the address of the mapped region. + * The memory area beyond the end of file to the end of page should be + * filled with zero. + * The changes beyond the end of file should not get written to the file. */ -#include <stdio.h> -#include <stdlib.h> -#include <sys/types.h> -#include <errno.h> -#include <unistd.h> -#include <fcntl.h> -#include <string.h> -#include <signal.h> -#include <stdint.h> -#include <sys/stat.h> -#include <sys/mman.h> -#include <sys/shm.h> - -#include "test.h" - -#define TEMPFILE "mmapfile" - -char *TCID = "mmap01"; -int TST_TOTAL = 1; + +#include "tst_test.h" + +#define TEMPFILE "mmapfile" static char *addr; static char *dummy; @@ -60,135 +33,83 @@ static size_t file_sz; static int fildes; static char cmd_buffer[BUFSIZ]; -static void setup(void); -static void cleanup(void); - -int main(int ac, char **av) +static void run(void) { - int lc; - - tst_parse_opts(ac, av, NULL, NULL); - - setup(); - - for (lc = 0; TEST_LOOPING(lc); lc++) { - - tst_count = 0; - - /* - * Call mmap to map the temporary file beyond EOF - * with write access. - */ - errno = 0; - addr = mmap(NULL, page_sz, PROT_READ | PROT_WRITE, - MAP_FILE | MAP_SHARED, fildes, 0); - - /* Check for the return value of mmap() */ - if (addr == MAP_FAILED) { - tst_resm(TFAIL | TERRNO, "mmap of %s failed", TEMPFILE); - continue; - } - - /* - * Check if mapped memory area beyond EOF are - * zeros and changes beyond EOF are not written - * to file. - */ - if (memcmp(&addr[file_sz], dummy, page_sz - file_sz)) { - tst_brkm(TFAIL, cleanup, - "mapped memory area contains invalid " - "data"); - } - - /* - * Initialize memory beyond file size - */ - addr[file_sz] = 'X'; - addr[file_sz + 1] = 'Y'; - addr[file_sz + 2] = 'Z'; - - /* - * Synchronize the mapped memory region - * with the file. - */ - if (msync(addr, page_sz, MS_SYNC) != 0) { - tst_brkm(TFAIL | TERRNO, cleanup, - "failed to synchronize mapped file"); - } - - /* - * Now, Search for the pattern 'XYZ' in the - * temporary file. The pattern should not be - * found and the return value should be 1. - */ - if (system(cmd_buffer) != 0) { - tst_resm(TPASS, - "Functionality of mmap() successful"); - } else { - tst_resm(TFAIL, - "Specified pattern found in file"); - } - - /* Clean up things in case we are looping */ - /* Unmap the mapped memory */ - if (munmap(addr, page_sz) != 0) { - tst_brkm(TFAIL | TERRNO, NULL, "munmap failed"); - } + addr = SAFE_MMAP(NULL, page_sz, PROT_READ | PROT_WRITE, + MAP_FILE | MAP_SHARED, fildes, 0); + + /* + * Check if mapped memory area beyond EOF are zeros and changes beyond + * EOF are not written to file. + */ + if (memcmp(&addr[file_sz], dummy, page_sz - file_sz)) + tst_brk(TFAIL, "mapped memory area contains invalid data"); + + /* + * Initialize memory beyond file size + */ + addr[file_sz] = 'X'; + addr[file_sz + 1] = 'Y'; + addr[file_sz + 2] = 'Z'; + + /* + * Synchronize the mapped memory region with the file. + */ + if (msync(addr, page_sz, MS_SYNC) != 0) { + tst_res(TFAIL | TERRNO, "failed to synchronize mapped file"); + return; } - cleanup(); - tst_exit(); + /* + * Now, search for the pattern 'XYZ' in the temporary file. + * The pattern should not be found and the return value should be 1. + */ + if (system(cmd_buffer) != 0) { + tst_res(TPASS, "Functionality of mmap() successful"); + } else { + tst_res(TFAIL, "Specified pattern found in file"); + return; + } + + SAFE_MUNMAP(addr, page_sz); +} + +static void cleanup(void) +{ + if (dummy) + free(dummy); + if (fildes > 0) + SAFE_CLOSE(fildes); } static void setup(void) { struct stat stat_buf; - char Path_name[PATH_MAX]; char write_buf[] = "hello world\n"; - tst_sig(FORK, DEF_HANDLER, cleanup); - - TEST_PAUSE; - - tst_tmpdir(); - - /* Get the path of temporary file to be created */ - if (getcwd(Path_name, sizeof(Path_name)) == NULL) { - tst_brkm(TFAIL | TERRNO, cleanup, - "getcwd failed to get current working directory"); - } - - /* Creat a temporary file used for mapping */ - if ((fildes = open(TEMPFILE, O_RDWR | O_CREAT, 0666)) < 0) { - tst_brkm(TFAIL, cleanup, "opening %s failed", TEMPFILE); - } + /* Create a temporary file used for mapping */ + fildes = SAFE_OPEN(TEMPFILE, O_RDWR | O_CREAT, 0666); /* Write some data into temporary file */ - if (write(fildes, write_buf, strlen(write_buf)) != (long)strlen(write_buf)) { - tst_brkm(TFAIL, cleanup, "writing to %s", TEMPFILE); - } + SAFE_WRITE(SAFE_WRITE_ALL, fildes, write_buf, strlen(write_buf)); /* Get the size of temporary file */ - if (stat(TEMPFILE, &stat_buf) < 0) { - tst_brkm(TFAIL | TERRNO, cleanup, "stat of %s failed", - TEMPFILE); - } + SAFE_STAT(TEMPFILE, &stat_buf); file_sz = stat_buf.st_size; page_sz = getpagesize(); /* Allocate and initialize dummy string of system page size bytes */ - if ((dummy = calloc(page_sz, sizeof(char))) == NULL) { - tst_brkm(TFAIL, cleanup, "calloc failed (dummy)"); - } + dummy = SAFE_CALLOC(page_sz, sizeof(char)); /* Create the command which will be executed in the test */ - sprintf(cmd_buffer, "grep XYZ %s/%s > /dev/null", Path_name, TEMPFILE); + snprintf(cmd_buffer, sizeof(cmd_buffer), + "grep XYZ %s > /dev/null", TEMPFILE); } -static void cleanup(void) -{ - close(fildes); - free(dummy); - tst_rmdir(); -} +static struct tst_test test = { + .setup = setup, + .cleanup = cleanup, + .needs_tmpdir = 1, + .test_all = run, +};