[PATCH 0/9] clean-includes: improve exclude list, run on cxl, vfio, tests

[PATCH 0/9] clean-includes: improve exclude list, run on cxl, vfio, tests

[Peter Maydell]

I realised that we haven't run clean-includes recently, and
unsurprisingly various violations of our include policy have
crept in to the tree. The exclude-list of files it shouldn't
be run on has also grown rather out of date.

While looking at this I realised that one reason the exclude list
is stale is that it's encoded in the script in a really awkward
single long line extended regex. So the main thing this patch
series does is fix that to instead use a list of regexes, one
per line, with comments permitted.

The other useful new feature here is that you can now point
the script at a directory (previously your only options were
an explicit list of files, or '--all' to scan everything).

The other changes to the script itself are minor cleanups.

Finally, I have a couple of patches which are the result of
running the script on some subdirectories. I do think that all
the changes that the script now suggests are correct (it wants
to make changes to 28 files other than these) but I wanted to
get the script changes through review first, and then perhaps
send those last changes a bit more broken up per-subsystem.

thanks
-- PMM

Peter Maydell (9):
  scripts/clean-includes: Allow directories on command line
  scripts/clean-includes: Remove outdated comment
  scripts/clean-includes: Make ignore-regexes one per line
  scripts/clean-includes: Do all our exclusions with REGEXFILE
  scripts/clean-includes: Give the args in git commit messages
  scripts/clean-includes: Update exclude list
  cxl: Clean up includes
  vfio: Clean up includes
  tests: Clean up includes

 hw/vfio-user/container.h             |  1 -
 hw/vfio-user/device.h                |  1 -
 hw/vfio/pci-quirks.h                 |  1 -
 tests/qtest/aspeed-hace-utils.h      |  1 -
 tests/qtest/aspeed-smc-utils.h       |  1 -
 hw/cxl/cxl-mailbox-utils.c           |  2 +-
 hw/mem/cxl_type3.c                   |  2 +-
 hw/vfio-user/container.c             |  2 +-
 hw/vfio-user/pci.c                   |  2 +-
 hw/vfio/ap.c                         |  1 -
 hw/vfio/container.c                  |  2 +-
 hw/vfio/cpr-legacy.c                 |  2 +-
 tests/qtest/aspeed_gpio-test.c       |  1 -
 tests/qtest/dbus-display-test.c      |  3 -
 tests/qtest/pnv-spi-seeprom-test.c   |  1 -
 tests/unit/test-cutils.c             |  2 +-
 tests/unit/test-error-report.c       |  1 -
 tests/unit/test-io-channel-command.c |  2 -
 scripts/clean-includes               | 88 ++++++++++++++++++----------
 19 files changed, 63 insertions(+), 53 deletions(-)

-- 
2.43.0

[PATCH 1/9] scripts/clean-includes: Allow directories on command line

[Peter Maydell]

Currently clean-includes supports two ways of specifying files to check:
 * --all to run on everything
 * specific files
There's no way to say "check everything in target/arm".

Add support for handling directory names, by always running
the arguments through git ls-files.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 scripts/clean-includes | 12 ++++++++----
 1 file changed, 8 insertions(+), 4 deletions(-)

diff --git a/scripts/clean-includes b/scripts/clean-includes
index 25dbf16c021..07c0fd44e46 100755
--- a/scripts/clean-includes
+++ b/scripts/clean-includes
@@ -14,7 +14,7 @@
 # the top-level directory.
 
 # Usage:
-#   clean-includes [--git subjectprefix] [--check-dup-head] file ...
+#   clean-includes [--git subjectprefix] [--check-dup-head] file-or-dir ...
 # or
 #   clean-includes [--git subjectprefix] [--check-dup-head] --all
 #
@@ -28,7 +28,8 @@
 #
 # Using --all will cause clean-includes to run on the whole source
 # tree (excluding certain directories which are known not to need
-# handling).
+# handling). This is equivalent to passing '.' as the directory to
+# scan.
 
 # This script requires Coccinelle to be installed.
 
