On 10/08/2015 06:26 AM, Markus Armbruster wrote: > Struct and union type members are generally named alike in QMP and C, > except for a simple union's implicit tag member, which is "type" in > QMP, and "kind" in C. Can't change QMP, so rename it in C. > > Since the implicit enumeration type is still called *Kind, this > doesn't clean up the type vs. kind mess completely. > > Signed-off-by: Markus Armbruster <arm...@redhat.com> > --- ... > scripts/qapi-types.py | 12 ++++-------- > scripts/qapi-visit.py | 15 ++++----------- > tests/test-qmp-commands.c | 2 +- > tests/test-qmp-input-visitor.c | 30 +++++++++++++++--------------- > tests/test-qmp-output-visitor.c | 8 ++++---- > tpm.c | 2 +- > ui/input-keymap.c | 10 +++++----- > ui/input-legacy.c | 2 +- > ui/input.c | 22 +++++++++++----------- > util/qemu-sockets.c | 12 ++++++------ > 33 files changed, 113 insertions(+), 123 deletions(-)
This touches a lot of files all in one commit (both your RFC and my v5
version), and then I get to touch the same files all over again when I
swap to a named rather than anonymous union in the C struct. So here's
what I'm currently playing with:
first patch: hack _just_ scripts/qapi*py to turn:
{ 'union':'Foo', 'data':{'a':'int','b':'bool'}}
into:
struct Foo {
union {
FooKind type;
FooKind kind; /* temporary hack */
};
union { /* union tag is @type */
void *data; /* will go away much later in series */
int64_t a;
bool b;
union {
void *data;
int64_t a;
int b;
} u;
};
};
so that old code accessing foo->kind and foo->a just works, but also
leaving the door open to access foo->type and foo->u.a. Then a series
of patches grouped by logical file sets (so no one patch is too huge to
review, and spreading the load among maintainers), and a final patch to
scripts/qapi*.py to get to the desired:
struct Foo {
FooKind type;
union { /* union tag is @type */
void *data; /* will go away much later in series */
int64_t a;
bool b;
} u;
};
at which point we've gotten rid of any collisions between tag value
(branch names) and QMP names, at the possible expense of a new collision
with 'u'. I'm also beefing up the testsuite and check_name() function
(or maybe somewhere else, still haven't actually written that part of my
planned series) to reject 'u' and anything spelled 'has_*' as member
names, to proactively avoid the need to worry about collisions with 'u'
or the added members for optional fields. I'll probably also reject
'*List' as a user-supplied type name, addressing the comment just barely
added in current qapi-next that we don't have a reserved namespace for
arrays.
The collision with 'data' is harder; I can't remove it until we delete
visit_start_union()/visit_end_union() which starts to get in the mess of
patches that work with qapi visitor interfaces, so that will be in a
later subset.
It may be a day or two before I can post the pending work. Meanwhile, I
previously posted subset C which is somewhat orthogonal (not sure if it
needs any minor rebasing to apply against current qapi-next), if you
want to dive into reviewing that, instead of waiting:
https://lists.gnu.org/archive/html/qemu-devel/2015-10/msg01980.html
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature
