From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:53547) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1WKmX5-00053U-5x for qemu-devel@nongnu.org; Tue, 04 Mar 2014 05:25:12 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1WKmX0-0006lB-Cp for qemu-devel@nongnu.org; Tue, 04 Mar 2014 05:25:07 -0500 Received: from mx1.redhat.com ([209.132.183.28]:32821) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1WKmX0-0006hp-48 for qemu-devel@nongnu.org; Tue, 04 Mar 2014 05:25:02 -0500 Message-ID: <5315A9F6.5060101@redhat.com> Date: Tue, 04 Mar 2014 11:24:54 +0100 From: Paolo Bonzini MIME-Version: 1.0 References: <1393901250-3922-1-git-send-email-xbing6@gmail.com> <1393901250-3922-6-git-send-email-xbing6@gmail.com> In-Reply-To: <1393901250-3922-6-git-send-email-xbing6@gmail.com> Content-Type: text/plain; charset=ISO-8859-15; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [Discussion 05/10] NEED_CPU_H: remove unnecessary inclusion of "cpu.h" in root List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Xuebing Wang , qemu-devel@nongnu.org Cc: afaerber@suse.de, stefanha@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 > --- > 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 . > */ > + > +#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 . > */ > > +#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 > > -#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 . > */ > + > +#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 > > -#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 > #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 . > */ > + > +#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 > #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 . > */ > + > +#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" >