@@ -86,11 +87,14 @@ if [ $# -eq 0 ]; then
     exit 1
 fi
 
+# --all means "scan everything starting from the current directory"
 if [ "$1" = "--all" ]; then
-    # We assume there are no files in the tree with spaces in their name
-    set -- $(git ls-files '*.[ch]' | grep -E -v "$XDIRREGEX")
+    set -- '.'
 fi
 
+# We assume there are no files in the tree with spaces in their name
+set -- $(git ls-files "$@" | grep '\.[ch]$' | grep -E -v "$XDIRREGEX")
+
 # Annoyingly coccinelle won't read a scriptfile unless its
 # name ends '.cocci', so write it out to a tempfile with the
 # right kind of name.
-- 
2.43.0

[PATCH 2/9] scripts/clean-includes: Remove outdated comment

[Peter Maydell]

Remove an old comment suggesting a manual shell line to use to find
files to run the script on. The script's exclude-list and its
support for directory names make this irrelevant.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 scripts/clean-includes | 9 ---------
 1 file changed, 9 deletions(-)

diff --git a/scripts/clean-includes b/scripts/clean-includes
index 07c0fd44e46..568033ee9c1 100755
--- a/scripts/clean-includes
+++ b/scripts/clean-includes
@@ -39,15 +39,6 @@
 # removed.
 # Other files (including C++ and ObjectiveC) can't be handled by this script.
 
-# The following one-liner may be handy for finding files to run this on.
-# However some caution is required regarding files that might be part
-# of the guest agent or standalone tests.
-
-# for i in $(git ls-tree --name-only HEAD) ; do test -f $i && \
-#   grep -E '^# *include' $i | head -1 | grep 'osdep.h' ; test $? != 0 && \
-#   echo $i ; done
-
-
 GIT=no
 DUPHEAD=no
 
-- 
2.43.0

[PATCH 3/9] scripts/clean-includes: Make ignore-regexes one per line

[Peter Maydell]

Currently we have a single extended regular expression defining
files that clean-includes should ignore. This is now very long
and awkward to read and edit.

Switch to having a list of newline-separated EREs that we write
to a file for grep's -f option, so we can express them more
legibly in the shell script. We allow for comments in the
regex list, which lets us document why we have put the
exclusions in.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 scripts/clean-includes | 28 ++++++++++++++++++++++------
 1 file changed, 22 insertions(+), 6 deletions(-)

diff --git a/scripts/clean-includes b/scripts/clean-includes
index 568033ee9c1..5ab3b071967 100755
--- a/scripts/clean-includes
+++ b/scripts/clean-includes
@@ -42,8 +42,6 @@
 GIT=no
 DUPHEAD=no
 
-# Extended regular expression defining files to ignore when using --all
-XDIRREGEX='^(tests/tcg|tests/multiboot|tests/fp|tests/plugin|tests/uefi-test-tools|pc-bios|subprojects|contrib/plugins|tools/ebpf|ebpf/rss.bpf.skeleton.h|linux-user/(mips64|x86_64)/(cpu_loop|signal).c)'
 
 while true
 do
@@ -83,15 +81,33 @@ if [ "$1" = "--all" ]; then
     set -- '.'
 fi
 
-# We assume there are no files in the tree with spaces in their name
-set -- $(git ls-files "$@" | grep '\.[ch]$' | grep -E -v "$XDIRREGEX")
-
 # Annoyingly coccinelle won't read a scriptfile unless its
 # name ends '.cocci', so write it out to a tempfile with the
 # right kind of name.
 COCCIFILE="$(mktemp --suffix=.cocci)"
+REGEXFILE="$(mktemp --suffix=.regex)"
 
-trap 'rm -f -- "$COCCIFILE"' INT TERM HUP EXIT
+trap 'rm -f -- "$COCCIFILE" "$REGEXFILE"' INT TERM HUP EXIT
+
+# List of extended regular expressions defining files to ignore
+# Comments starting with '#' are permitted
+grep -v '^#' >"$REGEXFILE" <<EOT
+# These tests are generally standalone binaries
+^tests/(tcg|multiboot|fp|plugin|uefi-test-tools)
+# BIOS sources and third-party subprojects don't follow our rules
+^pc-bios
+^subprojects
+# plugin binaries are standalone
+^contrib/plugins
+# the ebpf tool is standalone, and the skeleton header is autogenerated
+^tools/ebpf
+^ebpf/rss.bpf.skeleton.h
+# These files just include some other .c file and have no content themselves
+^linux-user/(mips64|x86_64)/(cpu_loop|signal).c
+EOT
+
+# We assume there are no files in the tree with spaces in their name
+set -- $(git ls-files "$@" | grep '\.[ch]$' | grep -E -v -f "$REGEXFILE")
 
 cat >"$COCCIFILE" <<EOT
 @@
-- 
2.43.0

[PATCH 4/9] scripts/clean-includes: Do all our exclusions with REGEXFILE

[Peter Maydell]

We currently have two mechanisms for excluding files:
 * the REGEXFILE which excludes by regex
 * special cases in the "loop over each file" which make
   us skip the file

Roll all the "skip this" cases into REGEXFILE, so we use
a single mechanism for identifying which files to exclude.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 scripts/clean-includes | 24 +++++++++---------------
 1 file changed, 9 insertions(+), 15 deletions(-)

diff --git a/scripts/clean-includes b/scripts/clean-includes
index 5ab3b071967..a45421d2ff7 100755
--- a/scripts/clean-includes
+++ b/scripts/clean-includes
@@ -104,6 +104,15 @@ grep -v '^#' >"$REGEXFILE" <<EOT
 ^ebpf/rss.bpf.skeleton.h
 # These files just include some other .c file and have no content themselves
 ^linux-user/(mips64|x86_64)/(cpu_loop|signal).c
+# These are autogenerated headers
+^include/standard-headers/
+# osdep.h itself and its friends are expected to include system headers
+^include/qemu/osdep.h
+^include/qemu/compiler.h
+^include/glib-compat.h
+^include/system/os-(posix|win32).h
+# This is for use by plugins, which are standalone binaries
+^include/qemu/qemu-plugin.h
 EOT
 
 # We assume there are no files in the tree with spaces in their name
@@ -137,21 +146,6 @@ for f in "$@"; do
     *.c)
       MODE=c
       ;;
