Hi Michael, Peter. On 12/7/18 6:57 PM, Michael S. Tsirkin wrote: > On Fri, Dec 07, 2018 at 06:03:56PM +0100, Philippe Mathieu-Daudé wrote: >> Signed-off-by: Philippe Mathieu-Daudé <phi...@redhat.com> >> --- >> hw/arm/virt-acpi-build.c | 1 - >> 1 file changed, 1 deletion(-) >> >> diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c >> index 5785fb697c..98d7f7cf20 100644 >> --- a/hw/arm/virt-acpi-build.c >> +++ b/hw/arm/virt-acpi-build.c >> @@ -35,7 +35,6 @@ >> #include "target/arm/cpu.h" >> #include "hw/acpi/acpi-defs.h" >> #include "hw/acpi/acpi.h" >> -#include "hw/nvram/fw_cfg.h" >> #include "hw/acpi/bios-linker-loader.h" >> #include "hw/loader.h" >> #include "hw/hw.h" > > > This file uses fw_cfg_add_file, looks wrong.
Previously I thought if another include (here hw/loader.h) already pulls this header, we can drop it, but Peter Maydell explained me this not safe for refactors: Generally I think that if a .c file directly uses function X declared in header Y it should #include Y, even if it happens that it already includes other header Z that includes Y. Otherwise if we refactor Z later such that it no longer needs to include Y, it will break compilation of the .c file. (That is, Z including Y is a detail of the implementation of Z, not a guarantee made by Z to its users.) https://lists.gnu.org/archive/html/qemu-devel/2018-11/msg02460.html I'm sorry I forgot to update this in the series :/ Regards, Phil.