Re: [PATCH v2 00/48] bsd-user style and reorg patches

[Warner Losh <imp@bsdimp.com>]

[-- Attachment #1: Type: text/plain, Size: 17927 bytes --]

This is a false positive. The files were wrong before, so it's detecting
residual wrongness in the incremental changes.

The cumulative diff of all 48 patches passes with only warnings:

Use of uninitialized value $acpi_testexpected in string eq at ../scripts/
checkpatch.pl line 1529.
WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
#2:
new file mode 100644

WARNING: architecture specific defines should be avoided
#1641: FILE: bsd-user/freebsd/host_os.h:20:
+#ifndef __HOST_OS_H_

WARNING: architecture specific defines should be avoided
#3123: FILE: bsd-user/netbsd/host_os.h:20:
+#ifndef __HOST_OS_H_

WARNING: architecture specific defines should be avoided
#3154: FILE: bsd-user/openbsd/host_os.h:20:
+#ifndef __HOST_OS_H_

total: 0 errors, 4 warnings, 4264 lines checked

Warner

On Sat, Apr 24, 2021 at 10:55 AM <no-reply@patchew.org> wrote:

> Patchew URL:
> https://patchew.org/QEMU/20210424160016.15200-1-imp@bsdimp.com/
>
>
>
> Hi,
>
> This series seems to have some coding style problems. See output below for
> more information:
>
> Type: series
> Message-id: 20210424160016.15200-1-imp@bsdimp.com
> Subject: [PATCH v2 00/48] bsd-user style and reorg patches
>
> === TEST SCRIPT BEGIN ===
> #!/bin/bash
> git rev-parse base > /dev/null || exit 0
> git config --local diff.renamelimit 0
> git config --local diff.renames True
> git config --local diff.algorithm histogram
> ./scripts/checkpatch.pl --mailback base..
> === TEST SCRIPT END ===
>
> Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
> From https://github.com/patchew-project/qemu
>  * [new tag]         patchew/20210424160016.15200-1-imp@bsdimp.com ->
> patchew/20210424160016.15200-1-imp@bsdimp.com
> Switched to a new branch 'test'
> be1f8df bsd-user: put back a break; that had gone missing...
> 19b2708 bsd-user: style tweak: don't assign in if statement.
> 1bf13fe bsd-user: style tweak: Return is not a function call.
> de239a3 bsd-user: style tweak: Use preferred {} in if/else statements.
> cf2e30f bsd-user: style tweak: use preferred block comments
> 1a93628 bsd-user: style tweak: fold long lines
> 1448c0f bsd-user: style tweak: spaces around operators and commas
> 11e0aae bsd-user: style tweak: don't assign in if statements
> 2c53346 bsd-user: style tweak: Use preferred block comments
> 8946d14 bsd-user: style tweak: spaces around =, remove stray space
> ab7c026 bsd-user: style tweak: use {} for all if statements, format else
> correctly
> 3a3b578 bsd-user: style tweak: don't assign in if statements
> d32211e bsd-user: style tweak: Use preferred block comments
> 695b534 bsd-user: style tweak: use {} for all if statements, format else
> correctly
> eca311e bsd-user: style tweak: don't assign in if statements
> 45bfe91 bsd-user: style tweak: Use preferred block comments
> d00c21e bsd-user: style tweak: remove spacing after '*' and add after }
> 780e9f4 bsd-user: style tweak: use {} for all if statements, format else
> correctly
> e92240f bsd-user: style tweak: fix block comments
> cb3234a bsd-user: style tweak: use {} correctly
> 522e458 bsd-user: style tweak: comments
> e7e00a4 bsd-user: style tweak: space pedantry
> 55da1fd bsd-user: move sparc cpu_loop into target_arch_cpu.h as
> target_cpu_loop
> c546aa9 bsd-user: move x86 (i386 and x86_64) cpu_loop to target_arch_cpu.h
> f19b57e bsd-user: create target_arch_cpu.h
> 7ed989c bsd-user: introduce host_os.h for bsd-specific code and defaults
> 9cb365d bsd-user: use qemu_strtoul in preference to strtol
> 4454689 bsd-user: style nits: return is not a function
> 0c748d0 bsd-user: style tweak: use {} consistently in for / if / else
> statements
> 3ce10b8 bsd-user: style tweak: move extern to header file
> 3aa06bf bsd-user: style tweak: Use preferred block comments
> 2e4fcda bsd-user: style tweak: Fix commentary issues
> 4416126 bsd-user: style tweak: Put {} around all if/else/for statements
> dc7ae60 bsd-user: style tweak: return is not a function, eliminate ()
> b84160d bsd-user: style tweak: Remove #if 0'd code
> df39a25 bsd-user: style tweak: Remove #if 0'd code
> 9d92841 bsd-user: style tweak: Remove #if 0'd code
> 7ef0035 bsd-user: style tweak: Remove #if 0'd code
> 00a0b5e bsd-user: Remove commented out code
> c87209f bsd-user: style tweak: use C not C++ comments
> 1284f86 bsd-user: style tweak: use C not C++ comments
> afedaf7 bsd-user: style tweak: keyword space (
> d903536 bsd-user: style tweak: keyword space (
> a9aa3e4 bsd-user: style tweak: keyword space (
> 64b81a2 bsd-user: style tweak: keyword space (
> 1c1ed78 bsd-user: whitespace changes
> 3d281e8 bsd-user: whitespace changes
> 6f7ee37 bsd-user: whitespace changes
>
> === OUTPUT BEGIN ===
> 1/48 Checking commit 6f7ee37e5c9f (bsd-user: whitespace changes)
> 2/48 Checking commit 3d281e896a43 (bsd-user: whitespace changes)
> WARNING: line over 80 characters
> #149: FILE: bsd-user/elfload.c:541:
> +#define TARGET_ELF_PAGESTART(_v) ((_v) & ~(unsigned
> long)(TARGET_ELF_EXEC_PAGESIZE - 1))
>
> ERROR: braces {} are necessary for all arms of this statement
> #331: FILE: bsd-user/elfload.c:867:
> +    if (sizeof(struct elf_phdr) * interp_elf_ex->e_phnum >
> TARGET_PAGE_SIZE)
> [...]
>
> ERROR: braces {} are necessary for all arms of this statement
> #341: FILE: bsd-user/elfload.c:873:
> +    if (!elf_phdata)
> [...]
>
> ERROR: space prohibited between function name and open parenthesis '('
> #382: FILE: bsd-user/elfload.c:894:
> +        free (elf_phdata);
>
> ERROR: spaces required around that '<' (ctx:VxV)
> #391: FILE: bsd-user/elfload.c:899:
> +    for (i = 0; i<interp_elf_ex->e_phnum; i++, eppnt++) {
>                   ^
>
> WARNING: Block comments use a leading /* on a separate line
> #409: FILE: bsd-user/elfload.c:905:
> +        /* in order to avoid hardcoding the interpreter load
>
> WARNING: Block comments use * on subsequent lines
> #410: FILE: bsd-user/elfload.c:906:
> +        /* in order to avoid hardcoding the interpreter load
> +           address in qemu, we allocate a big enough memory zone */
>
> WARNING: Block comments use a trailing */ on a separate line
> #410: FILE: bsd-user/elfload.c:906:
> +           address in qemu, we allocate a big enough memory zone */
>
> WARNING: line over 80 characters
> #442: FILE: bsd-user/elfload.c:934:
> +                                eppnt->p_filesz +
> TARGET_ELF_PAGEOFFSET(eppnt->p_vaddr),
>
> WARNING: line over 80 characters
> #446: FILE: bsd-user/elfload.c:938:
> +                                eppnt->p_offset -
> TARGET_ELF_PAGEOFFSET(eppnt->p_vaddr));
>
> ERROR: line over 90 characters
> #507: FILE: bsd-user/elfload.c:978:
> +    elf_bss = TARGET_ELF_PAGESTART(elf_bss + qemu_host_page_size - 1); /*
> What we have mapped so far */
>
> ERROR: space required after that ',' (ctx:VxV)
> #554: FILE: bsd-user/elfload.c:1186:
> +    bprm->p = copy_elf_strings(bprm->envc, bprm->envp,
> bprm->page,bprm->p);
>                                                                   ^
>
> ERROR: space required after that ',' (ctx:VxV)
> #555: FILE: bsd-user/elfload.c:1187:
> +    bprm->p = copy_elf_strings(bprm->argc, bprm->argv,
> bprm->page,bprm->p);
>                                                                   ^
>
> ERROR: space required after that ';' (ctx:VxV)
> #590: FILE: bsd-user/elfload.c:1230:
> +    for (i = 0;i < elf_ex.e_phnum; i++) {
>                ^
>
> ERROR: suspect code indent for conditional statements (12, 14)
> #626: FILE: bsd-user/elfload.c:1268:
> +            if (strcmp(elf_interpreter, "/usr/lib/libc.so.1") == 0 ||
> [...]
>                ibcs2_interpreter = 1;
>
> total: 9 errors, 6 warnings, 666 lines checked
>
> Patch 2/48 has style problems, please review.  If any of these errors
> are false positives report them to the maintainer, see
> CHECKPATCH in MAINTAINERS.
>
> 3/48 Checking commit 1c1ed7831e8a (bsd-user: whitespace changes)
> ERROR: consider using qemu_strtol in preference to strtol
> #102: FILE: bsd-user/main.c:835:
> +            guest_base = strtol(argv[optind++], NULL, 0);
>
> ERROR: braces {} are necessary for all arms of this statement
> #143: FILE: bsd-user/main.c:1126:
> +        for (i = 0; i < 8; i++)
> [...]
>
> ERROR: braces {} are necessary for all arms of this statement
> #146: FILE: bsd-user/main.c:1128:
> +        for (i = 0; i < 8; i++)
> [...]
>
> total: 3 errors, 0 warnings, 119 lines checked
>
> Patch 3/48 has style problems, please review.  If any of these errors
> are false positives report them to the maintainer, see
> CHECKPATCH in MAINTAINERS.
>
> 4/48 Checking commit 64b81a2f81d0 (bsd-user: style tweak: keyword space ()
> 5/48 Checking commit a9aa3e409bd8 (bsd-user: style tweak: keyword space ()
> 6/48 Checking commit d903536bcaa8 (bsd-user: style tweak: keyword space ()
> ERROR: space required after that ';' (ctx:VxV)
> #32: FILE: bsd-user/syscall.c:274:
> +    for (i = 0;i < count; i++) {
>                ^
>
> ERROR: space required after that ';' (ctx:VxV)
> #41: FILE: bsd-user/syscall.c:300:
> +    for (i = 0;i < count; i++) {
>                ^
>
> total: 2 errors, 0 warnings, 60 lines checked
>
> Patch 6/48 has style problems, please review.  If any of these errors
> are false positives report them to the maintainer, see
> CHECKPATCH in MAINTAINERS.
>
> 7/48 Checking commit afedaf7c29e2 (bsd-user: style tweak: keyword space ()
> 8/48 Checking commit 1284f86e9b7b (bsd-user: style tweak: use C not C++
> comments)
> 9/48 Checking commit c87209f1f0f9 (bsd-user: style tweak: use C not C++
> comments)
> 10/48 Checking commit 00a0b5ee4157 (bsd-user: Remove commented out code)
> 11/48 Checking commit 7ef00352a15f (bsd-user: style tweak: Remove #if 0'd
> code)
> 12/48 Checking commit 9d92841f7d3a (bsd-user: style tweak: Remove #if 0'd
> code)
> 13/48 Checking commit df39a2547ed7 (bsd-user: style tweak: Remove #if 0'd
> code)
> 14/48 Checking commit b84160d0a3db (bsd-user: style tweak: Remove #if 0'd
> code)
> 15/48 Checking commit dc7ae6073667 (bsd-user: style tweak: return is not a
> function, eliminate ())
> 16/48 Checking commit 4416126bd67b (bsd-user: style tweak: Put {} around
> all if/else/for statements)
> 17/48 Checking commit 2e4fcdafda97 (bsd-user: style tweak: Fix commentary
> issues)
> 18/48 Checking commit 3aa06bf4a169 (bsd-user: style tweak: Use preferred
> block comments)
> 19/48 Checking commit 3ce10b8d1073 (bsd-user: style tweak: move extern to
> header file)
> 20/48 Checking commit 0c748d0a6ea1 (bsd-user: style tweak: use {}
> consistently in for / if / else statements)
> 21/48 Checking commit 4454689b7dff (bsd-user: style nits: return is not a
> function)
> 22/48 Checking commit 9cb365d13f71 (bsd-user: use qemu_strtoul in
> preference to strtol)
> 23/48 Checking commit 7ed989c1d383 (bsd-user: introduce host_os.h for
> bsd-specific code and defaults)
> Use of uninitialized value $acpi_testexpected in string eq at ./scripts/
> checkpatch.pl line 1529.
> WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
> #24:
> new file mode 100644
>
> WARNING: architecture specific defines should be avoided
> #48: FILE: bsd-user/freebsd/host_os.h:20:
> +#ifndef __HOST_OS_H_
>
> WARNING: architecture specific defines should be avoided
> #101: FILE: bsd-user/netbsd/host_os.h:20:
> +#ifndef __HOST_OS_H_
>
> WARNING: architecture specific defines should be avoided
> #132: FILE: bsd-user/openbsd/host_os.h:20:
> +#ifndef __HOST_OS_H_
>
> total: 0 errors, 4 warnings, 91 lines checked
>
> Patch 23/48 has style problems, please review.  If any of these errors
> are false positives report them to the maintainer, see
> CHECKPATCH in MAINTAINERS.
> 24/48 Checking commit f19b57e6a271 (bsd-user: create target_arch_cpu.h)
> Use of uninitialized value $acpi_testexpected in string eq at ./scripts/
> checkpatch.pl line 1529.
> WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
> #31:
> new file mode 100644
>
> total: 0 errors, 1 warnings, 111 lines checked
>
> Patch 24/48 has style problems, please review.  If any of these errors
> are false positives report them to the maintainer, see
> CHECKPATCH in MAINTAINERS.
> 25/48 Checking commit c546aa906cbe (bsd-user: move x86 (i386 and x86_64)
> cpu_loop to target_arch_cpu.h)
> 26/48 Checking commit 55da1fd9d9a3 (bsd-user: move sparc cpu_loop into
> target_arch_cpu.h as target_cpu_loop)
> 27/48 Checking commit e7e00a4308a1 (bsd-user: style tweak: space pedantry)
> ERROR: spaces required around that '=' (ctx:WxV)
> #78: FILE: bsd-user/elfload.c:1226:
> +    start_data =n 0;
>                 ^
>
> total: 1 errors, 0 warnings, 73 lines checked
>
> Patch 27/48 has style problems, please review.  If any of these errors
> are false positives report them to the maintainer, see
> CHECKPATCH in MAINTAINERS.
>
> 28/48 Checking commit 522e4589fc17 (bsd-user: style tweak: comments)
> 29/48 Checking commit cb3234ad892f (bsd-user: style tweak: use {}
> correctly)
> 30/48 Checking commit e92240f03f40 (bsd-user: style tweak: fix block
> comments)
> 31/48 Checking commit 780e9f44ba8b (bsd-user: style tweak: use {} for all
> if statements, format else correctly)
> 32/48 Checking commit d00c21e0a91a (bsd-user: style tweak: remove spacing
> after '*' and add after })
> 33/48 Checking commit 45bfe917b433 (bsd-user: style tweak: Use preferred
> block comments)
> 34/48 Checking commit eca311e2127b (bsd-user: style tweak: don't assign in
> if statements)
> 35/48 Checking commit 695b53466022 (bsd-user: style tweak: use {} for all
> if statements, format else correctly)
> 36/48 Checking commit d32211e78858 (bsd-user: style tweak: Use preferred
> block comments)
> 37/48 Checking commit 3a3b578aff7b (bsd-user: style tweak: don't assign in
> if statements)
> 38/48 Checking commit ab7c026d39c0 (bsd-user: style tweak: use {} for all
> if statements, format else correctly)
> 39/48 Checking commit 8946d1425629 (bsd-user: style tweak: spaces around
> =, remove stray space)
> 40/48 Checking commit 2c5334681060 (bsd-user: style tweak: Use preferred
> block comments)
> 41/48 Checking commit 11e0aae47a39 (bsd-user: style tweak: don't assign in
> if statements)
> 42/48 Checking commit 1448c0f5067c (bsd-user: style tweak: spaces around
> operators and commas)
> WARNING: line over 80 characters
> #24: FILE: bsd-user/syscall.c:81:
> +                                        MAP_ANON | MAP_FIXED |
> MAP_PRIVATE, -1, 0));
>
> ERROR: line over 90 characters
> #85: FILE: bsd-user/syscall.c:396:
> +        ret = do_freebsd_syscall(cpu_env, arg1 & 0xffff, arg2, arg3,
> arg4, arg5, arg6, arg7, arg8, 0);
>
> WARNING: line over 80 characters
> #94: FILE: bsd-user/syscall.c:475:
> +        ret = do_netbsd_syscall(cpu_env, arg1 & 0xffff, arg2, arg3, arg4,
> arg5, arg6, 0);
>
> WARNING: line over 80 characters
> #103: FILE: bsd-user/syscall.c:554:
> +        ret = do_openbsd_syscall(cpu_env, arg1 & 0xffff, arg2, arg3,
> arg4, arg5, arg6, 0);
>
> total: 1 errors, 3 warnings, 81 lines checked
>
> Patch 42/48 has style problems, please review.  If any of these errors
> are false positives report them to the maintainer, see
> CHECKPATCH in MAINTAINERS.
>
> 43/48 Checking commit 1a936284fa25 (bsd-user: style tweak: fold long lines)
> 44/48 Checking commit cf2e30fc41ca (bsd-user: style tweak: use preferred
> block comments)
> 45/48 Checking commit de239a3b68bb (bsd-user: style tweak: Use preferred
> {} in if/else statements.)
> ERROR: do not use assignment in if condition
> #139: FILE: bsd-user/syscall.c:248:
> +    if (!(hnamep = lock_user(VERIFY_READ, namep, namelen, 1))) {
>
> ERROR: do not use assignment in if condition
> #143: FILE: bsd-user/syscall.c:251:
> +    if (newp && !(hnewp = lock_user(VERIFY_READ, newp, newlen, 1))) {
>
> ERROR: do not use assignment in if condition
> #147: FILE: bsd-user/syscall.c:254:
> +    if (!(holdp = lock_user(VERIFY_WRITE, oldp, oldlen, 0))) {
>
> ERROR: do not use assignment in if condition
> #210: FILE: bsd-user/syscall.c:371:
> +        if (!(p = lock_user(VERIFY_WRITE, arg2, arg3, 0))) {
>
> ERROR: do not use assignment in if condition
> #218: FILE: bsd-user/syscall.c:378:
> +        if (!(p = lock_user(VERIFY_READ, arg2, arg3, 1))) {
>
> ERROR: do not use assignment in if condition
> #238: FILE: bsd-user/syscall.c:398:
> +        if (!(p = lock_user_string(arg1))) {
>
> ERROR: do not use assignment in if condition
> #271: FILE: bsd-user/syscall.c:482:
> +        if (!(p = lock_user(VERIFY_WRITE, arg2, arg3, 0))) {
>
> ERROR: do not use assignment in if condition
> #279: FILE: bsd-user/syscall.c:489:
> +        if (!(p = lock_user(VERIFY_READ, arg2, arg3, 1))) {
>
> ERROR: do not use assignment in if condition
> #287: FILE: bsd-user/syscall.c:496:
> +        if (!(p = lock_user_string(arg1))) {
>
> ERROR: do not use assignment in if condition
> #320: FILE: bsd-user/syscall.c:568:
> +        if (!(p = lock_user(VERIFY_WRITE, arg2, arg3, 0))) {
>
> ERROR: do not use assignment in if condition
> #328: FILE: bsd-user/syscall.c:575:
> +        if (!(p = lock_user(VERIFY_READ, arg2, arg3, 1))) {
>
> ERROR: do not use assignment in if condition
> #336: FILE: bsd-user/syscall.c:582:
> +        if (!(p = lock_user_string(arg1))) {
>
> total: 12 errors, 0 warnings, 318 lines checked
>
> Patch 45/48 has style problems, please review.  If any of these errors
> are false positives report them to the maintainer, see
> CHECKPATCH in MAINTAINERS.
>
> 46/48 Checking commit 1bf13fe0ac6b (bsd-user: style tweak: Return is not a
> function call.)
> 47/48 Checking commit 19b2708a0763 (bsd-user: style tweak: don't assign in
> if statement.)
> 48/48 Checking commit be1f8df08151 (bsd-user: put back a break; that had
> gone missing...)
> === OUTPUT END ===
>
> Test command exited with code: 1
>
>
> The full log is available at
>
> http://patchew.org/logs/20210424160016.15200-1-imp@bsdimp.com/testing.checkpatch/?type=message
> .
> ---
> Email generated automatically by Patchew [https://patchew.org/].
> Please send your feedback to patchew-devel@redhat.com

[-- Attachment #2: Type: text/html, Size: 20885 bytes --]