-    *include/qemu/osdep.h | \
-    *include/qemu/compiler.h | \
-    *include/qemu/qemu-plugin.h | \
-    *include/glib-compat.h | \
-    *include/system/os-posix.h | \
-    *include/system/os-win32.h | \
-    *include/standard-headers/ )
-      # Removing include lines from osdep.h itself would be counterproductive.
-      echo "SKIPPING $f (special case header)"
-      continue
-      ;;
-    *include/standard-headers/*)
-      echo "SKIPPING $f (autogenerated header)"
-      continue
-      ;;
     *.h)
       MODE=h
       ;;
-- 
2.43.0

[PATCH 5/9] scripts/clean-includes: Give the args in git commit messages

[Peter Maydell]

If clean-includes is creating a git commit for its changes,
currently it says only "created with scripts/clean-includes".
Add the command line arguments the user passed us, as useful
extra information.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 scripts/clean-includes | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/scripts/clean-includes b/scripts/clean-includes
index a45421d2ff7..694e12f062c 100755
--- a/scripts/clean-includes
+++ b/scripts/clean-includes
@@ -42,6 +42,9 @@
 GIT=no
 DUPHEAD=no
 
+# Save the original arguments in case we want to put them in
+# a git commit message
+ORIGARGS="$*"
 
 while true
 do
@@ -198,7 +201,8 @@ if [ "$GIT" = "yes" ]; then
     git commit --signoff -F - <<EOF
 $GITSUBJ: Clean up includes
 
-This commit was created with scripts/clean-includes.
+This commit was created with scripts/clean-includes:
+ ./scripts/clean-includes $ORIGARGS
 
 All .c should include qemu/osdep.h first.  The script performs three
 related cleanups:
-- 
2.43.0

[PATCH 6/9] scripts/clean-includes: Update exclude list

[Peter Maydell]

Remove from the exclude list:
 * tests/plugin, which is a non-existent directory. This was
   probably intended to exclude tests/tcg/plugins/, which is caught
   by the tests/tcg exclude pattern anyway
Add to the exclude list:
 * rust/ -- the headers in here are purely for input to bindgen
 * target/hexagon has some standalone tools used at build time
 * linux-user/gen-vsdo.c -- another standalone tool
 * linux-user/mips64/elfload.c just includes mips/elfload.c
 * scripts/xen-detect.c is feature-detection code used by meson.build
 * tests/tracetool/simple.c is autogenerated
 * tests/unit/ has some "C file just includes another one" files
 * include/system/os-wasm.h is like os-posix.h and os-win32.h and
   shouldn't be adjusted

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 scripts/clean-includes | 19 +++++++++++++++++--
 1 file changed, 17 insertions(+), 2 deletions(-)

diff --git a/scripts/clean-includes b/scripts/clean-includes
index 694e12f062c..256ff5b5faa 100755
--- a/scripts/clean-includes
+++ b/scripts/clean-includes
@@ -96,10 +96,12 @@ trap 'rm -f -- "$COCCIFILE" "$REGEXFILE"' INT TERM HUP EXIT
 # Comments starting with '#' are permitted
 grep -v '^#' >"$REGEXFILE" <<EOT
 # These tests are generally standalone binaries
-^tests/(tcg|multiboot|fp|plugin|uefi-test-tools)
+^tests/(tcg|multiboot|fp|uefi-test-tools|qtest/migration/s390x)
 # BIOS sources and third-party subprojects don't follow our rules
 ^pc-bios
 ^subprojects
+# headers under rust are only used for input to bindgen
+^rust
 # plugin binaries are standalone
 ^contrib/plugins
 # the ebpf tool is standalone, and the skeleton header is autogenerated
@@ -107,15 +109,28 @@ grep -v '^#' >"$REGEXFILE" <<EOT
 ^ebpf/rss.bpf.skeleton.h
 # These files just include some other .c file and have no content themselves
 ^linux-user/(mips64|x86_64)/(cpu_loop|signal).c
+^linux-user/mips64/elfload.c
 # These are autogenerated headers
 ^include/standard-headers/
 # osdep.h itself and its friends are expected to include system headers
 ^include/qemu/osdep.h
 ^include/qemu/compiler.h
 ^include/glib-compat.h
-^include/system/os-(posix|win32).h
+^include/system/os-(posix|win32|wasm).h
 # This is for use by plugins, which are standalone binaries
 ^include/qemu/qemu-plugin.h
+# standalone tools used in building the hexagon target code
+^target/hexagon/(idef-parser|gen_semantics.c|gen_dectree_import.c)
+# standalone tool
+^target/s390x/gen-features.c
+# gen-vdso is a standalone tool
+^linux-user/gen-vdso.c
+# feature-detection code used by meson.bulid
+^scripts/xen-detect.c
+# autogenerated by tracetool
+^tests/tracetool/simple.c
+# these just include another C file
+^tests/unit/test-rcu-(simpleq|slist|tailq).c
 EOT
 
 # We assume there are no files in the tree with spaces in their name
-- 
2.43.0

[PATCH 7/9] cxl: Clean up includes

[Peter Maydell]

This commit was created with scripts/clean-includes:
 ./scripts/clean-includes --git cxl hw/cxl hw/mem

All .c should include qemu/osdep.h first.  The script performs three
related cleanups:

* Ensure .c files include qemu/osdep.h first.
* Including it in a .h is redundant, since the .c  already includes
  it.  Drop such inclusions.
* Likewise, including headers qemu/osdep.h includes is redundant.
  Drop these, too.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 hw/cxl/cxl-mailbox-utils.c | 2 +-
 hw/mem/cxl_type3.c         | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/hw/cxl/cxl-mailbox-utils.c b/hw/cxl/cxl-mailbox-utils.c
index 68c7cc98912..6cfdd98168f 100644
--- a/hw/cxl/cxl-mailbox-utils.c
+++ b/hw/cxl/cxl-mailbox-utils.c
@@ -7,9 +7,9 @@
  * COPYING file in the top-level directory.
  */
 
+#include "qemu/osdep.h"
 #include <math.h>
 
-#include "qemu/osdep.h"
 #include "hw/pci/msi.h"
 #include "hw/pci/msix.h"
 #include "hw/cxl/cxl.h"
diff --git a/hw/mem/cxl_type3.c b/hw/mem/cxl_type3.c
index be609ff9d03..4f3688a71b6 100644
--- a/hw/mem/cxl_type3.c
+++ b/hw/mem/cxl_type3.c
@@ -8,9 +8,9 @@
  *
  * SPDX-License-Identifier: GPL-v2-only
  */
+#include "qemu/osdep.h"
 #include <math.h>
 
-#include "qemu/osdep.h"
 #include "qemu/units.h"
 #include "qemu/error-report.h"
 #include "qapi/qapi-commands-cxl.h"
-- 
2.43.0

[PATCH 8/9] vfio: Clean up includes

[Peter Maydell]

This commit was created with scripts/clean-includes:
 ./scripts/clean-includes --git vfio hw/vfio hw/vfio-user

All .c should include qemu/osdep.h first.  The script performs three
related cleanups:

* Ensure .c files include qemu/osdep.h first.
* Including it in a .h is redundant, since the .c  already includes
  it.  Drop such inclusions.
