[PATCH v3 0/3] hw/core: Cleanup and reorder headers

[PATCH v3 0/3] hw/core: Cleanup and reorder headers

[Zhao Liu]

From: Zhao Liu <zhao1.liu@intel.com>

Hi list,

This is the v3 based on the commit 7489f7f3f81d ("Merge tag 'hw-misc-
20240309' of https://github.com/philmd/qemu into staging")

This series identifies unused headers by manually checking item (of
headers) calls in .c files, and is tested by full compilation (tested by
"./configure" and then "make").

Thanks and Best Regards,
Zhao

---
Changelog:

Changes since v2:
  * Dropped the reorder patches since QEMU has no (alphabetical) order
    requirement for included headers.
  * Re-included hw/core/cpu.h in hw/core/numa.c since
    CPU_UNSET_NUMA_NODE_ID is defined in this header.

Changes since v1:
  * Per Peter and Philippe's comments in v1, kept directly included
    headers to avoid implicit header inclusions.

v2: https://lore.kernel.org/qemu-devel/20240116074647.3644821-1-zhao1.liu@linux.intel.com/
v1: https://lore.kernel.org/qemu-devel/20240115094852.3597165-1-zhao1.liu@linux.intel.com/

---
Zhao Liu (3):
  hw/core: Cleanup unused included headers in cpu-common.c
  hw/core: Cleanup unused included header in machine-qmp-cmds.c
  hw/core: Cleanup unused included headers in numa.c

 hw/core/cpu-common.c       | 4 ----
 hw/core/machine-qmp-cmds.c | 1 -
 hw/core/numa.c             | 2 --
 3 files changed, 7 deletions(-)

-- 
2.34.1

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

[Zhao Liu]

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".

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"
 #include "exec/gdbstub.h"
-#include "qemu/error-report.h"
-#include "qemu/qemu-print.h"
 #include "sysemu/tcg.h"
 #include "hw/boards.h"
 #include "hw/qdev-properties.h"
-- 
2.34.1

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

[Philippe Mathieu-Daudé]

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

I'll keep it for now. No need to repost.

>   #include "exec/gdbstub.h"
> -#include "qemu/error-report.h"
> -#include "qemu/qemu-print.h"
>   #include "sysemu/tcg.h"
>   #include "hw/boards.h"
>   #include "hw/qdev-properties.h"

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

[Philippe Mathieu-Daudé]

On Mon, 11 Mar 2024 at 12:40, Philippe Mathieu-Daudé <philmd@linaro.org> wrote:
>
> 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
>
> I'll keep it for now. No need to repost.

Forgot:
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>

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

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

[Philippe Mathieu-Daudé]

On 11/3/24 16:44, Zhao Liu wrote:
> 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.

Yes, you are right, it might not be necessary.

For all other headers in your series I checked that no function /
definition is used in the source, but "exec/cpu-common.h" is too
big to do that manually. I mostly skipped it because it is odd to
have cpu-common.c not including the header with the same name...
Also, in another series I split / reworked "exec/cpu-common.h" and
IIRC a part of it will be included here. Bah, I'll stop writing
and take your patch unmodified.

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

[Zhao Liu]

> > 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.
> 
> Yes, you are right, it might not be necessary.
> 
> For all other headers in your series I checked that no function /
> definition is used in the source, but "exec/cpu-common.h" is too
> big to do that manually.

Thanks! I checked manually as well... In the future I'll also think
about if there's a more elegant way to do it.

> I mostly skipped it because it is odd to
> have cpu-common.c not including the header with the same name...

Yes, I think the "cpu-common.c" is the related .c file of
exec/cpu-common.h.

And the related header of "hw/core/cpu-common.c" should be
"hw/core/cpu.h".

Could we rename "hw/core/cpu-common.c" to "hw/core/cpu.c"? Then the
relationship could be clear.

> Also, in another series I split / reworked "exec/cpu-common.h" and
> IIRC a part of it will be included here. Bah, I'll stop writing
> and take your patch unmodified.

Many thanks!

Regards,
Zhao

[PATCH v3 2/3] hw/core: Cleanup unused included header in machine-qmp-cmds.c

[Zhao Liu]

From: Zhao Liu <zhao1.liu@intel.com>

Remove unused header (qemu/main-loop.h) in machine-qmp-cmds.c.

Tested by "./configure" and then "make".

Signed-off-by: Zhao Liu <zhao1.liu@intel.com>
---
 hw/core/machine-qmp-cmds.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/hw/core/machine-qmp-cmds.c b/hw/core/machine-qmp-cmds.c
index 3860a50c3b7b..4b72009cd3c3 100644
--- a/hw/core/machine-qmp-cmds.c
+++ b/hw/core/machine-qmp-cmds.c
@@ -19,7 +19,6 @@
 #include "qapi/qmp/qobject.h"
 #include "qapi/qobject-input-visitor.h"
 #include "qapi/type-helpers.h"
-#include "qemu/main-loop.h"
 #include "qemu/uuid.h"
 #include "qom/qom-qobject.h"
 #include "sysemu/hostmem.h"
-- 
2.34.1

Re: [PATCH v3 2/3] hw/core: Cleanup unused included header in machine-qmp-cmds.c

[Philippe Mathieu-Daudé]

On 11/3/24 08:56, Zhao Liu wrote:
> From: Zhao Liu <zhao1.liu@intel.com>
> 
> Remove unused header (qemu/main-loop.h) in machine-qmp-cmds.c.
> 
> Tested by "./configure" and then "make".
> 
> Signed-off-by: Zhao Liu <zhao1.liu@intel.com>
> ---
>   hw/core/machine-qmp-cmds.c | 1 -
>   1 file changed, 1 deletion(-)

Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>

[PATCH v3 3/3] hw/core: Cleanup unused included headers in numa.c

[Zhao Liu]

From: Zhao Liu <zhao1.liu@intel.com>

Remove unused header in numa.c:
* qemu/bitmap.h
* migration/vmstate.h

Note: Though parse_numa_hmat_lb() has the variable named "bitmap_copy",
it doesn't use the normal bitmap ops so that it's safe to exclude
qemu/bitmap.h header.

Tested by "./configure" and then "make".

Signed-off-by: Zhao Liu <zhao1.liu@intel.com>
---
v3: Re-included hw/core/cpu.h since CPU_UNSET_NUMA_NODE_ID is defined
    in this header.
---
 hw/core/numa.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/hw/core/numa.c b/hw/core/numa.c
index f08956ddb0ff..81d21243498e 100644
--- a/hw/core/numa.c
+++ b/hw/core/numa.c
@@ -28,7 +28,6 @@
 #include "sysemu/numa.h"
 #include "exec/cpu-common.h"
 #include "exec/ramlist.h"
-#include "qemu/bitmap.h"
 #include "qemu/error-report.h"
 #include "qapi/error.h"
 #include "qapi/opts-visitor.h"
@@ -36,7 +35,6 @@
 #include "sysemu/qtest.h"
 #include "hw/core/cpu.h"
 #include "hw/mem/pc-dimm.h"
-#include "migration/vmstate.h"
 #include "hw/boards.h"
 #include "hw/mem/memory-device.h"
 #include "qemu/option.h"
-- 
2.34.1

Re: [PATCH v3 3/3] hw/core: Cleanup unused included headers in numa.c

[Philippe Mathieu-Daudé]

On 11/3/24 08:56, Zhao Liu wrote:
> From: Zhao Liu <zhao1.liu@intel.com>
> 
> Remove unused header in numa.c:
> * qemu/bitmap.h
> * migration/vmstate.h
> 
> Note: Though parse_numa_hmat_lb() has the variable named "bitmap_copy",
> it doesn't use the normal bitmap ops so that it's safe to exclude
> qemu/bitmap.h header.
> 
> Tested by "./configure" and then "make".
> 
> Signed-off-by: Zhao Liu <zhao1.liu@intel.com>
> ---
> v3: Re-included hw/core/cpu.h since CPU_UNSET_NUMA_NODE_ID is defined
>      in this header.
> ---
>   hw/core/numa.c | 2 --
>   1 file changed, 2 deletions(-)

Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>

Re: [PATCH v3 0/3] hw/core: Cleanup and reorder headers

[Philippe Mathieu-Daudé]

On 11/3/24 08:56, Zhao Liu wrote:

> ---
> Zhao Liu (3):
>    hw/core: Cleanup unused included headers in cpu-common.c
>    hw/core: Cleanup unused included header in machine-qmp-cmds.c
>    hw/core: Cleanup unused included headers in numa.c

Thanks, series queued.