* [PATCH 0/2] replace sysconf(_SC_PAGESIZE) with qemu_real_host_page_size @ 2019-10-15 3:13 Wei Yang 2019-10-15 3:13 ` [PATCH 1/2] tests/tcg/multiarch: fix code style in function main of test-mmap.c Wei Yang ` (2 more replies) 0 siblings, 3 replies; 10+ messages in thread From: Wei Yang @ 2019-10-15 3:13 UTC (permalink / raw) To: kwolf, mreitz, jasowang, alex.bennee, dgilbert, richard.henderson Cc: qemu-devel, qemu-block, Wei Yang This is a following up patch to cleanup page size, suggested by "Dr. David Alan Gilbert" <dgilbert@redhat.com>. Patch 2 does the job, while during the cleanup I found test-mmap.c has quite a lot code style problem. To make the code looks good, patch 1 is introduced to make checkpatch.pl happy a little. Wei Yang (2): tests/tcg/multiarch: fix code style in function main of test-mmap.c core: replace sysconf(_SC_PAGESIZE) with qemu_real_host_page_size block/file-posix.c | 2 +- net/l2tpv3.c | 2 +- tests/tcg/multiarch/test-mmap.c | 67 ++++++++++++++++++--------------- 3 files changed, 38 insertions(+), 33 deletions(-) -- 2.17.1 ^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH 1/2] tests/tcg/multiarch: fix code style in function main of test-mmap.c 2019-10-15 3:13 [PATCH 0/2] replace sysconf(_SC_PAGESIZE) with qemu_real_host_page_size Wei Yang @ 2019-10-15 3:13 ` Wei Yang 2019-11-11 9:57 ` Stefano Garzarella 2019-11-11 10:25 ` Alex Bennée 2019-10-15 3:13 ` [PATCH 2/2] core: replace sysconf(_SC_PAGESIZE) with qemu_real_host_page_size Wei Yang 2019-11-09 1:25 ` [PATCH 0/2] " Wei Yang 2 siblings, 2 replies; 10+ messages in thread From: Wei Yang @ 2019-10-15 3:13 UTC (permalink / raw) To: kwolf, mreitz, jasowang, alex.bennee, dgilbert, richard.henderson Cc: qemu-devel, qemu-block, Wei Yang 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.yang@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); + } 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; } -- 2.17.1 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH 1/2] tests/tcg/multiarch: fix code style in function main of test-mmap.c 2019-10-15 3:13 ` [PATCH 1/2] tests/tcg/multiarch: fix code style in function main of test-mmap.c Wei Yang @ 2019-11-11 9:57 ` Stefano Garzarella 2019-11-11 10:25 ` Alex Bennée 1 sibling, 0 replies; 10+ messages in thread From: Stefano Garzarella @ 2019-11-11 9:57 UTC (permalink / raw) To: Wei Yang Cc: kwolf, qemu-block, qemu-devel, jasowang, richard.henderson, dgilbert, mreitz, alex.bennee 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.yang@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 ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 1/2] tests/tcg/multiarch: fix code style in function main of test-mmap.c 2019-10-15 3:13 ` [PATCH 1/2] tests/tcg/multiarch: fix code style in function main of test-mmap.c Wei Yang 2019-11-11 9:57 ` Stefano Garzarella @ 2019-11-11 10:25 ` Alex Bennée 2019-11-11 15:47 ` Wei Yang 1 sibling, 1 reply; 10+ messages in thread From: Alex Bennée @ 2019-11-11 10:25 UTC (permalink / raw) To: Wei Yang Cc: kwolf, qemu-block, qemu-devel, jasowang, richard.henderson, dgilbert, mreitz Wei Yang <richardw.yang@linux.intel.com> writes: > 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. NACK I'm afraid. The tests/tcg directory all build against glibc only to make them easier to cross-compile for the various targets. If you run check-tcg and have a non-native cross compiler setup you'll notice this fails to build: BUILD aarch64-linux-user guest-tests with aarch64-linux-gnu-gcc /home/alex/lsrc/qemu.git/tests/tcg/multiarch/test-mmap.c: In function ‘main’: /home/alex/lsrc/qemu.git/tests/tcg/multiarch/test-mmap.c:467:9: warning: implicit declaration of function ‘qemu_strtoul’; did you mean ‘strtoul’? [-Wimplicit-function-declaration] qemu_strtoul(argv[1], NULL, 0, &pagesize); ^~~~~~~~~~~~ strtoul /home/alex/lsrc/qemu.git/tests/tcg/multiarch/test-mmap.c:469:20: error: ‘qemu_real_host_page_size’ undeclared (first use in this function) pagesize = qemu_real_host_page_size; ^~~~~~~~~~~~~~~~~~~~~~~~ /home/alex/lsrc/qemu.git/tests/tcg/multiarch/test-mmap.c:469:20: note: each undeclared identifier is reported only once for each function it appears in make[2]: *** [../Makefile.target:103: test-mmap] Error 1 make[1]: *** [/home/alex/lsrc/qemu.git/tests/tcg/Makefile.qemu:33: cross-build-guest-tests] Error 2 make: *** [/home/alex/lsrc/qemu.git/tests/Makefile.include:1094: build-tcg-tests-aarch64-linux-user] Error 2 make: *** Waiting for unfinished jobs.... > > Signed-off-by: Wei Yang <richardw.yang@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); > + } 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; > } -- Alex Bennée ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 1/2] tests/tcg/multiarch: fix code style in function main of test-mmap.c 2019-11-11 10:25 ` Alex Bennée @ 2019-11-11 15:47 ` Wei Yang 2019-11-11 16:00 ` Alex Bennée 0 siblings, 1 reply; 10+ messages in thread From: Wei Yang @ 2019-11-11 15:47 UTC (permalink / raw) To: Alex Benn??e Cc: kwolf, qemu-block, jasowang, richard.henderson, qemu-devel, dgilbert, Wei Yang, mreitz On Mon, Nov 11, 2019 at 10:25:43AM +0000, Alex Benn??e wrote: > >Wei Yang <richardw.yang@linux.intel.com> writes: > >> 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. > > >NACK I'm afraid. > >The tests/tcg directory all build against glibc only to make them easier >to cross-compile for the various targets. If you run check-tcg and have >a non-native cross compiler setup you'll notice this fails to build: > > BUILD aarch64-linux-user guest-tests with aarch64-linux-gnu-gcc > /home/alex/lsrc/qemu.git/tests/tcg/multiarch/test-mmap.c: In function ???main???: > /home/alex/lsrc/qemu.git/tests/tcg/multiarch/test-mmap.c:467:9: warning: implicit declaration of function ???qemu_strtoul???; did you mean ???strtoul???? [-Wimplicit-function-declaration] > qemu_strtoul(argv[1], NULL, 0, &pagesize); > ^~~~~~~~~~~~ > strtoul > /home/alex/lsrc/qemu.git/tests/tcg/multiarch/test-mmap.c:469:20: error: ???qemu_real_host_page_size??? undeclared (first use in this function) > pagesize = qemu_real_host_page_size; > ^~~~~~~~~~~~~~~~~~~~~~~~ > /home/alex/lsrc/qemu.git/tests/tcg/multiarch/test-mmap.c:469:20: note: each undeclared identifier is reported only once for each function it appears in > make[2]: *** [../Makefile.target:103: test-mmap] Error 1 > make[1]: *** [/home/alex/lsrc/qemu.git/tests/tcg/Makefile.qemu:33: cross-build-guest-tests] Error 2 > make: *** [/home/alex/lsrc/qemu.git/tests/Makefile.include:1094: build-tcg-tests-aarch64-linux-user] Error 2 > make: *** Waiting for unfinished jobs.... > This output is from "make test" ? >> >> Signed-off-by: Wei Yang <richardw.yang@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); >> + } 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; >> } > > >-- >Alex Benn??e -- Wei Yang Help you, Help me ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 1/2] tests/tcg/multiarch: fix code style in function main of test-mmap.c 2019-11-11 15:47 ` Wei Yang @ 2019-11-11 16:00 ` Alex Bennée 0 siblings, 0 replies; 10+ messages in thread From: Alex Bennée @ 2019-11-11 16:00 UTC (permalink / raw) To: Wei Yang Cc: kwolf, qemu-block, jasowang, richard.henderson, qemu-devel, dgilbert, Wei Yang, mreitz Wei Yang <richard.weiyang@gmail.com> writes: > On Mon, Nov 11, 2019 at 10:25:43AM +0000, Alex Benn??e wrote: >> >>Wei Yang <richardw.yang@linux.intel.com> writes: >> >>> 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. >> >> >>NACK I'm afraid. >> >>The tests/tcg directory all build against glibc only to make them easier >>to cross-compile for the various targets. If you run check-tcg and have >>a non-native cross compiler setup you'll notice this fails to build: >> >> BUILD aarch64-linux-user guest-tests with aarch64-linux-gnu-gcc >> /home/alex/lsrc/qemu.git/tests/tcg/multiarch/test-mmap.c: In function ???main???: >> /home/alex/lsrc/qemu.git/tests/tcg/multiarch/test-mmap.c:467:9: warning: implicit declaration of function ???qemu_strtoul???; did you mean ???strtoul???? [-Wimplicit-function-declaration] >> qemu_strtoul(argv[1], NULL, 0, &pagesize); >> ^~~~~~~~~~~~ >> strtoul >> /home/alex/lsrc/qemu.git/tests/tcg/multiarch/test-mmap.c:469:20: error: ???qemu_real_host_page_size??? undeclared (first use in this function) >> pagesize = qemu_real_host_page_size; >> ^~~~~~~~~~~~~~~~~~~~~~~~ >> /home/alex/lsrc/qemu.git/tests/tcg/multiarch/test-mmap.c:469:20: note: each undeclared identifier is reported only once for each function it appears in >> make[2]: *** [../Makefile.target:103: test-mmap] Error 1 >> make[1]: *** [/home/alex/lsrc/qemu.git/tests/tcg/Makefile.qemu:33: cross-build-guest-tests] Error 2 >> make: *** [/home/alex/lsrc/qemu.git/tests/Makefile.include:1094: build-tcg-tests-aarch64-linux-user] Error 2 >> make: *** Waiting for unfinished jobs.... >> > > This output is from "make test" ? make check-tcg > >>> >>> Signed-off-by: Wei Yang <richardw.yang@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); >>> + } 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; >>> } >> >> >>-- >>Alex Benn??e -- Alex Bennée ^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH 2/2] core: replace sysconf(_SC_PAGESIZE) with qemu_real_host_page_size 2019-10-15 3:13 [PATCH 0/2] replace sysconf(_SC_PAGESIZE) with qemu_real_host_page_size Wei Yang 2019-10-15 3:13 ` [PATCH 1/2] tests/tcg/multiarch: fix code style in function main of test-mmap.c Wei Yang @ 2019-10-15 3:13 ` Wei Yang 2019-11-11 10:06 ` Stefano Garzarella 2019-11-09 1:25 ` [PATCH 0/2] " Wei Yang 2 siblings, 1 reply; 10+ messages in thread From: Wei Yang @ 2019-10-15 3:13 UTC (permalink / raw) To: kwolf, mreitz, jasowang, alex.bennee, dgilbert, richard.henderson Cc: qemu-devel, qemu-block, Wei Yang Signed-off-by: Wei Yang <richardw.yang@linux.intel.com> Suggested-by: "Dr. David Alan Gilbert" <dgilbert@redhat.com> CC: Richard Henderson <richard.henderson@linaro.org> --- block/file-posix.c | 2 +- net/l2tpv3.c | 2 +- tests/tcg/multiarch/test-mmap.c | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/block/file-posix.c b/block/file-posix.c index 5d1995a07c..853ed42134 100644 --- a/block/file-posix.c +++ b/block/file-posix.c @@ -2562,7 +2562,7 @@ static void check_cache_dropped(BlockDriverState *bs, Error **errp) off_t end; /* mincore(2) page status information requires 1 byte per page */ - page_size = sysconf(_SC_PAGESIZE); + page_size = qemu_real_host_page_size; vec = g_malloc(DIV_ROUND_UP(window_size, page_size)); end = raw_getlength(bs); diff --git a/net/l2tpv3.c b/net/l2tpv3.c index 55fea17c0f..5f843240de 100644 --- a/net/l2tpv3.c +++ b/net/l2tpv3.c @@ -41,7 +41,7 @@ * chosen to be sufficient to accommodate one packet with some headers */ -#define BUFFER_ALIGN sysconf(_SC_PAGESIZE) +#define BUFFER_ALIGN qemu_real_host_page_size #define BUFFER_SIZE 2048 #define IOVSIZE 2 #define MAX_L2TPV3_MSGCNT 64 diff --git a/tests/tcg/multiarch/test-mmap.c b/tests/tcg/multiarch/test-mmap.c index 9ea49e2307..370842e5c2 100644 --- a/tests/tcg/multiarch/test-mmap.c +++ b/tests/tcg/multiarch/test-mmap.c @@ -466,7 +466,7 @@ int main(int argc, char **argv) if (argc > 1) { qemu_strtoul(argv[1], NULL, 0, &pagesize); } else { - pagesize = sysconf(_SC_PAGESIZE); + pagesize = qemu_real_host_page_size; } /* Assume pagesize is a power of two. */ -- 2.17.1 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH 2/2] core: replace sysconf(_SC_PAGESIZE) with qemu_real_host_page_size 2019-10-15 3:13 ` [PATCH 2/2] core: replace sysconf(_SC_PAGESIZE) with qemu_real_host_page_size Wei Yang @ 2019-11-11 10:06 ` Stefano Garzarella 2019-11-11 15:46 ` Wei Yang 0 siblings, 1 reply; 10+ messages in thread From: Stefano Garzarella @ 2019-11-11 10:06 UTC (permalink / raw) To: Wei Yang Cc: kwolf, qemu-block, qemu-devel, jasowang, richard.henderson, dgilbert, mreitz, alex.bennee Why "core:" in the commit title? Perhaps to indicate that the patch concerns different subsystems, I'd use "qemu: ", but I'm not sure :-) On Tue, Oct 15, 2019 at 11:13:50AM +0800, Wei Yang wrote: > Signed-off-by: Wei Yang <richardw.yang@linux.intel.com> > Suggested-by: "Dr. David Alan Gilbert" <dgilbert@redhat.com> > CC: Richard Henderson <richard.henderson@linaro.org> > --- > block/file-posix.c | 2 +- > net/l2tpv3.c | 2 +- > tests/tcg/multiarch/test-mmap.c | 2 +- > 3 files changed, 3 insertions(+), 3 deletions(-) > > diff --git a/block/file-posix.c b/block/file-posix.c > index 5d1995a07c..853ed42134 100644 > --- a/block/file-posix.c > +++ b/block/file-posix.c > @@ -2562,7 +2562,7 @@ static void check_cache_dropped(BlockDriverState *bs, Error **errp) > off_t end; > > /* mincore(2) page status information requires 1 byte per page */ > - page_size = sysconf(_SC_PAGESIZE); > + page_size = qemu_real_host_page_size; > vec = g_malloc(DIV_ROUND_UP(window_size, page_size)); > > end = raw_getlength(bs); > diff --git a/net/l2tpv3.c b/net/l2tpv3.c > index 55fea17c0f..5f843240de 100644 > --- a/net/l2tpv3.c > +++ b/net/l2tpv3.c > @@ -41,7 +41,7 @@ > * chosen to be sufficient to accommodate one packet with some headers > */ > > -#define BUFFER_ALIGN sysconf(_SC_PAGESIZE) > +#define BUFFER_ALIGN qemu_real_host_page_size > #define BUFFER_SIZE 2048 > #define IOVSIZE 2 > #define MAX_L2TPV3_MSGCNT 64 > diff --git a/tests/tcg/multiarch/test-mmap.c b/tests/tcg/multiarch/test-mmap.c > index 9ea49e2307..370842e5c2 100644 > --- a/tests/tcg/multiarch/test-mmap.c > +++ b/tests/tcg/multiarch/test-mmap.c > @@ -466,7 +466,7 @@ int main(int argc, char **argv) > if (argc > 1) { > qemu_strtoul(argv[1], NULL, 0, &pagesize); > } else { > - pagesize = sysconf(_SC_PAGESIZE); > + pagesize = qemu_real_host_page_size; > } > > /* Assume pagesize is a power of two. */ The patch LGTM: Reviewed-by: Stefano Garzarella <sgarzare@redhat.com> Thanks, Stefano ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 2/2] core: replace sysconf(_SC_PAGESIZE) with qemu_real_host_page_size 2019-11-11 10:06 ` Stefano Garzarella @ 2019-11-11 15:46 ` Wei Yang 0 siblings, 0 replies; 10+ messages in thread From: Wei Yang @ 2019-11-11 15:46 UTC (permalink / raw) To: Stefano Garzarella Cc: kwolf, qemu-block, jasowang, richard.henderson, qemu-devel, dgilbert, Wei Yang, mreitz, alex.bennee On Mon, Nov 11, 2019 at 11:06:41AM +0100, Stefano Garzarella wrote: >Why "core:" in the commit title? > >Perhaps to indicate that the patch concerns different subsystems, >I'd use "qemu: ", but I'm not sure :-) > I didn't find a better one. Maybe "qemu" is better :-) >On Tue, Oct 15, 2019 at 11:13:50AM +0800, Wei Yang wrote: >> Signed-off-by: Wei Yang <richardw.yang@linux.intel.com> >> Suggested-by: "Dr. David Alan Gilbert" <dgilbert@redhat.com> >> CC: Richard Henderson <richard.henderson@linaro.org> >> --- >> block/file-posix.c | 2 +- >> net/l2tpv3.c | 2 +- >> tests/tcg/multiarch/test-mmap.c | 2 +- >> 3 files changed, 3 insertions(+), 3 deletions(-) >> >> diff --git a/block/file-posix.c b/block/file-posix.c >> index 5d1995a07c..853ed42134 100644 >> --- a/block/file-posix.c >> +++ b/block/file-posix.c >> @@ -2562,7 +2562,7 @@ static void check_cache_dropped(BlockDriverState *bs, Error **errp) >> off_t end; >> >> /* mincore(2) page status information requires 1 byte per page */ >> - page_size = sysconf(_SC_PAGESIZE); >> + page_size = qemu_real_host_page_size; >> vec = g_malloc(DIV_ROUND_UP(window_size, page_size)); >> >> end = raw_getlength(bs); >> diff --git a/net/l2tpv3.c b/net/l2tpv3.c >> index 55fea17c0f..5f843240de 100644 >> --- a/net/l2tpv3.c >> +++ b/net/l2tpv3.c >> @@ -41,7 +41,7 @@ >> * chosen to be sufficient to accommodate one packet with some headers >> */ >> >> -#define BUFFER_ALIGN sysconf(_SC_PAGESIZE) >> +#define BUFFER_ALIGN qemu_real_host_page_size >> #define BUFFER_SIZE 2048 >> #define IOVSIZE 2 >> #define MAX_L2TPV3_MSGCNT 64 >> diff --git a/tests/tcg/multiarch/test-mmap.c b/tests/tcg/multiarch/test-mmap.c >> index 9ea49e2307..370842e5c2 100644 >> --- a/tests/tcg/multiarch/test-mmap.c >> +++ b/tests/tcg/multiarch/test-mmap.c >> @@ -466,7 +466,7 @@ int main(int argc, char **argv) >> if (argc > 1) { >> qemu_strtoul(argv[1], NULL, 0, &pagesize); >> } else { >> - pagesize = sysconf(_SC_PAGESIZE); >> + pagesize = qemu_real_host_page_size; >> } >> >> /* Assume pagesize is a power of two. */ > >The patch LGTM: >Reviewed-by: Stefano Garzarella <sgarzare@redhat.com> > >Thanks, >Stefano > -- Wei Yang Help you, Help me ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 0/2] replace sysconf(_SC_PAGESIZE) with qemu_real_host_page_size 2019-10-15 3:13 [PATCH 0/2] replace sysconf(_SC_PAGESIZE) with qemu_real_host_page_size Wei Yang 2019-10-15 3:13 ` [PATCH 1/2] tests/tcg/multiarch: fix code style in function main of test-mmap.c Wei Yang 2019-10-15 3:13 ` [PATCH 2/2] core: replace sysconf(_SC_PAGESIZE) with qemu_real_host_page_size Wei Yang @ 2019-11-09 1:25 ` Wei Yang 2 siblings, 0 replies; 10+ messages in thread From: Wei Yang @ 2019-11-09 1:25 UTC (permalink / raw) To: Wei Yang Cc: kwolf, qemu-block, qemu-devel, jasowang, richard.henderson, dgilbert, mreitz, alex.bennee On Tue, Oct 15, 2019 at 11:13:48AM +0800, Wei Yang wrote: >This is a following up patch to cleanup page size, suggested by >"Dr. David Alan Gilbert" <dgilbert@redhat.com>. > >Patch 2 does the job, while during the cleanup I found test-mmap.c has quite a >lot code style problem. To make the code looks good, patch 1 is introduced to >make checkpatch.pl happy a little. Does this patch set look good? > >Wei Yang (2): > tests/tcg/multiarch: fix code style in function main of test-mmap.c > core: replace sysconf(_SC_PAGESIZE) with qemu_real_host_page_size > > block/file-posix.c | 2 +- > net/l2tpv3.c | 2 +- > tests/tcg/multiarch/test-mmap.c | 67 ++++++++++++++++++--------------- > 3 files changed, 38 insertions(+), 33 deletions(-) > >-- >2.17.1 > -- Wei Yang Help you, Help me ^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2019-11-11 16:06 UTC | newest] Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2019-10-15 3:13 [PATCH 0/2] replace sysconf(_SC_PAGESIZE) with qemu_real_host_page_size Wei Yang 2019-10-15 3:13 ` [PATCH 1/2] tests/tcg/multiarch: fix code style in function main of test-mmap.c Wei Yang 2019-11-11 9:57 ` Stefano Garzarella 2019-11-11 10:25 ` Alex Bennée 2019-11-11 15:47 ` Wei Yang 2019-11-11 16:00 ` Alex Bennée 2019-10-15 3:13 ` [PATCH 2/2] core: replace sysconf(_SC_PAGESIZE) with qemu_real_host_page_size Wei Yang 2019-11-11 10:06 ` Stefano Garzarella 2019-11-11 15:46 ` Wei Yang 2019-11-09 1:25 ` [PATCH 0/2] " Wei Yang
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).