* Likewise, including headers qemu/osdep.h includes is redundant.
  Drop these, too.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 hw/vfio-user/container.h | 1 -
 hw/vfio-user/device.h    | 1 -
 hw/vfio/pci-quirks.h     | 1 -
 hw/vfio-user/container.c | 2 +-
 hw/vfio-user/pci.c       | 2 +-
 hw/vfio/ap.c             | 1 -
 hw/vfio/container.c      | 2 +-
 hw/vfio/cpr-legacy.c     | 2 +-
 8 files changed, 4 insertions(+), 8 deletions(-)

diff --git a/hw/vfio-user/container.h b/hw/vfio-user/container.h
index a2b42e3169d..c952e090631 100644
--- a/hw/vfio-user/container.h
+++ b/hw/vfio-user/container.h
@@ -7,7 +7,6 @@
 #ifndef HW_VFIO_USER_CONTAINER_H
 #define HW_VFIO_USER_CONTAINER_H
 
-#include "qemu/osdep.h"
 
 #include "hw/vfio/vfio-container.h"
 #include "hw/vfio-user/proxy.h"
diff --git a/hw/vfio-user/device.h b/hw/vfio-user/device.h
index d183a3950e2..49c05848f1a 100644
--- a/hw/vfio-user/device.h
+++ b/hw/vfio-user/device.h
@@ -9,7 +9,6 @@
  * SPDX-License-Identifier: GPL-2.0-or-later
  */
 
-#include "qemu/osdep.h"
 #include "linux/vfio.h"
 
 #include "hw/vfio-user/proxy.h"
diff --git a/hw/vfio/pci-quirks.h b/hw/vfio/pci-quirks.h
index d1532e379b1..a6282e063a1 100644
--- a/hw/vfio/pci-quirks.h
+++ b/hw/vfio/pci-quirks.h
@@ -12,7 +12,6 @@
 #ifndef HW_VFIO_VFIO_PCI_QUIRKS_H
 #define HW_VFIO_VFIO_PCI_QUIRKS_H
 
-#include "qemu/osdep.h"
 #include "exec/memop.h"
 
 /*
diff --git a/hw/vfio-user/container.c b/hw/vfio-user/container.c
index e45192fef65..dab7a23224c 100644
--- a/hw/vfio-user/container.c
+++ b/hw/vfio-user/container.c
@@ -6,9 +6,9 @@
  * SPDX-License-Identifier: GPL-2.0-or-later
  */
 
+#include "qemu/osdep.h"
 #include <sys/ioctl.h>
 #include <linux/vfio.h>
-#include "qemu/osdep.h"
 
 #include "hw/vfio-user/container.h"
 #include "hw/vfio-user/device.h"
diff --git a/hw/vfio-user/pci.c b/hw/vfio-user/pci.c
index b53ed3b456f..353d07e7819 100644
--- a/hw/vfio-user/pci.c
+++ b/hw/vfio-user/pci.c
@@ -6,8 +6,8 @@
  * SPDX-License-Identifier: GPL-2.0-or-later
  */
 
-#include <sys/ioctl.h>
 #include "qemu/osdep.h"
+#include <sys/ioctl.h>
 #include "qapi-visit-sockets.h"
 #include "qemu/error-report.h"
 
diff --git a/hw/vfio/ap.c b/hw/vfio/ap.c
index 7719f245797..3368ac89150 100644
--- a/hw/vfio/ap.c
+++ b/hw/vfio/ap.c
@@ -10,7 +10,6 @@
  * directory.
  */
 
-#include <stdbool.h>
 #include "qemu/osdep.h"
 #include CONFIG_DEVICES /* CONFIG_IOMMUFD */
 #include <linux/vfio.h>
diff --git a/hw/vfio/container.c b/hw/vfio/container.c
index 9ddec300e35..013a691bc5a 100644
--- a/hw/vfio/container.c
+++ b/hw/vfio/container.c
@@ -10,10 +10,10 @@
  * SPDX-License-Identifier: GPL-2.0-or-later
  */
 
+#include "qemu/osdep.h"
 #include <sys/ioctl.h>
 #include <linux/vfio.h>
 
-#include "qemu/osdep.h"
 #include "system/tcg.h"
 #include "system/ram_addr.h"
 #include "qapi/error.h"
diff --git a/hw/vfio/cpr-legacy.c b/hw/vfio/cpr-legacy.c
index 7184c939912..273b5978806 100644
--- a/hw/vfio/cpr-legacy.c
+++ b/hw/vfio/cpr-legacy.c
@@ -4,9 +4,9 @@
  * SPDX-License-Identifier: GPL-2.0-or-later
  */
 
+#include "qemu/osdep.h"
 #include <sys/ioctl.h>
 #include <linux/vfio.h>
-#include "qemu/osdep.h"
 #include "hw/vfio/vfio-container-legacy.h"
 #include "hw/vfio/vfio-device.h"
 #include "hw/vfio/vfio-listener.h"
-- 
2.43.0

[PATCH 9/9] tests: Clean up includes

[Peter Maydell]

This commit was created with scripts/clean-includes:
 ./scripts/clean-includes --git tests tests

with one hand-edit to remove a now-empty #ifndef WIN32...#endif
from tests/qtest/dbus-display-test.c .

All .c should include qemu/osdep.h first.  The script performs three
related cleanups:

* Ensure .c files include qemu/osdep.h first.
* Including it in a .h is redundant, since the .c  already includes
  it.  Drop such inclusions.
* Likewise, including headers qemu/osdep.h includes is redundant.
  Drop these, too.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 tests/qtest/aspeed-hace-utils.h      | 1 -
 tests/qtest/aspeed-smc-utils.h       | 1 -
 tests/qtest/aspeed_gpio-test.c       | 1 -
 tests/qtest/dbus-display-test.c      | 3 ---
 tests/qtest/pnv-spi-seeprom-test.c   | 1 -
 tests/unit/test-cutils.c             | 2 +-
 tests/unit/test-error-report.c       | 1 -
 tests/unit/test-io-channel-command.c | 2 --
 8 files changed, 1 insertion(+), 11 deletions(-)

