Patchwork #include cleanliness

login
register
mail settings
Submitter Avi Kivity
Date May 19, 2011, 2:17 p.m.
Message ID <1305814662-16184-1-git-send-email-avi@redhat.com>
Download mbox | patch
Permalink /patch/96397/
State New
Headers show

Comments

Avi Kivity - May 19, 2011, 2:17 p.m.
My mother always told me to explicitly #include any headers need to compile
a file, instead of relying on other #includes to bring them in.  This patch
fixes up targphys.h and cpu-common.h in this regard.

Signed-off-by: Avi Kivity <avi@redhat.com>
---
 cpu-common.h |    4 ++++
 targphys.h   |    2 ++
 2 files changed, 6 insertions(+), 0 deletions(-)
Avi Kivity - May 29, 2011, 4:30 p.m.
On 05/19/2011 05:17 PM, Avi Kivity wrote:
> My mother always told me to explicitly #include any headers need to compile
> a file, instead of relying on other #includes to bring them in.  This patch
> fixes up targphys.h and cpu-common.h in this regard.

Can some qemu maintainer review this patch?  Stefan?
Anthony Liguori - May 29, 2011, 10:06 p.m.
On 05/19/2011 09:17 AM, Avi Kivity wrote:
> My mother always told me to explicitly #include any headers need to compile
> a file, instead of relying on other #includes to bring them in.  This patch
> fixes up targphys.h and cpu-common.h in this regard.
>
> Signed-off-by: Avi Kivity<avi@redhat.com>
> ---
>   cpu-common.h |    4 ++++
>   targphys.h   |    2 ++
>   2 files changed, 6 insertions(+), 0 deletions(-)
>
> diff --git a/cpu-common.h b/cpu-common.h
> index 151c32c..2009adc 100644
> --- a/cpu-common.h
> +++ b/cpu-common.h
> @@ -18,6 +18,10 @@
>   #include "bswap.h"
>   #include "qemu-queue.h"
>
> +#include "qemu-common.h"
> +
> +#include<stdbool.h>

qemu-common.h should include stdbool.

The idea behind qemu-common.h is to avoid direct includes to help with 
portability.

Regards,

Anthony Liguori

> +
>   #if !defined(CONFIG_USER_ONLY)
>
>   enum device_endian {
> diff --git a/targphys.h b/targphys.h
> index 95648d6..501add2 100644
> --- a/targphys.h
> +++ b/targphys.h
> @@ -3,6 +3,8 @@
>   #ifndef TARGPHYS_H
>   #define TARGPHYS_H
>
> +#include<stdint.h>
> +
>   #ifdef TARGET_PHYS_ADDR_BITS
>   /* target_phys_addr_t is the type of a physical address (its size can
>      be different from 'target_ulong').  */
Avi Kivity - May 30, 2011, 9:06 a.m.
On 05/30/2011 01:06 AM, Anthony Liguori wrote:
> On 05/19/2011 09:17 AM, Avi Kivity wrote:
>> My mother always told me to explicitly #include any headers need to 
>> compile
>> a file, instead of relying on other #includes to bring them in.  This 
>> patch
>> fixes up targphys.h and cpu-common.h in this regard.
>>
>> Signed-off-by: Avi Kivity<avi@redhat.com>
>> ---
>>   cpu-common.h |    4 ++++
>>   targphys.h   |    2 ++
>>   2 files changed, 6 insertions(+), 0 deletions(-)
>>
>> diff --git a/cpu-common.h b/cpu-common.h
>> index 151c32c..2009adc 100644
>> --- a/cpu-common.h
>> +++ b/cpu-common.h
>> @@ -18,6 +18,10 @@
>>   #include "bswap.h"
>>   #include "qemu-queue.h"
>>
>> +#include "qemu-common.h"
>> +
>> +#include<stdbool.h>
>
> qemu-common.h should include stdbool.
>
> The idea behind qemu-common.h is to avoid direct includes to help with 
> portability.

Okay.  But note qemu-common.h #includes cpu.h #includes qemu-common.h...

I think osdep.h matches the "help with portability" label better, no?
Anthony Liguori - May 31, 2011, 1:43 p.m.
On 05/30/2011 04:06 AM, Avi Kivity wrote:
> On 05/30/2011 01:06 AM, Anthony Liguori wrote:
>> On 05/19/2011 09:17 AM, Avi Kivity wrote:
>>> My mother always told me to explicitly #include any headers need to
>>> compile
>>> a file, instead of relying on other #includes to bring them in. This
>>> patch
>>> fixes up targphys.h and cpu-common.h in this regard.
>>>
>>> Signed-off-by: Avi Kivity<avi@redhat.com>
>>> ---
>>> cpu-common.h | 4 ++++
>>> targphys.h | 2 ++
>>> 2 files changed, 6 insertions(+), 0 deletions(-)
>>>
>>> diff --git a/cpu-common.h b/cpu-common.h
>>> index 151c32c..2009adc 100644
>>> --- a/cpu-common.h
>>> +++ b/cpu-common.h
>>> @@ -18,6 +18,10 @@
>>> #include "bswap.h"
>>> #include "qemu-queue.h"
>>>
>>> +#include "qemu-common.h"
>>> +
>>> +#include<stdbool.h>
>>
>> qemu-common.h should include stdbool.
>>
>> The idea behind qemu-common.h is to avoid direct includes to help with
>> portability.
>
> Okay. But note qemu-common.h #includes cpu.h #includes qemu-common.h...

Fortunately, guards will prevent infinite recursion here :-)

> I think osdep.h matches the "help with portability" label better, no?

I don't disagree, but that's not the purpose of osdep.h today. 
qemu-common.h is a header file that's more or less supposed to be 
included by everything.

It should prevent explicit #include's for system headers.

Regards,

Anthony Liguori

Patch

diff --git a/cpu-common.h b/cpu-common.h
index 151c32c..2009adc 100644
--- a/cpu-common.h
+++ b/cpu-common.h
@@ -18,6 +18,10 @@ 
 #include "bswap.h"
 #include "qemu-queue.h"
 
+#include "qemu-common.h"
+
+#include <stdbool.h>
+
 #if !defined(CONFIG_USER_ONLY)
 
 enum device_endian {
diff --git a/targphys.h b/targphys.h
index 95648d6..501add2 100644
--- a/targphys.h
+++ b/targphys.h
@@ -3,6 +3,8 @@ 
 #ifndef TARGPHYS_H
 #define TARGPHYS_H
 
+#include <stdint.h>
+
 #ifdef TARGET_PHYS_ADDR_BITS
 /* target_phys_addr_t is the type of a physical address (its size can
    be different from 'target_ulong').  */