| Message ID | 20170508233918.9043-4-f4bug@amsat.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 05/08/2017 06:39 PM, Philippe Mathieu-Daudé wrote: > Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org> > --- > include/hw/misc/unimp.h | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/include/hw/misc/unimp.h b/include/hw/misc/unimp.h > index 3462d85836..353ee19abf 100644 > --- a/include/hw/misc/unimp.h > +++ b/include/hw/misc/unimp.h > @@ -8,6 +8,9 @@ > #ifndef HW_MISC_UNIMP_H > #define HW_MISC_UNIMP_H > > +#include "qemu/osdep.h" NACK. .h files should not include osdep.h, because the .c file that is using the .h file should have already done so. This is mentioned in HACKING.
On 05/08/2017 08:56 PM, Eric Blake wrote: > On 05/08/2017 06:39 PM, Philippe Mathieu-Daudé wrote: >> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org> >> --- >> include/hw/misc/unimp.h | 3 +++ >> 1 file changed, 3 insertions(+) >> >> diff --git a/include/hw/misc/unimp.h b/include/hw/misc/unimp.h >> index 3462d85836..353ee19abf 100644 >> --- a/include/hw/misc/unimp.h >> +++ b/include/hw/misc/unimp.h >> @@ -8,6 +8,9 @@ >> #ifndef HW_MISC_UNIMP_H >> #define HW_MISC_UNIMP_H >> >> +#include "qemu/osdep.h" > > NACK. .h files should not include osdep.h, because the .c file that is > using the .h file should have already done so. This is mentioned in > HACKING. Ok! Indeed my .c doesn't include osdep.h, sorry for the noise.
Hi Eric, On 05/08/2017 09:56 PM, Philippe Mathieu-Daudé wrote: > On 05/08/2017 08:56 PM, Eric Blake wrote: >> On 05/08/2017 06:39 PM, Philippe Mathieu-Daudé wrote: >>> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org> >>> --- >>> include/hw/misc/unimp.h | 3 +++ >>> 1 file changed, 3 insertions(+) >>> >>> diff --git a/include/hw/misc/unimp.h b/include/hw/misc/unimp.h >>> index 3462d85836..353ee19abf 100644 >>> --- a/include/hw/misc/unimp.h >>> +++ b/include/hw/misc/unimp.h >>> @@ -8,6 +8,9 @@ >>> #ifndef HW_MISC_UNIMP_H >>> #define HW_MISC_UNIMP_H >>> >>> +#include "qemu/osdep.h" >> >> NACK. .h files should not include osdep.h, because the .c file that is >> using the .h file should have already done so. This is mentioned in >> HACKING. > > Ok! Indeed my .c doesn't include osdep.h, sorry for the noise. I attentively read the HACKING after a good night's sleep. What about the other include, "hw/sysbus.h"? The inlined function create_unimplemented_device() calls sysbus_mmio_map_overlap(). Anyone willing to use "hw/misc/unimp.h" will get a compilation failure if he does not include "hw/sysbus.h" and I don't think it belongs to "qemu/osdep.h". Regards, Phil.
On 05/09/2017 12:20 PM, Philippe Mathieu-Daudé wrote: > What about the other include, "hw/sysbus.h"? > The inlined function create_unimplemented_device() calls > sysbus_mmio_map_overlap(). Anyone willing to use "hw/misc/unimp.h" will > get a compilation failure if he does not include "hw/sysbus.h" and I > don't think it belongs to "qemu/osdep.h". That one's probably okay.
On 05/08/2017 08:56 PM, Eric Blake wrote: >> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org> >> --- >> include/hw/misc/unimp.h | 3 +++ >> 1 file changed, 3 insertions(+) >> >> diff --git a/include/hw/misc/unimp.h b/include/hw/misc/unimp.h >> index 3462d85836..353ee19abf 100644 >> --- a/include/hw/misc/unimp.h >> +++ b/include/hw/misc/unimp.h >> @@ -8,6 +8,9 @@ >> #ifndef HW_MISC_UNIMP_H >> #define HW_MISC_UNIMP_H >> >> +#include "qemu/osdep.h" > > NACK. .h files should not include osdep.h, because the .c file that is > using the .h file should have already done so. This is mentioned in > HACKING. I'm not sure this kind of mistake happens often enough, but it might worth add a such test in checkpatch.pl (easier to say than to implement :p)
On 5 September 2017 at 10:35, Philippe Mathieu-Daudé <f4bug@amsat.org> wrote: > I'm not sure this kind of mistake happens often enough, but it might worth > add a such test in checkpatch.pl (easier to say than to implement :p) It doesn't run automatically, but you can run scripts/clean-includes on your new files to remove unneeded includes and add osdep.h if it's missing. thanks -- PMM
diff --git a/include/hw/misc/unimp.h b/include/hw/misc/unimp.h index 3462d85836..353ee19abf 100644 --- a/include/hw/misc/unimp.h +++ b/include/hw/misc/unimp.h @@ -8,6 +8,9 @@ #ifndef HW_MISC_UNIMP_H #define HW_MISC_UNIMP_H +#include "qemu/osdep.h" +#include "hw/sysbus.h" + #define TYPE_UNIMPLEMENTED_DEVICE "unimplemented-device" /**
Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org> --- include/hw/misc/unimp.h | 3 +++ 1 file changed, 3 insertions(+)