Re: [Qemu-devel] [Discussion 05/10] NEED_CPU_H: remove unnecessary inclusion of "cpu.h" in root

[Paolo Bonzini <pbonzini@redhat.com>]

Il 04/03/2014 03:47, Xuebing Wang ha scritto:
> Enforce the strict associativity between NEED_CPU_H and "inclusion of cpu.h",
> for every appearance of '#include "cpu.h"', there must have the check of
> NEED_CPU_H, and conversely.

As mentioned earlier, the #ifndef is unnecessary.  It is already 
provided by include/exec/cpu-defs.h

> This patch only checks file in qemu root directory by:
> grep -nw "cpu.h" *.[ch] .
>
> Note: remove unnecessary inclusion of "cpu-all.h" too. "cpu-all.h" is
> architecture-specific, should ONLY be included from target-xxx/*
> - 'git grep -nw "cpu-all.h"' confirms this

Should be a separate patch.  I agree with removing unnecessary #includes 
in general, but please check that the include is _really_ removed and 
not just included indirectly now.

Paolo

> Signed-off-by: Xuebing Wang <xbing6@gmail.com>
> ---
>  arch_init.c      |    1 -
>  cpu-exec.c       |    7 ++++++-
>  cputlb.c         |    6 +++++-
>  disas.c          |    7 ++++++-
>  dump.c           |    2 --
>  exec.c           |    2 --
>  gdbstub.c        |    8 +++++++-
>  kvm-stub.c       |    1 -
>  memory_mapping.c |    6 +++---
>  monitor.c        |    1 -
>  translate-all.c  |    7 ++++++-
>  user-exec.c      |    7 ++++++-
>  12 files changed, 39 insertions(+), 16 deletions(-)
>
> diff --git a/arch_init.c b/arch_init.c
> index fe17279..76e8630 100644
> --- a/arch_init.c
> +++ b/arch_init.c
> @@ -47,7 +47,6 @@
>  #include "qemu/config-file.h"
>  #include "qmp-commands.h"
>  #include "trace.h"
> -#include "exec/cpu-all.h"
>  #include "exec/ram_addr.h"
>  #include "hw/acpi/acpi.h"
>  #include "qemu/host-utils.h"
> diff --git a/cpu-exec.c b/cpu-exec.c
> index 1b0f617..858825f 100644
> --- a/cpu-exec.c
> +++ b/cpu-exec.c
> @@ -16,8 +16,13 @@
>   * You should have received a copy of the GNU Lesser General Public
>   * License along with this library; if not, see <http://www.gnu.org/licenses/>.
>   */
> +
> +#ifndef NEED_CPU_H
> +#error target-xxx/cpu.h must be included because target-specific are required
> +#endif
> +#include "cpu.h" /* target-xxx/cpu.h, required for CPUArchState etc */
> +
>  #include "config.h"
> -#include "cpu.h"
>  #include "disas/disas.h"
>  #include "tcg.h"
>  #include "qemu/atomic.h"
> diff --git a/cputlb.c b/cputlb.c
> index 0fbaa39..3c6cb16 100644
> --- a/cputlb.c
> +++ b/cputlb.c
> @@ -17,8 +17,12 @@
>   * License along with this library; if not, see <http://www.gnu.org/licenses/>.
>   */
>
> +#ifndef NEED_CPU_H
> +#error target-xxx/cpu.h must be included because target-specific are required
> +#endif
> +#include "cpu.h" /* target-xxx/cpu.h, required for CPUArchState etc */
> +
>  #include "config.h"
> -#include "cpu.h"
>  #include "exec/exec-all.h"
>  #include "exec/memory.h"
>  #include "exec/address-spaces.h"
> diff --git a/disas.c b/disas.c
> index 79e6944..8dc51c8 100644
> --- a/disas.c
> +++ b/disas.c
> @@ -1,10 +1,15 @@
>  /* General "disassemble this chunk" code.  Used for debugging. */
> +
> +#ifndef NEED_CPU_H
> +#error target-xxx/cpu.h must be included because target-specific are required
> +#endif
> +#include "cpu.h" /* target-xxx/cpu.h, required for CPUArchState etc */
> +
>  #include "config.h"
>  #include "disas/bfd.h"
>  #include "elf.h"
>  #include <errno.h>
>
> -#include "cpu.h"
>  #include "disas/disas.h"
>
>  typedef struct CPUDebug {
> diff --git a/dump.c b/dump.c
> index 80a9116..d75c2ea 100644
> --- a/dump.c
> +++ b/dump.c
> @@ -13,8 +13,6 @@
>
>  #include "qemu-common.h"
>  #include "elf.h"
> -#include "cpu.h"
> -#include "exec/cpu-all.h"
>  #include "exec/hwaddr.h"
>  #include "monitor/monitor.h"
>  #include "sysemu/kvm.h"
> diff --git a/exec.c b/exec.c
> index b69fd29..3df2ffc 100644
> --- a/exec.c
> +++ b/exec.c
> @@ -25,7 +25,6 @@
>  #endif
>
>  #include "qemu-common.h"
> -#include "cpu.h"
>  #include "tcg.h"
>  #include "hw/hw.h"
>  #include "hw/qdev.h"
> @@ -44,7 +43,6 @@
>  #include "sysemu/xen-mapcache.h"
>  #include "trace.h"
>  #endif
> -#include "exec/cpu-all.h"
>
>  #include "exec/cputlb.h"
>  #include "translate-all.h"
> diff --git a/gdbstub.c b/gdbstub.c
> index e8ab0b2..a96add1 100644
> --- a/gdbstub.c
> +++ b/gdbstub.c
> @@ -16,6 +16,13 @@
>   * You should have received a copy of the GNU Lesser General Public
>   * License along with this library; if not, see <http://www.gnu.org/licenses/>.
>   */
> +
> +#ifndef NEED_CPU_H
> +#error target-xxx/cpu.h must be included because target-specific are required
> +#endif
> +#include "cpu.h" /* target-xxx/cpu.h, required for CPUArchState etc */
> +/* TODO: to remove using CPUArchState to make thie file arch-independent. */
> +
>  #include "config.h"
>  #include "qemu-common.h"
>  #ifdef CONFIG_USER_ONLY
> @@ -37,7 +44,6 @@
>
>  #define MAX_PACKET_LENGTH 4096
>
> -#include "cpu.h"
>  #include "qemu/sockets.h"
>  #include "sysemu/kvm.h"
>
> diff --git a/kvm-stub.c b/kvm-stub.c
> index e979f76..581350a 100644
> --- a/kvm-stub.c
> +++ b/kvm-stub.c
> @@ -12,7 +12,6 @@
>
>  #include "qemu-common.h"
>  #include "hw/hw.h"
> -#include "cpu.h"
>  #include "sysemu/kvm.h"
>
>  #ifndef CONFIG_USER_ONLY
> diff --git a/memory_mapping.c b/memory_mapping.c
> index 87a6ed5..b17eea1 100644
> --- a/memory_mapping.c
> +++ b/memory_mapping.c
> @@ -13,11 +13,11 @@
>
>  #include <glib.h>
>
> -#include "cpu.h"
> -#include "exec/cpu-all.h"
> -#include "sysemu/memory_mapping.h"
> +#include "qemu-common.h"
> +#include "exec/cpu-common.h" /* for ram_addr_t */
>  #include "exec/memory.h"
>  #include "exec/address-spaces.h"
> +#include "sysemu/memory_mapping.h"
>
>  //#define DEBUG_GUEST_PHYS_REGION_ADD
>
> diff --git a/monitor.c b/monitor.c
> index aebcbd8..1a8dd24 100644
> --- a/monitor.c
> +++ b/monitor.c
> @@ -58,7 +58,6 @@
>  #include "qapi/qmp/json-parser.h"
>  #include <qom/object_interfaces.h>
>  #include "qemu/osdep.h"
> -#include "cpu.h"
>  #include "trace.h"
>  #include "trace/control.h"
>  #ifdef CONFIG_TRACE_SIMPLE
> diff --git a/translate-all.c b/translate-all.c
> index 1ac0246..be220fb 100644
> --- a/translate-all.c
> +++ b/translate-all.c
> @@ -16,6 +16,12 @@
>   * You should have received a copy of the GNU Lesser General Public
>   * License along with this library; if not, see <http://www.gnu.org/licenses/>.
>   */
> +
> +#ifndef NEED_CPU_H
> +#error target-xxx/cpu.h must be included because target-specific are required
> +#endif
> +#include "cpu.h" /* target-xxx/cpu.h, required for CPUArchState etc */
> +
>  #ifdef _WIN32
>  #include <windows.h>
>  #else
> @@ -32,7 +38,6 @@
>
>  #include "qemu-common.h"
>  #define NO_CPU_IO_DEFS
> -#include "cpu.h"
>  #include "disas/disas.h"
>  #include "tcg.h"
>  #if defined(CONFIG_USER_ONLY)
> diff --git a/user-exec.c b/user-exec.c
> index 82bfa66..c4c1e3f 100644
> --- a/user-exec.c
> +++ b/user-exec.c
> @@ -16,8 +16,13 @@
>   * You should have received a copy of the GNU Lesser General Public
>   * License along with this library; if not, see <http://www.gnu.org/licenses/>.
>   */
> +
> +#ifndef NEED_CPU_H
> +#error target-xxx/cpu.h must be included because target-specific are required
> +#endif
> +#include "cpu.h" /* target-xxx/cpu.h, required for CPUArchState etc */
> +
>  #include "config.h"
> -#include "cpu.h"
>  #include "disas/disas.h"
>  #include "tcg.h"
>  #include "qemu/bitops.h"
>