Re: [PATCH v3 1/3] hw/core: Cleanup unused included headers in cpu-common.c

[Zhao Liu]

Hi Philippe,

On Mon, Mar 11, 2024 at 12:40:23PM +0100, Philippe Mathieu-Daudé wrote:
> Date: Mon, 11 Mar 2024 12:40:23 +0100
> From: Philippe Mathieu-Daudé <philmd@linaro.org>
> Subject: Re: [PATCH v3 1/3] hw/core: Cleanup unused included headers in
>  cpu-common.c
> 
> On 11/3/24 08:56, Zhao Liu wrote:
> > From: Zhao Liu <zhao1.liu@intel.com>
> > 
> > Remove unused headers in cpu-common.c:
> > * qemu/notify.h
> > * exec/cpu-common.h
> > * qemu/error-report.h
> > * qemu/qemu-print.h
> > 
> > Tested by "./configure" and then "make".
> 
> This isn't often enough. The safest way to catch implicit
> includes is to add #error in them and compile the source.
> 
> > 
> > Signed-off-by: Zhao Liu <zhao1.liu@intel.com>
> > ---
> >   hw/core/cpu-common.c | 4 ----
> >   1 file changed, 4 deletions(-)
> > 
> > diff --git a/hw/core/cpu-common.c b/hw/core/cpu-common.c
> > index 0108fb11dbc8..4bd9c70a83f1 100644
> > --- a/hw/core/cpu-common.c
> > +++ b/hw/core/cpu-common.c
> > @@ -22,14 +22,10 @@
> >   #include "qapi/error.h"
> >   #include "hw/core/cpu.h"
> >   #include "sysemu/hw_accel.h"
> > -#include "qemu/notify.h"
> >   #include "qemu/log.h"
> >   #include "qemu/main-loop.h"
> >   #include "exec/log.h"
> > -#include "exec/cpu-common.h"
> 
> Watch out, "exec/cpu-common.h" is implicitly included:
> 
> $ git diff -U0
> diff --git a/include/exec/cpu-common.h b/include/exec/cpu-common.h
> index 6346df17ce..27961bacc6 100644
> --- a/include/exec/cpu-common.h
> +++ b/include/exec/cpu-common.h
> @@ -2,0 +3 @@
> +#error
> 
> $ ninja libcommon.fa.p/hw_core_cpu-common.c.o
> In file included from ../../hw/core/cpu-common.c:34:
> In file included from include/hw/boards.h:6:
> In file included from include/exec/memory.h:19:
> include/exec/cpu-common.h:3:2: error:
> #error

Thanks for helpping me verify this!!

EMM, but I'm still not understanding how this approach distinguishes
whether hw/core/cpu-common.c needs the header (include/exec/cpu-common.h)
directly or just include/exec/memory.h needs that header? For the latter,
the header needn't be included in .c file.

Thanks,
Zhao