[PATCH v4 00/19] Clean up includes

[PATCH v4 00/19] Clean up includes

[Markus Armbruster]

Back in 2016, we discussed[1] rules for headers, and these were
generally liked:

1. Have a carefully curated header that's included everywhere first.  We
   got that already thanks to Peter: osdep.h.

2. Headers should normally include everything they need beyond osdep.h.
   If exceptions are needed for some reason, they must be documented in
   the header.  If all that's needed from a header is typedefs, put
   those into qemu/typedefs.h instead of including the header.

3. Cyclic inclusion is forbidden.

This series fixes violations of rule 2.  I may have split patches too
aggressively.  Let me know if you want some squashed together.

v4:
* PATCH 01-03: New
* PATCH 04-15: Previous version redone with scripts/clean-includes,
  	       result split up for review
* PATCH 16-19: New

v3:
* Rebased, old PATCH 1+2+4 are in master as commit
  881e019770..f07ceffdf5
* PATCH 1: Fix bsd-user

v2:
* Rebased
* PATCH 3: v1 posted separately
* PATCH 4: New

[1] Message-ID: <87h9g8j57d.fsf@blackfin.pond.sub.org>
    https://lists.nongnu.org/archive/html/qemu-devel/2016-03/msg03345.html

Markus Armbruster (19):
  scripts/clean-includes: Fully skip / ignore files
  scripts/clean-includes: Don't claim duplicate headers found when not
  scripts/clean-includes: Skip symbolic links
  bsd-user: Clean up includes
  crypto: Clean up includes
  hw/cxl: Clean up includes
  hw/input: Clean up includes
  hw/tricore: Clean up includes
  qga: Clean up includes
  migration: Clean up includes
  net: Clean up includes
  target/hexagon: Clean up includes
  riscv: Clean up includes
  block: Clean up includes
  accel: Clean up includes
  Fix non-first inclusions of qemu/osdep.h
  Don't include headers already included by qemu/osdep.h
  9p: Drop superfluous include of linux/limits.h
  Drop duplicate #include

 backends/tpm/tpm_ioctl.h          |  2 --
 bsd-user/bsd-proc.h               |  4 ----
 bsd-user/qemu.h                   |  1 -
 crypto/block-luks-priv.h          |  1 -
 fsdev/p9array.h                   |  2 --
 include/block/graph-lock.h        |  1 -
 include/block/write-threshold.h   |  2 --
 include/hw/arm/fsl-imx6ul.h       |  1 -
 include/hw/arm/fsl-imx7.h         |  1 -
 include/hw/cxl/cxl_component.h    |  2 --
 include/hw/cxl/cxl_host.h         |  1 -
 include/hw/cxl/cxl_pci.h          |  1 -
 include/hw/input/pl050.h          |  1 -
 include/hw/misc/aspeed_lpc.h      |  2 --
 include/hw/pci/pcie_doe.h         |  1 -
 include/hw/tricore/triboard.h     |  1 -
 include/qemu/async-teardown.h     |  2 --
 include/qemu/dbus.h               |  1 -
 include/qemu/host-utils.h         |  1 -
 include/qemu/userfaultfd.h        |  1 -
 include/sysemu/accel-blocker.h    |  1 -
 include/sysemu/event-loop-base.h  |  1 -
 net/vmnet_int.h                   |  1 -
 qga/cutils.h                      |  2 --
 target/hexagon/hex_arch_types.h   |  1 -
 target/hexagon/mmvec/macros.h     |  1 -
 target/riscv/pmu.h                |  1 -
 accel/tcg/cpu-exec.c              |  1 -
 audio/sndioaudio.c                |  2 +-
 backends/hostmem-epc.c            |  2 +-
 backends/tpm/tpm_emulator.c       |  1 -
 block/export/vduse-blk.c          |  2 +-
 block/qapi.c                      |  1 -
 bsd-user/arm/signal.c             |  1 +
 bsd-user/arm/target_arch_cpu.c    |  2 ++
 bsd-user/freebsd/os-sys.c         |  1 +
 bsd-user/i386/signal.c            |  1 +
 bsd-user/i386/target_arch_cpu.c   |  3 +--
 bsd-user/main.c                   |  4 +---
 bsd-user/strace.c                 |  1 -
 bsd-user/x86_64/signal.c          |  1 +
 bsd-user/x86_64/target_arch_cpu.c |  3 +--
 hw/9pfs/9p.c                      |  5 -----
 hw/acpi/piix4.c                   |  1 -
 hw/alpha/dp264.c                  |  1 -
 hw/arm/virt.c                     |  1 -
 hw/arm/xlnx-versal.c              |  1 -
 hw/block/pflash_cfi01.c           |  1 -
 hw/core/machine.c                 |  1 -
 hw/display/virtio-gpu-udmabuf.c   |  1 -
 hw/hppa/machine.c                 |  1 -
 hw/hyperv/syndbg.c                |  2 +-
 hw/i2c/pmbus_device.c             |  1 -
 hw/i386/acpi-build.c              |  1 -
 hw/input/tsc210x.c                |  1 -
 hw/loongarch/acpi-build.c         |  1 -
 hw/misc/macio/cuda.c              |  1 -
 hw/misc/macio/pmu.c               |  1 -
 hw/net/xilinx_axienet.c           |  1 -
 hw/ppc/ppc405_uc.c                |  2 --
 hw/ppc/ppc440_bamboo.c            |  1 -
 hw/ppc/spapr_drc.c                |  1 -
 hw/rdma/vmw/pvrdma_dev_ring.c     |  1 -
 hw/remote/machine.c               |  1 -
 hw/remote/proxy-memory-listener.c |  1 -
 hw/remote/remote-obj.c            |  1 -
 hw/rtc/mc146818rtc.c              |  1 -
 hw/s390x/virtio-ccw-serial.c      |  1 -
 hw/sensor/adm1272.c               |  1 -
 hw/usb/dev-storage-bot.c          |  1 -
 hw/usb/dev-storage-classic.c      |  1 -
 migration/postcopy-ram.c          |  2 --
 qga/commands-posix.c              |  1 -
 qga/cutils.c                      |  3 ++-
 softmmu/dirtylimit.c              |  1 -
 softmmu/runstate.c                |  1 -
 softmmu/vl.c                      |  3 ---
 target/loongarch/translate.c      |  1 -
 target/mips/tcg/translate.c       |  1 -
 target/nios2/translate.c          |  2 --
 tcg/tci.c                         |  1 -
 tests/unit/test-cutils.c          |  1 -
 tests/unit/test-seccomp.c         |  1 -
 ui/gtk.c                          |  1 -
 ui/udmabuf.c                      |  1 -
 util/async-teardown.c             | 12 ++++--------
 util/main-loop.c                  |  1 -
 util/oslib-posix.c                |  6 ------
 scripts/clean-includes            | 15 ++++++++++-----
 89 files changed, 29 insertions(+), 123 deletions(-)

-- 
2.39.0

[PATCH v4 01/19] scripts/clean-includes: Fully skip / ignore files

[Markus Armbruster]

When clean-includes claims to skip or ignore a file, only the part
that sanitizes use of qemu/osdep.h skips the file.  The part that
looks for duplicate #include does not, and neither does committing to
Git.

The latter can get unrelated stuff included in the commit, but only if
you run clean-includes in a dirty tree, which is unwise.  Messed up
when we added skipping in commit fd3e39a40c "scripts/clean-includes:
Enhance to handle header files".

The former can cause bogus reports for --check-dup-head.  Added in
commit d66253e46a "scripts/clean-includes: added duplicate #include
check", duplicating the prior mistake.

Fix the script to fully skip files.

Fixes: fd3e39a40ca2ee26b09a5de3149af8b056b85233
Fixes: d66253e46ae2b9c36a9dd90b2b74c0dfa5804b22
Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 scripts/clean-includes | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/scripts/clean-includes b/scripts/clean-includes
index d37bd4f692..86944f27fc 100755
--- a/scripts/clean-includes
+++ b/scripts/clean-includes
@@ -111,6 +111,7 @@ cat >"$COCCIFILE" <<EOT
 )
 EOT
 
+files=
 for f in "$@"; do
   case "$f" in
     *.c.inc)
@@ -144,6 +145,7 @@ for f in "$@"; do
       continue
       ;;
   esac
+  files="$files $f"
 
   if [ "$MODE" = "c" ]; then
     # First, use Coccinelle to add qemu/osdep.h before the first existing include
@@ -174,8 +176,8 @@ for f in "$@"; do
 
 done
 
-if [ "$DUPHEAD" = "yes" ]; then
-    egrep "^[[:space:]]*#[[:space:]]*include" "$@" | tr -d '[:blank:]' \
+if [ "$DUPHEAD" = "yes" ] && [ -n "$files" ]; then
+    egrep "^[[:space:]]*#[[:space:]]*include" $files | tr -d '[:blank:]' \
         | sort | uniq -c | awk '{if ($1 > 1) print $0}'
     if [ $? -eq 0 ]; then
         echo "Found duplicate header file includes. Please check the above files manually."
@@ -184,7 +186,7 @@ if [ "$DUPHEAD" = "yes" ]; then
 fi
 
 if [ "$GIT" = "yes" ]; then
-    git add -- "$@"
+    git add -- $files
     git commit --signoff -F - <<EOF
 $GITSUBJ: Clean up includes
 
-- 
2.39.0

[PATCH v4 02/19] scripts/clean-includes: Don't claim duplicate headers found when not

[Markus Armbruster]

When running with --check-dup-head, the script always claims it "Found
duplicate header file includes."  Fix to do it only when it actually
found some.

Fixes: d66253e46ae2b9c36a9dd90b2b74c0dfa5804b22
Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 scripts/clean-includes | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/scripts/clean-includes b/scripts/clean-includes
index 86944f27fc..8e8420d785 100755
--- a/scripts/clean-includes
+++ b/scripts/clean-includes
@@ -177,9 +177,8 @@ for f in "$@"; do
 done
 
 if [ "$DUPHEAD" = "yes" ] && [ -n "$files" ]; then
-    egrep "^[[:space:]]*#[[:space:]]*include" $files | tr -d '[:blank:]' \
-        | sort | uniq -c | awk '{if ($1 > 1) print $0}'
-    if [ $? -eq 0 ]; then
+    if egrep "^[[:space:]]*#[[:space:]]*include" $files | tr -d '[:blank:]' \
+        | sort | uniq -c | grep -v '^ *1 '; then
         echo "Found duplicate header file includes. Please check the above files manually."
         exit 1
     fi
-- 
2.39.0

[PATCH v4 03/19] scripts/clean-includes: Skip symbolic links

[Markus Armbruster]

When a symbolic link points to a file that needs cleaning, the script
replaces the link with a cleaned regular file.  Not wanted; skip them.

We have a few symbolic links under subprojects/libvduse/ and
subprojects/libvhost-user/.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 scripts/clean-includes | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/scripts/clean-includes b/scripts/clean-includes
index 8e8420d785..f0466a6262 100755
--- a/scripts/clean-includes
+++ b/scripts/clean-includes
@@ -113,6 +113,10 @@ EOT
 
 files=
 for f in "$@"; do
+  if [ -L "$f" ]; then
+      echo "SKIPPING $f (symbolic link)"
+      continue
+  fi
   case "$f" in
     *.c.inc)
       # These aren't standalone C source files
-- 
2.39.0

[PATCH v4 04/19] bsd-user: Clean up includes

[Markus Armbruster]

Clean up includes so that osdep.h is included first and headers
which it implies are not included manually.

This commit was created with scripts/clean-includes.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 bsd-user/bsd-proc.h               | 4 ----
 bsd-user/qemu.h                   | 1 -
 bsd-user/arm/signal.c             | 1 +
 bsd-user/arm/target_arch_cpu.c    | 2 ++
 bsd-user/freebsd/os-sys.c         | 1 +
 bsd-user/i386/signal.c            | 1 +
 bsd-user/i386/target_arch_cpu.c   | 3 +--
 bsd-user/main.c                   | 4 +---
 bsd-user/strace.c                 | 1 -
 bsd-user/x86_64/signal.c          | 1 +
 bsd-user/x86_64/target_arch_cpu.c | 3 +--
 11 files changed, 9 insertions(+), 13 deletions(-)

diff --git a/bsd-user/bsd-proc.h b/bsd-user/bsd-proc.h
index 68b66e571d..a1061bffb8 100644
--- a/bsd-user/bsd-proc.h
+++ b/bsd-user/bsd-proc.h
@@ -20,11 +20,7 @@
 #ifndef BSD_PROC_H_
 #define BSD_PROC_H_
 
-#include <sys/types.h>
-#include <sys/stat.h>
-#include <sys/time.h>
 #include <sys/resource.h>
-#include <unistd.h>
 
 /* exit(2) */
 static inline abi_long do_bsd_exit(void *cpu_env, abi_long arg1)
diff --git a/bsd-user/qemu.h b/bsd-user/qemu.h
index be6105385e..0ceecfb6df 100644
--- a/bsd-user/qemu.h
+++ b/bsd-user/qemu.h
@@ -17,7 +17,6 @@
 #ifndef QEMU_H
 #define QEMU_H
 
-#include "qemu/osdep.h"
 #include "cpu.h"
 #include "qemu/units.h"
 #include "exec/cpu_ldst.h"
diff --git a/bsd-user/arm/signal.c b/bsd-user/arm/signal.c
index 2b1dd745d1..9734407543 100644
--- a/bsd-user/arm/signal.c
+++ b/bsd-user/arm/signal.c
@@ -17,6 +17,7 @@
  *  along with this program; if not, see <http://www.gnu.org/licenses/>.
  */
 
+#include "qemu/osdep.h"
 #include "qemu.h"
 
 /*
diff --git a/bsd-user/arm/target_arch_cpu.c b/bsd-user/arm/target_arch_cpu.c
index 02bf9149d5..fe38ae2210 100644
--- a/bsd-user/arm/target_arch_cpu.c
+++ b/bsd-user/arm/target_arch_cpu.c
@@ -16,6 +16,8 @@
  *  You should have received a copy of the GNU General Public License
  *  along with this program; if not, see <http://www.gnu.org/licenses/>.
  */
+
+#include "qemu/osdep.h"
 #include "target_arch.h"
 
 void target_cpu_set_tls(CPUARMState *env, target_ulong newtls)
diff --git a/bsd-user/freebsd/os-sys.c b/bsd-user/freebsd/os-sys.c
index 309e27b9d6..1676ec10f8 100644
--- a/bsd-user/freebsd/os-sys.c
+++ b/bsd-user/freebsd/os-sys.c
@@ -17,6 +17,7 @@
  *  along with this program; if not, see <http://www.gnu.org/licenses/>.
  */
 
+#include "qemu/osdep.h"
 #include "qemu.h"
 #include "target_arch_sysarch.h"
 
diff --git a/bsd-user/i386/signal.c b/bsd-user/i386/signal.c
index 5dd975ce56..a3131047b8 100644
--- a/bsd-user/i386/signal.c
+++ b/bsd-user/i386/signal.c
@@ -17,6 +17,7 @@
  *  along with this program; if not, see <http://www.gnu.org/licenses/>.
  */
 
+#include "qemu/osdep.h"
 #include "qemu.h"
 
 /*
diff --git a/bsd-user/i386/target_arch_cpu.c b/bsd-user/i386/target_arch_cpu.c
index d349e45299..2a3af2ddef 100644
--- a/bsd-user/i386/target_arch_cpu.c
+++ b/bsd-user/i386/target_arch_cpu.c
@@ -17,9 +17,8 @@
  *  along with this program; if not, see <http://www.gnu.org/licenses/>.
  */
 
-#include <sys/types.h>
-
 #include "qemu/osdep.h"
+
 #include "cpu.h"
 #include "qemu.h"
 #include "qemu/timer.h"
diff --git a/bsd-user/main.c b/bsd-user/main.c
index 6f09180d65..41290e16f9 100644
--- a/bsd-user/main.c
+++ b/bsd-user/main.c
@@ -18,12 +18,10 @@
  *  along with this program; if not, see <http://www.gnu.org/licenses/>.
  */
 
-#include <sys/types.h>
-#include <sys/time.h>
+#include "qemu/osdep.h"
 #include <sys/resource.h>
 #include <sys/sysctl.h>
 
-#include "qemu/osdep.h"
 #include "qemu/help-texts.h"
 #include "qemu/units.h"
 #include "qemu/accel.h"
diff --git a/bsd-user/strace.c b/bsd-user/strace.c
index a77d10dd6b..96499751eb 100644
--- a/bsd-user/strace.c
+++ b/bsd-user/strace.c
@@ -20,7 +20,6 @@
 #include <sys/select.h>
 #include <sys/syscall.h>
 #include <sys/ioccom.h>
-#include <ctype.h>
 
 #include "qemu.h"
 
diff --git a/bsd-user/x86_64/signal.c b/bsd-user/x86_64/signal.c
index c3875bc4c6..46cb865180 100644
--- a/bsd-user/x86_64/signal.c
+++ b/bsd-user/x86_64/signal.c
@@ -16,6 +16,7 @@
  *  along with this program; if not, see <http://www.gnu.org/licenses/>.
  */
 
+#include "qemu/osdep.h"
 #include "qemu.h"
 
 /*
diff --git a/bsd-user/x86_64/target_arch_cpu.c b/bsd-user/x86_64/target_arch_cpu.c
index be7bd10720..1d32f18907 100644
--- a/bsd-user/x86_64/target_arch_cpu.c
+++ b/bsd-user/x86_64/target_arch_cpu.c
@@ -17,9 +17,8 @@
  *  along with this program; if not, see <http://www.gnu.org/licenses/>.
  */
 
-#include <sys/types.h>
-
 #include "qemu/osdep.h"
+
 #include "cpu.h"
 #include "qemu.h"
 #include "qemu/timer.h"
-- 
2.39.0

[PATCH v4 05/19] crypto: Clean up includes

[Markus Armbruster]

Clean up includes so that osdep.h is included first and headers
which it implies are not included manually.

This commit was created with scripts/clean-includes.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 crypto/block-luks-priv.h | 1 -
 1 file changed, 1 deletion(-)

diff --git a/crypto/block-luks-priv.h b/crypto/block-luks-priv.h
index 90a20d432b..1066df0307 100644
--- a/crypto/block-luks-priv.h
+++ b/crypto/block-luks-priv.h
@@ -18,7 +18,6 @@
  *
  */
 
-#include "qemu/osdep.h"
 #include "qapi/error.h"
 #include "qemu/bswap.h"
 
-- 
2.39.0

[PATCH v4 06/19] hw/cxl: Clean up includes

[Markus Armbruster]

Clean up includes so that osdep.h is included first and headers
which it implies are not included manually.

This commit was created with scripts/clean-includes.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 include/hw/cxl/cxl_component.h | 2 --
 include/hw/cxl/cxl_host.h      | 1 -
 include/hw/cxl/cxl_pci.h       | 1 -
 3 files changed, 4 deletions(-)

diff --git a/include/hw/cxl/cxl_component.h b/include/hw/cxl/cxl_component.h
index 5dca21e95b..692d7a5507 100644
--- a/include/hw/cxl/cxl_component.h
+++ b/include/hw/cxl/cxl_component.h
@@ -15,9 +15,7 @@
 #define CXL2_COMPONENT_CM_REGION_SIZE 0x1000
 #define CXL2_COMPONENT_BLOCK_SIZE 0x10000
 
-#include "qemu/compiler.h"
 #include "qemu/range.h"
-#include "qemu/typedefs.h"
 #include "hw/cxl/cxl_cdat.h"
 #include "hw/register.h"
 #include "qapi/error.h"
diff --git a/include/hw/cxl/cxl_host.h b/include/hw/cxl/cxl_host.h
index a1b662ce40..c9bc9c7c50 100644
--- a/include/hw/cxl/cxl_host.h
+++ b/include/hw/cxl/cxl_host.h
@@ -7,7 +7,6 @@
  * COPYING file in the top-level directory.
  */
 
-#include "qemu/osdep.h"
 #include "hw/cxl/cxl.h"
 #include "hw/boards.h"
 
diff --git a/include/hw/cxl/cxl_pci.h b/include/hw/cxl/cxl_pci.h
index 01e15ed5b4..407be95b9e 100644
--- a/include/hw/cxl/cxl_pci.h
+++ b/include/hw/cxl/cxl_pci.h
@@ -10,7 +10,6 @@
 #ifndef CXL_PCI_H
 #define CXL_PCI_H
 
-#include "qemu/compiler.h"
 
 #define CXL_VENDOR_ID 0x1e98
 
-- 
2.39.0

[PATCH v4 07/19] hw/input: Clean up includes

[Markus Armbruster]

Clean up includes so that osdep.h is included first and headers
which it implies are not included manually.

This commit was created with scripts/clean-includes.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 include/hw/input/pl050.h | 1 -
 hw/input/tsc210x.c       | 1 -
 2 files changed, 2 deletions(-)

diff --git a/include/hw/input/pl050.h b/include/hw/input/pl050.h
index 89ec4fafc9..4cb8985f31 100644
--- a/include/hw/input/pl050.h
+++ b/include/hw/input/pl050.h
@@ -10,7 +10,6 @@
 #ifndef HW_PL050_H
 #define HW_PL050_H
 
-#include "qemu/osdep.h"
 #include "hw/sysbus.h"
 #include "migration/vmstate.h"
 #include "hw/input/ps2.h"
diff --git a/hw/input/tsc210x.c b/hw/input/tsc210x.c
index fdd5ff87d9..7eae5989f7 100644
--- a/hw/input/tsc210x.c
+++ b/hw/input/tsc210x.c
@@ -20,7 +20,6 @@
  */
 
 #include "qemu/osdep.h"
-#include "qemu/log.h"
 #include "hw/hw.h"
 #include "audio/audio.h"
 #include "qemu/timer.h"
-- 
2.39.0

[PATCH v4 08/19] hw/tricore: Clean up includes

[Markus Armbruster]

Clean up includes so that osdep.h is included first and headers
which it implies are not included manually.

This commit was created with scripts/clean-includes.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 include/hw/tricore/triboard.h | 1 -
 1 file changed, 1 deletion(-)

diff --git a/include/hw/tricore/triboard.h b/include/hw/tricore/triboard.h
index 094c8bd563..4fdd2d7d97 100644
--- a/include/hw/tricore/triboard.h
+++ b/include/hw/tricore/triboard.h
@@ -18,7 +18,6 @@
  * License along with this library; if not, see <http://www.gnu.org/licenses/>.
  */
 
-#include "qemu/osdep.h"
 #include "qapi/error.h"
 #include "hw/boards.h"
 #include "sysemu/sysemu.h"
-- 
2.39.0

[PATCH v4 09/19] qga: Clean up includes

[Markus Armbruster]

Clean up includes so that osdep.h is included first and headers
which it implies are not included manually.

This commit was created with scripts/clean-includes.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 qga/cutils.h         | 2 --
 qga/commands-posix.c | 1 -
 qga/cutils.c         | 3 ++-
 3 files changed, 2 insertions(+), 4 deletions(-)

diff --git a/qga/cutils.h b/qga/cutils.h
index f0f30a7d28..c1f2f4b17a 100644
--- a/qga/cutils.h
+++ b/qga/cutils.h
@@ -1,8 +1,6 @@
 #ifndef CUTILS_H_
 #define CUTILS_H_
 
-#include "qemu/osdep.h"
-
 int qga_open_cloexec(const char *name, int flags, mode_t mode);
 
 #endif /* CUTILS_H_ */
diff --git a/qga/commands-posix.c b/qga/commands-posix.c
index ebd33a643c..079689d79a 100644
--- a/qga/commands-posix.c
+++ b/qga/commands-posix.c
@@ -51,7 +51,6 @@
 #else
 #include <net/ethernet.h>
 #endif
-#include <sys/types.h>
 #ifdef CONFIG_SOLARIS
 #include <sys/sockio.h>
 #endif
diff --git a/qga/cutils.c b/qga/cutils.c
index b8e142ef64..b21bcf3683 100644
--- a/qga/cutils.c
+++ b/qga/cutils.c
@@ -2,8 +2,9 @@
  * This work is licensed under the terms of the GNU GPL, version 2 or later.
  * See the COPYING file in the top-level directory.
  */
-#include "cutils.h"
 
