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"
+#include "hw/sysbus.h"
+
 #define TYPE_UNIMPLEMENTED_DEVICE "unimplemented-device"
 
 /**
-- 
2.11.0

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.

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org

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

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.

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org