diff --git a/tests/qtest/aspeed-hace-utils.h b/tests/qtest/aspeed-hace-utils.h
index c8b2ec45af2..27ab2bb9758 100644
--- a/tests/qtest/aspeed-hace-utils.h
+++ b/tests/qtest/aspeed-hace-utils.h
@@ -8,7 +8,6 @@
 #ifndef TESTS_ASPEED_HACE_UTILS_H
 #define TESTS_ASPEED_HACE_UTILS_H
 
-#include "qemu/osdep.h"
 #include "libqtest.h"
 #include "qemu/bitops.h"
 
diff --git a/tests/qtest/aspeed-smc-utils.h b/tests/qtest/aspeed-smc-utils.h
index b07870f3b8f..e2fd8ff1bd1 100644
--- a/tests/qtest/aspeed-smc-utils.h
+++ b/tests/qtest/aspeed-smc-utils.h
@@ -26,7 +26,6 @@
 #ifndef TESTS_ASPEED_SMC_UTILS_H
 #define TESTS_ASPEED_SMC_UTILS_H
 
-#include "qemu/osdep.h"
 #include "qemu/bswap.h"
 #include "libqtest-single.h"
 #include "qemu/bitops.h"
diff --git a/tests/qtest/aspeed_gpio-test.c b/tests/qtest/aspeed_gpio-test.c
index c2f9ca2298a..decbba23c8f 100644
--- a/tests/qtest/aspeed_gpio-test.c
+++ b/tests/qtest/aspeed_gpio-test.c
@@ -27,7 +27,6 @@
 #include "qemu/timer.h"
 #include "qobject/qdict.h"
 #include "libqtest-single.h"
-#include "qemu/typedefs.h"
 
 #define AST2600_GPIO_BASE 0x1E780000
 
diff --git a/tests/qtest/dbus-display-test.c b/tests/qtest/dbus-display-test.c
index f7fc873bfb3..1d5951b7117 100644
--- a/tests/qtest/dbus-display-test.c
+++ b/tests/qtest/dbus-display-test.c
@@ -7,9 +7,6 @@
 #include <gio/gio.h>
 #include <gio/gunixfdlist.h>
 #include "libqtest.h"
-#ifndef WIN32
-#include <sys/mman.h>
-#endif
 #include "ui/dbus-display1.h"
 
 static GDBusConnection*
diff --git a/tests/qtest/pnv-spi-seeprom-test.c b/tests/qtest/pnv-spi-seeprom-test.c
index 600493c425d..8033261758b 100644
--- a/tests/qtest/pnv-spi-seeprom-test.c
+++ b/tests/qtest/pnv-spi-seeprom-test.c
@@ -5,7 +5,6 @@
  *
  * SPDX-License-Identifier: GPL-2.0-or-later
  */
-#include <unistd.h>
 #include "qemu/osdep.h"
 #include "libqtest.h"
 #include "qemu/bswap.h"
diff --git a/tests/unit/test-cutils.c b/tests/unit/test-cutils.c
index 227acc59955..75fae29003a 100644
--- a/tests/unit/test-cutils.c
+++ b/tests/unit/test-cutils.c
@@ -25,9 +25,9 @@
  * THE SOFTWARE.
  */
 
+#include "qemu/osdep.h"
 #include <math.h>
 
-#include "qemu/osdep.h"
 #include "qemu/cutils.h"
 #include "qemu/units.h"
 
diff --git a/tests/unit/test-error-report.c b/tests/unit/test-error-report.c
index 0cbde3c4cf5..a8532fc58fc 100644
--- a/tests/unit/test-error-report.c
+++ b/tests/unit/test-error-report.c
@@ -8,7 +8,6 @@
  */
 
 #include "qemu/osdep.h"
-#include "glib-compat.h"
 #include <locale.h>
 
 #include "qemu/error-report.h"
diff --git a/tests/unit/test-io-channel-command.c b/tests/unit/test-io-channel-command.c
index 4f022617df0..964418b5cd9 100644
--- a/tests/unit/test-io-channel-command.c
+++ b/tests/unit/test-io-channel-command.c
@@ -20,8 +20,6 @@
 
 #include "qemu/osdep.h"
 #include <glib/gstdio.h>
-#include <sys/types.h>
-#include <sys/stat.h>
 #include "io/channel-command.h"
 #include "io-channel-helpers.h"
 #include "qapi/error.h"
-- 
2.43.0

Re: [PATCH 8/9] vfio: Clean up includes

[Cédric Le Goater]

On 11/4/25 17:09, Peter Maydell wrote:
> This commit was created with scripts/clean-includes:
>   ./scripts/clean-includes --git vfio hw/vfio hw/vfio-user
> 
> All .c should include qemu/osdep.h first.  The script performs three
> related cleanups:
> 
> * Ensure .c files include qemu/osdep.h first.
> * Including it in a .h is redundant, since the .c  already includes
>    it.  Drop such inclusions.
> * Likewise, including headers qemu/osdep.h includes is redundant.
>    Drop these, too.
> 
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
>   hw/vfio-user/container.h | 1 -
>   hw/vfio-user/device.h    | 1 -
>   hw/vfio/pci-quirks.h     | 1 -
>   hw/vfio-user/container.c | 2 +-
>   hw/vfio-user/pci.c       | 2 +-
>   hw/vfio/ap.c             | 1 -
>   hw/vfio/container.c      | 2 +-
>   hw/vfio/cpr-legacy.c     | 2 +-
>   8 files changed, 4 insertions(+), 8 deletions(-)
> 

Reviewed-by: Cédric Le Goater <clg@redhat.com>

Thanks,

C.

Re: [PATCH 9/9] tests: Clean up includes

[Cédric Le Goater]

