Patchwork Error: Fix build when qemu-common.h is not included

login
register
mail settings
Submitter Luiz Capitulino
Date July 7, 2011, 4:02 p.m.
Message ID <20110707130255.76f47cd0@doriath>
Download mbox | patch
Permalink /patch/103690/
State New
Headers show

Comments

Luiz Capitulino - July 7, 2011, 4:02 p.m.
Commit e4ea5e2d0e0e4c5188ab45b66f3195062ae059dc added the use of
the macro GCC_FMT_ATTR to error.h, however qemu-common.h is not
included by error.h

This will cause a build error when files including error.h
don't include qemu-common.h. Not an issue today because the only
file including it is json-parser.h and it does include
qemu-common.h, but let's fix it.

Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
---
 error.h |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)
Michael Roth - July 7, 2011, 4:17 p.m.
On 07/07/2011 11:02 AM, Luiz Capitulino wrote:
> Commit e4ea5e2d0e0e4c5188ab45b66f3195062ae059dc added the use of
> the macro GCC_FMT_ATTR to error.h, however qemu-common.h is not
> included by error.h
>
> This will cause a build error when files including error.h
> don't include qemu-common.h. Not an issue today because the only
> file including it is json-parser.h and it does include
> qemu-common.h, but let's fix it.
>
> Signed-off-by: Luiz Capitulino<lcapitulino@redhat.com>
> ---
>   error.h |    2 +-
>   1 files changed, 1 insertions(+), 1 deletions(-)
>
> diff --git a/error.h b/error.h
> index 0f92a6f..6e3bd98 100644
> --- a/error.h
> +++ b/error.h
> @@ -12,7 +12,7 @@
>   #ifndef ERROR_H
>   #define ERROR_H
>
> -#include<stdbool.h>
> +#include "qemu-common.h"
>
>   /**
>    * A class representing internal errors within QEMU.  An error has a string

Tested-by: Michael Roth <mdroth@linux.vnet.ibm.com>
Stefan Weil - July 7, 2011, 4:50 p.m.
Am 07.07.2011 18:17, schrieb Michael Roth:
> On 07/07/2011 11:02 AM, Luiz Capitulino wrote:
>> Commit e4ea5e2d0e0e4c5188ab45b66f3195062ae059dc added the use of
>> the macro GCC_FMT_ATTR to error.h, however qemu-common.h is not
>> included by error.h
>>
>> This will cause a build error when files including error.h
>> don't include qemu-common.h. Not an issue today because the only
>> file including it is json-parser.h and it does include
>> qemu-common.h, but let's fix it.
>>
>> Signed-off-by: Luiz Capitulino<lcapitulino@redhat.com>
>> ---
>>   error.h |    2 +-
>>   1 files changed, 1 insertions(+), 1 deletions(-)

The same argument could be applied to more QEMU *.h files
which also work only after qemu-common.h. Otherwise, including
qemu-common.h in *.c files would rarely be needed.

As far as I remember, the *.h files used to be more self-contained
some years ago, but then the strategy changed and central files
like qemu-common.h were introduced.

I personally prefer self-contained include files like error.h
(before my patch and after your patch), but I'm afraid that
the QEMU way is different.

Cheers,
Stefan W.
Luiz Capitulino - July 7, 2011, 5:32 p.m.
On Thu, 07 Jul 2011 18:50:28 +0200
Stefan Weil <weil@mail.berlios.de> wrote:

> Am 07.07.2011 18:17, schrieb Michael Roth:
> > On 07/07/2011 11:02 AM, Luiz Capitulino wrote:
> >> Commit e4ea5e2d0e0e4c5188ab45b66f3195062ae059dc added the use of
> >> the macro GCC_FMT_ATTR to error.h, however qemu-common.h is not
> >> included by error.h
> >>
> >> This will cause a build error when files including error.h
> >> don't include qemu-common.h. Not an issue today because the only
> >> file including it is json-parser.h and it does include
> >> qemu-common.h, but let's fix it.
> >>
> >> Signed-off-by: Luiz Capitulino<lcapitulino@redhat.com>
> >> ---
> >>   error.h |    2 +-
> >>   1 files changed, 1 insertions(+), 1 deletions(-)
> 
> The same argument could be applied to more QEMU *.h files
> which also work only after qemu-common.h. Otherwise, including
> qemu-common.h in *.c files would rarely be needed.
> 
> As far as I remember, the *.h files used to be more self-contained
> some years ago, but then the strategy changed and central files
> like qemu-common.h were introduced.
> 
> I personally prefer self-contained include files like error.h
> (before my patch and after your patch), but I'm afraid that
> the QEMU way is different.

The current way is broken. Some qemu header files are a complete mess.
qemu-common.h and sysemu.h are good examples. They include too many
things, if you change them a lot of code will be recompiled.

I could make the problem less worse by moving all compiler related
macros to a compiler.h header and then include it in qemu-common.h
and error.h. This way error.h won't be affected, although the real
problem will remain unsolved.

Patch

diff --git a/error.h b/error.h
index 0f92a6f..6e3bd98 100644
--- a/error.h
+++ b/error.h
@@ -12,7 +12,7 @@ 
 #ifndef ERROR_H
 #define ERROR_H
 
-#include <stdbool.h>
+#include "qemu-common.h"
 
 /**
  * A class representing internal errors within QEMU.  An error has a string