Re: [Qemu-devel] [PATCH v2 01/29] include: Make headers more self-contained

[Alex Bennée]

Markus Armbruster <address@hidden> 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 patch gets include/ closer to obeying 2.
>
> It's actually extracted from my "[RFC] Baby steps towards saner
> headers" series[2], which demonstrates a possible path towards
> checking 2 automatically.  It passes the RFC test there.
>
> [1] Message-ID: <address@hidden>
>     https://lists.nongnu.org/archive/html/qemu-devel/2016-03/msg03345.html
> [2] Message-Id: <address@hidden>
>     https://lists.nongnu.org/archive/html/qemu-devel/2019-07/msg02715.html
>
> Signed-off-by: Markus Armbruster <address@hidden>
> Reviewed-by: Alistair Francis <address@hidden>
> ---
<snip>
>  include/exec/cputlb.h                 | 2 ++
>  include/exec/exec-all.h               | 1 +
>  include/exec/ioport.h                 | 2 ++
>  include/exec/memory-internal.h        | 2 ++
>  include/exec/ram_addr.h               | 1 +
>  include/exec/softmmu-semi.h           | 2 ++
>  include/exec/tb-hash.h                | 2 ++
>  include/exec/user/thunk.h             | 1 +
>  include/fpu/softfloat-macros.h        | 2 ++
<snip>
>
>  /*
>   * bdrv_write_threshold_set:
> diff --git a/include/disas/disas.h b/include/disas/disas.h
> index 15da511f49..ba47e9197c 100644
> --- a/include/disas/disas.h
> +++ b/include/disas/disas.h
> @@ -1,6 +1,7 @@
>  #ifndef QEMU_DISAS_H
>  #define QEMU_DISAS_H
>
> +#include "exec/hwaddr.h"
>
>  #ifdef NEED_CPU_H
>  #include "cpu.h"
> diff --git a/include/exec/cputlb.h b/include/exec/cputlb.h
> index 5373188be3..23abd71579 100644
> --- a/include/exec/cputlb.h
> +++ b/include/exec/cputlb.h
> @@ -19,6 +19,8 @@
>  #ifndef CPUTLB_H
>  #define CPUTLB_H
>
> +#include "exec/cpu-common.h"
> +
>  #if !defined(CONFIG_USER_ONLY)
>  /* cputlb.c */
>  void tlb_protect_code(ram_addr_t ram_addr);
> diff --git a/include/exec/exec-all.h b/include/exec/exec-all.h
> index 16034ee651..135aeaab0d 100644
> --- a/include/exec/exec-all.h
> +++ b/include/exec/exec-all.h
> @@ -20,6 +20,7 @@
>  #ifndef EXEC_ALL_H
>  #define EXEC_ALL_H
>
> +#include "cpu.h"
>  #include "exec/tb-context.h"
>  #include "sysemu/cpus.h"
>
> diff --git a/include/exec/ioport.h b/include/exec/ioport.h
> index a298b89ce1..97feb296d2 100644
> --- a/include/exec/ioport.h
> +++ b/include/exec/ioport.h
> @@ -24,6 +24,8 @@
>  #ifndef IOPORT_H
>  #define IOPORT_H
>
> +#include "exec/memory.h"
> +
>  #define MAX_IOPORTS     (64 * 1024)
>  #define IOPORTS_MASK    (MAX_IOPORTS - 1)
>
> diff --git a/include/exec/memory-internal.h b/include/exec/memory-internal.h
> index d1a9dd1ec8..ef4fb92371 100644
> --- a/include/exec/memory-internal.h
> +++ b/include/exec/memory-internal.h
> @@ -20,6 +20,8 @@
>  #ifndef MEMORY_INTERNAL_H
>  #define MEMORY_INTERNAL_H
>
> +#include "cpu.h"
> +
>  #ifndef CONFIG_USER_ONLY
>  static inline AddressSpaceDispatch *flatview_to_dispatch(FlatView *fv)
>  {
> diff --git a/include/exec/ram_addr.h b/include/exec/ram_addr.h
> index b7b2e60ff6..a327a80cfe 100644
> --- a/include/exec/ram_addr.h
> +++ b/include/exec/ram_addr.h
> @@ -20,6 +20,7 @@
>  #define RAM_ADDR_H
>
>  #ifndef CONFIG_USER_ONLY
> +#include "cpu.h"
>  #include "hw/xen/xen.h"
>  #include "sysemu/tcg.h"
>  #include "exec/ramlist.h"
> diff --git a/include/exec/softmmu-semi.h b/include/exec/softmmu-semi.h
> index 970837992e..fbcae88f4b 100644
> --- a/include/exec/softmmu-semi.h
> +++ b/include/exec/softmmu-semi.h
> @@ -10,6 +10,8 @@
>  #ifndef SOFTMMU_SEMI_H
>  #define SOFTMMU_SEMI_H
>
> +#include "cpu.h"
> +
>  static inline uint64_t softmmu_tget64(CPUArchState *env, target_ulong addr)
>  {
>      uint64_t val;
> diff --git a/include/exec/tb-hash.h b/include/exec/tb-hash.h
> index 4f3a37d927..805235d321 100644
> --- a/include/exec/tb-hash.h
> +++ b/include/exec/tb-hash.h
> @@ -20,6 +20,8 @@
>  #ifndef EXEC_TB_HASH_H
>  #define EXEC_TB_HASH_H
>
> +#include "exec/cpu-defs.h"
> +#include "exec/exec-all.h"
>  #include "qemu/xxhash.h"
>
>  #ifdef CONFIG_SOFTMMU
> diff --git a/include/exec/user/thunk.h b/include/exec/user/thunk.h
> index 8d3af5a3be..d05a8a4dab 100644
> --- a/include/exec/user/thunk.h
> +++ b/include/exec/user/thunk.h
> @@ -20,6 +20,7 @@
>  #define THUNK_H
>
>  #include "cpu.h"
> +#include "exec/user/abitypes.h"
>
>  /* types enums definitions */

These all seem OK.

>
> diff --git a/include/fpu/softfloat-macros.h b/include/fpu/softfloat-macros.h
> index c55aa6d174..be83a833ec 100644
> --- a/include/fpu/softfloat-macros.h
> +++ b/include/fpu/softfloat-macros.h
> @@ -82,6 +82,8 @@ this code that are retained.
>  #ifndef FPU_SOFTFLOAT_MACROS_H
>  #define FPU_SOFTFLOAT_MACROS_H
>
> +#include "fpu/softfloat.h"
> +

What does softfloat-macros actually need from the core softfloat API?
These are lower level functions used by softfloat itself (and m68k for
it's own bit fiddling).

--
Alex Bennée