Commit e4ba22b3 authored by Eric Blake's avatar Eric Blake Committed by Markus Armbruster
Browse files

qapi: Finish converting to new qapi union layout 



We have two issues with our qapi union layout:
1) Even though the QMP wire format spells the tag 'type', the
C code spells it 'kind', requiring some hacks in the generator.
2) The C struct uses an anonymous union, which places all tag
values in the same namespace as all non-variant members. This
leads to spurious collisions if a tag value matches a non-variant
member's name.

This patch is the back end for a series that converts to a
saner qapi union layout.  Now that all clients have been
converted to use 'type' and 'obj->u.value', we can drop the
temporary parallel support for 'kind' and 'obj->value'.

Given a simple union qapi type:

{ 'union':'Foo', 'data': { 'a':'int', 'b':'bool' } }

this is the overall effect, when compared to the state before
this series of patches:

| struct Foo {
|-    FooKind kind;
|-    union { /* union tag is @kind */
|+    FooKind type;
|+    union { /* union tag is @type */
|         void *data;
|         int64_t a;
|         bool b;
|-    };
|+    } u;
| };

The testsuite still contains some examples of artificial restrictions
(see flat-union-clash-type.json, for example) that are no longer
technically necessary, now that there is no longer a collision between
enum tag values and non-variant member names; but fixing this will be
done in later patches, in part because some further changes are required
to keep QAPISchema*.check() from asserting.  Also, a later patch will
add a reservation for the member name 'u' to avoid a collision between a
user's non-variant names and our internal choice of C union name.

Note, however, that we do not rename the generated enum, which
is still 'FooKind'.  A further patch could generate implicit
enums as 'FooType', but while the generator already reserved
the '*Kind' namespace (commit 4dc2e690), there are already QMP
constructs with '*Type' naming, which means changing our
reservation namespace would have lots of churn to C code to
deal with a forced name change.

Signed-off-by: default avatarEric Blake <eblake@redhat.com>
Message-Id: <1445898903-12082-23-git-send-email-eblake@redhat.com>
[Commit message tweaked]
Signed-off-by: default avatarMarkus Armbruster <armbru@redhat.com>
parent ce21131a

scripts/qapi-types.py

+6 −22
Original line number Diff line number Diff line
@@ -149,23 +149,10 @@ struct %(c_name)s { 
    if base:

        ret += gen_struct_fields([], base)

    else:

        # TODO As a hack, we emit both 'kind' and 'type'. Ultimately, we

        # want to use only 'type', but the conversion is large enough to

        # require staging over several commits.

        ret += mcgen('''

    union {

        %(c_type)s kind;

        %(c_type)s type;

    };

''',

                     c_type=c_name(variants.tag_member.type.name))



    # TODO As a hack, we emit the union twice, once as an anonymous union

    # and once as a named union.  Ultimately, we want to use only the

    # named union version (as it avoids conflicts between tag values as

    # branch names competing with non-variant QMP names), but the conversion

    # is large enough to require staging over several commits.

    tmp = ''

        ret += gen_struct_field(variants.tag_member.name,

                                variants.tag_member.type,

                                False)



    # FIXME: What purpose does data serve, besides preventing a union that

    # has a branch named 'data'? We use it in qapi-visit.py to decide

    # whether to bypass the switch statement if visiting the discriminator
@@ -174,7 +161,7 @@ struct %(c_name)s { 
    # should not be any data leaks even without a data pointer.  Or, if

    # 'data' is merely added to guarantee we don't have an empty union,

    # shouldn't we enforce that at .json parse time?

    tmp += mcgen('''

    ret += mcgen('''

    union { /* union tag is @%(c_name)s */

        void *data;

''',
@@ -183,18 +170,15 @@ struct %(c_name)s { 
    for var in variants.variants:

        # Ugly special case for simple union TODO get rid of it

        typ = var.simple_union_type() or var.type

        tmp += mcgen('''

        ret += mcgen('''

        %(c_type)s %(c_name)s;

''',

                     c_type=typ.c_type(),

                     c_name=c_name(var.name))



    ret += tmp

    ret += '    ' + '\n    '.join(tmp.split('\n'))

    ret += mcgen('''

    } u;

};

};

''')



    return ret