On 11/4/25 17:09, Peter Maydell wrote:
> This commit was created with scripts/clean-includes:
>   ./scripts/clean-includes --git tests tests
> 
> with one hand-edit to remove a now-empty #ifndef WIN32...#endif
> from tests/qtest/dbus-display-test.c .
> 
> All .c should include qemu/osdep.h first.  The script performs three
> related cleanups:
> 
> * Ensure .c files include qemu/osdep.h first.
> * Including it in a .h is redundant, since the .c  already includes
>    it.  Drop such inclusions.
> * Likewise, including headers qemu/osdep.h includes is redundant.
>    Drop these, too.
> 
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
>   tests/qtest/aspeed-hace-utils.h      | 1 -
>   tests/qtest/aspeed-smc-utils.h       | 1 -
>   tests/qtest/aspeed_gpio-test.c       | 1 -
>   tests/qtest/dbus-display-test.c      | 3 ---
>   tests/qtest/pnv-spi-seeprom-test.c   | 1 -
>   tests/unit/test-cutils.c             | 2 +-
>   tests/unit/test-error-report.c       | 1 -
>   tests/unit/test-io-channel-command.c | 2 --
>   8 files changed, 1 insertion(+), 11 deletions(-)

Reviewed-by: Cédric Le Goater <clg@redhat.com>

Thanks,

C.

Re: [PATCH 7/9] cxl: Clean up includes

[Philippe Mathieu-Daudé]

On 4/11/25 17:09, Peter Maydell wrote:
> This commit was created with scripts/clean-includes:
>   ./scripts/clean-includes --git cxl hw/cxl hw/mem
> 
> All .c should include qemu/osdep.h first.  The script performs three
> related cleanups:
> 
> * Ensure .c files include qemu/osdep.h first.
> * Including it in a .h is redundant, since the .c  already includes
>    it.  Drop such inclusions.
> * Likewise, including headers qemu/osdep.h includes is redundant.
>    Drop these, too.
> 
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
>   hw/cxl/cxl-mailbox-utils.c | 2 +-
>   hw/mem/cxl_type3.c         | 2 +-
>   2 files changed, 2 insertions(+), 2 deletions(-)

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

Re: [PATCH 8/9] vfio: Clean up includes

[Philippe Mathieu-Daudé]

On 4/11/25 17:09, Peter Maydell wrote:
> This commit was created with scripts/clean-includes:
>   ./scripts/clean-includes --git vfio hw/vfio hw/vfio-user
> 
> All .c should include qemu/osdep.h first.  The script performs three
> related cleanups:
> 
> * Ensure .c files include qemu/osdep.h first.
> * Including it in a .h is redundant, since the .c  already includes
>    it.  Drop such inclusions.
> * Likewise, including headers qemu/osdep.h includes is redundant.
>    Drop these, too.
> 
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
>   hw/vfio-user/container.h | 1 -
>   hw/vfio-user/device.h    | 1 -
>   hw/vfio/pci-quirks.h     | 1 -
>   hw/vfio-user/container.c | 2 +-
>   hw/vfio-user/pci.c       | 2 +-
>   hw/vfio/ap.c             | 1 -
>   hw/vfio/container.c      | 2 +-
>   hw/vfio/cpr-legacy.c     | 2 +-
>   8 files changed, 4 insertions(+), 8 deletions(-)

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

Re: [PATCH 7/9] cxl: Clean up includes

[Jonathan Cameron via]

On Tue,  4 Nov 2025 16:09:41 +0000
Peter Maydell <peter.maydell@linaro.org> wrote:

> This commit was created with scripts/clean-includes:
>  ./scripts/clean-includes --git cxl hw/cxl hw/mem
> 
> All .c should include qemu/osdep.h first.  The script performs three
> related cleanups:
> 
> * Ensure .c files include qemu/osdep.h first.
> * Including it in a .h is redundant, since the .c  already includes
>   it.  Drop such inclusions.
> * Likewise, including headers qemu/osdep.h includes is redundant.
>   Drop these, too.
> 
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
FWIW

Acked-by: Jonathan Cameron <jonathan.cameron@huawei.com>

Re: [PATCH 5/9] scripts/clean-includes: Give the args in git commit messages

[Markus Armbruster]

Peter Maydell <peter.maydell@linaro.org> writes:

> If clean-includes is creating a git commit for its changes,
> currently it says only "created with scripts/clean-includes".
> Add the command line arguments the user passed us, as useful
> extra information.
>
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
>  scripts/clean-includes | 6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)
>
> diff --git a/scripts/clean-includes b/scripts/clean-includes
> index a45421d2ff7..694e12f062c 100755
> --- a/scripts/clean-includes
> +++ b/scripts/clean-includes
> @@ -42,6 +42,9 @@
>  GIT=no
>  DUPHEAD=no
>  
> +# Save the original arguments in case we want to put them in
> +# a git commit message
> +ORIGARGS="$*"
>  
>  while true
>  do
> @@ -198,7 +201,8 @@ if [ "$GIT" = "yes" ]; then
>      git commit --signoff -F - <<EOF
>  $GITSUBJ: Clean up includes
>  
> -This commit was created with scripts/clean-includes.
> +This commit was created with scripts/clean-includes:
> + ./scripts/clean-includes $ORIGARGS

Consider

    $ ./scripts/clean-includes --git 'a b c' a b c

>  
>  All .c should include qemu/osdep.h first.  The script performs three
>  related cleanups:

Re: [PATCH 5/9] scripts/clean-includes: Give the args in git commit messages

[Peter Maydell]

