mbox series

[v2,00/48] bsd-user style and reorg patches

Message ID 20210424160016.15200-1-imp@bsdimp.com (mailing list archive)
Headers show
Series bsd-user style and reorg patches | expand

Message

Warner Losh April 24, 2021, 3:59 p.m. UTC
From: Warner Losh <imp@bsdimp.com>

Version 2:

In version 2 I've fixed all the checkpatch.pl issues for all the files that I
touched. Since files are changed incrementally, some of the individual changes
may not pass checkpatch.pl due to pre-existing problems, but the cumulative
diff passes with just warnings. I've folded the 'if 0 should just be lost'
advice in the relevant hunks, as well as in a few places where things were
commented out.

I've included all the reviews I've got so far, but since this is my first time
with such a complex review here please let me know if I've missed following all
the conventions somehow.

I had to tweak patches 025 and 026, which had been previously reviewed (adding
static in 2 places to fix a warning). So I removed the Reviewed By: line from
it. I hope that's the proper protocol here, but if not I'm keen on suggestions.

Version 1:

This series starts to cleanup bsd-user. The current checkpatch.pl output is on
the order of 4500 lines long. These cleanups don't fix everything (there's still
plenty of errors, even in some of the files the patches touch). I've tried to
make things better in every case, but be advised that checkpatch.pl is unhappy
with many of the files still in the first 15 commits. I do plan on addressing
the issues in the future as I need to make real commits to those files. The
changes were ones that were trivial to do with scripts that I had to do for
main.c anyway. main.c is now completely clean. All the cleanup I've done
myself, and at the end of this patch trail the output is only 3500 lines...

Next, I've started to reorg the bsd-user sources. There's really 4 BSDs in the
wild (FreeBSD, NetBSD, OpenBSD and Dragonfly) that these could, in theory,
target. In the bsd-user branch, we've broken the emulation of these apart. It
largely preserves the other BSD's as is, and greatly expands FreeBSD so that we
can do package builds in user-land emulation. The other BSDs have not been
run-time tested, though the CI tooling builds some of them. It's my belief that
in the current state, even an old-school cat(1) would fail, though I am to fix
that at least for FreeBSD...

We've also added a number of other architectures than sparc and i386. Those
additions, though, are dependent on other things not yet reshuffled and or
merged, so they will be coming along in due time: mips (32 and 64 bit), arm,
aarch64 and riscv willl come in due time. powerpc might, but it's in a frightful
state. I'm open to other BSDs that wish to work along with me as well, though it
may be best to wait until after future patch sets as much is still set to
change. CHERI and related work may also happen, but that's still TBD.

In this installement, after the style stuff, I've created a
target_arch_cpu.h. Here, all the functions related to the cpu loop and similar
things are moved into there on a per-architecture basis to start to tame the
number of #ifdefs in mail. Linux-user did a similar thing years after we had
done it in the repo, and a number of different choices were made. Rather than
redo all the work from the bsd-user repo, I'm recreating / rebasing it on a
current qemu. Future patch series will address other aspects. Once they are
complete, we can look, potentially, at any refactoring between linux-user and
bsd-user. I very much explicitly want to push that to the end because otherwise
I'm completely recreating a lot of the work on the bsd-user branch rather than
just transitioning it forward to a newer qemu. This reorg was started by Stacey
Son and I've redone it with the latest code. I've added his signoff (with his
blanket permission) to those commits. Also, all of these commits pass
checkpatch.pl

Please let me know what you think, and how I might structure future patches if
there's ways I can do them better. I've switched to pull-requests with this series
since it will be easier to keep track of, especially in the future.

Warner

P.S. This has no relevance to 6.0 at all: we're too late and this feature
isn't fully integrated by this patch trail.



