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

[Warner Losh <imp@bsdimp.com>]

[-- 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 --]