+#include "qemu/osdep.h"
+#include "cutils.h"
 #include "qapi/error.h"
 
 /**
-- 
2.39.0

[PATCH v4 10/19] migration: Clean up includes

[Markus Armbruster]

Clean up includes so that osdep.h is included first and headers
which it implies are not included manually.

This commit was created with scripts/clean-includes.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 include/qemu/userfaultfd.h | 1 -
 1 file changed, 1 deletion(-)

diff --git a/include/qemu/userfaultfd.h b/include/qemu/userfaultfd.h
index 6b74f92792..55c95998e8 100644
--- a/include/qemu/userfaultfd.h
+++ b/include/qemu/userfaultfd.h
@@ -13,7 +13,6 @@
 #ifndef USERFAULTFD_H
 #define USERFAULTFD_H
 
-#include "qemu/osdep.h"
 #include "exec/hwaddr.h"
 #include <linux/userfaultfd.h>
 
-- 
2.39.0

[PATCH v4 11/19] net: Clean up includes

[Markus Armbruster]

Clean up includes so that osdep.h is included first and headers
which it implies are not included manually.

This commit was created with scripts/clean-includes.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 net/vmnet_int.h | 1 -
 1 file changed, 1 deletion(-)

diff --git a/net/vmnet_int.h b/net/vmnet_int.h
index adf6e8c20d..d0b90594f2 100644
--- a/net/vmnet_int.h
+++ b/net/vmnet_int.h
@@ -10,7 +10,6 @@
 #ifndef VMNET_INT_H
 #define VMNET_INT_H
 
-#include "qemu/osdep.h"
 #include "vmnet_int.h"
 #include "clients.h"
 
-- 
2.39.0

[PATCH v4 12/19] target/hexagon: Clean up includes

[Markus Armbruster]

Clean up includes so that osdep.h is included first and headers
which it implies are not included manually.

This commit was created with scripts/clean-includes.

Changes to standalone programs dropped, because I can't tell whether
them not using qemu/osdep.h is intentional:

    target/hexagon/gen_dectree_import.c
    target/hexagon/gen_semantics.c
    target/hexagon/idef-parser/idef-parser.h
    target/hexagon/idef-parser/parser-helpers.c
    target/hexagon/idef-parser/parser-helpers.h

Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 target/hexagon/hex_arch_types.h | 1 -
 target/hexagon/mmvec/macros.h   | 1 -
 2 files changed, 2 deletions(-)

diff --git a/target/hexagon/hex_arch_types.h b/target/hexagon/hex_arch_types.h
index 885f68f760..52a7f2b2f3 100644
--- a/target/hexagon/hex_arch_types.h
+++ b/target/hexagon/hex_arch_types.h
@@ -18,7 +18,6 @@
 #ifndef HEXAGON_HEX_ARCH_TYPES_H
 #define HEXAGON_HEX_ARCH_TYPES_H
 
-#include "qemu/osdep.h"
 #include "mmvec/mmvec.h"
 #include "qemu/int128.h"
 
diff --git a/target/hexagon/mmvec/macros.h b/target/hexagon/mmvec/macros.h
index 8c864e8c68..1201d778d0 100644
--- a/target/hexagon/mmvec/macros.h
+++ b/target/hexagon/mmvec/macros.h
@@ -18,7 +18,6 @@
 #ifndef HEXAGON_MMVEC_MACROS_H
 #define HEXAGON_MMVEC_MACROS_H
 
-#include "qemu/osdep.h"
 #include "qemu/host-utils.h"
 #include "arch.h"
 #include "mmvec/system_ext_mmvec.h"
-- 
2.39.0

[PATCH v4 13/19] riscv: Clean up includes

[Markus Armbruster]

Clean up includes so that osdep.h is included first and headers
which it implies are not included manually.

This commit was created with scripts/clean-includes.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 target/riscv/pmu.h | 1 -
 1 file changed, 1 deletion(-)

diff --git a/target/riscv/pmu.h b/target/riscv/pmu.h
index 3004ce37b6..0c819ca983 100644
--- a/target/riscv/pmu.h
+++ b/target/riscv/pmu.h
@@ -16,7 +16,6 @@
  * this program.  If not, see <http://www.gnu.org/licenses/>.
  */
 
-#include "qemu/osdep.h"
 #include "qemu/log.h"
 #include "cpu.h"
 #include "qemu/main-loop.h"
-- 
2.39.0

[PATCH v4 14/19] block: Clean up includes

[Markus Armbruster]

Clean up includes so that osdep.h is included first and headers
which it implies are not included manually.

This commit was created with scripts/clean-includes.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 include/block/graph-lock.h      | 1 -
 include/block/write-threshold.h | 2 --
 block/qapi.c                    | 1 -
 3 files changed, 4 deletions(-)

diff --git a/include/block/graph-lock.h b/include/block/graph-lock.h
index 4c92cd8edf..ed9263b84e 100644
--- a/include/block/graph-lock.h
+++ b/include/block/graph-lock.h
@@ -20,7 +20,6 @@
 #ifndef GRAPH_LOCK_H
 #define GRAPH_LOCK_H
 
-#include "qemu/osdep.h"
 #include "qemu/clang-tsa.h"
 
 #include "qemu/coroutine.h"
diff --git a/include/block/write-threshold.h b/include/block/write-threshold.h
index f50f923e7e..63d1583887 100644
--- a/include/block/write-threshold.h
+++ b/include/block/write-threshold.h
@@ -13,8 +13,6 @@
 #ifndef BLOCK_WRITE_THRESHOLD_H
 #define BLOCK_WRITE_THRESHOLD_H
 
-#include "qemu/typedefs.h"
-
 /*
  * bdrv_write_threshold_set:
  *
diff --git a/block/qapi.c b/block/qapi.c
index fea808425b..aede6a40d8 100644
--- a/block/qapi.c
+++ b/block/qapi.c
@@ -39,7 +39,6 @@
 #include "qapi/qmp/qstring.h"
 #include "qemu/qemu-print.h"
 #include "sysemu/block-backend.h"
-#include "qemu/cutils.h"
 
 BlockDeviceInfo *bdrv_block_device_info(BlockBackend *blk,
                                         BlockDriverState *bs,
-- 
2.39.0

[PATCH v4 15/19] accel: Clean up includes

[Markus Armbruster]

Clean up includes so that osdep.h is included first and headers
which it implies are not included manually.

This commit was created with scripts/clean-includes.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 include/sysemu/accel-blocker.h | 1 -
 1 file changed, 1 deletion(-)

diff --git a/include/sysemu/accel-blocker.h b/include/sysemu/accel-blocker.h
index 72020529ef..0733783bcc 100644
--- a/include/sysemu/accel-blocker.h
+++ b/include/sysemu/accel-blocker.h
@@ -14,7 +14,6 @@
 #ifndef ACCEL_BLOCKER_H
 #define ACCEL_BLOCKER_H
 
-#include "qemu/osdep.h"
 #include "sysemu/cpus.h"
 
 extern void accel_blocker_init(void);
-- 
2.39.0

[PATCH v4 16/19] Fix non-first inclusions of qemu/osdep.h

[Markus Armbruster]

This commit was created with scripts/clean-includes.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 audio/sndioaudio.c       |  2 +-
 backends/hostmem-epc.c   |  2 +-
 block/export/vduse-blk.c |  2 +-
 hw/hyperv/syndbg.c       |  2 +-
 util/async-teardown.c    | 12 ++++--------
 5 files changed, 8 insertions(+), 12 deletions(-)

diff --git a/audio/sndioaudio.c b/audio/sndioaudio.c
index 632b0e3825..3fde01fdbd 100644
--- a/audio/sndioaudio.c
+++ b/audio/sndioaudio.c
@@ -14,9 +14,9 @@
  * to recording, which is what guest systems expect.
  */
 
+#include "qemu/osdep.h"
 #include <poll.h>
 #include <sndio.h>
-#include "qemu/osdep.h"
 #include "qemu/main-loop.h"
 #include "audio.h"
 #include "trace.h"
diff --git a/backends/hostmem-epc.c b/backends/hostmem-epc.c
index 037292d267..4e162d6789 100644
--- a/backends/hostmem-epc.c
+++ b/backends/hostmem-epc.c
@@ -9,9 +9,9 @@
  * This work is licensed under the terms of the GNU GPL, version 2 or later.
  * See the COPYING file in the top-level directory.
  */
-#include <sys/ioctl.h>
 
 #include "qemu/osdep.h"
+#include <sys/ioctl.h>
 #include "qom/object_interfaces.h"
 #include "qapi/error.h"
 #include "sysemu/hostmem.h"
diff --git a/block/export/vduse-blk.c b/block/export/vduse-blk.c
index 350d6fdaf0..f7ae44e3ce 100644
--- a/block/export/vduse-blk.c
+++ b/block/export/vduse-blk.c
@@ -10,9 +10,9 @@
  * later.  See the COPYING file in the top-level directory.
  */
 
+#include "qemu/osdep.h"
 #include <sys/eventfd.h>
 
-#include "qemu/osdep.h"
 #include "qapi/error.h"
 #include "block/export.h"
 #include "qemu/error-report.h"
diff --git a/hw/hyperv/syndbg.c b/hw/hyperv/syndbg.c
index 16d04cfdc6..94fe1b534b 100644
--- a/hw/hyperv/syndbg.c
+++ b/hw/hyperv/syndbg.c
@@ -5,8 +5,8 @@
  * See the COPYING file in the top-level directory.
  */
 
-#include "qemu/ctype.h"
 #include "qemu/osdep.h"
+#include "qemu/ctype.h"
 #include "qemu/error-report.h"
 #include "qemu/main-loop.h"
 #include "qemu/sockets.h"
diff --git a/util/async-teardown.c b/util/async-teardown.c
index 62bfce1b3c..62cdeb0f20 100644
--- a/util/async-teardown.c
+++ b/util/async-teardown.c
@@ -10,16 +10,12 @@
  * option) any later version.  See the COPYING file in the top-level directory.
  *
  */
-#include <stdlib.h>
-#include <stdio.h>
-#include <sys/types.h>
-#include <dirent.h>
-#include <sys/prctl.h>
-#include <signal.h>
-#include <sched.h>
-#include <unistd.h>
 
 #include "qemu/osdep.h"
+#include <dirent.h>
+#include <sys/prctl.h>
+#include <sched.h>
+
 #include "qemu/async-teardown.h"
 
 #ifdef _SC_THREAD_STACK_MIN
-- 
2.39.0

[PATCH v4 17/19] Don't include headers already included by qemu/osdep.h

[Markus Armbruster]

This commit was created with scripts/clean-includes.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 backends/tpm/tpm_ioctl.h          | 2 --
 fsdev/p9array.h                   | 2 --
 include/hw/misc/aspeed_lpc.h      | 2 --
 include/hw/pci/pcie_doe.h         | 1 -
 include/qemu/async-teardown.h     | 2 --
 include/qemu/dbus.h               | 1 -
 include/qemu/host-utils.h         | 1 -
 include/sysemu/event-loop-base.h  | 1 -
 accel/tcg/cpu-exec.c              | 1 -
 hw/9pfs/9p.c                      | 2 --
 hw/display/virtio-gpu-udmabuf.c   | 1 -
 hw/i2c/pmbus_device.c             | 1 -
 hw/remote/proxy-memory-listener.c | 1 -
 hw/sensor/adm1272.c               | 1 -
 hw/usb/dev-storage-bot.c          | 1 -
 hw/usb/dev-storage-classic.c      | 1 -
 softmmu/vl.c                      | 2 --
 tcg/tci.c                         | 1 -
 tests/unit/test-seccomp.c         | 1 -
 ui/udmabuf.c                      | 1 -
 util/main-loop.c                  | 1 -
 util/oslib-posix.c                | 2 --
 22 files changed, 29 deletions(-)

diff --git a/backends/tpm/tpm_ioctl.h b/backends/tpm/tpm_ioctl.h
index e506ef5160..b1d31768a6 100644
--- a/backends/tpm/tpm_ioctl.h
+++ b/backends/tpm/tpm_ioctl.h
@@ -12,8 +12,6 @@
 # define __USE_LINUX_IOCTL_DEFS
 #endif
 
-#include <stdint.h>
-#include <sys/types.h>
 #ifndef _WIN32
 #include <sys/uio.h>
 #include <sys/ioctl.h>
diff --git a/fsdev/p9array.h b/fsdev/p9array.h
index 90e83a7c7b..50a1b15fe9 100644
--- a/fsdev/p9array.h
+++ b/fsdev/p9array.h
@@ -27,8 +27,6 @@
 #ifndef QEMU_P9ARRAY_H
 #define QEMU_P9ARRAY_H
 
-#include "qemu/compiler.h"
-
 /**
  * P9Array provides a mechanism to access arrays in common C-style (e.g. by
  * square bracket [] operator) in conjunction with reference variables that
diff --git a/include/hw/misc/aspeed_lpc.h b/include/hw/misc/aspeed_lpc.h
index fd228731d2..fa398959af 100644
--- a/include/hw/misc/aspeed_lpc.h
+++ b/include/hw/misc/aspeed_lpc.h
@@ -12,8 +12,6 @@
 
 #include "hw/sysbus.h"
 
-#include <stdint.h>
-
 #define TYPE_ASPEED_LPC "aspeed.lpc"
 #define ASPEED_LPC(obj) OBJECT_CHECK(AspeedLPCState, (obj), TYPE_ASPEED_LPC)
 
diff --git a/include/hw/pci/pcie_doe.h b/include/hw/pci/pcie_doe.h
index ba4d8b03bd..87dc17dcef 100644
--- a/include/hw/pci/pcie_doe.h
+++ b/include/hw/pci/pcie_doe.h
@@ -11,7 +11,6 @@
 #define PCIE_DOE_H
 
 #include "qemu/range.h"
-#include "qemu/typedefs.h"
 #include "hw/register.h"
 
 /*
diff --git a/include/qemu/async-teardown.h b/include/qemu/async-teardown.h
index 092e7a37e7..b281da005b 100644
--- a/include/qemu/async-teardown.h
+++ b/include/qemu/async-teardown.h
@@ -13,8 +13,6 @@
 #ifndef QEMU_ASYNC_TEARDOWN_H
 #define QEMU_ASYNC_TEARDOWN_H
 
-#include "config-host.h"
-
 #ifdef CONFIG_LINUX
 void init_async_teardown(void);
 #endif
diff --git a/include/qemu/dbus.h b/include/qemu/dbus.h
index 08f00dfd53..81d3de8a5a 100644
--- a/include/qemu/dbus.h
+++ b/include/qemu/dbus.h
@@ -15,7 +15,6 @@
 #include "qom/object.h"
 #include "chardev/char.h"
 #include "qemu/notify.h"
-#include "qemu/typedefs.h"
 
 /* glib/gio 2.68 */
 #define DBUS_METHOD_INVOCATION_HANDLED TRUE
diff --git a/include/qemu/host-utils.h b/include/qemu/host-utils.h
index 88d476161c..3ce62bf4a5 100644
--- a/include/qemu/host-utils.h
+++ b/include/qemu/host-utils.h
@@ -30,7 +30,6 @@
 #ifndef HOST_UTILS_H
 #define HOST_UTILS_H
 
-#include "qemu/compiler.h"
 #include "qemu/bswap.h"
 #include "qemu/int128.h"
 
diff --git a/include/sysemu/event-loop-base.h b/include/sysemu/event-loop-base.h
index 2748bf6ae1..a6c24f1351 100644
--- a/include/sysemu/event-loop-base.h
+++ b/include/sysemu/event-loop-base.h
@@ -14,7 +14,6 @@
 
 #include "qom/object.h"
 #include "block/aio.h"
