Now that the schema can be configured, it is crucial that all types
are configured the same. Make sure config-host.h is included, so
build-sys tracks the dependency and rebuilds the types, by including
osdep.h first.

Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
 scripts/qapi/types.py | 1 +
 1 file changed, 1 insertion(+)

diff --git a/scripts/qapi/types.py b/scripts/qapi/types.py
index fd7808103c..91f87d0b8f 100644
--- a/scripts/qapi/types.py
+++ b/scripts/qapi/types.py
@@ -201,6 +201,7 @@ class QAPISchemaGenTypeVisitor(QAPISchemaModularCVisitor):
 ''',
                                       types=types, visit=visit))
         self._genh.preamble_add(mcgen('''
+#include "qemu/osdep.h"
 #include "qapi/qapi-builtin-types.h"
 '''))
 
-- 
2.18.0.rc1

Marc-André Lureau <marcandre.lureau@redhat.com> writes:

> Now that the schema can be configured, it is crucial that all types
> are configured the same. Make sure config-host.h is included, so
> build-sys tracks the dependency and rebuilds the types, by including
> osdep.h first.
>
> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> ---
>  scripts/qapi/types.py | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/scripts/qapi/types.py b/scripts/qapi/types.py
> index fd7808103c..91f87d0b8f 100644
> --- a/scripts/qapi/types.py
> +++ b/scripts/qapi/types.py
> @@ -201,6 +201,7 @@ class QAPISchemaGenTypeVisitor(QAPISchemaModularCVisitor):
>  ''',
>                                        types=types, visit=visit))
>          self._genh.preamble_add(mcgen('''
> +#include "qemu/osdep.h"
>  #include "qapi/qapi-builtin-types.h"
>  '''))

No.  Every .c must include qemu/osdep.h first.  No .h may include it.
We clean this up periodically.  scripts/clean-includes can help with
that.  We currently have a few offenders in the tree.

Hi

On Tue, Dec 4, 2018 at 7:23 PM Markus Armbruster <armbru@redhat.com> wrote:
>
> Marc-André Lureau <marcandre.lureau@redhat.com> writes:
>
> > Now that the schema can be configured, it is crucial that all types
> > are configured the same. Make sure config-host.h is included, so
> > build-sys tracks the dependency and rebuilds the types, by including
> > osdep.h first.
> >
> > Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> > ---
> >  scripts/qapi/types.py | 1 +
> >  1 file changed, 1 insertion(+)
> >
> > diff --git a/scripts/qapi/types.py b/scripts/qapi/types.py
> > index fd7808103c..91f87d0b8f 100644
> > --- a/scripts/qapi/types.py
> > +++ b/scripts/qapi/types.py
> > @@ -201,6 +201,7 @@ class QAPISchemaGenTypeVisitor(QAPISchemaModularCVisitor):
> >  ''',
> >                                        types=types, visit=visit))
> >          self._genh.preamble_add(mcgen('''
> > +#include "qemu/osdep.h"
> >  #include "qapi/qapi-builtin-types.h"
> >  '''))
>
> No.  Every .c must include qemu/osdep.h first.  No .h may include it.
> We clean this up periodically.  scripts/clean-includes can help with
> that.  We currently have a few offenders in the tree.

Ok, I don't know the reason an internal header couldn't include osdep,
could you explain?

I think we can replace osdep.h by config-host.h for the same result,
I'd have to check.

On 12/4/18 9:32 AM, Marc-André Lureau wrote:

>>> +++ b/scripts/qapi/types.py
>>> @@ -201,6 +201,7 @@ class QAPISchemaGenTypeVisitor(QAPISchemaModularCVisitor):
>>>   ''',
>>>                                         types=types, visit=visit))
>>>           self._genh.preamble_add(mcgen('''
>>> +#include "qemu/osdep.h"
>>>   #include "qapi/qapi-builtin-types.h"
>>>   '''))
>>
>> No.  Every .c must include qemu/osdep.h first.  No .h may include it.
>> We clean this up periodically.  scripts/clean-includes can help with
>> that.  We currently have a few offenders in the tree.
> 
> Ok, I don't know the reason an internal header couldn't include osdep,
> could you explain?

Per HACKING:

>> Do not include "qemu/osdep.h" from header files since the .c file will have
>> already included it.


That is, if we follow the rule that every .c file included osdep.h 
before anything else, then by the time any other internal .h is included 
(necessarily second or later) from a .c, then osdep.h is already active, 
so no .h needs to reproduce the effort that has already occurred.

> 
> I think we can replace osdep.h by config-host.h for the same result,
> I'd have to check.

osdep.h includes config-host.h, so you don't need that either.

What are you really trying to accomplish by including a header from 
another header?  And why isn't it already happening by our coding rule 
that all .c files include osdep.h first?

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

Marc-André Lureau <marcandre.lureau@redhat.com> writes:

> Hi
>
> On Tue, Dec 4, 2018 at 7:23 PM Markus Armbruster <armbru@redhat.com> wrote:
>>
>> Marc-André Lureau <marcandre.lureau@redhat.com> writes:
>>
>> > Now that the schema can be configured, it is crucial that all types
>> > are configured the same. Make sure config-host.h is included, so
>> > build-sys tracks the dependency and rebuilds the types, by including
>> > osdep.h first.
>> >
>> > Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
>> > ---
>> >  scripts/qapi/types.py | 1 +
>> >  1 file changed, 1 insertion(+)
>> >
>> > diff --git a/scripts/qapi/types.py b/scripts/qapi/types.py
>> > index fd7808103c..91f87d0b8f 100644
>> > --- a/scripts/qapi/types.py
>> > +++ b/scripts/qapi/types.py
>> > @@ -201,6 +201,7 @@ class QAPISchemaGenTypeVisitor(QAPISchemaModularCVisitor):
>> >  ''',
>> >                                        types=types, visit=visit))
>> >          self._genh.preamble_add(mcgen('''
>> > +#include "qemu/osdep.h"
>> >  #include "qapi/qapi-builtin-types.h"
>> >  '''))
>>
>> No.  Every .c must include qemu/osdep.h first.  No .h may include it.
>> We clean this up periodically.  scripts/clean-includes can help with
>> that.  We currently have a few offenders in the tree.
>
> Ok, I don't know the reason an internal header couldn't include osdep,
> could you explain?
>
> I think we can replace osdep.h by config-host.h for the same result,
> I'd have to check.

Quote ./HACKING:

1.2. Include directives

Order include directives as follows:

#include "qemu/osdep.h"  /* Always first... */
#include <...>           /* then system headers... */
#include "..."           /* and finally QEMU headers. */

The "qemu/osdep.h" header contains preprocessor macros that affect the behavior
of core system headers like <stdint.h>.  It must be the first include so that
core system headers included by external libraries get the preprocessor macros
that QEMU depends on.

Do not include "qemu/osdep.h" from header files since the .c file will have
already included it.