1. Patchew
  2. QEMU
  3. [PATCH v3 0/3] hw/core: Cleanup and reorder headers
  4. View patch

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

Zhao Liu posted 3 patches 8 months, 2 weeks ago
Maintainers: Eduardo Habkost <eduardo@habkost.net>, Marcel Apfelbaum <marcel.apfelbaum@gmail.com>, "Philippe Mathieu-Daudé" <philmd@linaro.org>, Yanan Wang <wangyanan55@huawei.com>
[PATCH v3 1/3] hw/core: Cleanup unused included headers in cpu-common.c
Posted by Zhao Liu 8 months, 2 weeks ago 
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
Posted by Philippe Mathieu-Daudé 8 months, 2 weeks ago 
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
Posted by Zhao Liu 8 months, 2 weeks ago 
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
Posted by Philippe Mathieu-Daudé 8 months, 2 weeks ago 
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
Posted by Zhao Liu 8 months, 2 weeks ago 
> > 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
Re: [PATCH v3 1/3] hw/core: Cleanup unused included headers in cpu-common.c
Posted by Philippe Mathieu-Daudé 8 months, 2 weeks ago 
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>

Patchew 2.1-devel-b67e4c2

© 2016 - 2024 Red Hat, Inc.