-#include "qemu/typedefs.h"
 
 #define TYPE_EVENT_LOOP_BASE         "event-loop-base"
 OBJECT_DECLARE_TYPE(EventLoopBase, EventLoopBaseClass,
diff --git a/accel/tcg/cpu-exec.c b/accel/tcg/cpu-exec.c
index 8927092537..dd8f54a415 100644
--- a/accel/tcg/cpu-exec.c
+++ b/accel/tcg/cpu-exec.c
@@ -28,7 +28,6 @@
 #include "exec/exec-all.h"
 #include "tcg/tcg.h"
 #include "qemu/atomic.h"
-#include "qemu/compiler.h"
 #include "qemu/timer.h"
 #include "qemu/rcu.h"
 #include "exec/log.h"
diff --git a/hw/9pfs/9p.c b/hw/9pfs/9p.c
index 072cf67956..9621ec1341 100644
--- a/hw/9pfs/9p.c
+++ b/hw/9pfs/9p.c
@@ -19,8 +19,6 @@
 #include "qemu/osdep.h"
 #ifdef CONFIG_LINUX
 #include <linux/limits.h>
-#else
-#include <limits.h>
 #endif
 #include <glib/gprintf.h>
 #include "hw/virtio/virtio.h"
diff --git a/hw/display/virtio-gpu-udmabuf.c b/hw/display/virtio-gpu-udmabuf.c
index 8bdf4bac6e..847fa4c0cc 100644
--- a/hw/display/virtio-gpu-udmabuf.c
+++ b/hw/display/virtio-gpu-udmabuf.c
@@ -21,7 +21,6 @@
 #include "exec/ramblock.h"
 #include "sysemu/hostmem.h"
 #include <sys/ioctl.h>
-#include <fcntl.h>
 #include <linux/memfd.h>
 #include "qemu/memfd.h"
 #include "standard-headers/linux/udmabuf.h"
diff --git a/hw/i2c/pmbus_device.c b/hw/i2c/pmbus_device.c
index 4071a88cfc..c3d6046784 100644
--- a/hw/i2c/pmbus_device.c
+++ b/hw/i2c/pmbus_device.c
@@ -8,7 +8,6 @@
 
 #include "qemu/osdep.h"
 #include <math.h>
-#include <string.h>
 #include "hw/i2c/pmbus_device.h"
 #include "migration/vmstate.h"
 #include "qemu/module.h"
diff --git a/hw/remote/proxy-memory-listener.c b/hw/remote/proxy-memory-listener.c
index eb9918fe72..18d96a1d04 100644
--- a/hw/remote/proxy-memory-listener.c
+++ b/hw/remote/proxy-memory-listener.c
@@ -8,7 +8,6 @@
 
 #include "qemu/osdep.h"
 
-#include "qemu/compiler.h"
 #include "qemu/int128.h"
 #include "qemu/range.h"
 #include "exec/memory.h"
diff --git a/hw/sensor/adm1272.c b/hw/sensor/adm1272.c
index 7310c769be..8f4a1c2cd4 100644
--- a/hw/sensor/adm1272.c
+++ b/hw/sensor/adm1272.c
@@ -8,7 +8,6 @@
  */
 
 #include "qemu/osdep.h"
-#include <string.h>
 #include "hw/i2c/pmbus_device.h"
 #include "hw/irq.h"
 #include "migration/vmstate.h"
diff --git a/hw/usb/dev-storage-bot.c b/hw/usb/dev-storage-bot.c
index b24b3148c2..1e5c5c711f 100644
--- a/hw/usb/dev-storage-bot.c
+++ b/hw/usb/dev-storage-bot.c
@@ -8,7 +8,6 @@
  */
 
 #include "qemu/osdep.h"
-#include "qemu/typedefs.h"
 #include "qapi/error.h"
 #include "hw/usb.h"
 #include "hw/usb/desc.h"
diff --git a/hw/usb/dev-storage-classic.c b/hw/usb/dev-storage-classic.c
index 00f25bade2..84d19752b5 100644
--- a/hw/usb/dev-storage-classic.c
+++ b/hw/usb/dev-storage-classic.c
@@ -8,7 +8,6 @@
  */
 
 #include "qemu/osdep.h"
-#include "qemu/typedefs.h"
 #include "qapi/error.h"
 #include "qapi/visitor.h"
 #include "hw/usb.h"
diff --git a/softmmu/vl.c b/softmmu/vl.c
index 9177d95d4e..5355a7fe5a 100644
--- a/softmmu/vl.c
+++ b/softmmu/vl.c
@@ -136,8 +136,6 @@
 #include "qemu/guest-random.h"
 #include "qemu/keyval.h"
 
-#include "config-host.h"
-
 #define MAX_VIRTIO_CONSOLES 1
 
 typedef struct BlockdevOptionsQueueEntry {
diff --git a/tcg/tci.c b/tcg/tci.c
index 05a24163d3..e7ac74cab0 100644
--- a/tcg/tci.c
+++ b/tcg/tci.c
@@ -21,7 +21,6 @@
 #include "exec/cpu_ldst.h"
 #include "tcg/tcg-op.h"
 #include "tcg/tcg-ldst.h"
-#include "qemu/compiler.h"
 #include <ffi.h>
 
 
diff --git a/tests/unit/test-seccomp.c b/tests/unit/test-seccomp.c
index 3d7771e46c..f02c79cafd 100644
--- a/tests/unit/test-seccomp.c
+++ b/tests/unit/test-seccomp.c
@@ -25,7 +25,6 @@
 #include "qapi/error.h"
 #include "qemu/module.h"
 
-#include <unistd.h>
 #include <sys/syscall.h>
 
 static void test_seccomp_helper(const char *args, bool killed,
diff --git a/ui/udmabuf.c b/ui/udmabuf.c
index cebceb2610..cbf4357bb1 100644
--- a/ui/udmabuf.c
+++ b/ui/udmabuf.c
@@ -8,7 +8,6 @@
 #include "qapi/error.h"
 #include "ui/console.h"
 
-#include <fcntl.h>
 #include <sys/ioctl.h>
 
 int udmabuf_fd(void)
diff --git a/util/main-loop.c b/util/main-loop.c
index 58f776a8c9..3c0f525192 100644
--- a/util/main-loop.c
+++ b/util/main-loop.c
@@ -33,7 +33,6 @@
 #include "block/thread-pool.h"
 #include "qemu/error-report.h"
 #include "qemu/queue.h"
-#include "qemu/compiler.h"
 #include "qom/object.h"
 
 #ifndef _WIN32
diff --git a/util/oslib-posix.c b/util/oslib-posix.c
index 59a891b6a8..fd03fd32c8 100644
--- a/util/oslib-posix.c
+++ b/util/oslib-posix.c
@@ -40,7 +40,6 @@
 #include "qemu/thread.h"
 #include <libgen.h>
 #include "qemu/cutils.h"
-#include "qemu/compiler.h"
 #include "qemu/units.h"
 #include "qemu/thread-context.h"
 
@@ -50,7 +49,6 @@
 
 #ifdef __FreeBSD__
 #include <sys/thr.h>
-#include <sys/types.h>
 #include <sys/user.h>
 #include <libutil.h>
 #endif
-- 
2.39.0

[PATCH v4 18/19] 9p: Drop superfluous include of linux/limits.h

[Markus Armbruster]

Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 hw/9pfs/9p.c | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/hw/9pfs/9p.c b/hw/9pfs/9p.c
index 9621ec1341..aa736af380 100644
--- a/hw/9pfs/9p.c
+++ b/hw/9pfs/9p.c
@@ -17,9 +17,6 @@
  */
 
 #include "qemu/osdep.h"
-#ifdef CONFIG_LINUX
-#include <linux/limits.h>
-#endif
 #include <glib/gprintf.h>
 #include "hw/virtio/virtio.h"
 #include "qapi/error.h"
-- 
2.39.0

[PATCH v4 19/19] Drop duplicate #include

[Markus Armbruster]

Tracked down with the help of scripts/clean-includes.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 include/hw/arm/fsl-imx6ul.h   | 1 -
 include/hw/arm/fsl-imx7.h     | 1 -
 backends/tpm/tpm_emulator.c   | 1 -
 hw/acpi/piix4.c               | 1 -
 hw/alpha/dp264.c              | 1 -
 hw/arm/virt.c                 | 1 -
 hw/arm/xlnx-versal.c          | 1 -
 hw/block/pflash_cfi01.c       | 1 -
 hw/core/machine.c             | 1 -
 hw/hppa/machine.c             | 1 -
 hw/i386/acpi-build.c          | 1 -
 hw/loongarch/acpi-build.c     | 1 -
 hw/misc/macio/cuda.c          | 1 -
 hw/misc/macio/pmu.c           | 1 -
 hw/net/xilinx_axienet.c       | 1 -
 hw/ppc/ppc405_uc.c            | 2 --
 hw/ppc/ppc440_bamboo.c        | 1 -
 hw/ppc/spapr_drc.c            | 1 -
 hw/rdma/vmw/pvrdma_dev_ring.c | 1 -
 hw/remote/machine.c           | 1 -
 hw/remote/remote-obj.c        | 1 -
 hw/rtc/mc146818rtc.c          | 1 -
 hw/s390x/virtio-ccw-serial.c  | 1 -
 migration/postcopy-ram.c      | 2 --
 softmmu/dirtylimit.c          | 1 -
 softmmu/runstate.c            | 1 -
 softmmu/vl.c                  | 1 -
 target/loongarch/translate.c  | 1 -
 target/mips/tcg/translate.c   | 1 -
 target/nios2/translate.c      | 2 --
 tests/unit/test-cutils.c      | 1 -
 ui/gtk.c                      | 1 -
 util/oslib-posix.c            | 4 ----
 33 files changed, 39 deletions(-)

diff --git a/include/hw/arm/fsl-imx6ul.h b/include/hw/arm/fsl-imx6ul.h
index 7812e516a5..1952cb984d 100644
--- a/include/hw/arm/fsl-imx6ul.h
+++ b/include/hw/arm/fsl-imx6ul.h
@@ -30,7 +30,6 @@
 #include "hw/timer/imx_gpt.h"
 #include "hw/timer/imx_epit.h"
 #include "hw/i2c/imx_i2c.h"
-#include "hw/gpio/imx_gpio.h"
 #include "hw/sd/sdhci.h"
 #include "hw/ssi/imx_spi.h"
 #include "hw/net/imx_fec.h"
diff --git a/include/hw/arm/fsl-imx7.h b/include/hw/arm/fsl-imx7.h
index 4e5e071864..355bd8ea83 100644
--- a/include/hw/arm/fsl-imx7.h
+++ b/include/hw/arm/fsl-imx7.h
@@ -32,7 +32,6 @@
 #include "hw/timer/imx_gpt.h"
 #include "hw/timer/imx_epit.h"
 #include "hw/i2c/imx_i2c.h"
-#include "hw/gpio/imx_gpio.h"
 #include "hw/sd/sdhci.h"
 #include "hw/ssi/imx_spi.h"
 #include "hw/net/imx_fec.h"
diff --git a/backends/tpm/tpm_emulator.c b/backends/tpm/tpm_emulator.c
index 49cc3d749d..2b440d2c9a 100644
--- a/backends/tpm/tpm_emulator.c
+++ b/backends/tpm/tpm_emulator.c
@@ -35,7 +35,6 @@
 #include "sysemu/runstate.h"
 #include "sysemu/tpm_backend.h"
 #include "sysemu/tpm_util.h"
-#include "sysemu/runstate.h"
 #include "tpm_int.h"
 #include "tpm_ioctl.h"
 #include "migration/blocker.h"
diff --git a/hw/acpi/piix4.c b/hw/acpi/piix4.c
index 0a81f1ad93..df39f91294 100644
--- a/hw/acpi/piix4.c
+++ b/hw/acpi/piix4.c
@@ -35,7 +35,6 @@
 #include "sysemu/xen.h"
 #include "qapi/error.h"
 #include "qemu/range.h"
-#include "hw/acpi/pcihp.h"
 #include "hw/acpi/cpu_hotplug.h"
 #include "hw/acpi/cpu.h"
 #include "hw/hotplug.h"
diff --git a/hw/alpha/dp264.c b/hw/alpha/dp264.c
index c502c8c62a..4161f559a7 100644
--- a/hw/alpha/dp264.c
+++ b/hw/alpha/dp264.c
@@ -18,7 +18,6 @@
 #include "net/net.h"
 #include "qemu/cutils.h"
 #include "qemu/datadir.h"
-#include "net/net.h"
 
 static uint64_t cpu_alpha_superpage_to_phys(void *opaque, uint64_t addr)
 {
diff --git a/hw/arm/virt.c b/hw/arm/virt.c
index ea2413a0ba..d3849d7233 100644
--- a/hw/arm/virt.c
+++ b/hw/arm/virt.c
@@ -33,7 +33,6 @@
 #include "qemu/units.h"
 #include "qemu/option.h"
 #include "monitor/qdev.h"
-#include "qapi/error.h"
 #include "hw/sysbus.h"
 #include "hw/arm/boot.h"
 #include "hw/arm/primecell.h"
diff --git a/hw/arm/xlnx-versal.c b/hw/arm/xlnx-versal.c
index 57276e1506..69b1b99e93 100644
--- a/hw/arm/xlnx-versal.c
+++ b/hw/arm/xlnx-versal.c
@@ -22,7 +22,6 @@
 #include "hw/misc/unimp.h"
 #include "hw/arm/xlnx-versal.h"
 #include "qemu/log.h"
-#include "hw/sysbus.h"
 
 #define XLNX_VERSAL_ACPU_TYPE ARM_CPU_TYPE_NAME("cortex-a72")
 #define XLNX_VERSAL_RCPU_TYPE ARM_CPU_TYPE_NAME("cortex-r5f")
diff --git a/hw/block/pflash_cfi01.c b/hw/block/pflash_cfi01.c
index 0cbc2fb4cb..d11406eada 100644
--- a/hw/block/pflash_cfi01.c
+++ b/hw/block/pflash_cfi01.c
@@ -45,7 +45,6 @@
 #include "qapi/error.h"
 #include "qemu/error-report.h"
 #include "qemu/bitops.h"
-#include "qemu/error-report.h"
 #include "qemu/host-utils.h"
 #include "qemu/log.h"
 #include "qemu/module.h"
diff --git a/hw/core/machine.c b/hw/core/machine.c
index 616f3a207c..67cf9f9dcd 100644
--- a/hw/core/machine.c
+++ b/hw/core/machine.c
@@ -39,7 +39,6 @@
 #include "exec/confidential-guest-support.h"
 #include "hw/virtio/virtio.h"
 #include "hw/virtio/virtio-pci.h"
-#include "qom/object_interfaces.h"
 
 GlobalProperty hw_compat_7_2[] = {};
 const size_t hw_compat_7_2_len = G_N_ELEMENTS(hw_compat_7_2);
diff --git a/hw/hppa/machine.c b/hw/hppa/machine.c
index de1cc7ab71..7ac68c943f 100644
--- a/hw/hppa/machine.c
+++ b/hw/hppa/machine.c
@@ -28,7 +28,6 @@
 #include "qapi/error.h"
 #include "net/net.h"
 #include "qemu/log.h"
-#include "net/net.h"
 
 #define MIN_SEABIOS_HPPA_VERSION 6 /* require at least this fw version */
 
diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
index 127c4e2d50..14f6f75454 100644
--- a/hw/i386/acpi-build.c
+++ b/hw/i386/acpi-build.c
@@ -76,7 +76,6 @@
 
 #include "hw/acpi/hmat.h"
 #include "hw/acpi/viot.h"
-#include "hw/acpi/cxl.h"
 
 #include CONFIG_DEVICES
 
diff --git a/hw/loongarch/acpi-build.c b/hw/loongarch/acpi-build.c
index c2b237736d..f551296a0e 100644
--- a/hw/loongarch/acpi-build.c
+++ b/hw/loongarch/acpi-build.c
@@ -22,7 +22,6 @@
 /* Supported chipsets: */
 #include "hw/pci-host/ls7a.h"
 #include "hw/loongarch/virt.h"
-#include "hw/acpi/aml-build.h"
 
 #include "hw/acpi/utils.h"
 #include "hw/acpi/pci.h"
diff --git a/hw/misc/macio/cuda.c b/hw/misc/macio/cuda.c
index 853e88bfed..29a8e5ed19 100644
--- a/hw/misc/macio/cuda.c
+++ b/hw/misc/macio/cuda.c
@@ -30,7 +30,6 @@
 #include "hw/input/adb.h"
 #include "hw/misc/mos6522.h"
 #include "hw/misc/macio/cuda.h"
-#include "qapi/error.h"
 #include "qemu/timer.h"
 #include "sysemu/runstate.h"
 #include "sysemu/rtc.h"
diff --git a/hw/misc/macio/pmu.c b/hw/misc/macio/pmu.c
index 97ef8c771b..5a788e595a 100644
--- a/hw/misc/macio/pmu.c
+++ b/hw/misc/macio/pmu.c
@@ -36,7 +36,6 @@
 #include "hw/misc/mos6522.h"
 #include "hw/misc/macio/gpio.h"
 #include "hw/misc/macio/pmu.h"
-#include "qapi/error.h"
 #include "qemu/timer.h"
 #include "sysemu/runstate.h"
 #include "sysemu/rtc.h"
diff --git a/hw/net/xilinx_axienet.c b/hw/net/xilinx_axienet.c
index 990ff3a1c2..673af7da26 100644
--- a/hw/net/xilinx_axienet.c
+++ b/hw/net/xilinx_axienet.c
@@ -31,7 +31,6 @@
 #include "net/net.h"
 #include "net/checksum.h"
 
-#include "hw/hw.h"
 #include "hw/irq.h"
 #include "hw/qdev-properties.h"
 #include "hw/stream.h"
diff --git a/hw/ppc/ppc405_uc.c b/hw/ppc/ppc405_uc.c
index c973cfb04e..0cc68178ad 100644
--- a/hw/ppc/ppc405_uc.c
+++ b/hw/ppc/ppc405_uc.c
@@ -38,8 +38,6 @@
 #include "sysemu/sysemu.h"
 #include "exec/address-spaces.h"
 #include "hw/intc/ppc-uic.h"
-#include "hw/qdev-properties.h"
-#include "qapi/error.h"
 #include "trace.h"
 
 /*****************************************************************************/
diff --git a/hw/ppc/ppc440_bamboo.c b/hw/ppc/ppc440_bamboo.c
index 81d71adf34..2880c81cb1 100644
--- a/hw/ppc/ppc440_bamboo.c
+++ b/hw/ppc/ppc440_bamboo.c
@@ -13,7 +13,6 @@
 
 #include "qemu/osdep.h"
 #include "qemu/units.h"
-#include "qemu/error-report.h"
 #include "qemu/datadir.h"
 #include "qemu/error-report.h"
 #include "net/net.h"
diff --git a/hw/ppc/spapr_drc.c b/hw/ppc/spapr_drc.c
index 4923435a8b..b5c400a94d 100644
--- a/hw/ppc/spapr_drc.c
+++ b/hw/ppc/spapr_drc.c
@@ -17,7 +17,6 @@
 #include "hw/ppc/spapr_drc.h"
 #include "qom/object.h"
 #include "migration/vmstate.h"
-#include "qapi/error.h"
 #include "qapi/qapi-events-qdev.h"
 #include "qapi/visitor.h"
 #include "qemu/error-report.h"
diff --git a/hw/rdma/vmw/pvrdma_dev_ring.c b/hw/rdma/vmw/pvrdma_dev_ring.c
index 598e6adc5e..30ce22a5be 100644
--- a/hw/rdma/vmw/pvrdma_dev_ring.c
+++ b/hw/rdma/vmw/pvrdma_dev_ring.c
@@ -14,7 +14,6 @@
  */
 
 #include "qemu/osdep.h"
-#include "qemu/cutils.h"
 #include "hw/pci/pci.h"
 #include "cpu.h"
 #include "qemu/cutils.h"
diff --git a/hw/remote/machine.c b/hw/remote/machine.c
index 519f855ec1..fdc6c441bb 100644
--- a/hw/remote/machine.c
+++ b/hw/remote/machine.c
@@ -22,7 +22,6 @@
 #include "hw/remote/iohub.h"
 #include "hw/remote/iommu.h"
 #include "hw/qdev-core.h"
-#include "hw/remote/iommu.h"
 #include "hw/remote/vfio-user-obj.h"
 #include "hw/pci/msi.h"
 
diff --git a/hw/remote/remote-obj.c b/hw/remote/remote-obj.c
index 333e5ac443..65b6f7cc86 100644
--- a/hw/remote/remote-obj.c
+++ b/hw/remote/remote-obj.c
@@ -12,7 +12,6 @@
 #include "qemu/error-report.h"
 #include "qemu/notify.h"
 #include "qom/object_interfaces.h"
-#include "hw/qdev-core.h"
 #include "io/channel.h"
 #include "hw/qdev-core.h"
 #include "hw/remote/machine.h"
diff --git a/hw/rtc/mc146818rtc.c b/hw/rtc/mc146818rtc.c
index bc1192b7ae..ba612a151d 100644
--- a/hw/rtc/mc146818rtc.c
+++ b/hw/rtc/mc146818rtc.c
@@ -43,7 +43,6 @@
 #include "qapi/error.h"
 #include "qapi/qapi-events-misc.h"
 #include "qapi/visitor.h"
-#include "hw/rtc/mc146818rtc_regs.h"
 
 //#define DEBUG_CMOS
 //#define DEBUG_COALESCED
diff --git a/hw/s390x/virtio-ccw-serial.c b/hw/s390x/virtio-ccw-serial.c
index bf8057880f..8f8d2302f8 100644
--- a/hw/s390x/virtio-ccw-serial.c
+++ b/hw/s390x/virtio-ccw-serial.c
@@ -15,7 +15,6 @@
 #include "hw/qdev-properties.h"
 #include "hw/virtio/virtio-serial.h"
 #include "virtio-ccw.h"
-#include "hw/virtio/virtio-serial.h"
 
 #define TYPE_VIRTIO_SERIAL_CCW "virtio-serial-ccw"
 OBJECT_DECLARE_SIMPLE_TYPE(VirtioSerialCcw, VIRTIO_SERIAL_CCW)
diff --git a/migration/postcopy-ram.c b/migration/postcopy-ram.c
index b9a37ef255..8b7d1af75d 100644
--- a/migration/postcopy-ram.c
+++ b/migration/postcopy-ram.c
@@ -17,7 +17,6 @@
  */
 
 #include "qemu/osdep.h"
-#include "qemu/rcu.h"
 #include "qemu/madvise.h"
 #include "exec/target_page.h"
 #include "migration.h"
@@ -34,7 +33,6 @@
 #include "hw/boards.h"
 #include "exec/ramblock.h"
 #include "socket.h"
-#include "qemu-file.h"
 #include "yank_functions.h"
 #include "tls.h"
 
diff --git a/softmmu/dirtylimit.c b/softmmu/dirtylimit.c
index 12668555f2..c56f0f58c8 100644
--- a/softmmu/dirtylimit.c
+++ b/softmmu/dirtylimit.c
@@ -11,7 +11,6 @@
  */
 
 #include "qemu/osdep.h"
-#include "qapi/error.h"
 #include "qemu/main-loop.h"
 #include "qapi/qapi-commands-migration.h"
 #include "qapi/qmp/qdict.h"
diff --git a/softmmu/runstate.c b/softmmu/runstate.c
index cab9f6fc07..f9ad88e6a7 100644
--- a/softmmu/runstate.c
+++ b/softmmu/runstate.c
@@ -41,7 +41,6 @@
 #include "qapi/qapi-commands-run-state.h"
 #include "qapi/qapi-events-run-state.h"
 #include "qemu/error-report.h"
-#include "qemu/log.h"
 #include "qemu/job.h"
 #include "qemu/log.h"
 #include "qemu/module.h"
diff --git a/softmmu/vl.c b/softmmu/vl.c
index 5355a7fe5a..b2ee3fee3f 100644
--- a/softmmu/vl.c
+++ b/softmmu/vl.c
@@ -129,7 +129,6 @@
 #include "qapi/qapi-commands-misc.h"
 #include "qapi/qapi-visit-qom.h"
 #include "qapi/qapi-commands-ui.h"
-#include "qapi/qmp/qdict.h"
 #include "block/qdict.h"
 #include "qapi/qmp/qerror.h"
 #include "sysemu/iothread.h"
diff --git a/target/loongarch/translate.c b/target/loongarch/translate.c
index 38ced69803..72a6275665 100644
--- a/target/loongarch/translate.c
+++ b/target/loongarch/translate.c
@@ -12,7 +12,6 @@
 #include "exec/helper-proto.h"
 #include "exec/helper-gen.h"
 
-#include "exec/translator.h"
 #include "exec/log.h"
 #include "qemu/qemu-print.h"
 #include "fpu/softfloat.h"
diff --git a/target/mips/tcg/translate.c b/target/mips/tcg/translate.c
index 624e6b7786..aa12bb708a 100644
--- a/target/mips/tcg/translate.c
+++ b/target/mips/tcg/translate.c
@@ -32,7 +32,6 @@
 #include "semihosting/semihost.h"
 
 #include "trace.h"
-#include "exec/translator.h"
 #include "exec/log.h"
 #include "qemu/qemu-print.h"
 #include "fpu_helper.h"
diff --git a/target/nios2/translate.c b/target/nios2/translate.c
index 4db8b47744..7aee65a089 100644
--- a/target/nios2/translate.c
+++ b/target/nios2/translate.c
@@ -938,8 +938,6 @@ static const char * const cr_regnames[NUM_CR_REGS] = {
 };
 #endif
 
-#include "exec/gen-icount.h"
-
 /* generate intermediate code for basic block 'tb'.  */
 static void nios2_tr_init_disas_context(DisasContextBase *dcbase, CPUState *cs)
 {
diff --git a/tests/unit/test-cutils.c b/tests/unit/test-cutils.c
index 2126b46391..3c4f875420 100644
--- a/tests/unit/test-cutils.c
+++ b/tests/unit/test-cutils.c
@@ -26,7 +26,6 @@
  */
 
 #include "qemu/osdep.h"
-#include "qemu/units.h"
 #include "qemu/cutils.h"
 #include "qemu/units.h"
 
diff --git a/ui/gtk.c b/ui/gtk.c
index 4817623c8f..7f752d8b7d 100644
--- a/ui/gtk.c
+++ b/ui/gtk.c
@@ -53,7 +53,6 @@
 #include <math.h>
 
 #include "trace.h"
-#include "qemu/cutils.h"
 #include "ui/input.h"
 #include "sysemu/runstate.h"
 #include "sysemu/sysemu.h"
diff --git a/util/oslib-posix.c b/util/oslib-posix.c
index fd03fd32c8..77d882e681 100644
--- a/util/oslib-posix.c
+++ b/util/oslib-posix.c
@@ -59,10 +59,6 @@
 
 #include "qemu/mmap-alloc.h"
 
-#ifdef CONFIG_DEBUG_STACK_USAGE
-#include "qemu/error-report.h"
-#endif
-
 #define MAX_MEM_PREALLOC_THREAD_COUNT 16
 
 struct MemsetThread;
-- 
2.39.0

Re: [PATCH v4 09/19] qga: Clean up includes

[Konstantin Kostiuk]

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

Reviewed-by: Konstantin Kostiuk <kkostiuk@redhat.com>



On Thu, Jan 19, 2023 at 9:00 AM Markus Armbruster <armbru@redhat.com> wrote:

> Clean up includes so that osdep.h is included first and headers
> which it implies are not included manually.
>
> This commit was created with scripts/clean-includes.
>
> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> ---
>  qga/cutils.h         | 2 --
>  qga/commands-posix.c | 1 -
>  qga/cutils.c         | 3 ++-
>  3 files changed, 2 insertions(+), 4 deletions(-)
>
> diff --git a/qga/cutils.h b/qga/cutils.h
> index f0f30a7d28..c1f2f4b17a 100644
> --- a/qga/cutils.h
> +++ b/qga/cutils.h
> @@ -1,8 +1,6 @@
>  #ifndef CUTILS_H_
>  #define CUTILS_H_
>
> -#include "qemu/osdep.h"
> -
>  int qga_open_cloexec(const char *name, int flags, mode_t mode);
>
>  #endif /* CUTILS_H_ */
> diff --git a/qga/commands-posix.c b/qga/commands-posix.c
> index ebd33a643c..079689d79a 100644
> --- a/qga/commands-posix.c
> +++ b/qga/commands-posix.c
> @@ -51,7 +51,6 @@
>  #else
>  #include <net/ethernet.h>
>  #endif
> -#include <sys/types.h>
>  #ifdef CONFIG_SOLARIS
>  #include <sys/sockio.h>
>  #endif
> diff --git a/qga/cutils.c b/qga/cutils.c
> index b8e142ef64..b21bcf3683 100644
> --- a/qga/cutils.c
> +++ b/qga/cutils.c
> @@ -2,8 +2,9 @@
>   * This work is licensed under the terms of the GNU GPL, version 2 or
> later.
>   * See the COPYING file in the top-level directory.
>   */
> -#include "cutils.h"
>
> +#include "qemu/osdep.h"
> +#include "cutils.h"
>  #include "qapi/error.h"
>
>  /**
> --
> 2.39.0
>
>

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

Re: [PATCH v4 00/19] Clean up includes

[Markus Armbruster]

Markus Armbruster <armbru@redhat.com> writes:

> Back in 2016, we discussed[1] rules for headers, and these were
> generally liked:
>
> 1. Have a carefully curated header that's included everywhere first.  We
>    got that already thanks to Peter: osdep.h.
>
> 2. Headers should normally include everything they need beyond osdep.h.
>    If exceptions are needed for some reason, they must be documented in
>    the header.  If all that's needed from a header is typedefs, put
>    those into qemu/typedefs.h instead of including the header.
>
> 3. Cyclic inclusion is forbidden.
>
> This series fixes violations of rule 2.  I may have split patches too
> aggressively.  Let me know if you want some squashed together.
>
> v4:
> * PATCH 01-03: New
> * PATCH 04-15: Previous version redone with scripts/clean-includes,
>   	       result split up for review

Copying the R-bys for v3 to these patches is tempting.  But I didn't.

> * PATCH 16-19: New
>
> v3:
> * Rebased, old PATCH 1+2+4 are in master as commit
>   881e019770..f07ceffdf5
> * PATCH 1: Fix bsd-user
>
> v2:
> * Rebased
> * PATCH 3: v1 posted separately
> * PATCH 4: New
>
> [1] Message-ID: <87h9g8j57d.fsf@blackfin.pond.sub.org>
>     https://lists.nongnu.org/archive/html/qemu-devel/2016-03/msg03345.html

Re: [PATCH v4 10/19] migration: Clean up includes

[Dr. David Alan Gilbert]

* Markus Armbruster (armbru@redhat.com) wrote:
> Clean up includes so that osdep.h is included first and headers
> which it implies are not included manually.

That change doesn't seem to match the message; the patch is removing the
osdep.h include.

Dave

> This commit was created with scripts/clean-includes.
> 
> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> ---
>  include/qemu/userfaultfd.h | 1 -
>  1 file changed, 1 deletion(-)
> 
> diff --git a/include/qemu/userfaultfd.h b/include/qemu/userfaultfd.h
> index 6b74f92792..55c95998e8 100644
> --- a/include/qemu/userfaultfd.h
> +++ b/include/qemu/userfaultfd.h
> @@ -13,7 +13,6 @@
>  #ifndef USERFAULTFD_H
>  #define USERFAULTFD_H
>  
> -#include "qemu/osdep.h"
>  #include "exec/hwaddr.h"
>  #include <linux/userfaultfd.h>
>  
> -- 
> 2.39.0
> 
-- 
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK

Re: [PATCH v4 18/19] 9p: Drop superfluous include of linux/limits.h

[Christian Schoenebeck]

On Thursday, January 19, 2023 7:59:58 AM CET Markus Armbruster wrote:
> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> ---
>  hw/9pfs/9p.c | 3 ---
>  1 file changed, 3 deletions(-)
> 
> diff --git a/hw/9pfs/9p.c b/hw/9pfs/9p.c
> index 9621ec1341..aa736af380 100644
> --- a/hw/9pfs/9p.c
> +++ b/hw/9pfs/9p.c
> @@ -17,9 +17,6 @@
>   */
>  
>  #include "qemu/osdep.h"
> -#ifdef CONFIG_LINUX
> -#include <linux/limits.h>
> -#endif
>  #include <glib/gprintf.h>
>  #include "hw/virtio/virtio.h"
>  #include "qapi/error.h"
> 

Where did that base version come from? I don't see it anywhere in history. 
Last relevant change in context was a136d17590a.

Best regards,
Christian Schoenebeck

Re: [PATCH v4 19/19] Drop duplicate #include

[Dr. David Alan Gilbert]

* Markus Armbruster (armbru@redhat.com) wrote:
> Tracked down with the help of scripts/clean-includes.
> 
> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> ---
>  include/hw/arm/fsl-imx6ul.h   | 1 -
>  include/hw/arm/fsl-imx7.h     | 1 -
>  backends/tpm/tpm_emulator.c   | 1 -
>  hw/acpi/piix4.c               | 1 -
>  hw/alpha/dp264.c              | 1 -
>  hw/arm/virt.c                 | 1 -
>  hw/arm/xlnx-versal.c          | 1 -
>  hw/block/pflash_cfi01.c       | 1 -
>  hw/core/machine.c             | 1 -
>  hw/hppa/machine.c             | 1 -
>  hw/i386/acpi-build.c          | 1 -
>  hw/loongarch/acpi-build.c     | 1 -
>  hw/misc/macio/cuda.c          | 1 -
>  hw/misc/macio/pmu.c           | 1 -
>  hw/net/xilinx_axienet.c       | 1 -
>  hw/ppc/ppc405_uc.c            | 2 --
>  hw/ppc/ppc440_bamboo.c        | 1 -
>  hw/ppc/spapr_drc.c            | 1 -
>  hw/rdma/vmw/pvrdma_dev_ring.c | 1 -
>  hw/remote/machine.c           | 1 -
>  hw/remote/remote-obj.c        | 1 -
>  hw/rtc/mc146818rtc.c          | 1 -
>  hw/s390x/virtio-ccw-serial.c  | 1 -
>  migration/postcopy-ram.c      | 2 --
>  softmmu/dirtylimit.c          | 1 -
>  softmmu/runstate.c            | 1 -
>  softmmu/vl.c                  | 1 -
>  target/loongarch/translate.c  | 1 -
>  target/mips/tcg/translate.c   | 1 -
>  target/nios2/translate.c      | 2 --
>  tests/unit/test-cutils.c      | 1 -
>  ui/gtk.c                      | 1 -
>  util/oslib-posix.c            | 4 ----
>  33 files changed, 39 deletions(-)
> 
> diff --git a/include/hw/arm/fsl-imx6ul.h b/include/hw/arm/fsl-imx6ul.h
> index 7812e516a5..1952cb984d 100644
> --- a/include/hw/arm/fsl-imx6ul.h
> +++ b/include/hw/arm/fsl-imx6ul.h
> @@ -30,7 +30,6 @@
>  #include "hw/timer/imx_gpt.h"
>  #include "hw/timer/imx_epit.h"
>  #include "hw/i2c/imx_i2c.h"
> -#include "hw/gpio/imx_gpio.h"
>  #include "hw/sd/sdhci.h"
>  #include "hw/ssi/imx_spi.h"
>  #include "hw/net/imx_fec.h"
> diff --git a/include/hw/arm/fsl-imx7.h b/include/hw/arm/fsl-imx7.h
> index 4e5e071864..355bd8ea83 100644
> --- a/include/hw/arm/fsl-imx7.h
> +++ b/include/hw/arm/fsl-imx7.h
> @@ -32,7 +32,6 @@
>  #include "hw/timer/imx_gpt.h"
>  #include "hw/timer/imx_epit.h"
>  #include "hw/i2c/imx_i2c.h"
> -#include "hw/gpio/imx_gpio.h"
>  #include "hw/sd/sdhci.h"
>  #include "hw/ssi/imx_spi.h"
>  #include "hw/net/imx_fec.h"
> diff --git a/backends/tpm/tpm_emulator.c b/backends/tpm/tpm_emulator.c
> index 49cc3d749d..2b440d2c9a 100644
> --- a/backends/tpm/tpm_emulator.c
> +++ b/backends/tpm/tpm_emulator.c
> @@ -35,7 +35,6 @@
>  #include "sysemu/runstate.h"
>  #include "sysemu/tpm_backend.h"
>  #include "sysemu/tpm_util.h"
> -#include "sysemu/runstate.h"
>  #include "tpm_int.h"
>  #include "tpm_ioctl.h"
>  #include "migration/blocker.h"
> diff --git a/hw/acpi/piix4.c b/hw/acpi/piix4.c
> index 0a81f1ad93..df39f91294 100644
> --- a/hw/acpi/piix4.c
> +++ b/hw/acpi/piix4.c
> @@ -35,7 +35,6 @@
>  #include "sysemu/xen.h"
>  #include "qapi/error.h"
>  #include "qemu/range.h"
> -#include "hw/acpi/pcihp.h"
>  #include "hw/acpi/cpu_hotplug.h"
>  #include "hw/acpi/cpu.h"
>  #include "hw/hotplug.h"
> diff --git a/hw/alpha/dp264.c b/hw/alpha/dp264.c
> index c502c8c62a..4161f559a7 100644
> --- a/hw/alpha/dp264.c
> +++ b/hw/alpha/dp264.c
> @@ -18,7 +18,6 @@
>  #include "net/net.h"
>  #include "qemu/cutils.h"
>  #include "qemu/datadir.h"
> -#include "net/net.h"
>  
>  static uint64_t cpu_alpha_superpage_to_phys(void *opaque, uint64_t addr)
>  {
> diff --git a/hw/arm/virt.c b/hw/arm/virt.c
> index ea2413a0ba..d3849d7233 100644
> --- a/hw/arm/virt.c
> +++ b/hw/arm/virt.c
> @@ -33,7 +33,6 @@
>  #include "qemu/units.h"
>  #include "qemu/option.h"
>  #include "monitor/qdev.h"
> -#include "qapi/error.h"
>  #include "hw/sysbus.h"
>  #include "hw/arm/boot.h"
>  #include "hw/arm/primecell.h"
> diff --git a/hw/arm/xlnx-versal.c b/hw/arm/xlnx-versal.c
> index 57276e1506..69b1b99e93 100644
> --- a/hw/arm/xlnx-versal.c
> +++ b/hw/arm/xlnx-versal.c
> @@ -22,7 +22,6 @@
>  #include "hw/misc/unimp.h"
>  #include "hw/arm/xlnx-versal.h"
>  #include "qemu/log.h"
> -#include "hw/sysbus.h"
>  
>  #define XLNX_VERSAL_ACPU_TYPE ARM_CPU_TYPE_NAME("cortex-a72")
>  #define XLNX_VERSAL_RCPU_TYPE ARM_CPU_TYPE_NAME("cortex-r5f")
> diff --git a/hw/block/pflash_cfi01.c b/hw/block/pflash_cfi01.c
> index 0cbc2fb4cb..d11406eada 100644
> --- a/hw/block/pflash_cfi01.c
> +++ b/hw/block/pflash_cfi01.c
> @@ -45,7 +45,6 @@
>  #include "qapi/error.h"
>  #include "qemu/error-report.h"
>  #include "qemu/bitops.h"
> -#include "qemu/error-report.h"
>  #include "qemu/host-utils.h"
>  #include "qemu/log.h"
>  #include "qemu/module.h"
> diff --git a/hw/core/machine.c b/hw/core/machine.c
> index 616f3a207c..67cf9f9dcd 100644
> --- a/hw/core/machine.c
> +++ b/hw/core/machine.c
> @@ -39,7 +39,6 @@
>  #include "exec/confidential-guest-support.h"
>  #include "hw/virtio/virtio.h"
>  #include "hw/virtio/virtio-pci.h"
> -#include "qom/object_interfaces.h"
>  
>  GlobalProperty hw_compat_7_2[] = {};
>  const size_t hw_compat_7_2_len = G_N_ELEMENTS(hw_compat_7_2);
> diff --git a/hw/hppa/machine.c b/hw/hppa/machine.c
> index de1cc7ab71..7ac68c943f 100644
> --- a/hw/hppa/machine.c
> +++ b/hw/hppa/machine.c
> @@ -28,7 +28,6 @@
>  #include "qapi/error.h"
>  #include "net/net.h"
>  #include "qemu/log.h"
> -#include "net/net.h"
>  
>  #define MIN_SEABIOS_HPPA_VERSION 6 /* require at least this fw version */
>  
> diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
> index 127c4e2d50..14f6f75454 100644
> --- a/hw/i386/acpi-build.c
> +++ b/hw/i386/acpi-build.c
> @@ -76,7 +76,6 @@
>  
>  #include "hw/acpi/hmat.h"
>  #include "hw/acpi/viot.h"
> -#include "hw/acpi/cxl.h"
>  
>  #include CONFIG_DEVICES
>  
> diff --git a/hw/loongarch/acpi-build.c b/hw/loongarch/acpi-build.c
> index c2b237736d..f551296a0e 100644
> --- a/hw/loongarch/acpi-build.c
> +++ b/hw/loongarch/acpi-build.c
> @@ -22,7 +22,6 @@
>  /* Supported chipsets: */
>  #include "hw/pci-host/ls7a.h"
>  #include "hw/loongarch/virt.h"
> -#include "hw/acpi/aml-build.h"
>  
>  #include "hw/acpi/utils.h"
>  #include "hw/acpi/pci.h"
> diff --git a/hw/misc/macio/cuda.c b/hw/misc/macio/cuda.c
> index 853e88bfed..29a8e5ed19 100644
> --- a/hw/misc/macio/cuda.c
> +++ b/hw/misc/macio/cuda.c
> @@ -30,7 +30,6 @@
>  #include "hw/input/adb.h"
>  #include "hw/misc/mos6522.h"
>  #include "hw/misc/macio/cuda.h"
> -#include "qapi/error.h"
>  #include "qemu/timer.h"
>  #include "sysemu/runstate.h"
>  #include "sysemu/rtc.h"
> diff --git a/hw/misc/macio/pmu.c b/hw/misc/macio/pmu.c
> index 97ef8c771b..5a788e595a 100644
> --- a/hw/misc/macio/pmu.c
> +++ b/hw/misc/macio/pmu.c
> @@ -36,7 +36,6 @@
>  #include "hw/misc/mos6522.h"
>  #include "hw/misc/macio/gpio.h"
>  #include "hw/misc/macio/pmu.h"
> -#include "qapi/error.h"
>  #include "qemu/timer.h"
>  #include "sysemu/runstate.h"
>  #include "sysemu/rtc.h"
> diff --git a/hw/net/xilinx_axienet.c b/hw/net/xilinx_axienet.c
> index 990ff3a1c2..673af7da26 100644
> --- a/hw/net/xilinx_axienet.c
> +++ b/hw/net/xilinx_axienet.c
> @@ -31,7 +31,6 @@
>  #include "net/net.h"
>  #include "net/checksum.h"
>  
> -#include "hw/hw.h"
>  #include "hw/irq.h"
>  #include "hw/qdev-properties.h"
>  #include "hw/stream.h"
> diff --git a/hw/ppc/ppc405_uc.c b/hw/ppc/ppc405_uc.c
> index c973cfb04e..0cc68178ad 100644
> --- a/hw/ppc/ppc405_uc.c
> +++ b/hw/ppc/ppc405_uc.c
> @@ -38,8 +38,6 @@
>  #include "sysemu/sysemu.h"
>  #include "exec/address-spaces.h"
>  #include "hw/intc/ppc-uic.h"
> -#include "hw/qdev-properties.h"
> -#include "qapi/error.h"
>  #include "trace.h"
>  
>  /*****************************************************************************/
> diff --git a/hw/ppc/ppc440_bamboo.c b/hw/ppc/ppc440_bamboo.c
> index 81d71adf34..2880c81cb1 100644
> --- a/hw/ppc/ppc440_bamboo.c
> +++ b/hw/ppc/ppc440_bamboo.c
> @@ -13,7 +13,6 @@
>  
>  #include "qemu/osdep.h"
>  #include "qemu/units.h"
> -#include "qemu/error-report.h"
>  #include "qemu/datadir.h"
>  #include "qemu/error-report.h"
>  #include "net/net.h"
> diff --git a/hw/ppc/spapr_drc.c b/hw/ppc/spapr_drc.c
> index 4923435a8b..b5c400a94d 100644
> --- a/hw/ppc/spapr_drc.c
> +++ b/hw/ppc/spapr_drc.c
> @@ -17,7 +17,6 @@
>  #include "hw/ppc/spapr_drc.h"
>  #include "qom/object.h"
>  #include "migration/vmstate.h"
> -#include "qapi/error.h"
>  #include "qapi/qapi-events-qdev.h"
>  #include "qapi/visitor.h"
>  #include "qemu/error-report.h"
> diff --git a/hw/rdma/vmw/pvrdma_dev_ring.c b/hw/rdma/vmw/pvrdma_dev_ring.c
> index 598e6adc5e..30ce22a5be 100644
> --- a/hw/rdma/vmw/pvrdma_dev_ring.c
> +++ b/hw/rdma/vmw/pvrdma_dev_ring.c
> @@ -14,7 +14,6 @@
>   */
>  
>  #include "qemu/osdep.h"
> -#include "qemu/cutils.h"
>  #include "hw/pci/pci.h"
>  #include "cpu.h"
>  #include "qemu/cutils.h"
> diff --git a/hw/remote/machine.c b/hw/remote/machine.c
> index 519f855ec1..fdc6c441bb 100644
> --- a/hw/remote/machine.c
> +++ b/hw/remote/machine.c
> @@ -22,7 +22,6 @@
>  #include "hw/remote/iohub.h"
>  #include "hw/remote/iommu.h"
>  #include "hw/qdev-core.h"
> -#include "hw/remote/iommu.h"
>  #include "hw/remote/vfio-user-obj.h"
>  #include "hw/pci/msi.h"
>  
> diff --git a/hw/remote/remote-obj.c b/hw/remote/remote-obj.c
> index 333e5ac443..65b6f7cc86 100644
> --- a/hw/remote/remote-obj.c
> +++ b/hw/remote/remote-obj.c
> @@ -12,7 +12,6 @@
>  #include "qemu/error-report.h"
>  #include "qemu/notify.h"
>  #include "qom/object_interfaces.h"
> -#include "hw/qdev-core.h"
>  #include "io/channel.h"
>  #include "hw/qdev-core.h"
>  #include "hw/remote/machine.h"
> diff --git a/hw/rtc/mc146818rtc.c b/hw/rtc/mc146818rtc.c
> index bc1192b7ae..ba612a151d 100644
> --- a/hw/rtc/mc146818rtc.c
> +++ b/hw/rtc/mc146818rtc.c
> @@ -43,7 +43,6 @@
>  #include "qapi/error.h"
>  #include "qapi/qapi-events-misc.h"
>  #include "qapi/visitor.h"
> -#include "hw/rtc/mc146818rtc_regs.h"
>  
>  //#define DEBUG_CMOS
>  //#define DEBUG_COALESCED
> diff --git a/hw/s390x/virtio-ccw-serial.c b/hw/s390x/virtio-ccw-serial.c
> index bf8057880f..8f8d2302f8 100644
> --- a/hw/s390x/virtio-ccw-serial.c
> +++ b/hw/s390x/virtio-ccw-serial.c
> @@ -15,7 +15,6 @@
>  #include "hw/qdev-properties.h"
>  #include "hw/virtio/virtio-serial.h"
>  #include "virtio-ccw.h"
> -#include "hw/virtio/virtio-serial.h"
>  
>  #define TYPE_VIRTIO_SERIAL_CCW "virtio-serial-ccw"
>  OBJECT_DECLARE_SIMPLE_TYPE(VirtioSerialCcw, VIRTIO_SERIAL_CCW)
> diff --git a/migration/postcopy-ram.c b/migration/postcopy-ram.c
> index b9a37ef255..8b7d1af75d 100644
> --- a/migration/postcopy-ram.c
> +++ b/migration/postcopy-ram.c
> @@ -17,7 +17,6 @@
>   */
>  
>  #include "qemu/osdep.h"
> -#include "qemu/rcu.h"
>  #include "qemu/madvise.h"
>  #include "exec/target_page.h"
>  #include "migration.h"
> @@ -34,7 +33,6 @@
>  #include "hw/boards.h"
>  #include "exec/ramblock.h"
>  #include "socket.h"
> -#include "qemu-file.h"
>  #include "yank_functions.h"
>  #include "tls.h"

Acked-by: Dr. David Alan Gilbert <dgilbert@redhat.com>

>  
> diff --git a/softmmu/dirtylimit.c b/softmmu/dirtylimit.c
> index 12668555f2..c56f0f58c8 100644
> --- a/softmmu/dirtylimit.c
> +++ b/softmmu/dirtylimit.c
> @@ -11,7 +11,6 @@
>   */
>  
>  #include "qemu/osdep.h"
> -#include "qapi/error.h"
>  #include "qemu/main-loop.h"
>  #include "qapi/qapi-commands-migration.h"
>  #include "qapi/qmp/qdict.h"
> diff --git a/softmmu/runstate.c b/softmmu/runstate.c
> index cab9f6fc07..f9ad88e6a7 100644
> --- a/softmmu/runstate.c
> +++ b/softmmu/runstate.c
> @@ -41,7 +41,6 @@
>  #include "qapi/qapi-commands-run-state.h"
>  #include "qapi/qapi-events-run-state.h"
>  #include "qemu/error-report.h"
> -#include "qemu/log.h"
>  #include "qemu/job.h"
>  #include "qemu/log.h"
>  #include "qemu/module.h"
> diff --git a/softmmu/vl.c b/softmmu/vl.c
> index 5355a7fe5a..b2ee3fee3f 100644
> --- a/softmmu/vl.c
> +++ b/softmmu/vl.c
> @@ -129,7 +129,6 @@
>  #include "qapi/qapi-commands-misc.h"
>  #include "qapi/qapi-visit-qom.h"
>  #include "qapi/qapi-commands-ui.h"
> -#include "qapi/qmp/qdict.h"
>  #include "block/qdict.h"
>  #include "qapi/qmp/qerror.h"
>  #include "sysemu/iothread.h"
> diff --git a/target/loongarch/translate.c b/target/loongarch/translate.c
> index 38ced69803..72a6275665 100644
> --- a/target/loongarch/translate.c
> +++ b/target/loongarch/translate.c
> @@ -12,7 +12,6 @@
>  #include "exec/helper-proto.h"
>  #include "exec/helper-gen.h"
>  
> -#include "exec/translator.h"
>  #include "exec/log.h"
>  #include "qemu/qemu-print.h"
>  #include "fpu/softfloat.h"
> diff --git a/target/mips/tcg/translate.c b/target/mips/tcg/translate.c
> index 624e6b7786..aa12bb708a 100644
> --- a/target/mips/tcg/translate.c
> +++ b/target/mips/tcg/translate.c
> @@ -32,7 +32,6 @@
>  #include "semihosting/semihost.h"
>  
>  #include "trace.h"
> -#include "exec/translator.h"
>  #include "exec/log.h"
>  #include "qemu/qemu-print.h"
>  #include "fpu_helper.h"
> diff --git a/target/nios2/translate.c b/target/nios2/translate.c
> index 4db8b47744..7aee65a089 100644
> --- a/target/nios2/translate.c
> +++ b/target/nios2/translate.c
> @@ -938,8 +938,6 @@ static const char * const cr_regnames[NUM_CR_REGS] = {
>  };
>  #endif
>  
> -#include "exec/gen-icount.h"
> -
>  /* generate intermediate code for basic block 'tb'.  */
>  static void nios2_tr_init_disas_context(DisasContextBase *dcbase, CPUState *cs)
>  {
> diff --git a/tests/unit/test-cutils.c b/tests/unit/test-cutils.c
> index 2126b46391..3c4f875420 100644
> --- a/tests/unit/test-cutils.c
> +++ b/tests/unit/test-cutils.c
> @@ -26,7 +26,6 @@
>   */
>  
>  #include "qemu/osdep.h"
> -#include "qemu/units.h"
>  #include "qemu/cutils.h"
>  #include "qemu/units.h"
>  
> diff --git a/ui/gtk.c b/ui/gtk.c
> index 4817623c8f..7f752d8b7d 100644
> --- a/ui/gtk.c
> +++ b/ui/gtk.c
> @@ -53,7 +53,6 @@
>  #include <math.h>
>  
>  #include "trace.h"
> -#include "qemu/cutils.h"
>  #include "ui/input.h"
>  #include "sysemu/runstate.h"
>  #include "sysemu/sysemu.h"
> diff --git a/util/oslib-posix.c b/util/oslib-posix.c
> index fd03fd32c8..77d882e681 100644
> --- a/util/oslib-posix.c
> +++ b/util/oslib-posix.c
> @@ -59,10 +59,6 @@
>  
>  #include "qemu/mmap-alloc.h"
>  
> -#ifdef CONFIG_DEBUG_STACK_USAGE
> -#include "qemu/error-report.h"
> -#endif
> -
>  #define MAX_MEM_PREALLOC_THREAD_COUNT 16
>  
>  struct MemsetThread;
> -- 
> 2.39.0
> 
-- 
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK

Re: [PATCH v4 10/19] migration: Clean up includes

[Markus Armbruster]

"Dr. David Alan Gilbert" <dgilbert@redhat.com> writes:

> * Markus Armbruster (armbru@redhat.com) wrote:
>> Clean up includes so that osdep.h is included first and headers
>> which it implies are not included manually.
>
> That change doesn't seem to match the message; the patch is removing the
> osdep.h include.

It's the commit message scripts/clean-includes creates :)

I can throw in another patch to the script so it mentions it also
removes qemu/osdep.h from headers.

>> This commit was created with scripts/clean-includes.
>> 
>> Signed-off-by: Markus Armbruster <armbru@redhat.com>
>> ---
>>  include/qemu/userfaultfd.h | 1 -
>>  1 file changed, 1 deletion(-)
>> 
>> diff --git a/include/qemu/userfaultfd.h b/include/qemu/userfaultfd.h
>> index 6b74f92792..55c95998e8 100644
>> --- a/include/qemu/userfaultfd.h
>> +++ b/include/qemu/userfaultfd.h
>> @@ -13,7 +13,6 @@
>>  #ifndef USERFAULTFD_H
>>  #define USERFAULTFD_H
>>  
>> -#include "qemu/osdep.h"
>>  #include "exec/hwaddr.h"
>>  #include <linux/userfaultfd.h>
>>  
>> -- 
>> 2.39.0
>> 

Re: [PATCH v4 08/19] hw/tricore: Clean up includes

[Philippe Mathieu-Daudé]

On 19/1/23 07:59, Markus Armbruster wrote:
> Clean up includes so that osdep.h is included first and headers
> which it implies are not included manually.
> 
> This commit was created with scripts/clean-includes.
> 
> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> ---
>   include/hw/tricore/triboard.h | 1 -
>   1 file changed, 1 deletion(-)

Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>

Re: [PATCH v4 05/19] crypto: Clean up includes

[Philippe Mathieu-Daudé]

On 19/1/23 07:59, Markus Armbruster wrote:
> Clean up includes so that osdep.h is included first and headers
> which it implies are not included manually.
> 
> This commit was created with scripts/clean-includes.
> 
> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> ---
>   crypto/block-luks-priv.h | 1 -
>   1 file changed, 1 deletion(-)

Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>

Re: [PATCH v4 11/19] net: Clean up includes

[Philippe Mathieu-Daudé]

On 19/1/23 07:59, Markus Armbruster wrote:
> Clean up includes so that osdep.h is included first and headers
> which it implies are not included manually.
> 
> This commit was created with scripts/clean-includes.
> 
> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> ---
>   net/vmnet_int.h | 1 -
>   1 file changed, 1 deletion(-)

Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>

Re: [PATCH v4 15/19] accel: Clean up includes

[Philippe Mathieu-Daudé]

On 19/1/23 07:59, Markus Armbruster wrote:
> Clean up includes so that osdep.h is included first and headers
> which it implies are not included manually.
> 
> This commit was created with scripts/clean-includes.
> 
> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> ---
>   include/sysemu/accel-blocker.h | 1 -
>   1 file changed, 1 deletion(-)

Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>

Re: [PATCH v4 18/19] 9p: Drop superfluous include of linux/limits.h

[Markus Armbruster]

Christian Schoenebeck <qemu_oss@crudebyte.com> writes:

> On Thursday, January 19, 2023 7:59:58 AM CET Markus Armbruster wrote:
>> Signed-off-by: Markus Armbruster <armbru@redhat.com>
>> ---
>>  hw/9pfs/9p.c | 3 ---
>>  1 file changed, 3 deletions(-)
>> 
>> diff --git a/hw/9pfs/9p.c b/hw/9pfs/9p.c
>> index 9621ec1341..aa736af380 100644
>> --- a/hw/9pfs/9p.c
>> +++ b/hw/9pfs/9p.c
>> @@ -17,9 +17,6 @@
>>   */
>>  
>>  #include "qemu/osdep.h"
>> -#ifdef CONFIG_LINUX
>> -#include <linux/limits.h>
>> -#endif
>>  #include <glib/gprintf.h>
>>  #include "hw/virtio/virtio.h"
>>  #include "qapi/error.h"
>> 
>
> Where did that base version come from? I don't see it anywhere in history. 
> Last relevant change in context was a136d17590a.

Current master (7ec8aeb6048) has

    #include "qemu/osdep.h"
    #ifdef CONFIG_LINUX
    #include <linux/limits.h>
    #else
    #include <limits.h>
    #endif
    #include <glib/gprintf.h>

The previous commit changes it to

    #include "qemu/osdep.h"
    #ifdef CONFIG_LINUX
    #include <linux/limits.h>
    #endif
    #include <glib/gprintf.h>

because "qemu/osdep.h" already includes <limits.h>.

Clearer now?

Re: [PATCH v4 13/19] riscv: Clean up includes

[Philippe Mathieu-Daudé]

On 19/1/23 07:59, Markus Armbruster wrote:
> Clean up includes so that osdep.h is included first and headers
> which it implies are not included manually.
> 
> This commit was created with scripts/clean-includes.
> 
> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> ---
>   target/riscv/pmu.h | 1 -
>   1 file changed, 1 deletion(-)

Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>

Re: [PATCH v4 16/19] Fix non-first inclusions of qemu/osdep.h

[Philippe Mathieu-Daudé]

On 19/1/23 07:59, Markus Armbruster wrote:
> This commit was created with scripts/clean-includes.
> 
> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> ---
>   audio/sndioaudio.c       |  2 +-
>   backends/hostmem-epc.c   |  2 +-
>   block/export/vduse-blk.c |  2 +-
>   hw/hyperv/syndbg.c       |  2 +-
>   util/async-teardown.c    | 12 ++++--------
>   5 files changed, 8 insertions(+), 12 deletions(-)
> 
> diff --git a/audio/sndioaudio.c b/audio/sndioaudio.c
> index 632b0e3825..3fde01fdbd 100644
> --- a/audio/sndioaudio.c
> +++ b/audio/sndioaudio.c
> @@ -14,9 +14,9 @@
>    * to recording, which is what guest systems expect.
>    */
>   
> +#include "qemu/osdep.h"
>   #include <poll.h>
>   #include <sndio.h>
> -#include "qemu/osdep.h"
>   #include "qemu/main-loop.h"
>   #include "audio.h"
>   #include "trace.h"
> diff --git a/backends/hostmem-epc.c b/backends/hostmem-epc.c
> index 037292d267..4e162d6789 100644
> --- a/backends/hostmem-epc.c
> +++ b/backends/hostmem-epc.c
> @@ -9,9 +9,9 @@
>    * This work is licensed under the terms of the GNU GPL, version 2 or later.
>    * See the COPYING file in the top-level directory.
>    */
> -#include <sys/ioctl.h>
>   
>   #include "qemu/osdep.h"
> +#include <sys/ioctl.h>
>   #include "qom/object_interfaces.h"
>   #include "qapi/error.h"
>   #include "sysemu/hostmem.h"
> diff --git a/block/export/vduse-blk.c b/block/export/vduse-blk.c
> index 350d6fdaf0..f7ae44e3ce 100644
> --- a/block/export/vduse-blk.c
> +++ b/block/export/vduse-blk.c
> @@ -10,9 +10,9 @@
>    * later.  See the COPYING file in the top-level directory.
>    */
>   
> +#include "qemu/osdep.h"
>   #include <sys/eventfd.h>
>   
> -#include "qemu/osdep.h"
>   #include "qapi/error.h"
>   #include "block/export.h"
>   #include "qemu/error-report.h"
> diff --git a/hw/hyperv/syndbg.c b/hw/hyperv/syndbg.c
> index 16d04cfdc6..94fe1b534b 100644
> --- a/hw/hyperv/syndbg.c
> +++ b/hw/hyperv/syndbg.c
> @@ -5,8 +5,8 @@
>    * See the COPYING file in the top-level directory.
>    */
>   
> -#include "qemu/ctype.h"
>   #include "qemu/osdep.h"
> +#include "qemu/ctype.h"
>   #include "qemu/error-report.h"
>   #include "qemu/main-loop.h"
>   #include "qemu/sockets.h"

Up to here:

Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>

> diff --git a/util/async-teardown.c b/util/async-teardown.c
> index 62bfce1b3c..62cdeb0f20 100644
> --- a/util/async-teardown.c
> +++ b/util/async-teardown.c
> @@ -10,16 +10,12 @@
>    * option) any later version.  See the COPYING file in the top-level directory.
>    *
>    */
> -#include <stdlib.h>
> -#include <stdio.h>
> -#include <sys/types.h>
> -#include <dirent.h>
> -#include <sys/prctl.h>
> -#include <signal.h>
> -#include <sched.h>
> -#include <unistd.h>
>   
>   #include "qemu/osdep.h"
> +#include <dirent.h>
> +#include <sys/prctl.h>
> +#include <sched.h>
> +
>   #include "qemu/async-teardown.h"

This file has more changes.

Re: [PATCH v4 16/19] Fix non-first inclusions of qemu/osdep.h

[Markus Armbruster]

Philippe Mathieu-Daudé <philmd@linaro.org> writes:

> On 19/1/23 07:59, Markus Armbruster wrote:
>> This commit was created with scripts/clean-includes.
>> Signed-off-by: Markus Armbruster <armbru@redhat.com>

[...]

> Up to here:
>
> Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>
>> diff --git a/util/async-teardown.c b/util/async-teardown.c
>> index 62bfce1b3c..62cdeb0f20 100644
>> --- a/util/async-teardown.c
>> +++ b/util/async-teardown.c
>> @@ -10,16 +10,12 @@
>>    * option) any later version.  See the COPYING file in the top-level directory.
>>    *
>>    */
>> -#include <stdlib.h>
>> -#include <stdio.h>
>> -#include <sys/types.h>
>> -#include <dirent.h>
>> -#include <sys/prctl.h>
>> -#include <signal.h>
>> -#include <sched.h>
>> -#include <unistd.h>
>>  
>>  #include "qemu/osdep.h"
>> +#include <dirent.h>
>> +#include <sys/prctl.h>
>> +#include <sched.h>
>> +
>>   #include "qemu/async-teardown.h"
>
> This file has more changes.

I'm not sure I understand.

The patch does two related things:

1. It puts qemu/osdep.h first.  The diff makes it look like we leave it
   in place and move other stuff across, but that's the same.

2. It deletes inclusions of headers qemu/osdep.h already includes:

    <stdlib.h>
    <stdio.h>
    <sys/types.h>
    <signal.h>
    <unistd.h>

What would you like me to change or explain?

Re: [PATCH v4 10/19] migration: Clean up includes

[Dr. David Alan Gilbert]

* Markus Armbruster (armbru@redhat.com) wrote:
> "Dr. David Alan Gilbert" <dgilbert@redhat.com> writes:
> 
> > * Markus Armbruster (armbru@redhat.com) wrote:
> >> Clean up includes so that osdep.h is included first and headers
> >> which it implies are not included manually.
> >
> > That change doesn't seem to match the message; the patch is removing the
> > osdep.h include.
> 
> It's the commit message scripts/clean-includes creates :)
> 
> I can throw in another patch to the script so it mentions it also
> removes qemu/osdep.h from headers.

Oh hmm it would be clearer; but OK then, so 

Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>

> >> This commit was created with scripts/clean-includes.
> >> 
> >> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> >> ---
> >>  include/qemu/userfaultfd.h | 1 -
> >>  1 file changed, 1 deletion(-)
> >> 
> >> diff --git a/include/qemu/userfaultfd.h b/include/qemu/userfaultfd.h
> >> index 6b74f92792..55c95998e8 100644
> >> --- a/include/qemu/userfaultfd.h
> >> +++ b/include/qemu/userfaultfd.h
> >> @@ -13,7 +13,6 @@
> >>  #ifndef USERFAULTFD_H
> >>  #define USERFAULTFD_H
> >>  
> >> -#include "qemu/osdep.h"
> >>  #include "exec/hwaddr.h"
> >>  #include <linux/userfaultfd.h>
> >>  
> >> -- 
> >> 2.39.0
> >> 
> 
-- 
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK

Re: [PATCH v4 00/19] Clean up includes

[Michael S. Tsirkin]

On Thu, Jan 19, 2023 at 07:59:40AM +0100, Markus Armbruster wrote:
> Back in 2016, we discussed[1] rules for headers, and these were
> generally liked:
> 
> 1. Have a carefully curated header that's included everywhere first.  We
>    got that already thanks to Peter: osdep.h.
> 
> 2. Headers should normally include everything they need beyond osdep.h.
>    If exceptions are needed for some reason, they must be documented in
>    the header.  If all that's needed from a header is typedefs, put
>    those into qemu/typedefs.h instead of including the header.
> 
> 3. Cyclic inclusion is forbidden.
> 
> This series fixes violations of rule 2.  I may have split patches too
> aggressively.  Let me know if you want some squashed together.

series
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>

feel free to merge.

> v4:
> * PATCH 01-03: New
> * PATCH 04-15: Previous version redone with scripts/clean-includes,
>   	       result split up for review
> * PATCH 16-19: New
> 
> v3:
> * Rebased, old PATCH 1+2+4 are in master as commit
>   881e019770..f07ceffdf5
> * PATCH 1: Fix bsd-user
> 
> v2:
> * Rebased
> * PATCH 3: v1 posted separately
> * PATCH 4: New
> 
> [1] Message-ID: <87h9g8j57d.fsf@blackfin.pond.sub.org>
>     https://lists.nongnu.org/archive/html/qemu-devel/2016-03/msg03345.html
> 
> Markus Armbruster (19):
>   scripts/clean-includes: Fully skip / ignore files
>   scripts/clean-includes: Don't claim duplicate headers found when not
>   scripts/clean-includes: Skip symbolic links
>   bsd-user: Clean up includes
>   crypto: Clean up includes
>   hw/cxl: Clean up includes
>   hw/input: Clean up includes
>   hw/tricore: Clean up includes
>   qga: Clean up includes
>   migration: Clean up includes
>   net: Clean up includes
>   target/hexagon: Clean up includes
>   riscv: Clean up includes
>   block: Clean up includes
>   accel: Clean up includes
>   Fix non-first inclusions of qemu/osdep.h
>   Don't include headers already included by qemu/osdep.h
>   9p: Drop superfluous include of linux/limits.h
>   Drop duplicate #include
> 
>  backends/tpm/tpm_ioctl.h          |  2 --
>  bsd-user/bsd-proc.h               |  4 ----
>  bsd-user/qemu.h                   |  1 -
>  crypto/block-luks-priv.h          |  1 -
>  fsdev/p9array.h                   |  2 --
>  include/block/graph-lock.h        |  1 -
>  include/block/write-threshold.h   |  2 --
>  include/hw/arm/fsl-imx6ul.h       |  1 -
>  include/hw/arm/fsl-imx7.h         |  1 -
>  include/hw/cxl/cxl_component.h    |  2 --
>  include/hw/cxl/cxl_host.h         |  1 -
>  include/hw/cxl/cxl_pci.h          |  1 -
>  include/hw/input/pl050.h          |  1 -
>  include/hw/misc/aspeed_lpc.h      |  2 --
>  include/hw/pci/pcie_doe.h         |  1 -
>  include/hw/tricore/triboard.h     |  1 -
>  include/qemu/async-teardown.h     |  2 --
>  include/qemu/dbus.h               |  1 -
>  include/qemu/host-utils.h         |  1 -
>  include/qemu/userfaultfd.h        |  1 -
>  include/sysemu/accel-blocker.h    |  1 -
>  include/sysemu/event-loop-base.h  |  1 -
>  net/vmnet_int.h                   |  1 -
>  qga/cutils.h                      |  2 --
>  target/hexagon/hex_arch_types.h   |  1 -
>  target/hexagon/mmvec/macros.h     |  1 -
>  target/riscv/pmu.h                |  1 -
>  accel/tcg/cpu-exec.c              |  1 -
>  audio/sndioaudio.c                |  2 +-
>  backends/hostmem-epc.c            |  2 +-
>  backends/tpm/tpm_emulator.c       |  1 -
>  block/export/vduse-blk.c          |  2 +-
>  block/qapi.c                      |  1 -
>  bsd-user/arm/signal.c             |  1 +
>  bsd-user/arm/target_arch_cpu.c    |  2 ++
>  bsd-user/freebsd/os-sys.c         |  1 +
>  bsd-user/i386/signal.c            |  1 +
>  bsd-user/i386/target_arch_cpu.c   |  3 +--
>  bsd-user/main.c                   |  4 +---
>  bsd-user/strace.c                 |  1 -
>  bsd-user/x86_64/signal.c          |  1 +
>  bsd-user/x86_64/target_arch_cpu.c |  3 +--
>  hw/9pfs/9p.c                      |  5 -----
>  hw/acpi/piix4.c                   |  1 -
>  hw/alpha/dp264.c                  |  1 -
>  hw/arm/virt.c                     |  1 -
>  hw/arm/xlnx-versal.c              |  1 -
>  hw/block/pflash_cfi01.c           |  1 -
>  hw/core/machine.c                 |  1 -
>  hw/display/virtio-gpu-udmabuf.c   |  1 -
>  hw/hppa/machine.c                 |  1 -
>  hw/hyperv/syndbg.c                |  2 +-
>  hw/i2c/pmbus_device.c             |  1 -
>  hw/i386/acpi-build.c              |  1 -
>  hw/input/tsc210x.c                |  1 -
>  hw/loongarch/acpi-build.c         |  1 -
>  hw/misc/macio/cuda.c              |  1 -
>  hw/misc/macio/pmu.c               |  1 -
>  hw/net/xilinx_axienet.c           |  1 -
>  hw/ppc/ppc405_uc.c                |  2 --
>  hw/ppc/ppc440_bamboo.c            |  1 -
>  hw/ppc/spapr_drc.c                |  1 -
>  hw/rdma/vmw/pvrdma_dev_ring.c     |  1 -
>  hw/remote/machine.c               |  1 -
>  hw/remote/proxy-memory-listener.c |  1 -
>  hw/remote/remote-obj.c            |  1 -
>  hw/rtc/mc146818rtc.c              |  1 -
>  hw/s390x/virtio-ccw-serial.c      |  1 -
>  hw/sensor/adm1272.c               |  1 -
>  hw/usb/dev-storage-bot.c          |  1 -
>  hw/usb/dev-storage-classic.c      |  1 -
>  migration/postcopy-ram.c          |  2 --
>  qga/commands-posix.c              |  1 -
>  qga/cutils.c                      |  3 ++-
>  softmmu/dirtylimit.c              |  1 -
>  softmmu/runstate.c                |  1 -
>  softmmu/vl.c                      |  3 ---
>  target/loongarch/translate.c      |  1 -
>  target/mips/tcg/translate.c       |  1 -
>  target/nios2/translate.c          |  2 --
>  tcg/tci.c                         |  1 -
>  tests/unit/test-cutils.c          |  1 -
>  tests/unit/test-seccomp.c         |  1 -
>  ui/gtk.c                          |  1 -
>  ui/udmabuf.c                      |  1 -
>  util/async-teardown.c             | 12 ++++--------
>  util/main-loop.c                  |  1 -
>  util/oslib-posix.c                |  6 ------
>  scripts/clean-includes            | 15 ++++++++++-----
>  89 files changed, 29 insertions(+), 123 deletions(-)
> 
> -- 
> 2.39.0

Re: [PATCH v4 16/19] Fix non-first inclusions of qemu/osdep.h

[Philippe Mathieu-Daudé]

On 19/1/23 12:41, Markus Armbruster wrote:
> Philippe Mathieu-Daudé <philmd@linaro.org> writes:
> 
>> On 19/1/23 07:59, Markus Armbruster wrote:
>>> This commit was created with scripts/clean-includes.
>>> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> 
> [...]
> 
>> Up to here:
>>
>> Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>>
>>> diff --git a/util/async-teardown.c b/util/async-teardown.c
>>> index 62bfce1b3c..62cdeb0f20 100644
>>> --- a/util/async-teardown.c
>>> +++ b/util/async-teardown.c
>>> @@ -10,16 +10,12 @@
>>>     * option) any later version.  See the COPYING file in the top-level directory.
>>>     *
>>>     */
>>> -#include <stdlib.h>
>>> -#include <stdio.h>
>>> -#include <sys/types.h>
>>> -#include <dirent.h>
>>> -#include <sys/prctl.h>
>>> -#include <signal.h>
>>> -#include <sched.h>
>>> -#include <unistd.h>
>>>   
>>>   #include "qemu/osdep.h"
>>> +#include <dirent.h>
>>> +#include <sys/prctl.h>
>>> +#include <sched.h>
>>> +
>>>    #include "qemu/async-teardown.h"
>>
>> This file has more changes.
> 
> I'm not sure I understand.
> 
> The patch does two related things:
> 
> 1. It puts qemu/osdep.h first.  The diff makes it look like we leave it
>     in place and move other stuff across, but that's the same.
> 
> 2. It deletes inclusions of headers qemu/osdep.h already includes:
> 
>      <stdlib.h>
>      <stdio.h>
>      <sys/types.h>
>      <signal.h>
>      <unistd.h>

Ah, the other files get this done in the "Drop duplicate #include" patch.

Re: [PATCH v4 18/19] 9p: Drop superfluous include of linux/limits.h

[Christian Schoenebeck]

On Thursday, January 19, 2023 11:37:00 AM CET Markus Armbruster wrote:
> Christian Schoenebeck <qemu_oss@crudebyte.com> writes:
> 
> > On Thursday, January 19, 2023 7:59:58 AM CET Markus Armbruster wrote:
> >> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> >> ---
> >>  hw/9pfs/9p.c | 3 ---
> >>  1 file changed, 3 deletions(-)
> >> 
> >> diff --git a/hw/9pfs/9p.c b/hw/9pfs/9p.c
> >> index 9621ec1341..aa736af380 100644
> >> --- a/hw/9pfs/9p.c
> >> +++ b/hw/9pfs/9p.c
> >> @@ -17,9 +17,6 @@
> >>   */
> >>  
> >>  #include "qemu/osdep.h"
> >> -#ifdef CONFIG_LINUX
> >> -#include <linux/limits.h>
> >> -#endif
> >>  #include <glib/gprintf.h>
> >>  #include "hw/virtio/virtio.h"
> >>  #include "qapi/error.h"
> >> 
> >
> > Where did that base version come from? I don't see it anywhere in history. 
> > Last relevant change in context was a136d17590a.
> 
> Current master (7ec8aeb6048) has
> 
>     #include "qemu/osdep.h"
>     #ifdef CONFIG_LINUX
>     #include <linux/limits.h>
>     #else
>     #include <limits.h>
>     #endif
>     #include <glib/gprintf.h>
> 
> The previous commit changes it to
> 
>     #include "qemu/osdep.h"
>     #ifdef CONFIG_LINUX
>     #include <linux/limits.h>
>     #endif
>     #include <glib/gprintf.h>
> 
> because "qemu/osdep.h" already includes <limits.h>.
> 
> Clearer now?

Ah, right I missed that in your previous patch. Thanks!

Reviewed-by: Christian Schoenebeck <qemu_oss@crudebyte.com>

Best regards,
Christian Schoenebeck

Re: [PATCH v4 17/19] Don't include headers already included by qemu/osdep.h

[Christian Schoenebeck]

On Thursday, January 19, 2023 7:59:57 AM CET Markus Armbruster wrote:
> This commit was created with scripts/clean-includes.
> 
> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> ---

For 9p changes:

Acked-by: Christian Schoenebeck <qemu_oss@crudebyte.com>

>  backends/tpm/tpm_ioctl.h          | 2 --
>  fsdev/p9array.h                   | 2 --
>  include/hw/misc/aspeed_lpc.h      | 2 --
>  include/hw/pci/pcie_doe.h         | 1 -
>  include/qemu/async-teardown.h     | 2 --
>  include/qemu/dbus.h               | 1 -
>  include/qemu/host-utils.h         | 1 -
>  include/sysemu/event-loop-base.h  | 1 -
>  accel/tcg/cpu-exec.c              | 1 -
>  hw/9pfs/9p.c                      | 2 --
>  hw/display/virtio-gpu-udmabuf.c   | 1 -
>  hw/i2c/pmbus_device.c             | 1 -
>  hw/remote/proxy-memory-listener.c | 1 -
>  hw/sensor/adm1272.c               | 1 -
>  hw/usb/dev-storage-bot.c          | 1 -
>  hw/usb/dev-storage-classic.c      | 1 -
>  softmmu/vl.c                      | 2 --
>  tcg/tci.c                         | 1 -
>  tests/unit/test-seccomp.c         | 1 -
>  ui/udmabuf.c                      | 1 -
>  util/main-loop.c                  | 1 -
>  util/oslib-posix.c                | 2 --
>  22 files changed, 29 deletions(-)
> 
> diff --git a/backends/tpm/tpm_ioctl.h b/backends/tpm/tpm_ioctl.h
> index e506ef5160..b1d31768a6 100644
> --- a/backends/tpm/tpm_ioctl.h
> +++ b/backends/tpm/tpm_ioctl.h
> @@ -12,8 +12,6 @@
>  # define __USE_LINUX_IOCTL_DEFS
>  #endif
>  
> -#include <stdint.h>
> -#include <sys/types.h>
>  #ifndef _WIN32
>  #include <sys/uio.h>
>  #include <sys/ioctl.h>
> diff --git a/fsdev/p9array.h b/fsdev/p9array.h
> index 90e83a7c7b..50a1b15fe9 100644
> --- a/fsdev/p9array.h
> +++ b/fsdev/p9array.h
> @@ -27,8 +27,6 @@
>  #ifndef QEMU_P9ARRAY_H
>  #define QEMU_P9ARRAY_H
>  
> -#include "qemu/compiler.h"
> -
>  /**
>   * P9Array provides a mechanism to access arrays in common C-style (e.g. by
>   * square bracket [] operator) in conjunction with reference variables that
> diff --git a/include/hw/misc/aspeed_lpc.h b/include/hw/misc/aspeed_lpc.h
> index fd228731d2..fa398959af 100644
> --- a/include/hw/misc/aspeed_lpc.h
> +++ b/include/hw/misc/aspeed_lpc.h
> @@ -12,8 +12,6 @@
>  
>  #include "hw/sysbus.h"
>  
> -#include <stdint.h>
> -
>  #define TYPE_ASPEED_LPC "aspeed.lpc"
>  #define ASPEED_LPC(obj) OBJECT_CHECK(AspeedLPCState, (obj), TYPE_ASPEED_LPC)
>  
> diff --git a/include/hw/pci/pcie_doe.h b/include/hw/pci/pcie_doe.h
> index ba4d8b03bd..87dc17dcef 100644
> --- a/include/hw/pci/pcie_doe.h
> +++ b/include/hw/pci/pcie_doe.h
> @@ -11,7 +11,6 @@
>  #define PCIE_DOE_H
>  
>  #include "qemu/range.h"
> -#include "qemu/typedefs.h"
>  #include "hw/register.h"
>  
>  /*
> diff --git a/include/qemu/async-teardown.h b/include/qemu/async-teardown.h
> index 092e7a37e7..b281da005b 100644
> --- a/include/qemu/async-teardown.h
> +++ b/include/qemu/async-teardown.h
> @@ -13,8 +13,6 @@
>  #ifndef QEMU_ASYNC_TEARDOWN_H
>  #define QEMU_ASYNC_TEARDOWN_H
>  
> -#include "config-host.h"
> -
>  #ifdef CONFIG_LINUX
>  void init_async_teardown(void);
>  #endif
> diff --git a/include/qemu/dbus.h b/include/qemu/dbus.h
> index 08f00dfd53..81d3de8a5a 100644
> --- a/include/qemu/dbus.h
> +++ b/include/qemu/dbus.h
> @@ -15,7 +15,6 @@
>  #include "qom/object.h"
>  #include "chardev/char.h"
>  #include "qemu/notify.h"
> -#include "qemu/typedefs.h"
>  
>  /* glib/gio 2.68 */
>  #define DBUS_METHOD_INVOCATION_HANDLED TRUE
> diff --git a/include/qemu/host-utils.h b/include/qemu/host-utils.h
> index 88d476161c..3ce62bf4a5 100644
> --- a/include/qemu/host-utils.h
> +++ b/include/qemu/host-utils.h
> @@ -30,7 +30,6 @@
>  #ifndef HOST_UTILS_H
>  #define HOST_UTILS_H
>  
> -#include "qemu/compiler.h"
>  #include "qemu/bswap.h"
>  #include "qemu/int128.h"
>  
> diff --git a/include/sysemu/event-loop-base.h b/include/sysemu/event-loop-base.h
> index 2748bf6ae1..a6c24f1351 100644
> --- a/include/sysemu/event-loop-base.h
> +++ b/include/sysemu/event-loop-base.h
> @@ -14,7 +14,6 @@
>  
>  #include "qom/object.h"
>  #include "block/aio.h"
> -#include "qemu/typedefs.h"
>  
>  #define TYPE_EVENT_LOOP_BASE         "event-loop-base"
>  OBJECT_DECLARE_TYPE(EventLoopBase, EventLoopBaseClass,
> diff --git a/accel/tcg/cpu-exec.c b/accel/tcg/cpu-exec.c
> index 8927092537..dd8f54a415 100644
> --- a/accel/tcg/cpu-exec.c
> +++ b/accel/tcg/cpu-exec.c
> @@ -28,7 +28,6 @@
>  #include "exec/exec-all.h"
>  #include "tcg/tcg.h"
>  #include "qemu/atomic.h"
> -#include "qemu/compiler.h"
>  #include "qemu/timer.h"
>  #include "qemu/rcu.h"
>  #include "exec/log.h"
> diff --git a/hw/9pfs/9p.c b/hw/9pfs/9p.c
> index 072cf67956..9621ec1341 100644
> --- a/hw/9pfs/9p.c
> +++ b/hw/9pfs/9p.c
> @@ -19,8 +19,6 @@
>  #include "qemu/osdep.h"
>  #ifdef CONFIG_LINUX
>  #include <linux/limits.h>
> -#else
> -#include <limits.h>
>  #endif
>  #include <glib/gprintf.h>
>  #include "hw/virtio/virtio.h"
> diff --git a/hw/display/virtio-gpu-udmabuf.c b/hw/display/virtio-gpu-udmabuf.c
> index 8bdf4bac6e..847fa4c0cc 100644
> --- a/hw/display/virtio-gpu-udmabuf.c
> +++ b/hw/display/virtio-gpu-udmabuf.c
> @@ -21,7 +21,6 @@
>  #include "exec/ramblock.h"
>  #include "sysemu/hostmem.h"
>  #include <sys/ioctl.h>
> -#include <fcntl.h>
>  #include <linux/memfd.h>
>  #include "qemu/memfd.h"
>  #include "standard-headers/linux/udmabuf.h"
> diff --git a/hw/i2c/pmbus_device.c b/hw/i2c/pmbus_device.c
> index 4071a88cfc..c3d6046784 100644
> --- a/hw/i2c/pmbus_device.c
> +++ b/hw/i2c/pmbus_device.c
> @@ -8,7 +8,6 @@
>  
>  #include "qemu/osdep.h"
>  #include <math.h>
> -#include <string.h>
>  #include "hw/i2c/pmbus_device.h"
>  #include "migration/vmstate.h"
>  #include "qemu/module.h"
> diff --git a/hw/remote/proxy-memory-listener.c b/hw/remote/proxy-memory-listener.c
> index eb9918fe72..18d96a1d04 100644
> --- a/hw/remote/proxy-memory-listener.c
> +++ b/hw/remote/proxy-memory-listener.c
> @@ -8,7 +8,6 @@
>  
>  #include "qemu/osdep.h"
>  
> -#include "qemu/compiler.h"
>  #include "qemu/int128.h"
>  #include "qemu/range.h"
>  #include "exec/memory.h"
> diff --git a/hw/sensor/adm1272.c b/hw/sensor/adm1272.c
> index 7310c769be..8f4a1c2cd4 100644
> --- a/hw/sensor/adm1272.c
> +++ b/hw/sensor/adm1272.c
> @@ -8,7 +8,6 @@
>   */
>  
>  #include "qemu/osdep.h"
> -#include <string.h>
>  #include "hw/i2c/pmbus_device.h"
>  #include "hw/irq.h"
>  #include "migration/vmstate.h"
> diff --git a/hw/usb/dev-storage-bot.c b/hw/usb/dev-storage-bot.c
> index b24b3148c2..1e5c5c711f 100644
> --- a/hw/usb/dev-storage-bot.c
> +++ b/hw/usb/dev-storage-bot.c
> @@ -8,7 +8,6 @@
>   */
>  
>  #include "qemu/osdep.h"
> -#include "qemu/typedefs.h"
>  #include "qapi/error.h"
>  #include "hw/usb.h"
>  #include "hw/usb/desc.h"
> diff --git a/hw/usb/dev-storage-classic.c b/hw/usb/dev-storage-classic.c
> index 00f25bade2..84d19752b5 100644
> --- a/hw/usb/dev-storage-classic.c
> +++ b/hw/usb/dev-storage-classic.c
> @@ -8,7 +8,6 @@
>   */
>  
>  #include "qemu/osdep.h"
> -#include "qemu/typedefs.h"
>  #include "qapi/error.h"
>  #include "qapi/visitor.h"
>  #include "hw/usb.h"
> diff --git a/softmmu/vl.c b/softmmu/vl.c
> index 9177d95d4e..5355a7fe5a 100644
> --- a/softmmu/vl.c
> +++ b/softmmu/vl.c
> @@ -136,8 +136,6 @@
>  #include "qemu/guest-random.h"
>  #include "qemu/keyval.h"
>  
> -#include "config-host.h"
> -
>  #define MAX_VIRTIO_CONSOLES 1
>  
>  typedef struct BlockdevOptionsQueueEntry {
> diff --git a/tcg/tci.c b/tcg/tci.c
> index 05a24163d3..e7ac74cab0 100644
> --- a/tcg/tci.c
> +++ b/tcg/tci.c
> @@ -21,7 +21,6 @@
>  #include "exec/cpu_ldst.h"
>  #include "tcg/tcg-op.h"
>  #include "tcg/tcg-ldst.h"
> -#include "qemu/compiler.h"
>  #include <ffi.h>
>  
>  
> diff --git a/tests/unit/test-seccomp.c b/tests/unit/test-seccomp.c
> index 3d7771e46c..f02c79cafd 100644
> --- a/tests/unit/test-seccomp.c
> +++ b/tests/unit/test-seccomp.c
> @@ -25,7 +25,6 @@
>  #include "qapi/error.h"
>  #include "qemu/module.h"
>  
> -#include <unistd.h>
>  #include <sys/syscall.h>
>  
>  static void test_seccomp_helper(const char *args, bool killed,
> diff --git a/ui/udmabuf.c b/ui/udmabuf.c
> index cebceb2610..cbf4357bb1 100644
> --- a/ui/udmabuf.c
> +++ b/ui/udmabuf.c
> @@ -8,7 +8,6 @@
>  #include "qapi/error.h"
>  #include "ui/console.h"
>  
> -#include <fcntl.h>
>  #include <sys/ioctl.h>
>  
>  int udmabuf_fd(void)
> diff --git a/util/main-loop.c b/util/main-loop.c
> index 58f776a8c9..3c0f525192 100644
> --- a/util/main-loop.c
> +++ b/util/main-loop.c
> @@ -33,7 +33,6 @@
>  #include "block/thread-pool.h"
>  #include "qemu/error-report.h"
>  #include "qemu/queue.h"
> -#include "qemu/compiler.h"
>  #include "qom/object.h"
>  
>  #ifndef _WIN32
> diff --git a/util/oslib-posix.c b/util/oslib-posix.c
> index 59a891b6a8..fd03fd32c8 100644
> --- a/util/oslib-posix.c
> +++ b/util/oslib-posix.c
> @@ -40,7 +40,6 @@
>  #include "qemu/thread.h"
>  #include <libgen.h>
>  #include "qemu/cutils.h"
> -#include "qemu/compiler.h"
>  #include "qemu/units.h"
>  #include "qemu/thread-context.h"
>  
> @@ -50,7 +49,6 @@
>  
>  #ifdef __FreeBSD__
>  #include <sys/thr.h>
> -#include <sys/types.h>
>  #include <sys/user.h>
>  #include <libutil.h>
>  #endif
> 

Re: [PATCH v4 16/19] Fix non-first inclusions of qemu/osdep.h

[Markus Armbruster]

Philippe Mathieu-Daudé <philmd@linaro.org> writes:

> On 19/1/23 12:41, Markus Armbruster wrote:
>> Philippe Mathieu-Daudé <philmd@linaro.org> writes:
>> 
>>> On 19/1/23 07:59, Markus Armbruster wrote:
>>>> This commit was created with scripts/clean-includes.
>>>> Signed-off-by: Markus Armbruster <armbru@redhat.com>
>> [...]
>> 
>>> Up to here:
>>>
>>> Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>>>
>>>> diff --git a/util/async-teardown.c b/util/async-teardown.c
>>>> index 62bfce1b3c..62cdeb0f20 100644
>>>> --- a/util/async-teardown.c
>>>> +++ b/util/async-teardown.c
>>>> @@ -10,16 +10,12 @@
>>>>     * option) any later version.  See the COPYING file in the top-level directory.
>>>>     *
>>>>     */
>>>> -#include <stdlib.h>
>>>> -#include <stdio.h>
>>>> -#include <sys/types.h>
>>>> -#include <dirent.h>
>>>> -#include <sys/prctl.h>
>>>> -#include <signal.h>
>>>> -#include <sched.h>
>>>> -#include <unistd.h>
>>>>  #include "qemu/osdep.h"
>>>> +#include <dirent.h>
>>>> +#include <sys/prctl.h>
>>>> +#include <sched.h>
>>>> +
>>>>    #include "qemu/async-teardown.h"
>>>
>>> This file has more changes.
>> I'm not sure I understand.
>> The patch does two related things:
>> 1. It puts qemu/osdep.h first.  The diff makes it look like we leave it
>>     in place and move other stuff across, but that's the same.
>> 2. It deletes inclusions of headers qemu/osdep.h already includes:
>>      <stdlib.h>
>>      <stdio.h>
>>      <sys/types.h>
>>      <signal.h>
>>      <unistd.h>
>
> Ah, the other files get this done in the "Drop duplicate #include" patch.

Right!

Re: [PATCH v4 19/19] Drop duplicate #include

[Greg Kurz]

On Thu, 19 Jan 2023 07:59:59 +0100
Markus Armbruster <armbru@redhat.com> wrote:

> Tracked down with the help of scripts/clean-includes.
> 
> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> ---

For ppc changes.

Reviewed-by: Greg Kurz <groug@kaod.org>

>  include/hw/arm/fsl-imx6ul.h   | 1 -
>  include/hw/arm/fsl-imx7.h     | 1 -
>  backends/tpm/tpm_emulator.c   | 1 -
>  hw/acpi/piix4.c               | 1 -
>  hw/alpha/dp264.c              | 1 -
>  hw/arm/virt.c                 | 1 -
>  hw/arm/xlnx-versal.c          | 1 -
>  hw/block/pflash_cfi01.c       | 1 -
>  hw/core/machine.c             | 1 -
>  hw/hppa/machine.c             | 1 -
>  hw/i386/acpi-build.c          | 1 -
>  hw/loongarch/acpi-build.c     | 1 -
>  hw/misc/macio/cuda.c          | 1 -
>  hw/misc/macio/pmu.c           | 1 -
>  hw/net/xilinx_axienet.c       | 1 -
>  hw/ppc/ppc405_uc.c            | 2 --
>  hw/ppc/ppc440_bamboo.c        | 1 -
>  hw/ppc/spapr_drc.c            | 1 -
>  hw/rdma/vmw/pvrdma_dev_ring.c | 1 -
>  hw/remote/machine.c           | 1 -
>  hw/remote/remote-obj.c        | 1 -
>  hw/rtc/mc146818rtc.c          | 1 -
>  hw/s390x/virtio-ccw-serial.c  | 1 -
>  migration/postcopy-ram.c      | 2 --
>  softmmu/dirtylimit.c          | 1 -
>  softmmu/runstate.c            | 1 -
>  softmmu/vl.c                  | 1 -
>  target/loongarch/translate.c  | 1 -
>  target/mips/tcg/translate.c   | 1 -
>  target/nios2/translate.c      | 2 --
>  tests/unit/test-cutils.c      | 1 -
>  ui/gtk.c                      | 1 -
>  util/oslib-posix.c            | 4 ----
>  33 files changed, 39 deletions(-)
> 
> diff --git a/include/hw/arm/fsl-imx6ul.h b/include/hw/arm/fsl-imx6ul.h
> index 7812e516a5..1952cb984d 100644
> --- a/include/hw/arm/fsl-imx6ul.h
> +++ b/include/hw/arm/fsl-imx6ul.h
> @@ -30,7 +30,6 @@
>  #include "hw/timer/imx_gpt.h"
>  #include "hw/timer/imx_epit.h"
>  #include "hw/i2c/imx_i2c.h"
> -#include "hw/gpio/imx_gpio.h"
>  #include "hw/sd/sdhci.h"
>  #include "hw/ssi/imx_spi.h"
>  #include "hw/net/imx_fec.h"
> diff --git a/include/hw/arm/fsl-imx7.h b/include/hw/arm/fsl-imx7.h
> index 4e5e071864..355bd8ea83 100644
> --- a/include/hw/arm/fsl-imx7.h
> +++ b/include/hw/arm/fsl-imx7.h
> @@ -32,7 +32,6 @@
>  #include "hw/timer/imx_gpt.h"
>  #include "hw/timer/imx_epit.h"
>  #include "hw/i2c/imx_i2c.h"
> -#include "hw/gpio/imx_gpio.h"
>  #include "hw/sd/sdhci.h"
>  #include "hw/ssi/imx_spi.h"
>  #include "hw/net/imx_fec.h"
> diff --git a/backends/tpm/tpm_emulator.c b/backends/tpm/tpm_emulator.c
> index 49cc3d749d..2b440d2c9a 100644
> --- a/backends/tpm/tpm_emulator.c
> +++ b/backends/tpm/tpm_emulator.c
> @@ -35,7 +35,6 @@
>  #include "sysemu/runstate.h"
>  #include "sysemu/tpm_backend.h"
>  #include "sysemu/tpm_util.h"
> -#include "sysemu/runstate.h"
>  #include "tpm_int.h"
>  #include "tpm_ioctl.h"
>  #include "migration/blocker.h"
> diff --git a/hw/acpi/piix4.c b/hw/acpi/piix4.c
> index 0a81f1ad93..df39f91294 100644
> --- a/hw/acpi/piix4.c
> +++ b/hw/acpi/piix4.c
> @@ -35,7 +35,6 @@
>  #include "sysemu/xen.h"
>  #include "qapi/error.h"
>  #include "qemu/range.h"
> -#include "hw/acpi/pcihp.h"
>  #include "hw/acpi/cpu_hotplug.h"
>  #include "hw/acpi/cpu.h"
>  #include "hw/hotplug.h"
> diff --git a/hw/alpha/dp264.c b/hw/alpha/dp264.c
> index c502c8c62a..4161f559a7 100644
> --- a/hw/alpha/dp264.c
> +++ b/hw/alpha/dp264.c
> @@ -18,7 +18,6 @@
>  #include "net/net.h"
>  #include "qemu/cutils.h"
>  #include "qemu/datadir.h"
> -#include "net/net.h"
>  
>  static uint64_t cpu_alpha_superpage_to_phys(void *opaque, uint64_t addr)
>  {
> diff --git a/hw/arm/virt.c b/hw/arm/virt.c
> index ea2413a0ba..d3849d7233 100644
> --- a/hw/arm/virt.c
> +++ b/hw/arm/virt.c
> @@ -33,7 +33,6 @@
>  #include "qemu/units.h"
>  #include "qemu/option.h"
>  #include "monitor/qdev.h"
> -#include "qapi/error.h"
>  #include "hw/sysbus.h"
>  #include "hw/arm/boot.h"
>  #include "hw/arm/primecell.h"
> diff --git a/hw/arm/xlnx-versal.c b/hw/arm/xlnx-versal.c
> index 57276e1506..69b1b99e93 100644
> --- a/hw/arm/xlnx-versal.c
> +++ b/hw/arm/xlnx-versal.c
> @@ -22,7 +22,6 @@
>  #include "hw/misc/unimp.h"
>  #include "hw/arm/xlnx-versal.h"
>  #include "qemu/log.h"
> -#include "hw/sysbus.h"
>  
>  #define XLNX_VERSAL_ACPU_TYPE ARM_CPU_TYPE_NAME("cortex-a72")
>  #define XLNX_VERSAL_RCPU_TYPE ARM_CPU_TYPE_NAME("cortex-r5f")
> diff --git a/hw/block/pflash_cfi01.c b/hw/block/pflash_cfi01.c
> index 0cbc2fb4cb..d11406eada 100644
> --- a/hw/block/pflash_cfi01.c
> +++ b/hw/block/pflash_cfi01.c
> @@ -45,7 +45,6 @@
>  #include "qapi/error.h"
>  #include "qemu/error-report.h"
>  #include "qemu/bitops.h"
> -#include "qemu/error-report.h"
>  #include "qemu/host-utils.h"
>  #include "qemu/log.h"
>  #include "qemu/module.h"
> diff --git a/hw/core/machine.c b/hw/core/machine.c
> index 616f3a207c..67cf9f9dcd 100644
> --- a/hw/core/machine.c
> +++ b/hw/core/machine.c
> @@ -39,7 +39,6 @@
>  #include "exec/confidential-guest-support.h"
>  #include "hw/virtio/virtio.h"
>  #include "hw/virtio/virtio-pci.h"
> -#include "qom/object_interfaces.h"
>  
>  GlobalProperty hw_compat_7_2[] = {};
>  const size_t hw_compat_7_2_len = G_N_ELEMENTS(hw_compat_7_2);
> diff --git a/hw/hppa/machine.c b/hw/hppa/machine.c
> index de1cc7ab71..7ac68c943f 100644
> --- a/hw/hppa/machine.c
> +++ b/hw/hppa/machine.c
> @@ -28,7 +28,6 @@
>  #include "qapi/error.h"
>  #include "net/net.h"
>  #include "qemu/log.h"
> -#include "net/net.h"
>  
>  #define MIN_SEABIOS_HPPA_VERSION 6 /* require at least this fw version */
>  
> diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
> index 127c4e2d50..14f6f75454 100644
> --- a/hw/i386/acpi-build.c
> +++ b/hw/i386/acpi-build.c
> @@ -76,7 +76,6 @@
>  
>  #include "hw/acpi/hmat.h"
>  #include "hw/acpi/viot.h"
> -#include "hw/acpi/cxl.h"
>  
>  #include CONFIG_DEVICES
>  
> diff --git a/hw/loongarch/acpi-build.c b/hw/loongarch/acpi-build.c
> index c2b237736d..f551296a0e 100644
> --- a/hw/loongarch/acpi-build.c
> +++ b/hw/loongarch/acpi-build.c
> @@ -22,7 +22,6 @@
>  /* Supported chipsets: */
>  #include "hw/pci-host/ls7a.h"
>  #include "hw/loongarch/virt.h"
> -#include "hw/acpi/aml-build.h"
>  
>  #include "hw/acpi/utils.h"
>  #include "hw/acpi/pci.h"
> diff --git a/hw/misc/macio/cuda.c b/hw/misc/macio/cuda.c
> index 853e88bfed..29a8e5ed19 100644
> --- a/hw/misc/macio/cuda.c
> +++ b/hw/misc/macio/cuda.c
> @@ -30,7 +30,6 @@
>  #include "hw/input/adb.h"
>  #include "hw/misc/mos6522.h"
>  #include "hw/misc/macio/cuda.h"
> -#include "qapi/error.h"
>  #include "qemu/timer.h"
>  #include "sysemu/runstate.h"
>  #include "sysemu/rtc.h"
> diff --git a/hw/misc/macio/pmu.c b/hw/misc/macio/pmu.c
> index 97ef8c771b..5a788e595a 100644
> --- a/hw/misc/macio/pmu.c
> +++ b/hw/misc/macio/pmu.c
> @@ -36,7 +36,6 @@
>  #include "hw/misc/mos6522.h"
>  #include "hw/misc/macio/gpio.h"
>  #include "hw/misc/macio/pmu.h"
> -#include "qapi/error.h"
>  #include "qemu/timer.h"
>  #include "sysemu/runstate.h"
>  #include "sysemu/rtc.h"
> diff --git a/hw/net/xilinx_axienet.c b/hw/net/xilinx_axienet.c
> index 990ff3a1c2..673af7da26 100644
> --- a/hw/net/xilinx_axienet.c
> +++ b/hw/net/xilinx_axienet.c
> @@ -31,7 +31,6 @@
>  #include "net/net.h"
>  #include "net/checksum.h"
>  
> -#include "hw/hw.h"
>  #include "hw/irq.h"
>  #include "hw/qdev-properties.h"
>  #include "hw/stream.h"
> diff --git a/hw/ppc/ppc405_uc.c b/hw/ppc/ppc405_uc.c
> index c973cfb04e..0cc68178ad 100644
> --- a/hw/ppc/ppc405_uc.c
> +++ b/hw/ppc/ppc405_uc.c
> @@ -38,8 +38,6 @@
>  #include "sysemu/sysemu.h"
>  #include "exec/address-spaces.h"
>  #include "hw/intc/ppc-uic.h"
> -#include "hw/qdev-properties.h"
> -#include "qapi/error.h"
>  #include "trace.h"
>  
>  /*****************************************************************************/
> diff --git a/hw/ppc/ppc440_bamboo.c b/hw/ppc/ppc440_bamboo.c
> index 81d71adf34..2880c81cb1 100644
> --- a/hw/ppc/ppc440_bamboo.c
> +++ b/hw/ppc/ppc440_bamboo.c
> @@ -13,7 +13,6 @@
>  
>  #include "qemu/osdep.h"
>  #include "qemu/units.h"
> -#include "qemu/error-report.h"
>  #include "qemu/datadir.h"
>  #include "qemu/error-report.h"
>  #include "net/net.h"
> diff --git a/hw/ppc/spapr_drc.c b/hw/ppc/spapr_drc.c
> index 4923435a8b..b5c400a94d 100644
> --- a/hw/ppc/spapr_drc.c
> +++ b/hw/ppc/spapr_drc.c
> @@ -17,7 +17,6 @@
>  #include "hw/ppc/spapr_drc.h"
>  #include "qom/object.h"
>  #include "migration/vmstate.h"
> -#include "qapi/error.h"
>  #include "qapi/qapi-events-qdev.h"
>  #include "qapi/visitor.h"
>  #include "qemu/error-report.h"
> diff --git a/hw/rdma/vmw/pvrdma_dev_ring.c b/hw/rdma/vmw/pvrdma_dev_ring.c
> index 598e6adc5e..30ce22a5be 100644
> --- a/hw/rdma/vmw/pvrdma_dev_ring.c
> +++ b/hw/rdma/vmw/pvrdma_dev_ring.c
> @@ -14,7 +14,6 @@
>   */
>  
>  #include "qemu/osdep.h"
> -#include "qemu/cutils.h"
>  #include "hw/pci/pci.h"
>  #include "cpu.h"
>  #include "qemu/cutils.h"
> diff --git a/hw/remote/machine.c b/hw/remote/machine.c
> index 519f855ec1..fdc6c441bb 100644
> --- a/hw/remote/machine.c
> +++ b/hw/remote/machine.c
> @@ -22,7 +22,6 @@
>  #include "hw/remote/iohub.h"
>  #include "hw/remote/iommu.h"
>  #include "hw/qdev-core.h"
> -#include "hw/remote/iommu.h"
>  #include "hw/remote/vfio-user-obj.h"
>  #include "hw/pci/msi.h"
>  
> diff --git a/hw/remote/remote-obj.c b/hw/remote/remote-obj.c
> index 333e5ac443..65b6f7cc86 100644
> --- a/hw/remote/remote-obj.c
> +++ b/hw/remote/remote-obj.c
> @@ -12,7 +12,6 @@
>  #include "qemu/error-report.h"
>  #include "qemu/notify.h"
>  #include "qom/object_interfaces.h"
> -#include "hw/qdev-core.h"
>  #include "io/channel.h"
>  #include "hw/qdev-core.h"
>  #include "hw/remote/machine.h"
> diff --git a/hw/rtc/mc146818rtc.c b/hw/rtc/mc146818rtc.c
> index bc1192b7ae..ba612a151d 100644
> --- a/hw/rtc/mc146818rtc.c
> +++ b/hw/rtc/mc146818rtc.c
> @@ -43,7 +43,6 @@
>  #include "qapi/error.h"
>  #include "qapi/qapi-events-misc.h"
>  #include "qapi/visitor.h"
> -#include "hw/rtc/mc146818rtc_regs.h"
>  
>  //#define DEBUG_CMOS
>  //#define DEBUG_COALESCED
> diff --git a/hw/s390x/virtio-ccw-serial.c b/hw/s390x/virtio-ccw-serial.c
> index bf8057880f..8f8d2302f8 100644
> --- a/hw/s390x/virtio-ccw-serial.c
> +++ b/hw/s390x/virtio-ccw-serial.c
> @@ -15,7 +15,6 @@
>  #include "hw/qdev-properties.h"
>  #include "hw/virtio/virtio-serial.h"
>  #include "virtio-ccw.h"
> -#include "hw/virtio/virtio-serial.h"
>  
>  #define TYPE_VIRTIO_SERIAL_CCW "virtio-serial-ccw"
>  OBJECT_DECLARE_SIMPLE_TYPE(VirtioSerialCcw, VIRTIO_SERIAL_CCW)
> diff --git a/migration/postcopy-ram.c b/migration/postcopy-ram.c
> index b9a37ef255..8b7d1af75d 100644
> --- a/migration/postcopy-ram.c
> +++ b/migration/postcopy-ram.c
> @@ -17,7 +17,6 @@
>   */
>  
>  #include "qemu/osdep.h"
> -#include "qemu/rcu.h"
>  #include "qemu/madvise.h"
>  #include "exec/target_page.h"
>  #include "migration.h"
> @@ -34,7 +33,6 @@
>  #include "hw/boards.h"
>  #include "exec/ramblock.h"
>  #include "socket.h"
> -#include "qemu-file.h"
>  #include "yank_functions.h"
>  #include "tls.h"
>  
> diff --git a/softmmu/dirtylimit.c b/softmmu/dirtylimit.c
> index 12668555f2..c56f0f58c8 100644
> --- a/softmmu/dirtylimit.c
> +++ b/softmmu/dirtylimit.c
> @@ -11,7 +11,6 @@
>   */
>  
>  #include "qemu/osdep.h"
> -#include "qapi/error.h"
>  #include "qemu/main-loop.h"
>  #include "qapi/qapi-commands-migration.h"
>  #include "qapi/qmp/qdict.h"
> diff --git a/softmmu/runstate.c b/softmmu/runstate.c
> index cab9f6fc07..f9ad88e6a7 100644
> --- a/softmmu/runstate.c
> +++ b/softmmu/runstate.c
> @@ -41,7 +41,6 @@
>  #include "qapi/qapi-commands-run-state.h"
>  #include "qapi/qapi-events-run-state.h"
>  #include "qemu/error-report.h"
> -#include "qemu/log.h"
>  #include "qemu/job.h"
>  #include "qemu/log.h"
>  #include "qemu/module.h"
> diff --git a/softmmu/vl.c b/softmmu/vl.c
> index 5355a7fe5a..b2ee3fee3f 100644
> --- a/softmmu/vl.c
> +++ b/softmmu/vl.c
> @@ -129,7 +129,6 @@
>  #include "qapi/qapi-commands-misc.h"
>  #include "qapi/qapi-visit-qom.h"
>  #include "qapi/qapi-commands-ui.h"
> -#include "qapi/qmp/qdict.h"
>  #include "block/qdict.h"
>  #include "qapi/qmp/qerror.h"
>  #include "sysemu/iothread.h"
> diff --git a/target/loongarch/translate.c b/target/loongarch/translate.c
> index 38ced69803..72a6275665 100644
> --- a/target/loongarch/translate.c
> +++ b/target/loongarch/translate.c
> @@ -12,7 +12,6 @@
>  #include "exec/helper-proto.h"
>  #include "exec/helper-gen.h"
>  
> -#include "exec/translator.h"
>  #include "exec/log.h"
>  #include "qemu/qemu-print.h"
>  #include "fpu/softfloat.h"
> diff --git a/target/mips/tcg/translate.c b/target/mips/tcg/translate.c
> index 624e6b7786..aa12bb708a 100644
> --- a/target/mips/tcg/translate.c
> +++ b/target/mips/tcg/translate.c
> @@ -32,7 +32,6 @@
>  #include "semihosting/semihost.h"
>  
>  #include "trace.h"
> -#include "exec/translator.h"
>  #include "exec/log.h"
>  #include "qemu/qemu-print.h"
>  #include "fpu_helper.h"
> diff --git a/target/nios2/translate.c b/target/nios2/translate.c
> index 4db8b47744..7aee65a089 100644
> --- a/target/nios2/translate.c
> +++ b/target/nios2/translate.c
> @@ -938,8 +938,6 @@ static const char * const cr_regnames[NUM_CR_REGS] = {
>  };
>  #endif
>  
> -#include "exec/gen-icount.h"
> -
>  /* generate intermediate code for basic block 'tb'.  */
>  static void nios2_tr_init_disas_context(DisasContextBase *dcbase, CPUState *cs)
>  {
> diff --git a/tests/unit/test-cutils.c b/tests/unit/test-cutils.c
> index 2126b46391..3c4f875420 100644
> --- a/tests/unit/test-cutils.c
> +++ b/tests/unit/test-cutils.c
> @@ -26,7 +26,6 @@
>   */
>  
>  #include "qemu/osdep.h"
> -#include "qemu/units.h"
>  #include "qemu/cutils.h"
>  #include "qemu/units.h"
>  
> diff --git a/ui/gtk.c b/ui/gtk.c
> index 4817623c8f..7f752d8b7d 100644
> --- a/ui/gtk.c
> +++ b/ui/gtk.c
> @@ -53,7 +53,6 @@
>  #include <math.h>
>  
>  #include "trace.h"
> -#include "qemu/cutils.h"
>  #include "ui/input.h"
>  #include "sysemu/runstate.h"
>  #include "sysemu/sysemu.h"
> diff --git a/util/oslib-posix.c b/util/oslib-posix.c
> index fd03fd32c8..77d882e681 100644
> --- a/util/oslib-posix.c
> +++ b/util/oslib-posix.c
> @@ -59,10 +59,6 @@
>  
>  #include "qemu/mmap-alloc.h"
>  
> -#ifdef CONFIG_DEBUG_STACK_USAGE
> -#include "qemu/error-report.h"
> -#endif
> -
>  #define MAX_MEM_PREALLOC_THREAD_COUNT 16
>  
>  struct MemsetThread;

Re: [PATCH v4 04/19] bsd-user: Clean up includes

[Warner Losh]

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

On Thu, Jan 19, 2023 at 12:00 AM Markus Armbruster <armbru@redhat.com>
wrote:

> Clean up includes so that osdep.h is included first and headers
> which it implies are not included manually.
>
> This commit was created with scripts/clean-includes.
>
> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> ---
>  bsd-user/bsd-proc.h               | 4 ----
>  bsd-user/qemu.h                   | 1 -
>  bsd-user/arm/signal.c             | 1 +
>  bsd-user/arm/target_arch_cpu.c    | 2 ++
>  bsd-user/freebsd/os-sys.c         | 1 +
>  bsd-user/i386/signal.c            | 1 +
>  bsd-user/i386/target_arch_cpu.c   | 3 +--
>  bsd-user/main.c                   | 4 +---
>  bsd-user/strace.c                 | 1 -
>  bsd-user/x86_64/signal.c          | 1 +
>  bsd-user/x86_64/target_arch_cpu.c | 3 +--
>  11 files changed, 9 insertions(+), 13 deletions(-)
>
> diff --git a/bsd-user/bsd-proc.h b/bsd-user/bsd-proc.h
> index 68b66e571d..a1061bffb8 100644
> --- a/bsd-user/bsd-proc.h
> +++ b/bsd-user/bsd-proc.h
> @@ -20,11 +20,7 @@
>  #ifndef BSD_PROC_H_
>  #define BSD_PROC_H_
>
> -#include <sys/types.h>
> -#include <sys/stat.h>
> -#include <sys/time.h>
>  #include <sys/resource.h>
>

Did you test this on FreeBSD 12? FreeBSD 13 has started to climb the hill
where all includes are independent, but much of that hasn't been merged to
12 yet. sys/types.h, at least, is documented as a prereq for both
sys/stat.h and sys/resource.h.

I know many of these are in os-dep.h, and I've had trouble in the bsd-user
fork with that and the layout of the code which has arguably way too much
in the .h files.

Also, why didn't you move sys/resource.h and other such files to os-dep.h?
I'm struggling to understand the rules around what is or isn't included
where?


> -#include <unistd.h>
>
>  /* exit(2) */
>  static inline abi_long do_bsd_exit(void *cpu_env, abi_long arg1)
> diff --git a/bsd-user/qemu.h b/bsd-user/qemu.h
> index be6105385e..0ceecfb6df 100644
> --- a/bsd-user/qemu.h
> +++ b/bsd-user/qemu.h
> @@ -17,7 +17,6 @@
>  #ifndef QEMU_H
>  #define QEMU_H
>
> -#include "qemu/osdep.h"
>  #include "cpu.h"
>  #include "qemu/units.h"
>  #include "exec/cpu_ldst.h"
> diff --git a/bsd-user/arm/signal.c b/bsd-user/arm/signal.c
> index 2b1dd745d1..9734407543 100644
> --- a/bsd-user/arm/signal.c
> +++ b/bsd-user/arm/signal.c
> @@ -17,6 +17,7 @@
>   *  along with this program; if not, see <http://www.gnu.org/licenses/>.
>   */
>
> +#include "qemu/osdep.h"
>  #include "qemu.h"
>
>  /*
> diff --git a/bsd-user/arm/target_arch_cpu.c
> b/bsd-user/arm/target_arch_cpu.c
> index 02bf9149d5..fe38ae2210 100644
> --- a/bsd-user/arm/target_arch_cpu.c
> +++ b/bsd-user/arm/target_arch_cpu.c
> @@ -16,6 +16,8 @@
>   *  You should have received a copy of the GNU General Public License
>   *  along with this program; if not, see <http://www.gnu.org/licenses/>.
>   */
> +
> +#include "qemu/osdep.h"
>  #include "target_arch.h"
>
>  void target_cpu_set_tls(CPUARMState *env, target_ulong newtls)
> diff --git a/bsd-user/freebsd/os-sys.c b/bsd-user/freebsd/os-sys.c
> index 309e27b9d6..1676ec10f8 100644
> --- a/bsd-user/freebsd/os-sys.c
> +++ b/bsd-user/freebsd/os-sys.c
> @@ -17,6 +17,7 @@
>   *  along with this program; if not, see <http://www.gnu.org/licenses/>.
>   */
>
> +#include "qemu/osdep.h"
>  #include "qemu.h"
>  #include "target_arch_sysarch.h"
>
> diff --git a/bsd-user/i386/signal.c b/bsd-user/i386/signal.c
> index 5dd975ce56..a3131047b8 100644
> --- a/bsd-user/i386/signal.c
> +++ b/bsd-user/i386/signal.c
> @@ -17,6 +17,7 @@
>   *  along with this program; if not, see <http://www.gnu.org/licenses/>.
>   */
>
> +#include "qemu/osdep.h"
>  #include "qemu.h"
>
>  /*
> diff --git a/bsd-user/i386/target_arch_cpu.c
> b/bsd-user/i386/target_arch_cpu.c
> index d349e45299..2a3af2ddef 100644
> --- a/bsd-user/i386/target_arch_cpu.c
> +++ b/bsd-user/i386/target_arch_cpu.c
> @@ -17,9 +17,8 @@
>   *  along with this program; if not, see <http://www.gnu.org/licenses/>.
>   */
>
> -#include <sys/types.h>
> -
>  #include "qemu/osdep.h"
> +
>  #include "cpu.h"
>  #include "qemu.h"
>  #include "qemu/timer.h"
> diff --git a/bsd-user/main.c b/bsd-user/main.c
> index 6f09180d65..41290e16f9 100644
> --- a/bsd-user/main.c
> +++ b/bsd-user/main.c
> @@ -18,12 +18,10 @@
>   *  along with this program; if not, see <http://www.gnu.org/licenses/>.
>   */
>
> -#include <sys/types.h>
> -#include <sys/time.h>
> +#include "qemu/osdep.h"
>  #include <sys/resource.h>
>  #include <sys/sysctl.h>
>
> -#include "qemu/osdep.h"
>  #include "qemu/help-texts.h"
>  #include "qemu/units.h"
>  #include "qemu/accel.h"
> diff --git a/bsd-user/strace.c b/bsd-user/strace.c
> index a77d10dd6b..96499751eb 100644
> --- a/bsd-user/strace.c
> +++ b/bsd-user/strace.c
> @@ -20,7 +20,6 @@
>  #include <sys/select.h>
>  #include <sys/syscall.h>
>  #include <sys/ioccom.h>
> -#include <ctype.h>
>
>  #include "qemu.h"
>
> diff --git a/bsd-user/x86_64/signal.c b/bsd-user/x86_64/signal.c
> index c3875bc4c6..46cb865180 100644
> --- a/bsd-user/x86_64/signal.c
> +++ b/bsd-user/x86_64/signal.c
> @@ -16,6 +16,7 @@
>   *  along with this program; if not, see <http://www.gnu.org/licenses/>.
>   */
>
> +#include "qemu/osdep.h"
>  #include "qemu.h"
>
>  /*
> diff --git a/bsd-user/x86_64/target_arch_cpu.c
> b/bsd-user/x86_64/target_arch_cpu.c
> index be7bd10720..1d32f18907 100644
> --- a/bsd-user/x86_64/target_arch_cpu.c
> +++ b/bsd-user/x86_64/target_arch_cpu.c
> @@ -17,9 +17,8 @@
>   *  along with this program; if not, see <http://www.gnu.org/licenses/>.
>   */
>
> -#include <sys/types.h>
> -
>  #include "qemu/osdep.h"
> +
>  #include "cpu.h"
>  #include "qemu.h"
>  #include "qemu/timer.h"
>

I suppose these are fine. How do I run the cleanup script? I have 2x the
number of files not upstream in the bsd-user fork that I'd like to cleanup
as well and they are likely a bigger mess and I'll just upstream them in
the messy state unless I understand how to keep the neat :).

Warner

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

Re: [PATCH v4 04/19] bsd-user: Clean up includes

[Markus Armbruster]

Warner Losh <imp@bsdimp.com> writes:

> On Thu, Jan 19, 2023 at 12:00 AM Markus Armbruster <armbru@redhat.com>
> wrote:
>
>> Clean up includes so that osdep.h is included first and headers
>> which it implies are not included manually.
>>
>> This commit was created with scripts/clean-includes.
>>
>> Signed-off-by: Markus Armbruster <armbru@redhat.com>
>> ---
>>  bsd-user/bsd-proc.h               | 4 ----
>>  bsd-user/qemu.h                   | 1 -
>>  bsd-user/arm/signal.c             | 1 +
>>  bsd-user/arm/target_arch_cpu.c    | 2 ++
>>  bsd-user/freebsd/os-sys.c         | 1 +
>>  bsd-user/i386/signal.c            | 1 +
>>  bsd-user/i386/target_arch_cpu.c   | 3 +--
>>  bsd-user/main.c                   | 4 +---
>>  bsd-user/strace.c                 | 1 -
>>  bsd-user/x86_64/signal.c          | 1 +
>>  bsd-user/x86_64/target_arch_cpu.c | 3 +--
>>  11 files changed, 9 insertions(+), 13 deletions(-)
>>
>> diff --git a/bsd-user/bsd-proc.h b/bsd-user/bsd-proc.h
>> index 68b66e571d..a1061bffb8 100644
>> --- a/bsd-user/bsd-proc.h
>> +++ b/bsd-user/bsd-proc.h
>> @@ -20,11 +20,7 @@
>>  #ifndef BSD_PROC_H_
>>  #define BSD_PROC_H_
>>
>> -#include <sys/types.h>
>> -#include <sys/stat.h>
>> -#include <sys/time.h>
>>  #include <sys/resource.h>
>>
>
> Did you test this on FreeBSD 12? FreeBSD 13 has started to climb the hill
> where all includes are independent, but much of that hasn't been merged to
> 12 yet. sys/types.h, at least, is documented as a prereq for both
> sys/stat.h and sys/resource.h.
>
> I know many of these are in os-dep.h, and I've had trouble in the bsd-user
> fork with that and the layout of the code which has arguably way too much
> in the .h files.

No, I didn't test on FreeBSD 12.

Any .c must include qemu/osdep.h *first*.  Any further inclusions of
headers qemu/osdep.h includes already are no-ops unless the headers in
question lack multiple inclusion guards.

> Also, why didn't you move sys/resource.h and other such files to os-dep.h?
> I'm struggling to understand the rules around what is or isn't included
> where?

This series is "run scripts/clean-includes, split it into reviewable
chunks, tidy up blank lines".  Moving more includes into qemu/osdep.h is
out of scope.

I sympathize with your complaint that QEMU does too much in headers in
general, and in qemu/osdep.h in particular.

>> -#include <unistd.h>
>>
>>  /* exit(2) */
>>  static inline abi_long do_bsd_exit(void *cpu_env, abi_long arg1)
>> diff --git a/bsd-user/qemu.h b/bsd-user/qemu.h
>> index be6105385e..0ceecfb6df 100644
>> --- a/bsd-user/qemu.h
>> +++ b/bsd-user/qemu.h
>> @@ -17,7 +17,6 @@
>>  #ifndef QEMU_H
>>  #define QEMU_H
>>
>> -#include "qemu/osdep.h"
>>  #include "cpu.h"
>>  #include "qemu/units.h"
>>  #include "exec/cpu_ldst.h"
>> diff --git a/bsd-user/arm/signal.c b/bsd-user/arm/signal.c
>> index 2b1dd745d1..9734407543 100644
>> --- a/bsd-user/arm/signal.c
>> +++ b/bsd-user/arm/signal.c
>> @@ -17,6 +17,7 @@
>>   *  along with this program; if not, see <http://www.gnu.org/licenses/>.
>>   */
>>
>> +#include "qemu/osdep.h"
>>  #include "qemu.h"
>>
>>  /*
>> diff --git a/bsd-user/arm/target_arch_cpu.c
>> b/bsd-user/arm/target_arch_cpu.c
>> index 02bf9149d5..fe38ae2210 100644
>> --- a/bsd-user/arm/target_arch_cpu.c
>> +++ b/bsd-user/arm/target_arch_cpu.c
>> @@ -16,6 +16,8 @@
>>   *  You should have received a copy of the GNU General Public License
>>   *  along with this program; if not, see <http://www.gnu.org/licenses/>.
>>   */
>> +
>> +#include "qemu/osdep.h"
>>  #include "target_arch.h"
>>
>>  void target_cpu_set_tls(CPUARMState *env, target_ulong newtls)
>> diff --git a/bsd-user/freebsd/os-sys.c b/bsd-user/freebsd/os-sys.c
>> index 309e27b9d6..1676ec10f8 100644
>> --- a/bsd-user/freebsd/os-sys.c
>> +++ b/bsd-user/freebsd/os-sys.c
>> @@ -17,6 +17,7 @@
>>   *  along with this program; if not, see <http://www.gnu.org/licenses/>.
>>   */
>>
>> +#include "qemu/osdep.h"
>>  #include "qemu.h"
>>  #include "target_arch_sysarch.h"
>>
>> diff --git a/bsd-user/i386/signal.c b/bsd-user/i386/signal.c
>> index 5dd975ce56..a3131047b8 100644
>> --- a/bsd-user/i386/signal.c
>> +++ b/bsd-user/i386/signal.c
>> @@ -17,6 +17,7 @@
>>   *  along with this program; if not, see <http://www.gnu.org/licenses/>.
>>   */
>>
>> +#include "qemu/osdep.h"
>>  #include "qemu.h"
>>
>>  /*
>> diff --git a/bsd-user/i386/target_arch_cpu.c
>> b/bsd-user/i386/target_arch_cpu.c
>> index d349e45299..2a3af2ddef 100644
>> --- a/bsd-user/i386/target_arch_cpu.c
>> +++ b/bsd-user/i386/target_arch_cpu.c
>> @@ -17,9 +17,8 @@
>>   *  along with this program; if not, see <http://www.gnu.org/licenses/>.
>>   */
>>
>> -#include <sys/types.h>
>> -
>>  #include "qemu/osdep.h"
>> +
>>  #include "cpu.h"
>>  #include "qemu.h"
>>  #include "qemu/timer.h"
>> diff --git a/bsd-user/main.c b/bsd-user/main.c
>> index 6f09180d65..41290e16f9 100644
>> --- a/bsd-user/main.c
>> +++ b/bsd-user/main.c
>> @@ -18,12 +18,10 @@
>>   *  along with this program; if not, see <http://www.gnu.org/licenses/>.
>>   */
>>
>> -#include <sys/types.h>
>> -#include <sys/time.h>
>> +#include "qemu/osdep.h"
>>  #include <sys/resource.h>
>>  #include <sys/sysctl.h>
>>
>> -#include "qemu/osdep.h"
>>  #include "qemu/help-texts.h"
>>  #include "qemu/units.h"
>>  #include "qemu/accel.h"
>> diff --git a/bsd-user/strace.c b/bsd-user/strace.c
>> index a77d10dd6b..96499751eb 100644
>> --- a/bsd-user/strace.c
>> +++ b/bsd-user/strace.c
>> @@ -20,7 +20,6 @@
>>  #include <sys/select.h>
>>  #include <sys/syscall.h>
>>  #include <sys/ioccom.h>
>> -#include <ctype.h>
>>
>>  #include "qemu.h"
>>
>> diff --git a/bsd-user/x86_64/signal.c b/bsd-user/x86_64/signal.c
>> index c3875bc4c6..46cb865180 100644
>> --- a/bsd-user/x86_64/signal.c
>> +++ b/bsd-user/x86_64/signal.c
>> @@ -16,6 +16,7 @@
>>   *  along with this program; if not, see <http://www.gnu.org/licenses/>.
>>   */
>>
>> +#include "qemu/osdep.h"
>>  #include "qemu.h"
>>
>>  /*
>> diff --git a/bsd-user/x86_64/target_arch_cpu.c
>> b/bsd-user/x86_64/target_arch_cpu.c
>> index be7bd10720..1d32f18907 100644
>> --- a/bsd-user/x86_64/target_arch_cpu.c
>> +++ b/bsd-user/x86_64/target_arch_cpu.c
>> @@ -17,9 +17,8 @@
>>   *  along with this program; if not, see <http://www.gnu.org/licenses/>.
>>   */
>>
>> -#include <sys/types.h>
>> -
>>  #include "qemu/osdep.h"
>> +
>>  #include "cpu.h"
>>  #include "qemu.h"
>>  #include "qemu/timer.h"
>>
>
> I suppose these are fine. How do I run the cleanup script? I have 2x the
> number of files not upstream in the bsd-user fork that I'd like to cleanup
> as well and they are likely a bigger mess and I'll just upstream them in
> the messy state unless I understand how to keep the neat :).

I used

    $ scripts/clean-includes --check-dup-head --all

Which resulted in a big mess I didn't dare to submit for review :)  So I
split it up.  Easiest way was to identify useful sets of files, add
files that include headers from the set, transitively, then run

    $ scripts/clean-includes FILES...

--check-dup-head reports possible duplicate inclusions.  It is prone to
false positives.  That's why it merely reports them.  You may want to
not use it for now.

There's a big usage comment at the beginning of the script.

Hope this helps!

Re: [PATCH v4 04/19] bsd-user: Clean up includes

[Warner Losh]

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

On Thu, Jan 19, 2023 at 9:42 AM Markus Armbruster <armbru@redhat.com> wrote:

> Warner Losh <imp@bsdimp.com> writes:
>
> > On Thu, Jan 19, 2023 at 12:00 AM Markus Armbruster <armbru@redhat.com>
> > wrote:
> >
> >> Clean up includes so that osdep.h is included first and headers
> >> which it implies are not included manually.
> >>
> >> This commit was created with scripts/clean-includes.
> >>
> >> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> >> ---
> >>  bsd-user/bsd-proc.h               | 4 ----
> >>  bsd-user/qemu.h                   | 1 -
> >>  bsd-user/arm/signal.c             | 1 +
> >>  bsd-user/arm/target_arch_cpu.c    | 2 ++
> >>  bsd-user/freebsd/os-sys.c         | 1 +
> >>  bsd-user/i386/signal.c            | 1 +
> >>  bsd-user/i386/target_arch_cpu.c   | 3 +--
> >>  bsd-user/main.c                   | 4 +---
> >>  bsd-user/strace.c                 | 1 -
> >>  bsd-user/x86_64/signal.c          | 1 +
> >>  bsd-user/x86_64/target_arch_cpu.c | 3 +--
> >>  11 files changed, 9 insertions(+), 13 deletions(-)
> >>
> >> diff --git a/bsd-user/bsd-proc.h b/bsd-user/bsd-proc.h
> >> index 68b66e571d..a1061bffb8 100644
> >> --- a/bsd-user/bsd-proc.h
> >> +++ b/bsd-user/bsd-proc.h
> >> @@ -20,11 +20,7 @@
> >>  #ifndef BSD_PROC_H_
> >>  #define BSD_PROC_H_
> >>
> >> -#include <sys/types.h>
> >> -#include <sys/stat.h>
> >> -#include <sys/time.h>
> >>  #include <sys/resource.h>
> >>
> >
> > Did you test this on FreeBSD 12? FreeBSD 13 has started to climb the hill
> > where all includes are independent, but much of that hasn't been merged
> to
> > 12 yet. sys/types.h, at least, is documented as a prereq for both
> > sys/stat.h and sys/resource.h.
> >
> > I know many of these are in os-dep.h, and I've had trouble in the
> bsd-user
> > fork with that and the layout of the code which has arguably way too much
> > in the .h files.
>
> No, I didn't test on FreeBSD 12.
>

OK. Fair enough. If it passes the CI tests, then it's likely fine (though
if I hit issues, I'll submit patches).


> Any .c must include qemu/osdep.h *first*.  Any further inclusions of
> headers qemu/osdep.h includes already are no-ops unless the headers in
> question lack multiple inclusion guards.
>

OK.


> > Also, why didn't you move sys/resource.h and other such files to
> os-dep.h?
> > I'm struggling to understand the rules around what is or isn't included
> > where?
>
> This series is "run scripts/clean-includes, split it into reviewable
> chunks, tidy up blank lines".  Moving more includes into qemu/osdep.h is
> out of scope.
>

OK. Fair point. I'm happy with that answer since it tells me I could move
things there too, if need be.


> I sympathize with your complaint that QEMU does too much in headers in
> general, and in qemu/osdep.h in particular.
>

Yea, I'm not entirely sure  it's a complaint, or if it's an observation of
difficulties relative to other code bases... I go back and forth on my
opinion of it...


> >> -#include <unistd.h>
> >>
> >>  /* exit(2) */
> >>  static inline abi_long do_bsd_exit(void *cpu_env, abi_long arg1)
> >> diff --git a/bsd-user/qemu.h b/bsd-user/qemu.h
> >> index be6105385e..0ceecfb6df 100644
> >> --- a/bsd-user/qemu.h
> >> +++ b/bsd-user/qemu.h
> >> @@ -17,7 +17,6 @@
> >>  #ifndef QEMU_H
> >>  #define QEMU_H
> >>
> >> -#include "qemu/osdep.h"
> >>  #include "cpu.h"
> >>  #include "qemu/units.h"
> >>  #include "exec/cpu_ldst.h"
> >> diff --git a/bsd-user/arm/signal.c b/bsd-user/arm/signal.c
> >> index 2b1dd745d1..9734407543 100644
> >> --- a/bsd-user/arm/signal.c
> >> +++ b/bsd-user/arm/signal.c
> >> @@ -17,6 +17,7 @@
> >>   *  along with this program; if not, see <http://www.gnu.org/licenses/
> >.
> >>   */
> >>
> >> +#include "qemu/osdep.h"
> >>  #include "qemu.h"
> >>
> >>  /*
> >> diff --git a/bsd-user/arm/target_arch_cpu.c
> >> b/bsd-user/arm/target_arch_cpu.c
> >> index 02bf9149d5..fe38ae2210 100644
> >> --- a/bsd-user/arm/target_arch_cpu.c
> >> +++ b/bsd-user/arm/target_arch_cpu.c
> >> @@ -16,6 +16,8 @@
> >>   *  You should have received a copy of the GNU General Public License
> >>   *  along with this program; if not, see <http://www.gnu.org/licenses/
> >.
> >>   */
> >> +
> >> +#include "qemu/osdep.h"
> >>  #include "target_arch.h"
> >>
> >>  void target_cpu_set_tls(CPUARMState *env, target_ulong newtls)
> >> diff --git a/bsd-user/freebsd/os-sys.c b/bsd-user/freebsd/os-sys.c
> >> index 309e27b9d6..1676ec10f8 100644
> >> --- a/bsd-user/freebsd/os-sys.c
> >> +++ b/bsd-user/freebsd/os-sys.c
> >> @@ -17,6 +17,7 @@
> >>   *  along with this program; if not, see <http://www.gnu.org/licenses/
> >.
> >>   */
> >>
> >> +#include "qemu/osdep.h"
> >>  #include "qemu.h"
> >>  #include "target_arch_sysarch.h"
> >>
> >> diff --git a/bsd-user/i386/signal.c b/bsd-user/i386/signal.c
> >> index 5dd975ce56..a3131047b8 100644
> >> --- a/bsd-user/i386/signal.c
> >> +++ b/bsd-user/i386/signal.c
> >> @@ -17,6 +17,7 @@
> >>   *  along with this program; if not, see <http://www.gnu.org/licenses/
> >.
> >>   */
> >>
> >> +#include "qemu/osdep.h"
> >>  #include "qemu.h"
> >>
> >>  /*
> >> diff --git a/bsd-user/i386/target_arch_cpu.c
> >> b/bsd-user/i386/target_arch_cpu.c
> >> index d349e45299..2a3af2ddef 100644
> >> --- a/bsd-user/i386/target_arch_cpu.c
> >> +++ b/bsd-user/i386/target_arch_cpu.c
> >> @@ -17,9 +17,8 @@
> >>   *  along with this program; if not, see <http://www.gnu.org/licenses/
> >.
> >>   */
> >>
> >> -#include <sys/types.h>
> >> -
> >>  #include "qemu/osdep.h"
> >> +
> >>  #include "cpu.h"
> >>  #include "qemu.h"
> >>  #include "qemu/timer.h"
> >> diff --git a/bsd-user/main.c b/bsd-user/main.c
> >> index 6f09180d65..41290e16f9 100644
> >> --- a/bsd-user/main.c
> >> +++ b/bsd-user/main.c
> >> @@ -18,12 +18,10 @@
> >>   *  along with this program; if not, see <http://www.gnu.org/licenses/
> >.
> >>   */
> >>
> >> -#include <sys/types.h>
> >> -#include <sys/time.h>
> >> +#include "qemu/osdep.h"
> >>  #include <sys/resource.h>
> >>  #include <sys/sysctl.h>
> >>
> >> -#include "qemu/osdep.h"
> >>  #include "qemu/help-texts.h"
> >>  #include "qemu/units.h"
> >>  #include "qemu/accel.h"
> >> diff --git a/bsd-user/strace.c b/bsd-user/strace.c
> >> index a77d10dd6b..96499751eb 100644
> >> --- a/bsd-user/strace.c
> >> +++ b/bsd-user/strace.c
> >> @@ -20,7 +20,6 @@
> >>  #include <sys/select.h>
> >>  #include <sys/syscall.h>
> >>  #include <sys/ioccom.h>
> >> -#include <ctype.h>
> >>
> >>  #include "qemu.h"
> >>
> >> diff --git a/bsd-user/x86_64/signal.c b/bsd-user/x86_64/signal.c
> >> index c3875bc4c6..46cb865180 100644
> >> --- a/bsd-user/x86_64/signal.c
> >> +++ b/bsd-user/x86_64/signal.c
> >> @@ -16,6 +16,7 @@
> >>   *  along with this program; if not, see <http://www.gnu.org/licenses/
> >.
> >>   */
> >>
> >> +#include "qemu/osdep.h"
> >>  #include "qemu.h"
> >>
> >>  /*
> >> diff --git a/bsd-user/x86_64/target_arch_cpu.c
> >> b/bsd-user/x86_64/target_arch_cpu.c
> >> index be7bd10720..1d32f18907 100644
> >> --- a/bsd-user/x86_64/target_arch_cpu.c
> >> +++ b/bsd-user/x86_64/target_arch_cpu.c
> >> @@ -17,9 +17,8 @@
> >>   *  along with this program; if not, see <http://www.gnu.org/licenses/
> >.
> >>   */
> >>
> >> -#include <sys/types.h>
> >> -
> >>  #include "qemu/osdep.h"
> >> +
> >>  #include "cpu.h"
> >>  #include "qemu.h"
> >>  #include "qemu/timer.h"
> >>
> >
> > I suppose these are fine. How do I run the cleanup script? I have 2x the
> > number of files not upstream in the bsd-user fork that I'd like to
> cleanup
> > as well and they are likely a bigger mess and I'll just upstream them in
> > the messy state unless I understand how to keep the neat :).
>
> I used
>
>     $ scripts/clean-includes --check-dup-head --all
>
> Which resulted in a big mess I didn't dare to submit for review :)  So I
> split it up.  Easiest way was to identify useful sets of files, add
> files that include headers from the set, transitively, then run
>
>     $ scripts/clean-includes FILES...
>
> --check-dup-head reports possible duplicate inclusions.  It is prone to
> false positives.  That's why it merely reports them.  You may want to
> not use it for now.
>
> There's a big usage comment at the beginning of the script.
>
> Hope this helps!
>

It does. After your changes land, I'll merge, and run this on the branch. I
have a few changes queued up, and have been contemplating changes to my
upstreaming workflow to speed the process along...

So I'm happy with it. Thanks for the cleanup and the time to answer my
questions.

Reviewed-by: Warner Losh <imp@bsdimp.com>

Warner

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

RE: [PATCH v4 12/19] target/hexagon: Clean up includes

[Taylor Simpson]

> -----Original Message-----
> From: Markus Armbruster <armbru@redhat.com>
> Sent: Thursday, January 19, 2023 1:00 AM
> To: qemu-devel@nongnu.org
> Cc: richard.henderson@linaro.org; pbonzini@redhat.com;
> kwolf@redhat.com; hreitz@redhat.com; imp@bsdimp.com;
> kevans@freebsd.org; berrange@redhat.com; groug@kaod.org;
> qemu_oss@crudebyte.com; mst@redhat.com; philmd@linaro.org;
> peter.maydell@linaro.org; alistair@alistair23.me; jasowang@redhat.com;
> jonathan.cameron@huawei.com; kbastian@mail.uni-paderborn.de;
> quintela@redhat.com; dgilbert@redhat.com; michael.roth@amd.com;
> kkostiuk@redhat.com; Taylor Simpson <tsimpson@quicinc.com>;
> palmer@dabbelt.com; bin.meng@windriver.com; qemu-block@nongnu.org;
> qemu-arm@nongnu.org; qemu-riscv@nongnu.org
> Subject: [PATCH v4 12/19] target/hexagon: Clean up includes
> 
> Clean up includes so that osdep.h is included first and headers which it
> implies are not included manually.
> 
> This commit was created with scripts/clean-includes.
> 
> Changes to standalone programs dropped, because I can't tell whether them
> not using qemu/osdep.h is intentional:
> 
>     target/hexagon/gen_dectree_import.c
>     target/hexagon/gen_semantics.c
>     target/hexagon/idef-parser/idef-parser.h
>     target/hexagon/idef-parser/parser-helpers.c
>     target/hexagon/idef-parser/parser-helpers.h

Correct.  These are standalone programs not built with the full QEMU context.

> 
> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> ---
>  target/hexagon/hex_arch_types.h | 1 -
>  target/hexagon/mmvec/macros.h   | 1 -
>  2 files changed, 2 deletions(-)

Reviewed-by: Taylor Simpson <tsimpson@quicinc.com>

Re: [PATCH v4 13/19] riscv: Clean up includes

[Alistair Francis]

On Thu, Jan 19, 2023 at 5:10 PM Markus Armbruster <armbru@redhat.com> wrote:
>
> Clean up includes so that osdep.h is included first and headers
> which it implies are not included manually.
>
> This commit was created with scripts/clean-includes.
>
> Signed-off-by: Markus Armbruster <armbru@redhat.com>

Reviewed-by: Alistair Francis <alistair.francis@wdc.com>

Alistair

> ---
>  target/riscv/pmu.h | 1 -
>  1 file changed, 1 deletion(-)
>
> diff --git a/target/riscv/pmu.h b/target/riscv/pmu.h
> index 3004ce37b6..0c819ca983 100644
> --- a/target/riscv/pmu.h
> +++ b/target/riscv/pmu.h
> @@ -16,7 +16,6 @@
>   * this program.  If not, see <http://www.gnu.org/licenses/>.
>   */
>
> -#include "qemu/osdep.h"
>  #include "qemu/log.h"
>  #include "cpu.h"
>  #include "qemu/main-loop.h"
> --
> 2.39.0
>
>

Re: [PATCH v4 10/19] migration: Clean up includes

[Markus Armbruster]

"Dr. David Alan Gilbert" <dgilbert@redhat.com> writes:

> * Markus Armbruster (armbru@redhat.com) wrote:
>> "Dr. David Alan Gilbert" <dgilbert@redhat.com> writes:
>> 
>> > * Markus Armbruster (armbru@redhat.com) wrote:
>> >> Clean up includes so that osdep.h is included first and headers
>> >> which it implies are not included manually.
>> >
>> > That change doesn't seem to match the message; the patch is removing the
>> > osdep.h include.
>> 
>> It's the commit message scripts/clean-includes creates :)
>> 
>> I can throw in another patch to the script so it mentions it also
>> removes qemu/osdep.h from headers.
>
> Oh hmm it would be clearer;

What about

    $GITSUBJ: Clean up includes

    Clean up includes so that osdep.h is included first in .c and not in
    .h, and headers which it implies are not included manually.

    This commit was created with scripts/clean-includes.


>                             but OK then, so 
>
> Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>

Thanks!

Re: [PATCH v4 12/19] target/hexagon: Clean up includes

[Markus Armbruster]

Taylor Simpson <tsimpson@quicinc.com> writes:

>> -----Original Message-----
>> From: Markus Armbruster <armbru@redhat.com>
>> Sent: Thursday, January 19, 2023 1:00 AM
>> To: qemu-devel@nongnu.org
>> Cc: richard.henderson@linaro.org; pbonzini@redhat.com;
>> kwolf@redhat.com; hreitz@redhat.com; imp@bsdimp.com;
>> kevans@freebsd.org; berrange@redhat.com; groug@kaod.org;
>> qemu_oss@crudebyte.com; mst@redhat.com; philmd@linaro.org;
>> peter.maydell@linaro.org; alistair@alistair23.me; jasowang@redhat.com;
>> jonathan.cameron@huawei.com; kbastian@mail.uni-paderborn.de;
>> quintela@redhat.com; dgilbert@redhat.com; michael.roth@amd.com;
>> kkostiuk@redhat.com; Taylor Simpson <tsimpson@quicinc.com>;
>> palmer@dabbelt.com; bin.meng@windriver.com; qemu-block@nongnu.org;
>> qemu-arm@nongnu.org; qemu-riscv@nongnu.org
>> Subject: [PATCH v4 12/19] target/hexagon: Clean up includes
>> 
>> Clean up includes so that osdep.h is included first and headers which it
>> implies are not included manually.
>> 
>> This commit was created with scripts/clean-includes.
>> 
>> Changes to standalone programs dropped, because I can't tell whether them
>> not using qemu/osdep.h is intentional:
>> 
>>     target/hexagon/gen_dectree_import.c
>>     target/hexagon/gen_semantics.c
>>     target/hexagon/idef-parser/idef-parser.h
>>     target/hexagon/idef-parser/parser-helpers.c
>>     target/hexagon/idef-parser/parser-helpers.h
>
> Correct.  These are standalone programs not built with the full QEMU context.

I'll tweak the commit message like this:

   Changes to standalone programs dropped, because these intentionally
   don't use qemu/osdep.h:

>> Signed-off-by: Markus Armbruster <armbru@redhat.com>
>> ---
>>  target/hexagon/hex_arch_types.h | 1 -
>>  target/hexagon/mmvec/macros.h   | 1 -
>>  2 files changed, 2 deletions(-)
>
> Reviewed-by: Taylor Simpson <tsimpson@quicinc.com>

Thanks!

Re: [PATCH v4 04/19] bsd-user: Clean up includes

[Markus Armbruster]

Warner Losh <imp@bsdimp.com> writes:

[...]

> So I'm happy with it. Thanks for the cleanup and the time to answer my
> questions.
>
> Reviewed-by: Warner Losh <imp@bsdimp.com>

Thank *you* for reviewing my patch :)

Re: [PATCH v4 16/19] Fix non-first inclusions of qemu/osdep.h

[Markus Armbruster]

Philippe Mathieu-Daudé <philmd@linaro.org> writes:

> On 19/1/23 12:41, Markus Armbruster wrote:
>> Philippe Mathieu-Daudé <philmd@linaro.org> writes:
>> 
>>> On 19/1/23 07:59, Markus Armbruster wrote:
>>>> This commit was created with scripts/clean-includes.
>>>> Signed-off-by: Markus Armbruster <armbru@redhat.com>
>> [...]
>> 
>>> Up to here:
>>>
>>> Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>>>
>>>> diff --git a/util/async-teardown.c b/util/async-teardown.c
>>>> index 62bfce1b3c..62cdeb0f20 100644
>>>> --- a/util/async-teardown.c
>>>> +++ b/util/async-teardown.c
>>>> @@ -10,16 +10,12 @@
>>>>     * option) any later version.  See the COPYING file in the top-level directory.
>>>>     *
>>>>     */
>>>> -#include <stdlib.h>
>>>> -#include <stdio.h>
>>>> -#include <sys/types.h>
>>>> -#include <dirent.h>
>>>> -#include <sys/prctl.h>
>>>> -#include <signal.h>
>>>> -#include <sched.h>
>>>> -#include <unistd.h>
>>>>     #include "qemu/osdep.h"
>>>> +#include <dirent.h>
>>>> +#include <sys/prctl.h>
>>>> +#include <sched.h>
>>>> +
>>>>    #include "qemu/async-teardown.h"
>>>
>>> This file has more changes.
>> I'm not sure I understand.
>> The patch does two related things:
>> 1. It puts qemu/osdep.h first.  The diff makes it look like we leave it
>>     in place and move other stuff across, but that's the same.
>> 2. It deletes inclusions of headers qemu/osdep.h already includes:
>>      <stdlib.h>
>>      <stdio.h>
>>      <sys/types.h>
>>      <signal.h>
>>      <unistd.h>
>
> Ah, the other files get this done in the "Drop duplicate #include" patch.

I assume this extends your R-by to the complete patch.  Correct me if
I'm wrong, please.

Re: [PATCH v4 04/19] bsd-user: Clean up includes

[Peter Maydell]

On Thu, 19 Jan 2023 at 14:42, Warner Losh <imp@bsdimp.com> wrote:
>
> Also, why didn't you move sys/resource.h and other such files
> to os-dep.h? I'm struggling to understand the rules around what
> is or isn't included where?

The rough rule of thumb is that if some OS needs a compatibility
fixup or workaround for a system header (eg not every mmap.h
defines MAP_ANONYMOUS; on Windows unistd.h has to come before
time.h) then we put that header include and the compat workaround
into osdep.h. This avoids "only fails on obscure platform" issues
where somebody puts a header include into some specific .c file
but not the compat workaround, and it works on the Linux host
that most people develop and test on and we only find the
problem later.

There's also no doubt some includes there for historical
reasons, and some which really are "everybody needs these"
convenience ones. But we should probably not add new
includes to osdep.h unless they fall into the "working around
system header issues" bucket.

thanks
-- PMM

Re: [PATCH v4 04/19] bsd-user: Clean up includes

[Michael S. Tsirkin]

On Fri, Jan 27, 2023 at 02:54:30PM +0000, Peter Maydell wrote:
> On Thu, 19 Jan 2023 at 14:42, Warner Losh <imp@bsdimp.com> wrote:
> >
> > Also, why didn't you move sys/resource.h and other such files
> > to os-dep.h? I'm struggling to understand the rules around what
> > is or isn't included where?
> 
> The rough rule of thumb is that if some OS needs a compatibility
> fixup or workaround for a system header (eg not every mmap.h
> defines MAP_ANONYMOUS; on Windows unistd.h has to come before
> time.h) then we put that header include and the compat workaround
> into osdep.h. This avoids "only fails on obscure platform" issues
> where somebody puts a header include into some specific .c file
> but not the compat workaround, and it works on the Linux host
> that most people develop and test on and we only find the
> problem later.
> 
> There's also no doubt some includes there for historical
> reasons, and some which really are "everybody needs these"
> convenience ones. But we should probably not add new
> includes to osdep.h unless they fall into the "working around
> system header issues" bucket.
> 
> thanks
> -- PMM


BTW maybe we should teach checkpatch about that rule:
if a header is in osdep do not include it directly.

-- 
MST

Re: [PATCH v4 01/19] scripts/clean-includes: Fully skip / ignore files

[Eric Blake]

On Thu, Jan 19, 2023 at 07:59:41AM +0100, Markus Armbruster wrote:
...
> 
> Fix the script to fully skip files.
> 
> Fixes: fd3e39a40ca2ee26b09a5de3149af8b056b85233
> Fixes: d66253e46ae2b9c36a9dd90b2b74c0dfa5804b22
> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> ---
>  scripts/clean-includes | 8 +++++---
>  1 file changed, 5 insertions(+), 3 deletions(-)
> 
> diff --git a/scripts/clean-includes b/scripts/clean-includes
> index d37bd4f692..86944f27fc 100755
> --- a/scripts/clean-includes
> +++ b/scripts/clean-includes
> @@ -111,6 +111,7 @@ cat >"$COCCIFILE" <<EOT
>  )
>  EOT
>  
> +files=
>  for f in "$@"; do
>    case "$f" in
>      *.c.inc)
> @@ -144,6 +145,7 @@ for f in "$@"; do
>        continue
>        ;;
>    esac
> +  files="$files $f"

Bash's += might perform faster here, but this is a #!/bin/sh script.

Reviewed-by: Eric Blake <eblake@redhat.com>

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org

Re: [PATCH v4 02/19] scripts/clean-includes: Don't claim duplicate headers found when not

[Eric Blake]

On Thu, Jan 19, 2023 at 07:59:42AM +0100, Markus Armbruster wrote:
> When running with --check-dup-head, the script always claims it "Found
> duplicate header file includes."  Fix to do it only when it actually
> found some.
> 
> Fixes: d66253e46ae2b9c36a9dd90b2b74c0dfa5804b22
> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> ---
>  scripts/clean-includes | 5 ++---
>  1 file changed, 2 insertions(+), 3 deletions(-)
> 
> diff --git a/scripts/clean-includes b/scripts/clean-includes
> index 86944f27fc..8e8420d785 100755
> --- a/scripts/clean-includes
> +++ b/scripts/clean-includes
> @@ -177,9 +177,8 @@ for f in "$@"; do
>  done
>  
>  if [ "$DUPHEAD" = "yes" ] && [ -n "$files" ]; then
> -    egrep "^[[:space:]]*#[[:space:]]*include" $files | tr -d '[:blank:]' \
> -        | sort | uniq -c | awk '{if ($1 > 1) print $0}'
> -    if [ $? -eq 0 ]; then
> +    if egrep "^[[:space:]]*#[[:space:]]*include" $files | tr -d '[:blank:]' \
> +        | sort | uniq -c | grep -v '^ *1 '; then

Indeed - the awk script was succeeding even if it printed nothing,
whereas the grep -v is a bit lighter-weight and has the intended
semantics of exit status based on output.

Reviewed-by: Eric Blake <eblake@redhat.com>

>          echo "Found duplicate header file includes. Please check the above files manually."
>          exit 1
>      fi
> -- 
> 2.39.0
> 
> 

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org

Re: [PATCH v4 03/19] scripts/clean-includes: Skip symbolic links

[Eric Blake]

On Thu, Jan 19, 2023 at 07:59:43AM +0100, Markus Armbruster wrote:
> When a symbolic link points to a file that needs cleaning, the script
> replaces the link with a cleaned regular file.  Not wanted; skip them.
> 
> We have a few symbolic links under subprojects/libvduse/ and
> subprojects/libvhost-user/.
> 
> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> ---
>  scripts/clean-includes | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/scripts/clean-includes b/scripts/clean-includes
> index 8e8420d785..f0466a6262 100755
> --- a/scripts/clean-includes
> +++ b/scripts/clean-includes
> @@ -113,6 +113,10 @@ EOT
>  
>  files=
>  for f in "$@"; do
> +  if [ -L "$f" ]; then

I don't see -L used with test very often, but POSIX requires it, so it
is safe for our choice of /bin/sh.

Reviewed-by: Eric Blake <eblake@redhat.com>

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org

Re: [PATCH v4 14/19] block: Clean up includes

[Eric Blake]

On Thu, Jan 19, 2023 at 07:59:54AM +0100, Markus Armbruster wrote:
> Clean up includes so that osdep.h is included first and headers
> which it implies are not included manually.
> 
> This commit was created with scripts/clean-includes.
> 
> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> ---
>  include/block/graph-lock.h      | 1 -
>  include/block/write-threshold.h | 2 --
>  block/qapi.c                    | 1 -
>  3 files changed, 4 deletions(-)

Reviewed-by: Eric Blake <eblake@redhat.com>

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org

Re: [PATCH v4 03/19] scripts/clean-includes: Skip symbolic links

[Warner Losh]

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

On Fri, Jan 27, 2023 at 3:47 PM Eric Blake <eblake@redhat.com> wrote:

> On Thu, Jan 19, 2023 at 07:59:43AM +0100, Markus Armbruster wrote:
> > When a symbolic link points to a file that needs cleaning, the script
> > replaces the link with a cleaned regular file.  Not wanted; skip them.
> >
> > We have a few symbolic links under subprojects/libvduse/ and
> > subprojects/libvhost-user/.
> >
> > Signed-off-by: Markus Armbruster <armbru@redhat.com>
> > ---
> >  scripts/clean-includes | 4 ++++
> >  1 file changed, 4 insertions(+)
> >
> > diff --git a/scripts/clean-includes b/scripts/clean-includes
> > index 8e8420d785..f0466a6262 100755
> > --- a/scripts/clean-includes
> > +++ b/scripts/clean-includes
> > @@ -113,6 +113,10 @@ EOT
> >
> >  files=
> >  for f in "$@"; do
> > +  if [ -L "$f" ]; then
>
> I don't see -L used with test very often, but POSIX requires it, so it
> is safe for our choice of /bin/sh.
>

FYI: -L is in FreeBSD, NetBSD, OpenBSD, etc. It's been in all these trees
since the mid 90s. It wasn't in 4.4BSD, but all these projects have
imported the code from pdksh's test.

So in addition to POSIX, it's been widely implemented, at least in the BSD
world, for over 20 years.

Warner

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

Re: [PATCH v4 01/19] scripts/clean-includes: Fully skip / ignore files

[Michael S. Tsirkin]

On Thu, Jan 19, 2023 at 07:59:41AM +0100, Markus Armbruster wrote:
> When clean-includes claims to skip or ignore a file, only the part
> that sanitizes use of qemu/osdep.h skips the file.  The part that
> looks for duplicate #include does not, and neither does committing to
> Git.
> 
> The latter can get unrelated stuff included in the commit, but only if
> you run clean-includes in a dirty tree, which is unwise.  Messed up
> when we added skipping in commit fd3e39a40c "scripts/clean-includes:
> Enhance to handle header files".
> 
> The former can cause bogus reports for --check-dup-head.  Added in
> commit d66253e46a "scripts/clean-includes: added duplicate #include
> check", duplicating the prior mistake.
> 
> Fix the script to fully skip files.
> 
> Fixes: fd3e39a40ca2ee26b09a5de3149af8b056b85233
> Fixes: d66253e46ae2b9c36a9dd90b2b74c0dfa5804b22

Isn't
Fixes: %h (\"%s\")

the accepted format for this?

> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> ---
>  scripts/clean-includes | 8 +++++---
>  1 file changed, 5 insertions(+), 3 deletions(-)
> 
> diff --git a/scripts/clean-includes b/scripts/clean-includes
> index d37bd4f692..86944f27fc 100755
> --- a/scripts/clean-includes
> +++ b/scripts/clean-includes
> @@ -111,6 +111,7 @@ cat >"$COCCIFILE" <<EOT
>  )
>  EOT
>  
> +files=
>  for f in "$@"; do
>    case "$f" in
>      *.c.inc)
> @@ -144,6 +145,7 @@ for f in "$@"; do
>        continue
>        ;;
>    esac
> +  files="$files $f"
>  
>    if [ "$MODE" = "c" ]; then
>      # First, use Coccinelle to add qemu/osdep.h before the first existing include
> @@ -174,8 +176,8 @@ for f in "$@"; do
>  
>  done
>  
> -if [ "$DUPHEAD" = "yes" ]; then
> -    egrep "^[[:space:]]*#[[:space:]]*include" "$@" | tr -d '[:blank:]' \
> +if [ "$DUPHEAD" = "yes" ] && [ -n "$files" ]; then
> +    egrep "^[[:space:]]*#[[:space:]]*include" $files | tr -d '[:blank:]' \
>          | sort | uniq -c | awk '{if ($1 > 1) print $0}'
>      if [ $? -eq 0 ]; then
>          echo "Found duplicate header file includes. Please check the above files manually."
> @@ -184,7 +186,7 @@ if [ "$DUPHEAD" = "yes" ]; then
>  fi
>  
>  if [ "$GIT" = "yes" ]; then
> -    git add -- "$@"
> +    git add -- $files
>      git commit --signoff -F - <<EOF
>  $GITSUBJ: Clean up includes
>  
> -- 
> 2.39.0

