| Message ID | 20240115094852.3597165-3-zhao1.liu@linux.intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | hw/core: Cleanup and reorder headers | expand |
On Mon, 15 Jan 2024 at 09:37, Zhao Liu <zhao1.liu@linux.intel.com> wrote: > > From: Zhao Liu <zhao1.liu@intel.com> > > Remove unused headers in cpu-common.c: > * qemu/notify.h > * qemu/log.h > * qemu/main-loop.h > * exec/cpu-common.h > * qemu/error-report.h > * qemu/qemu-print.h > > Though hw/core/cpu.h has been included by sysemu/hw_accel.h, to keep > the dependency clear, still directly include hw/core/cpu.h in this file. > > Tested by "./configure" and then "make". > > Signed-off-by: Zhao Liu <zhao1.liu@intel.com> > --- > hw/core/cpu-common.c | 7 +------ > 1 file changed, 1 insertion(+), 6 deletions(-) Something seems to be wrong with your analysis of what includes it is OK to drop. For instance, this file uses the function qemu_log(), which is why it includes qemu/log.h. thanks -- PMM
Hi Peter, On Mon, Jan 15, 2024 at 10:41:48AM +0000, Peter Maydell wrote: > Date: Mon, 15 Jan 2024 10:41:48 +0000 > From: Peter Maydell <peter.maydell@linaro.org> > Subject: Re: [PATCH 02/11] hw/core: Cleanup unused included headers in > cpu-common.c > > On Mon, 15 Jan 2024 at 09:37, Zhao Liu <zhao1.liu@linux.intel.com> wrote: > > > > From: Zhao Liu <zhao1.liu@intel.com> > > > > Remove unused headers in cpu-common.c: > > * qemu/notify.h > > * qemu/log.h > > * qemu/main-loop.h > > * exec/cpu-common.h > > * qemu/error-report.h > > * qemu/qemu-print.h > > > > Though hw/core/cpu.h has been included by sysemu/hw_accel.h, to keep > > the dependency clear, still directly include hw/core/cpu.h in this file. > > > > Tested by "./configure" and then "make". > > > > Signed-off-by: Zhao Liu <zhao1.liu@intel.com> > > --- > > hw/core/cpu-common.c | 7 +------ > > 1 file changed, 1 insertion(+), 6 deletions(-) > > Something seems to be wrong with your analysis of what > includes it is OK to drop. For instance, this file uses > the function qemu_log(), which is why it includes > qemu/log.h. > I'm not sure about this, since qemu/log.h has been included by exec/log.h, so could we just include exec/log.h and omit qemu/log.h in this file? It seems enough for the compilation to omit qemu/log.h and only include exec/log.h. Regards, Zhao
On 15/1/24 15:45, Zhao Liu wrote: > Hi Peter, > > On Mon, Jan 15, 2024 at 10:41:48AM +0000, Peter Maydell wrote: >> Date: Mon, 15 Jan 2024 10:41:48 +0000 >> From: Peter Maydell <peter.maydell@linaro.org> >> Subject: Re: [PATCH 02/11] hw/core: Cleanup unused included headers in >> cpu-common.c >> >> On Mon, 15 Jan 2024 at 09:37, Zhao Liu <zhao1.liu@linux.intel.com> wrote: >>> >>> From: Zhao Liu <zhao1.liu@intel.com> >>> >>> Remove unused headers in cpu-common.c: >>> * qemu/notify.h >>> * qemu/log.h >>> * qemu/main-loop.h >>> * exec/cpu-common.h >>> * qemu/error-report.h >>> * qemu/qemu-print.h >>> >>> Though hw/core/cpu.h has been included by sysemu/hw_accel.h, to keep >>> the dependency clear, still directly include hw/core/cpu.h in this file. >>> >>> Tested by "./configure" and then "make". >>> >>> Signed-off-by: Zhao Liu <zhao1.liu@intel.com> >>> --- >>> hw/core/cpu-common.c | 7 +------ >>> 1 file changed, 1 insertion(+), 6 deletions(-) >> >> Something seems to be wrong with your analysis of what >> includes it is OK to drop. For instance, this file uses >> the function qemu_log(), which is why it includes >> qemu/log.h. >> > > I'm not sure about this, since qemu/log.h has been included by exec/log.h, > so could we just include exec/log.h and omit qemu/log.h in this file? We try to avoid implicit header inclusions, because if "exec/log.h" is reworked and "qemu/log.h" removed, then files using declarations implicitly declared start to fail building, and we need to clean unrelated files. > It seems enough for the compilation to omit qemu/log.h and only include > exec/log.h. > > Regards, > Zhao >
On Mon, Jan 15, 2024 at 05:07:52PM +0100, Philippe Mathieu-Daudé wrote: > Date: Mon, 15 Jan 2024 17:07:52 +0100 > From: Philippe Mathieu-Daudé <philmd@linaro.org> > Subject: Re: [PATCH 02/11] hw/core: Cleanup unused included headers in > cpu-common.c > > On 15/1/24 15:45, Zhao Liu wrote: > > Hi Peter, > > > > On Mon, Jan 15, 2024 at 10:41:48AM +0000, Peter Maydell wrote: > > > Date: Mon, 15 Jan 2024 10:41:48 +0000 > > > From: Peter Maydell <peter.maydell@linaro.org> > > > Subject: Re: [PATCH 02/11] hw/core: Cleanup unused included headers in > > > cpu-common.c > > > > > > On Mon, 15 Jan 2024 at 09:37, Zhao Liu <zhao1.liu@linux.intel.com> wrote: > > > > > > > > From: Zhao Liu <zhao1.liu@intel.com> > > > > > > > > Remove unused headers in cpu-common.c: > > > > * qemu/notify.h > > > > * qemu/log.h > > > > * qemu/main-loop.h > > > > * exec/cpu-common.h > > > > * qemu/error-report.h > > > > * qemu/qemu-print.h > > > > > > > > Though hw/core/cpu.h has been included by sysemu/hw_accel.h, to keep > > > > the dependency clear, still directly include hw/core/cpu.h in this file. > > > > > > > > Tested by "./configure" and then "make". > > > > > > > > Signed-off-by: Zhao Liu <zhao1.liu@intel.com> > > > > --- > > > > hw/core/cpu-common.c | 7 +------ > > > > 1 file changed, 1 insertion(+), 6 deletions(-) > > > > > > Something seems to be wrong with your analysis of what > > > includes it is OK to drop. For instance, this file uses > > > the function qemu_log(), which is why it includes > > > qemu/log.h. > > > > > > > I'm not sure about this, since qemu/log.h has been included by exec/log.h, > > so could we just include exec/log.h and omit qemu/log.h in this file? > > We try to avoid implicit header inclusions, because if "exec/log.h" is > reworked and "qemu/log.h" removed, then files using declarations > implicitly declared start to fail building, and we need to clean > unrelated files. Thanks! I see. Let me rework this series. Regards, Zhao
diff --git a/hw/core/cpu-common.c b/hw/core/cpu-common.c index 3ccfe882e2c3..bb21dfc03029 100644 --- a/hw/core/cpu-common.c +++ b/hw/core/cpu-common.c @@ -19,16 +19,11 @@ */ #include "qemu/osdep.h" + #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 "qemu/error-report.h" -#include "qemu/qemu-print.h" #include "sysemu/tcg.h" #include "hw/boards.h" #include "hw/qdev-properties.h"