From 02a57ae32b08e8981b59979b80e682c9a153e94d Mon Sep 17 00:00:00 2001 From: Eric Blake Date: Wed, 17 Feb 2016 23:48:16 -0700 Subject: [PATCH] qapi: Forbid empty unions and useless alternates Empty unions serve no purpose, and while we compile with gcc which permits them, strict C99 forbids them. We happen to inject a dummy 'void *data' member into the C unions that represent QAPI unions and alternates, but we want to get rid of that member (it pollutes the namespace for no good reason), which would leave us with an empty union if the user didn't provide any branches. While empty structs make sense in QAPI, empty unions don't add any expressiveness to the QMP language. So prohibit them at parse time. Update the documentation and testsuite to match. Note that the documentation already mentioned that alternates should have "two or more JSON data types"; so this also fixes the code to enforce that. However, we have existing uses of a union type with only one branch, so the 2-or-more strictness is intentionally limited to alternates. Signed-off-by: Eric Blake Message-Id: <1455778109-6278-3-git-send-email-eblake@redhat.com> Signed-off-by: Markus Armbruster --- docs/qapi-code-gen.txt | 15 ++++++++------- scripts/qapi.py | 12 ++++++++++-- tests/qapi-schema/alternate-empty.err | 1 + tests/qapi-schema/alternate-empty.exit | 2 +- tests/qapi-schema/alternate-empty.json | 2 +- tests/qapi-schema/alternate-empty.out | 5 ----- tests/qapi-schema/flat-union-empty.err | 1 + tests/qapi-schema/flat-union-empty.exit | 2 +- tests/qapi-schema/flat-union-empty.json | 2 +- tests/qapi-schema/flat-union-empty.out | 9 --------- tests/qapi-schema/union-empty.err | 1 + tests/qapi-schema/union-empty.exit | 2 +- tests/qapi-schema/union-empty.json | 2 +- tests/qapi-schema/union-empty.out | 6 ------ 14 files changed, 27 insertions(+), 35 deletions(-) diff --git a/docs/qapi-code-gen.txt b/docs/qapi-code-gen.txt index 128f074a2de..999f3b98f02 100644 --- a/docs/qapi-code-gen.txt +++ b/docs/qapi-code-gen.txt @@ -187,11 +187,11 @@ prevent incomplete include files. Usage: { 'struct': STRING, 'data': DICT, '*base': STRUCT-NAME } -A struct is a dictionary containing a single 'data' key whose -value is a dictionary. This corresponds to a struct in C or an Object -in JSON. Each value of the 'data' dictionary must be the name of a -type, or a one-element array containing a type name. An example of a -struct is: +A struct is a dictionary containing a single 'data' key whose value is +a dictionary; the dictionary may be empty. This corresponds to a +struct in C or an Object in JSON. Each value of the 'data' dictionary +must be the name of a type, or a one-element array containing a type +name. An example of a struct is: { 'struct': 'MyType', 'data': { 'member1': 'str', 'member2': 'int', '*member3': 'str' } } @@ -288,9 +288,10 @@ or: { 'union': STRING, 'data': DICT, 'base': STRUCT-NAME, Union types are used to let the user choose between several different variants for an object. There are two flavors: simple (no -discriminator or base), flat (both discriminator and base). A union +discriminator or base), and flat (both discriminator and base). A union type is defined using a data dictionary as explained in the following -paragraphs. +paragraphs. The data dictionary for either type of union must not +be empty. A simple union type defines a mapping from automatic discriminator values to data types like in this example: diff --git a/scripts/qapi.py b/scripts/qapi.py index f40dc9e777d..f97236f5098 100644 --- a/scripts/qapi.py +++ b/scripts/qapi.py @@ -590,7 +590,10 @@ def check_union(expr, expr_info): "Discriminator '%s' must be of enumeration " "type" % discriminator) - # Check every branch + # Check every branch; don't allow an empty union + if len(members) == 0: + raise QAPIExprError(expr_info, + "Union '%s' cannot have empty 'data'" % name) for (key, value) in members.items(): check_name(expr_info, "Member of union '%s'" % name, key) @@ -613,7 +616,11 @@ def check_alternate(expr, expr_info): members = expr['data'] types_seen = {} - # Check every branch + # Check every branch; require at least two branches + if len(members) < 2: + raise QAPIExprError(expr_info, + "Alternate '%s' should have at least two branches " + "in 'data'" % name) for (key, value) in members.items(): check_name(expr_info, "Member of alternate '%s'" % name, key) @@ -1059,6 +1066,7 @@ class QAPISchemaObjectTypeVariants(object): assert bool(tag_member) != bool(tag_name) assert (isinstance(tag_name, str) or isinstance(tag_member, QAPISchemaObjectTypeMember)) + assert len(variants) > 0 for v in variants: assert isinstance(v, QAPISchemaObjectTypeVariant) self.tag_name = tag_name diff --git a/tests/qapi-schema/alternate-empty.err b/tests/qapi-schema/alternate-empty.err index e69de29bb2d..bb06c5bfec0 100644 --- a/tests/qapi-schema/alternate-empty.err +++ b/tests/qapi-schema/alternate-empty.err @@ -0,0 +1 @@ +tests/qapi-schema/alternate-empty.json:2: Alternate 'Alt' should have at least two branches in 'data' diff --git a/tests/qapi-schema/alternate-empty.exit b/tests/qapi-schema/alternate-empty.exit index 573541ac970..d00491fd7e5 100644 --- a/tests/qapi-schema/alternate-empty.exit +++ b/tests/qapi-schema/alternate-empty.exit @@ -1 +1 @@ -0 +1 diff --git a/tests/qapi-schema/alternate-empty.json b/tests/qapi-schema/alternate-empty.json index db3820f841a..fff15baf16c 100644 --- a/tests/qapi-schema/alternate-empty.json +++ b/tests/qapi-schema/alternate-empty.json @@ -1,2 +1,2 @@ -# FIXME - alternates should list at least two types to be useful +# alternates must list at least two types to be useful { 'alternate': 'Alt', 'data': { 'i': 'int' } } diff --git a/tests/qapi-schema/alternate-empty.out b/tests/qapi-schema/alternate-empty.out index f78f174111c..e69de29bb2d 100644 --- a/tests/qapi-schema/alternate-empty.out +++ b/tests/qapi-schema/alternate-empty.out @@ -1,5 +0,0 @@ -object :empty -alternate Alt - case i: int -enum QType ['none', 'qnull', 'qint', 'qstring', 'qdict', 'qlist', 'qfloat', 'qbool'] - prefix QTYPE diff --git a/tests/qapi-schema/flat-union-empty.err b/tests/qapi-schema/flat-union-empty.err index e69de29bb2d..15754f54eb9 100644 --- a/tests/qapi-schema/flat-union-empty.err +++ b/tests/qapi-schema/flat-union-empty.err @@ -0,0 +1 @@ +tests/qapi-schema/flat-union-empty.json:4: Union 'Union' cannot have empty 'data' diff --git a/tests/qapi-schema/flat-union-empty.exit b/tests/qapi-schema/flat-union-empty.exit index 573541ac970..d00491fd7e5 100644 --- a/tests/qapi-schema/flat-union-empty.exit +++ b/tests/qapi-schema/flat-union-empty.exit @@ -1 +1 @@ -0 +1 diff --git a/tests/qapi-schema/flat-union-empty.json b/tests/qapi-schema/flat-union-empty.json index 67dd2978eb1..77f1d9abfb8 100644 --- a/tests/qapi-schema/flat-union-empty.json +++ b/tests/qapi-schema/flat-union-empty.json @@ -1,4 +1,4 @@ -# FIXME - flat unions should not be empty +# flat unions cannot be empty { 'enum': 'Empty', 'data': [ ] } { 'struct': 'Base', 'data': { 'type': 'Empty' } } { 'union': 'Union', 'base': 'Base', 'discriminator': 'type', 'data': { } } diff --git a/tests/qapi-schema/flat-union-empty.out b/tests/qapi-schema/flat-union-empty.out index eade2d5ac6e..e69de29bb2d 100644 --- a/tests/qapi-schema/flat-union-empty.out +++ b/tests/qapi-schema/flat-union-empty.out @@ -1,9 +0,0 @@ -object :empty -object Base - member type: Empty optional=False -enum Empty [] -enum QType ['none', 'qnull', 'qint', 'qstring', 'qdict', 'qlist', 'qfloat', 'qbool'] - prefix QTYPE -object Union - base Base - tag type diff --git a/tests/qapi-schema/union-empty.err b/tests/qapi-schema/union-empty.err index e69de29bb2d..12c20221bdf 100644 --- a/tests/qapi-schema/union-empty.err +++ b/tests/qapi-schema/union-empty.err @@ -0,0 +1 @@ +tests/qapi-schema/union-empty.json:2: Union 'Union' cannot have empty 'data' diff --git a/tests/qapi-schema/union-empty.exit b/tests/qapi-schema/union-empty.exit index 573541ac970..d00491fd7e5 100644 --- a/tests/qapi-schema/union-empty.exit +++ b/tests/qapi-schema/union-empty.exit @@ -1 +1 @@ -0 +1 diff --git a/tests/qapi-schema/union-empty.json b/tests/qapi-schema/union-empty.json index 17850071133..1f0b13ca21a 100644 --- a/tests/qapi-schema/union-empty.json +++ b/tests/qapi-schema/union-empty.json @@ -1,2 +1,2 @@ -# FIXME - unions should not be empty +# unions cannot be empty { 'union': 'Union', 'data': { } } diff --git a/tests/qapi-schema/union-empty.out b/tests/qapi-schema/union-empty.out index bdf17e5b295..e69de29bb2d 100644 --- a/tests/qapi-schema/union-empty.out +++ b/tests/qapi-schema/union-empty.out @@ -1,6 +0,0 @@ -object :empty -enum QType ['none', 'qnull', 'qint', 'qstring', 'qdict', 'qlist', 'qfloat', 'qbool'] - prefix QTYPE -object Union - member type: UnionKind optional=False -enum UnionKind [] -- 2.39.5