[v8,10/17] 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, for now it is still marked FIXME.

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); that also means we no longer have a collision with an
alternate branch named 'max'.  Next, the generated visitor is
fixed to properly detect unexpected qtypes in the switch
statement.  This is done via the use of a new
QAPISchemaAlternateTypeTag subclass and the use of a new
member.c_type() method when producing qapi-types.  The new
subtype also allows us to clean up a TODO left in the previous
commit.

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->type.  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>

---
v8: no change
v7: rebase onto earlier changes, rework how subtype makes things work
v6: rebase onto tag_member subclass, testsuite, gen_err_check(),
and info improvements
---
 docs/qapi-code-gen.txt                  |  3 ---
 include/qapi/visitor-impl.h             |  3 ++-
 include/qapi/visitor.h                  |  8 +++++-
 qapi/qapi-visit-core.c                  |  4 +--
 qapi/qmp-input-visitor.c                |  4 +--
 scripts/qapi-types.py                   | 36 +--------------------------
 scripts/qapi-visit.py                   | 12 +++++----
 scripts/qapi.py                         | 43 ++++++++++++++++++++++-----------
 tests/qapi-schema/alternate-empty.out   |  1 -
 tests/qapi-schema/qapi-schema-test.json |  2 +-
 tests/qapi-schema/qapi-schema-test.out  | 10 +-------
 tests/test-qmp-input-visitor.c          | 33 +++++++++++++------------
 tests/test-qmp-output-visitor.c         | 21 ++++++++++++----
 13 files changed, 86 insertions(+), 94 deletions(-)

[v8,10/17] qapi: Simplify visiting of alternate types

Commit Message

Comments

Patch