[RFC,v2,06/12] qapi: Simplify visiting of alternate types

Previously, working with alternates required two enums, and
some indirection: for type Foo, we created Foo_qtypes[] which
maps each qtype to a member of FooKind_lookup[], then use
FooKind_lookup[] like we do for other union types.

This has a subtle bug: since the values of FooKind_lookup
start at zero, all entries of Foo_qtypes that were not
explicitly initialized map to the same branch of the union as
the first member of the alternate, rather than triggering a
failure in visit_get_next_type().  Fortunately, the bug
seldom bites; the very next thing the input visitor does is
try to parse the incoming JSON with the wrong parser, which
fails; the output visitor is not used with a C struct in that
state, and the dealloc visitor has nothing to clean up (so
there is no leak).

However, it IS observable in one case: the behavior of an
alternate that contains a 'number' member but no 'int' member
differs according to whether the 'number' was first in the
qapi definition, and when the input being parsed is an integer;
this is because the 'number' parser accepts QTYPE_QINT in
addition to the expected QTYPE_QFLOAT.  A later patch will worry
about fixing alternates to parse all inputs that a non-alternate
'number' would accept.

This patch fixes the validation bug by deleting the indirection,
and modifying get_next_type() to directly return a qtype code.
There is no longer a need to generate an implicit FooKind array
associated with the alternate type (since the QMP wire format
never uses the stringized counterparts of the C union member
names).  Next, the generated visitor is fixed to properly detect
unexpected qtypes in the switch statement.  Things got a bit
tricky with validating QAPISchemaObjectTypeVariants, which now
has three different initialization paths; but I didn't think it
was confusing enough to need to create different sub-classes.

Callers now have to know the QTYPE_* mapping when looking at the
discriminator; but so far, only the testsuite was even using the
C struct of an alternate types.  If that gets too confusing, we
could reintroduce FooKind, but initialize it differently than
most generated arrays, as in:
  typedef enum FooKind {
      FOO_KIND_A = QTYPE_QDICT,
      FOO_KIND_B = QTYPE_QINT,
  } FooKind;
to create nicer aliases for knowing when to use foo->a or foo->b
when inspecting foo->kind.  But without a current client, I
didn't see the point of doing it now.

There is a user-visible side effect to this change, but I
consider it to be an improvement. Previously,
the invalid QMP command:
  {"execute":"blockdev-add", "arguments":{"options":
    {"driver":"raw", "id":"a", "file":true}}}
failed with:
  {"error": {"class": "GenericError",
    "desc": "Invalid parameter type for 'file', expected: QDict"}}
Now it fails with:
  {"error": {"class": "GenericError",
    "desc": "Invalid parameter type for 'file', expected: BlockdevRef"}}

Signed-off-by: Eric Blake <eblake@redhat.com>
---
 docs/qapi-code-gen.txt                 |  3 ---
 include/qapi/visitor-impl.h            |  2 +-
 include/qapi/visitor.h                 |  2 +-
 qapi/qapi-visit-core.c                 |  4 ++--
 qapi/qmp-input-visitor.c               |  4 ++--
 scripts/qapi-types.py                  | 39 ++++++----------------------------
 scripts/qapi-visit.py                  | 12 ++++++-----
 scripts/qapi.py                        | 21 +++++++++++-------
 tests/qapi-schema/alternate-good.out   |  1 -
 tests/qapi-schema/qapi-schema-test.out |  8 -------
 tests/test-qmp-input-visitor.c         | 26 +++++++++++------------
 tests/test-qmp-output-visitor.c        | 21 +++++++++++++-----
 12 files changed, 61 insertions(+), 82 deletions(-)

[RFC,v2,06/12] qapi: Simplify visiting of alternate types

Commit Message

Patch