Warner Losh (48):
  bsd-user: whitespace changes
  bsd-user: whitespace changes
  bsd-user: whitespace changes
  bsd-user: style tweak: keyword space (
  bsd-user: style tweak: keyword space (
  bsd-user: style tweak: keyword space (
  bsd-user: style tweak: keyword space (
  bsd-user: style tweak: use C not C++ comments
  bsd-user: style tweak: use C not C++ comments
  bsd-user: Remove commented out code
  bsd-user: style tweak: Remove #if 0'd code
  bsd-user: style tweak: Remove #if 0'd code
  bsd-user: style tweak: Remove #if 0'd code
  bsd-user: style tweak: Remove #if 0'd code
  bsd-user: style tweak: return is not a function, eliminate ()
  bsd-user: style tweak: Put {} around all if/else/for statements
  bsd-user: style tweak: Fix commentary issues
  bsd-user: style tweak: Use preferred block comments
  bsd-user: style tweak: move extern to header file
  bsd-user: style tweak: use {} consistently in for / if / else
    statements
  bsd-user: style nits: return is not a function
  bsd-user: use qemu_strtoul in preference to strtol
  bsd-user: introduce host_os.h for bsd-specific code and defaults
  bsd-user: create target_arch_cpu.h
  bsd-user: move x86 (i386 and x86_64) cpu_loop to target_arch_cpu.h
  bsd-user: move sparc cpu_loop into target_arch_cpu.h as
    target_cpu_loop
  bsd-user: style tweak: space pedantry
  bsd-user: style tweak: comments
  bsd-user: style tweak: use {} correctly
  bsd-user: style tweak: fix block comments
  bsd-user: style tweak: use {} for all if statements, format else
    correctly
  bsd-user: style tweak: remove spacing after '*' and add after }
  bsd-user: style tweak: Use preferred block comments
  bsd-user: style tweak: don't assign in if statements
  bsd-user: style tweak: use {} for all if statements, format else
    correctly
  bsd-user: style tweak: Use preferred block comments
  bsd-user: style tweak: don't assign in if statements
  bsd-user: style tweak: use {} for all if statements, format else
    correctly
  bsd-user: style tweak: spaces around =, remove stray space
  bsd-user: style tweak: Use preferred block comments
  bsd-user: style tweak: don't assign in if statements
  bsd-user: style tweak: spaces around operators and commas
  bsd-user: style tweak: fold long lines
  bsd-user: style tweak: use preferred block comments
  bsd-user: style tweak: Use preferred {} in if/else statements.
  bsd-user: style tweak: Return is not a function call.
  bsd-user: style tweak: don't assign in if statement.
  bsd-user: put back a break; that had gone missing...

 bsd-user/arm/target_arch_cpu.h     |  22 +
 bsd-user/bsd-mman.h                |  42 +-
 bsd-user/bsdload.c                 |  55 +--
 bsd-user/elfload.c                 | 677 ++++++++++++++++-------------
 bsd-user/freebsd/host_os.h         |  25 ++
 bsd-user/i386/target_arch_cpu.h    | 197 +++++++++
 bsd-user/main.c                    | 657 +++-------------------------
 bsd-user/mmap.c                    | 185 +++++---
 bsd-user/netbsd/host_os.h          |  25 ++
 bsd-user/openbsd/host_os.h         |  25 ++
 bsd-user/qemu.h                    | 128 +++---
 bsd-user/sparc/target_arch_cpu.h   | 287 ++++++++++++
 bsd-user/sparc64/target_arch_cpu.h |  19 +
 bsd-user/strace.c                  |  21 +-
 bsd-user/syscall.c                 | 222 ++++++----
 bsd-user/uaccess.c                 |  27 +-
 bsd-user/x86_64/target_arch_cpu.h  |  19 +
 bsd-user/x86_64/target_syscall.h   |  15 -
 18 files changed, 1449 insertions(+), 1199 deletions(-)
 create mode 100644 bsd-user/arm/target_arch_cpu.h
 create mode 100644 bsd-user/freebsd/host_os.h
 create mode 100644 bsd-user/i386/target_arch_cpu.h
 create mode 100644 bsd-user/netbsd/host_os.h
 create mode 100644 bsd-user/openbsd/host_os.h
 create mode 100644 bsd-user/sparc/target_arch_cpu.h
 create mode 100644 bsd-user/sparc64/target_arch_cpu.h
 create mode 100644 bsd-user/x86_64/target_arch_cpu.h

Comments

no-reply@patchew.org April 24, 2021, 4:55 p.m. UTC | #1
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
Warner Losh April 24, 2021, 5 p.m. UTC | #2
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