[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH for-4.0 v7 01/27] qapi: make sure osdep.h is inc
From: |
Marc-André Lureau |
Subject: |
Re: [Qemu-devel] [PATCH for-4.0 v7 01/27] qapi: make sure osdep.h is included in type headers |
Date: |
Mon, 10 Dec 2018 15:13:47 +0400 |
Hi
On Mon, Dec 10, 2018 at 1:52 PM Markus Armbruster <address@hidden> wrote:
>
> Marc-André Lureau <address@hidden> 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, by
> > checking osdep.h inclusion. The build-sys tracks the dependency and
> > rebuilds the types if the configuration changed.
> >
> > Signed-off-by: Marc-André Lureau <address@hidden>
> > ---
> > scripts/qapi/types.py | 3 +++
> > 1 file changed, 3 insertions(+)
> >
> > diff --git a/scripts/qapi/types.py b/scripts/qapi/types.py
> > index fd7808103c..3bb9cb6d47 100644
> > --- a/scripts/qapi/types.py
> > +++ b/scripts/qapi/types.py
> > @@ -201,6 +201,9 @@ class
> > QAPISchemaGenTypeVisitor(QAPISchemaModularCVisitor):
> > ''',
> > types=types, visit=visit))
> > self._genh.preamble_add(mcgen('''
> > +#ifndef QEMU_OSDEP_H
> > +#error Please include osdep.h first!
> > +#endif
> > #include "qapi/qapi-builtin-types.h"
> > '''))
>
> I understand why you propose this patch. The QAPI-generated headers use
> #ifdef CONFIG_FOO. The configuration header "qemu/osdep.h" must be
> consistently included before the generated headers, or else you end up
> with nasty bugs, such as the same enum having different values in
> different .o, or the same struct having a different layout.
>
> But this applies to *all* headers that use #ifdef. Why check it here,
> but not there? What makes the QAPI-generated headers special?
>
It's the discussion about #if in headers (and enums in particular)
that started this. We want to make sure all compilation units share
the same data structure/ABI. I proposed to include osdep.h in qapi
headers, but that was rejected.
The warning is a different approach. I agree it could apply to all
headers. Do you think I should update all headers as well?
--
Marc-André Lureau
[Qemu-devel] [PATCH for-4.0 v7 02/27] qapi: do not define enumeration value explicitly, Marc-André Lureau, 2018/12/08
[Qemu-devel] [PATCH for-4.0 v7 03/27] qapi: rename QAPISchemaEnumType.values to .members, Marc-André Lureau, 2018/12/08
[Qemu-devel] [PATCH for-4.0 v7 04/27] qapi: change enum visitor and gen_enum* to take QAPISchemaMember, Marc-André Lureau, 2018/12/08