Warner Losh <i...@bsdimp.com> writes: > On Thu, Jan 19, 2023 at 12:00 AM Markus Armbruster <arm...@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 <arm...@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!