Re: [PATCH v4 04/19] bsd-user: Clean up includes

[Michael S. Tsirkin]

On Fri, Jan 27, 2023 at 10:01:57AM -0500, Michael S. Tsirkin wrote:
> On Fri, Jan 27, 2023 at 02:54:30PM +0000, Peter Maydell wrote:
> > On Thu, 19 Jan 2023 at 14:42, Warner Losh <imp@bsdimp.com> wrote:
> > >
> > > Also, why didn't you move sys/resource.h and other such files
> > > to os-dep.h? I'm struggling to understand the rules around what
> > > is or isn't included where?
> > 
> > The rough rule of thumb is that if some OS needs a compatibility
> > fixup or workaround for a system header (eg not every mmap.h
> > defines MAP_ANONYMOUS; on Windows unistd.h has to come before
> > time.h) then we put that header include and the compat workaround
> > into osdep.h. This avoids "only fails on obscure platform" issues
> > where somebody puts a header include into some specific .c file
> > but not the compat workaround, and it works on the Linux host
> > that most people develop and test on and we only find the
> > problem later.
> > 
> > There's also no doubt some includes there for historical
> > reasons, and some which really are "everybody needs these"
> > convenience ones. But we should probably not add new
> > includes to osdep.h unless they fall into the "working around
> > system header issues" bucket.
> > 
> > thanks
> > -- PMM
> 
> 
> BTW maybe we should teach checkpatch about that rule:
> if a header is in osdep do not include it directly.