On Fri, 7 Nov 2025 at 12:05, Markus Armbruster <armbru@redhat.com> wrote:
>
> Peter Maydell <peter.maydell@linaro.org> writes:
>
> > If clean-includes is creating a git commit for its changes,
> > currently it says only "created with scripts/clean-includes".
> > Add the command line arguments the user passed us, as useful
> > extra information.
> >
> > Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> > ---
> >  scripts/clean-includes | 6 +++++-
> >  1 file changed, 5 insertions(+), 1 deletion(-)
> >
> > diff --git a/scripts/clean-includes b/scripts/clean-includes
> > index a45421d2ff7..694e12f062c 100755
> > --- a/scripts/clean-includes
> > +++ b/scripts/clean-includes
> > @@ -42,6 +42,9 @@
> >  GIT=no
> >  DUPHEAD=no
> >
> > +# Save the original arguments in case we want to put them in
> > +# a git commit message
> > +ORIGARGS="$*"
> >
> >  while true
> >  do
> > @@ -198,7 +201,8 @@ if [ "$GIT" = "yes" ]; then
> >      git commit --signoff -F - <<EOF
> >  $GITSUBJ: Clean up includes
> >
> > -This commit was created with scripts/clean-includes.
> > +This commit was created with scripts/clean-includes:
> > + ./scripts/clean-includes $ORIGARGS
>
> Consider
>
>     $ ./scripts/clean-includes --git 'a b c' a b c

Yes. Do you have a way to deal with that that isn't
more complex than justified by what we're doing here?

thanks
-- PMM

Re: [PATCH 5/9] scripts/clean-includes: Give the args in git commit messages

[Markus Armbruster]

Peter Maydell <peter.maydell@linaro.org> writes:

> On Fri, 7 Nov 2025 at 12:05, Markus Armbruster <armbru@redhat.com> wrote:
>>
>> Peter Maydell <peter.maydell@linaro.org> writes:
>>
>> > If clean-includes is creating a git commit for its changes,
>> > currently it says only "created with scripts/clean-includes".
>> > Add the command line arguments the user passed us, as useful
>> > extra information.
>> >
>> > Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
>> > ---
>> >  scripts/clean-includes | 6 +++++-
>> >  1 file changed, 5 insertions(+), 1 deletion(-)
>> >
>> > diff --git a/scripts/clean-includes b/scripts/clean-includes
>> > index a45421d2ff7..694e12f062c 100755
>> > --- a/scripts/clean-includes
>> > +++ b/scripts/clean-includes
>> > @@ -42,6 +42,9 @@
>> >  GIT=no
>> >  DUPHEAD=no
>> >
>> > +# Save the original arguments in case we want to put them in
>> > +# a git commit message
>> > +ORIGARGS="$*"
>> >
>> >  while true
>> >  do
>> > @@ -198,7 +201,8 @@ if [ "$GIT" = "yes" ]; then
>> >      git commit --signoff -F - <<EOF
>> >  $GITSUBJ: Clean up includes
>> >
>> > -This commit was created with scripts/clean-includes.
>> > +This commit was created with scripts/clean-includes:
>> > + ./scripts/clean-includes $ORIGARGS
>>
>> Consider
>>
>>     $ ./scripts/clean-includes --git 'a b c' a b c
>
> Yes. Do you have a way to deal with that that isn't
> more complex than justified by what we're doing here?
>
> thanks
> -- PMM

This doesn't feel too onerous:

    args=
    for i
    do args+=" '""$i""'"
    done

However, += is bash.

Re: [PATCH 5/9] scripts/clean-includes: Give the args in git commit messages

[Markus Armbruster]

Markus Armbruster <armbru@redhat.com> writes:

> Peter Maydell <peter.maydell@linaro.org> writes:
>
>> On Fri, 7 Nov 2025 at 12:05, Markus Armbruster <armbru@redhat.com> wrote:
>>>
>>> Peter Maydell <peter.maydell@linaro.org> writes:
>>>
>>> > If clean-includes is creating a git commit for its changes,
>>> > currently it says only "created with scripts/clean-includes".
>>> > Add the command line arguments the user passed us, as useful
>>> > extra information.
>>> >
>>> > Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
>>> > ---
>>> >  scripts/clean-includes | 6 +++++-
>>> >  1 file changed, 5 insertions(+), 1 deletion(-)
>>> >
>>> > diff --git a/scripts/clean-includes b/scripts/clean-includes
>>> > index a45421d2ff7..694e12f062c 100755
>>> > --- a/scripts/clean-includes
>>> > +++ b/scripts/clean-includes
>>> > @@ -42,6 +42,9 @@
>>> >  GIT=no
>>> >  DUPHEAD=no
>>> >
>>> > +# Save the original arguments in case we want to put them in
>>> > +# a git commit message
>>> > +ORIGARGS="$*"
>>> >
>>> >  while true
>>> >  do
>>> > @@ -198,7 +201,8 @@ if [ "$GIT" = "yes" ]; then
>>> >      git commit --signoff -F - <<EOF
>>> >  $GITSUBJ: Clean up includes
>>> >
>>> > -This commit was created with scripts/clean-includes.
>>> > +This commit was created with scripts/clean-includes:
>>> > + ./scripts/clean-includes $ORIGARGS
>>>
>>> Consider
>>>
>>>     $ ./scripts/clean-includes --git 'a b c' a b c
>>
>> Yes. Do you have a way to deal with that that isn't
>> more complex than justified by what we're doing here?
>>
>> thanks
>> -- PMM
>
> This doesn't feel too onerous:
>
>     args=
>     for i
>     do args+=" '""$i""'"
>     done
>
> However, += is bash.

Have a look at configure:

    printf "exec" >>config.status
    for i in "$0" "$@"; do
      test "$i" = --skip-meson || printf " %s" "$(quote_sh "$i")" >>config.status
    done

with shell function

    quote_sh() {
        printf "%s" "$1" | sed "s,','\\\\'',g; s,.*,'&',"
    }

Re: [PATCH 0/9] clean-includes: improve exclude list, run on cxl, vfio, tests

[Peter Maydell]

