On Tue, Oct 15, 2019 at 11:13:49AM +0800, Wei Yang wrote: > This file uses quite a different code style and changing just one line > would leads to some awkward appearance. > > This is a preparation for the following replacement of > sysconf(_SC_PAGESIZE). > > BTW, to depress ERROR message from checkpatch.pl, this patch replaces > strtoul with qemu_strtoul. > > Signed-off-by: Wei Yang <richardw.y...@linux.intel.com> > --- > tests/tcg/multiarch/test-mmap.c | 67 ++++++++++++++++++--------------- > 1 file changed, 36 insertions(+), 31 deletions(-) > > diff --git a/tests/tcg/multiarch/test-mmap.c b/tests/tcg/multiarch/test-mmap.c > index 11d0e777b1..9ea49e2307 100644 > --- a/tests/tcg/multiarch/test-mmap.c > +++ b/tests/tcg/multiarch/test-mmap.c > @@ -456,49 +456,54 @@ void check_invalid_mmaps(void) > > int main(int argc, char **argv) > { > - char tempname[] = "/tmp/.cmmapXXXXXX"; > - unsigned int i; > - > - /* Trust the first argument, otherwise probe the system for our > - pagesize. */ > - if (argc > 1) > - pagesize = strtoul(argv[1], NULL, 0); > - else > - pagesize = sysconf(_SC_PAGESIZE); > + char tempname[] = "/tmp/.cmmapXXXXXX"; > + unsigned int i; > + > + /* > + * Trust the first argument, otherwise probe the system for our > + * pagesize. > + */ > + if (argc > 1) { > + qemu_strtoul(argv[1], NULL, 0, &pagesize);
What do you think about doing this change in a separate patch? > + } else { > + pagesize = sysconf(_SC_PAGESIZE); > + } > > - /* Assume pagesize is a power of two. */ > - pagemask = pagesize - 1; > - dummybuf = malloc (pagesize); > - printf ("pagesize=%u pagemask=%x\n", pagesize, pagemask); > + /* Assume pagesize is a power of two. */ > + pagemask = pagesize - 1; > + dummybuf = malloc(pagesize); > + printf("pagesize=%u pagemask=%x\n", pagesize, pagemask); > > - test_fd = mkstemp(tempname); > - unlink(tempname); > + test_fd = mkstemp(tempname); > + unlink(tempname); > > - /* Fill the file with int's counting from zero and up. */ > + /* Fill the file with int's counting from zero and up. */ > for (i = 0; i < (pagesize * 4) / sizeof i; i++) { > checked_write(test_fd, &i, sizeof i); > } > > - /* Append a few extra writes to make the file end at non > - page boundary. */ > + /* > + * Append a few extra writes to make the file end at non > + * page boundary. > + */ > checked_write(test_fd, &i, sizeof i); i++; > checked_write(test_fd, &i, sizeof i); i++; > checked_write(test_fd, &i, sizeof i); i++; > > - test_fsize = lseek(test_fd, 0, SEEK_CUR); > + test_fsize = lseek(test_fd, 0, SEEK_CUR); > > - /* Run the tests. */ > - check_aligned_anonymous_unfixed_mmaps(); > - check_aligned_anonymous_unfixed_colliding_mmaps(); > - check_aligned_anonymous_fixed_mmaps(); > - check_file_unfixed_mmaps(); > - check_file_fixed_mmaps(); > - check_file_fixed_eof_mmaps(); > - check_file_unfixed_eof_mmaps(); > - check_invalid_mmaps(); > + /* Run the tests. */ > + check_aligned_anonymous_unfixed_mmaps(); > + check_aligned_anonymous_unfixed_colliding_mmaps(); > + check_aligned_anonymous_fixed_mmaps(); > + check_file_unfixed_mmaps(); > + check_file_fixed_mmaps(); > + check_file_fixed_eof_mmaps(); > + check_file_unfixed_eof_mmaps(); > + check_invalid_mmaps(); > > - /* Fails at the moment. */ > - /* check_aligned_anonymous_fixed_mmaps_collide_with_host(); */ > + /* Fails at the moment. */ > + /* check_aligned_anonymous_fixed_mmaps_collide_with_host(); */ > > - return EXIT_SUCCESS; > + return EXIT_SUCCESS; > } The rest looks good to me. Thanks, Stefano