To be more precise, make checkpatch run clean-includes somehow?
Or just make CI run clean-includes on the tree and verify result
is empty?

> -- 
> MST

Re: [PATCH v4 10/19] migration: Clean up includes

[Juan Quintela]

Markus Armbruster <armbru@redhat.com> wrote:
> "Dr. David Alan Gilbert" <dgilbert@redhat.com> writes:
>
>> * Markus Armbruster (armbru@redhat.com) wrote:
>>> "Dr. David Alan Gilbert" <dgilbert@redhat.com> writes:
>>> 
>>> > * Markus Armbruster (armbru@redhat.com) wrote:
>>> >> Clean up includes so that osdep.h is included first and headers
>>> >> which it implies are not included manually.
>>> >
>>> > That change doesn't seem to match the message; the patch is removing the
>>> > osdep.h include.
>>> 
>>> It's the commit message scripts/clean-includes creates :)
>>> 
>>> I can throw in another patch to the script so it mentions it also
>>> removes qemu/osdep.h from headers.
>>
>> Oh hmm it would be clearer;
>
> What about
>
>     $GITSUBJ: Clean up includes
>
>     Clean up includes so that osdep.h is included first in .c and not in
>     .h, and headers which it implies are not included manually.
>
>     This commit was created with scripts/clean-includes.
>
>
>>                             but OK then, so 
>>
>> Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>