On Tue, 4 Nov 2025 at 16:09, Peter Maydell <peter.maydell@linaro.org> wrote:
>
> I realised that we haven't run clean-includes recently, and
> unsurprisingly various violations of our include policy have
> crept in to the tree. The exclude-list of files it shouldn't
> be run on has also grown rather out of date.
>
> While looking at this I realised that one reason the exclude list
> is stale is that it's encoded in the script in a really awkward
> single long line extended regex. So the main thing this patch
> series does is fix that to instead use a list of regexes, one
> per line, with comments permitted.
>
> The other useful new feature here is that you can now point
> the script at a directory (previously your only options were
> an explicit list of files, or '--all' to scan everything).
>
> The other changes to the script itself are minor cleanups.
>
> Finally, I have a couple of patches which are the result of
> running the script on some subdirectories. I do think that all
> the changes that the script now suggests are correct (it wants
> to make changes to 28 files other than these) but I wanted to
> get the script changes through review first, and then perhaps
> send those last changes a bit more broken up per-subsystem.
>
> thanks
> -- PMM
>
> Peter Maydell (9):
>   scripts/clean-includes: Allow directories on command line
>   scripts/clean-includes: Remove outdated comment
>   scripts/clean-includes: Make ignore-regexes one per line
>   scripts/clean-includes: Do all our exclusions with REGEXFILE
>   scripts/clean-includes: Give the args in git commit messages
>   scripts/clean-includes: Update exclude list
>   cxl: Clean up includes
>   vfio: Clean up includes
>   tests: Clean up includes

I've taken the "actually clean some include files" patches
into target-arm.next, since those have been reviewed.
Review of the actual script changes would still be
appreciated.

thanks
-- PMM

Re: [PATCH 1/9] scripts/clean-includes: Allow directories on command line

[Richard Henderson]

On 11/4/25 17:09, Peter Maydell wrote:
> Currently clean-includes supports two ways of specifying files to check:
>   * --all to run on everything
>   * specific files
> There's no way to say "check everything in target/arm".
> 
> Add support for handling directory names, by always running
> the arguments through git ls-files.
> 
> Signed-off-by: Peter Maydell<peter.maydell@linaro.org>
> ---
>   scripts/clean-includes | 12 ++++++++----
>   1 file changed, 8 insertions(+), 4 deletions(-)

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

r~

Re: [PATCH 2/9] scripts/clean-includes: Remove outdated comment

[Richard Henderson]

On 11/4/25 17:09, Peter Maydell wrote:
> Remove an old comment suggesting a manual shell line to use to find
> files to run the script on. The script's exclude-list and its
> support for directory names make this irrelevant.
> 
> Signed-off-by: Peter Maydell<peter.maydell@linaro.org>
> ---
>   scripts/clean-includes | 9 ---------
>   1 file changed, 9 deletions(-)

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

r~

Re: [PATCH 3/9] scripts/clean-includes: Make ignore-regexes one per line

[Richard Henderson]

On 11/4/25 17:09, Peter Maydell wrote:
> Currently we have a single extended regular expression defining
> files that clean-includes should ignore. This is now very long
> and awkward to read and edit.
> 
> Switch to having a list of newline-separated EREs that we write
> to a file for grep's -f option, so we can express them more
> legibly in the shell script. We allow for comments in the
> regex list, which lets us document why we have put the
> exclusions in.
> 
> Signed-off-by: Peter Maydell<peter.maydell@linaro.org>
> ---
>   scripts/clean-includes | 28 ++++++++++++++++++++++------
>   1 file changed, 22 insertions(+), 6 deletions(-)

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

r~

Re: [PATCH 4/9] scripts/clean-includes: Do all our exclusions with REGEXFILE

[Richard Henderson]

On 11/4/25 17:09, Peter Maydell wrote:
> We currently have two mechanisms for excluding files:
>   * the REGEXFILE which excludes by regex
>   * special cases in the "loop over each file" which make
>     us skip the file
> 
> Roll all the "skip this" cases into REGEXFILE, so we use
> a single mechanism for identifying which files to exclude.
> 
> Signed-off-by: Peter Maydell<peter.maydell@linaro.org>
> ---
>   scripts/clean-includes | 24 +++++++++---------------
>   1 file changed, 9 insertions(+), 15 deletions(-)

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

r~

Re: [PATCH 6/9] scripts/clean-includes: Update exclude list

[Richard Henderson]

On 11/4/25 17:09, Peter Maydell wrote:
> Remove from the exclude list:
>   * tests/plugin, which is a non-existent directory. This was
>     probably intended to exclude tests/tcg/plugins/, which is caught
>     by the tests/tcg exclude pattern anyway
> Add to the exclude list:
>   * rust/ -- the headers in here are purely for input to bindgen
>   * target/hexagon has some standalone tools used at build time
>   * linux-user/gen-vsdo.c -- another standalone tool
>   * linux-user/mips64/elfload.c just includes mips/elfload.c
>   * scripts/xen-detect.c is feature-detection code used by meson.build
>   * tests/tracetool/simple.c is autogenerated
>   * tests/unit/ has some "C file just includes another one" files
>   * include/system/os-wasm.h is like os-posix.h and os-win32.h and
>     shouldn't be adjusted
> 
> Signed-off-by: Peter Maydell<peter.maydell@linaro.org>
> ---
>   scripts/clean-includes | 19 +++++++++++++++++--
>   1 file changed, 17 insertions(+), 2 deletions(-)

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

r~

Re: [PATCH 7/9] cxl: Clean up includes

[Richard Henderson]

On 11/4/25 17:09, Peter Maydell wrote:
> This commit was created with scripts/clean-includes:
>   ./scripts/clean-includes --git cxl hw/cxl hw/mem
> 
> All .c should include qemu/osdep.h first.  The script performs three
> related cleanups:
> 
> * Ensure .c files include qemu/osdep.h first.
> * Including it in a .h is redundant, since the .c  already includes
>    it.  Drop such inclusions.
> * Likewise, including headers qemu/osdep.h includes is redundant.
>    Drop these, too.
> 
> Signed-off-by: Peter Maydell<peter.maydell@linaro.org>
> ---
>   hw/cxl/cxl-mailbox-utils.c | 2 +-
>   hw/mem/cxl_type3.c         | 2 +-
>   2 files changed, 2 insertions(+), 2 deletions(-)

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

r~