From 907771516ec52a2c18e0ab9a01e2fc5939a23df8 Mon Sep 17 00:00:00 2001 From: Eric Blake Date: Tue, 20 Feb 2018 15:45:21 -0500 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. Backports commit 02a57ae32b08e8981b59979b80e682c9a153e94d from qemu --- qemu/scripts/qapi.py | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/qemu/scripts/qapi.py b/qemu/scripts/qapi.py index 85398d07..7ce32875 100644 --- a/qemu/scripts/qapi.py +++ b/qemu/scripts/qapi.py @@ -592,7 +592,11 @@ 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) @@ -615,7 +619,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) @@ -1062,6 +1070,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