Sounds ok to me.

Reviewed-by: Juan Quintela <quintela@redhat.com>

Re: [PATCH v4 16/19] Fix non-first inclusions of qemu/osdep.h

[Juan Quintela]

Markus Armbruster <armbru@redhat.com> wrote:
> This commit was created with scripts/clean-includes.
>
> Signed-off-by: Markus Armbruster <armbru@redhat.com>

Reviewed-by: Juan Quintela <quintela@redhat.com>

Re: [PATCH v4 19/19] Drop duplicate #include

[Juan Quintela]

Markus Armbruster <armbru@redhat.com> wrote:
> Tracked down with the help of scripts/clean-includes.
>
> Signed-off-by: Markus Armbruster <armbru@redhat.com>


> diff --git a/migration/postcopy-ram.c b/migration/postcopy-ram.c
> index b9a37ef255..8b7d1af75d 100644
> --- a/migration/postcopy-ram.c
> +++ b/migration/postcopy-ram.c
> @@ -17,7 +17,6 @@
>   */
>  
>  #include "qemu/osdep.h"
> -#include "qemu/rcu.h"
>  #include "qemu/madvise.h"
>  #include "exec/target_page.h"
>  #include "migration.h"
> @@ -34,7 +33,6 @@
>  #include "hw/boards.h"
>  #include "exec/ramblock.h"
>  #include "socket.h"
> -#include "qemu-file.h"
>  #include "yank_functions.h"
>  #include "tls.h"

Ouch

Reviewed-by: Juan Quintela <quintela@redhat.com>

I don't want to look into the logs.  They *could* say that it was me the
one doing the mess.

Re: [PATCH v4 01/19] scripts/clean-includes: Fully skip / ignore files

[Markus Armbruster]

"Michael S. Tsirkin" <mst@redhat.com> writes:

> On Thu, Jan 19, 2023 at 07:59:41AM +0100, Markus Armbruster wrote:
>> When clean-includes claims to skip or ignore a file, only the part
>> that sanitizes use of qemu/osdep.h skips the file.  The part that
>> looks for duplicate #include does not, and neither does committing to
>> Git.
>> 
>> The latter can get unrelated stuff included in the commit, but only if
>> you run clean-includes in a dirty tree, which is unwise.  Messed up
>> when we added skipping in commit fd3e39a40c "scripts/clean-includes:
>> Enhance to handle header files".
>> 
>> The former can cause bogus reports for --check-dup-head.  Added in
>> commit d66253e46a "scripts/clean-includes: added duplicate #include
>> check", duplicating the prior mistake.
>> 
>> Fix the script to fully skip files.
>> 
>> Fixes: fd3e39a40ca2ee26b09a5de3149af8b056b85233
>> Fixes: d66253e46ae2b9c36a9dd90b2b74c0dfa5804b22
>
> Isn't
> Fixes: %h (\"%s\")
>
> the accepted format for this?

It seems to be common these days.  I'll adjust.

Re: [PATCH v4 04/19] bsd-user: Clean up includes

[Markus Armbruster]

"Michael S. Tsirkin" <mst@redhat.com> writes:

> On Fri, Jan 27, 2023 at 10:01:57AM -0500, Michael S. Tsirkin wrote:
>> On Fri, Jan 27, 2023 at 02:54:30PM +0000, Peter Maydell wrote:
>> > On Thu, 19 Jan 2023 at 14:42, Warner Losh <imp@bsdimp.com> wrote:
>> > >
>> > > Also, why didn't you move sys/resource.h and other such files
>> > > to os-dep.h? I'm struggling to understand the rules around what
>> > > is or isn't included where?
>> > 
>> > The rough rule of thumb is that if some OS needs a compatibility
>> > fixup or workaround for a system header (eg not every mmap.h
>> > defines MAP_ANONYMOUS; on Windows unistd.h has to come before
>> > time.h) then we put that header include and the compat workaround
>> > into osdep.h. This avoids "only fails on obscure platform" issues
>> > where somebody puts a header include into some specific .c file
>> > but not the compat workaround, and it works on the Linux host
>> > that most people develop and test on and we only find the
>> > problem later.
>> > 
>> > There's also no doubt some includes there for historical
>> > reasons, and some which really are "everybody needs these"
>> > convenience ones. But we should probably not add new
>> > includes to osdep.h unless they fall into the "working around
>> > system header issues" bucket.
>> > 
>> > thanks
>> > -- PMM
>> 
>> 
>> BTW maybe we should teach checkpatch about that rule:
>> if a header is in osdep do not include it directly.
>
> To be more precise, make checkpatch run clean-includes somehow?
> Or just make CI run clean-includes on the tree and verify result
> is empty?

scripts/clean-includes isn't quite happy even after my series.
Offenders:

    ebpf/rss.bpf.skeleton.h
    subprojects/libvduse/libvduse.h
    subprojects/libvhost-user/libvhost-user-glib.h
    subprojects/libvhost-user/libvhost-user.h
    target/hexagon/idef-parser/idef-parser.h
    target/hexagon/idef-parser/parser-helpers.h
    tests/fp/platform.h
    contrib/plugins/cache.c
    contrib/plugins/drcov.c
    contrib/plugins/execlog.c
    contrib/plugins/hotblocks.c
    contrib/plugins/hotpages.c
    contrib/plugins/howvec.c
    contrib/plugins/hwprofile.c
    contrib/plugins/lockstep.c
    linux-user/mips64/cpu_loop.c
    linux-user/mips64/signal.c
    linux-user/x86_64/cpu_loop.c
    linux-user/x86_64/signal.c
    plugins/core.c
    plugins/loader.c
    scripts/xen-detect.c
    subprojects/libvduse/libvduse.c
    subprojects/libvhost-user/libvhost-user-glib.c
    subprojects/libvhost-user/libvhost-user.c
    subprojects/libvhost-user/link-test.c
    target/hexagon/gen_dectree_import.c
    target/hexagon/gen_semantics.c
    target/hexagon/idef-parser/parser-helpers.c
    target/s390x/gen-features.c
    tests/migration/s390x/a-b-bios.c
    tests/plugin/bb.c
    tests/plugin/empty.c
    tests/plugin/insn.c
    tests/plugin/mem.c
    tests/plugin/syscall.c
    tests/uefi-test-tools/UefiTestToolsPkg/BiosTablesTest/BiosTablesTest.c
    tests/unit/test-rcu-simpleq.c
    tests/unit/test-rcu-slist.c
    tests/unit/test-rcu-tailq.c
    tools/ebpf/rss.bpf.c

To support automatic checking, we'd have to fix the ones that need need
fixing, and add the remainder to the script's XDIRREGEX.