qapi: Fix missing 'if' checks in struct, union, alternate 'data'

Commit 87adbbffd4..3e270dcacc "qapi: Add 'if' to (implicit
struct|union|alternate) members" (v4.0.0) neglected test coverage, and
promptly failed to check the conditions.  Review fail.

Recent commit "tests/qapi-schema: Demonstrate insufficient 'if'
checking" added test coverage, demonstrating the bug.  Fix it by add
the missing check_if().

Signed-off-by: Markus Armbruster <armbru@redhat.com>
Message-Id: <20190914153506.2151-13-armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>

diff --git a/scripts/qapi/common.py b/scripts/qapi/common.py
index 2b461648545d831876d5d82f17b4d17600e77494..cacee9b8bbac6e0af0eb71d3a63830b76118478f 100644 (file)
--- a/scripts/qapi/common.py
+++ b/scripts/qapi/common.py
@@ -803,6 +803,7 @@ def check_type(info, source, value,
         # an optional argument.
         check_known_keys(info, "member '%s' of %s" % (key, source),
                          arg, ['type'], ['if'])
+        check_if(arg, info)
         check_type(info, "Member '%s' of %s" % (key, source),
                    arg['type'], allow_array=True,
                    allow_metas=['built-in', 'union', 'alternate', 'struct',
@@ -902,6 +903,7 @@ def check_union(expr, info):
 
         check_known_keys(info, "member '%s' of union '%s'" % (key, name),
                          value, ['type'], ['if'])
+        check_if(value, info)
         # Each value must name a known type
         check_type(info, "Member '%s' of union '%s'" % (key, name),
                    value['type'],
@@ -930,6 +932,7 @@ def check_alternate(expr, info):
         check_known_keys(info,
                          "member '%s' of alternate '%s'" % (key, name),
                          value, ['type'], ['if'])
+        check_if(value, info)
         typ = value['type']
 
         # Ensure alternates have no type conflicts.

diff --git a/tests/qapi-schema/alternate-branch-if-invalid.err b/tests/qapi-schema/alternate-branch-if-invalid.err
index e69de29bb2d1d6434b8b29ae775ad8c2e48c5391..f1d6c10e001b12c47b022d1c3ad793de0ccb36f7 100644 (file)
--- a/tests/qapi-schema/alternate-branch-if-invalid.err
+++ b/tests/qapi-schema/alternate-branch-if-invalid.err
@@ -0,0 +1 @@
+tests/qapi-schema/alternate-branch-if-invalid.json:2: 'if' condition ' ' makes no sense

diff --git a/tests/qapi-schema/alternate-branch-if-invalid.exit b/tests/qapi-schema/alternate-branch-if-invalid.exit
index 573541ac9702dd3969c9bc859d2b91ec1f7e6e56..d00491fd7e5bb6fa28c517a0bb32b8b506539d4d 100644 (file)
--- a/tests/qapi-schema/alternate-branch-if-invalid.exit
+++ b/tests/qapi-schema/alternate-branch-if-invalid.exit
@@ -1 +1 @@
-0
+1

diff --git a/tests/qapi-schema/alternate-branch-if-invalid.json b/tests/qapi-schema/alternate-branch-if-invalid.json
index 6497f53475d8cb75e7bf65ba27784d624a17ca56..fea6d9080cf8887a50fc830361e7eabc370a61bd 100644 (file)
--- a/tests/qapi-schema/alternate-branch-if-invalid.json
+++ b/tests/qapi-schema/alternate-branch-if-invalid.json
@@ -1,4 +1,3 @@
 # Cover alternative with invalid 'if'
-# FIXME not rejected, would generate '#if  \n'
 { 'alternate': 'Alt',
   'data': { 'branch': { 'type': 'int', 'if': ' ' } } }

diff --git a/tests/qapi-schema/alternate-branch-if-invalid.out b/tests/qapi-schema/alternate-branch-if-invalid.out
index 89305d7f21d676b6d6249315aa5236f5e8fb7089..e69de29bb2d1d6434b8b29ae775ad8c2e48c5391 100644 (file)
--- a/tests/qapi-schema/alternate-branch-if-invalid.out
+++ b/tests/qapi-schema/alternate-branch-if-invalid.out
@@ -1,16 +0,0 @@
-module None
-object q_empty
-enum QType
-    prefix QTYPE
-    member none
-    member qnull
-    member qnum
-    member qstring
-    member qdict
-    member qlist
-    member qbool
-module alternate-branch-if-invalid.json
-alternate Alt
-    tag type
-    case branch: int
-        if [' ']

diff --git a/tests/qapi-schema/struct-member-if-invalid.err b/tests/qapi-schema/struct-member-if-invalid.err
index e69de29bb2d1d6434b8b29ae775ad8c2e48c5391..bfd65db97b9e1ed63e7d148d8811ff92b135f702 100644 (file)
--- a/tests/qapi-schema/struct-member-if-invalid.err
+++ b/tests/qapi-schema/struct-member-if-invalid.err
@@ -0,0 +1 @@
+tests/qapi-schema/struct-member-if-invalid.json:2: 'if' condition must be a string or a list of strings

diff --git a/tests/qapi-schema/struct-member-if-invalid.exit b/tests/qapi-schema/struct-member-if-invalid.exit
index 573541ac9702dd3969c9bc859d2b91ec1f7e6e56..d00491fd7e5bb6fa28c517a0bb32b8b506539d4d 100644 (file)
--- a/tests/qapi-schema/struct-member-if-invalid.exit
+++ b/tests/qapi-schema/struct-member-if-invalid.exit
@@ -1 +1 @@
-0
+1

diff --git a/tests/qapi-schema/struct-member-if-invalid.json b/tests/qapi-schema/struct-member-if-invalid.json
index 73987e04fc4c3487d0355786962bfaf8c28fa858..35078bd6600b4b28ae7e76250e9c04f195602ebf 100644 (file)
--- a/tests/qapi-schema/struct-member-if-invalid.json
+++ b/tests/qapi-schema/struct-member-if-invalid.json
@@ -1,4 +1,3 @@
 # Cover member with invalid 'if'
-# FIXME not rejected, would generate '#if True\n'
 { 'struct': 'Stru',
   'data': { 'member': { 'type': 'int', 'if': true } } }

diff --git a/tests/qapi-schema/struct-member-if-invalid.out b/tests/qapi-schema/struct-member-if-invalid.out
index 8fbb97985c91929cc480c744b9ed737404c55a02..e69de29bb2d1d6434b8b29ae775ad8c2e48c5391 100644 (file)
--- a/tests/qapi-schema/struct-member-if-invalid.out
+++ b/tests/qapi-schema/struct-member-if-invalid.out
@@ -1,15 +0,0 @@
-module None
-object q_empty
-enum QType
-    prefix QTYPE
-    member none
-    member qnull
-    member qnum
-    member qstring
-    member qdict
-    member qlist
-    member qbool
-module struct-member-if-invalid.json
-object Stru
-    member member: int optional=False
-        if [True]

diff --git a/tests/qapi-schema/union-branch-if-invalid.err b/tests/qapi-schema/union-branch-if-invalid.err
index e69de29bb2d1d6434b8b29ae775ad8c2e48c5391..607edee382ec419a07e5f7f2c8193f5df47ad7d7 100644 (file)
--- a/tests/qapi-schema/union-branch-if-invalid.err
+++ b/tests/qapi-schema/union-branch-if-invalid.err
@@ -0,0 +1 @@
+tests/qapi-schema/union-branch-if-invalid.json:4: 'if' condition '' makes no sense

diff --git a/tests/qapi-schema/union-branch-if-invalid.exit b/tests/qapi-schema/union-branch-if-invalid.exit
index 573541ac9702dd3969c9bc859d2b91ec1f7e6e56..d00491fd7e5bb6fa28c517a0bb32b8b506539d4d 100644 (file)
--- a/tests/qapi-schema/union-branch-if-invalid.exit
+++ b/tests/qapi-schema/union-branch-if-invalid.exit
@@ -1 +1 @@
-0
+1

diff --git a/tests/qapi-schema/union-branch-if-invalid.json b/tests/qapi-schema/union-branch-if-invalid.json
index 859b63b61069293eab545738f6a7c012c3e8eca9..46d4239af6f8a530fcb3b65dfb0ccaf2f4a6e6ec 100644 (file)
--- a/tests/qapi-schema/union-branch-if-invalid.json
+++ b/tests/qapi-schema/union-branch-if-invalid.json
@@ -1,5 +1,4 @@
 # Cover branch with invalid 'if'
-# FIXME not rejected, would generate '#if \n'
 { 'enum': 'Branches', 'data': ['branch1'] }
 { 'struct': 'Stru', 'data': { 'member': 'str' } }
 { 'union': 'Uni',

diff --git a/tests/qapi-schema/union-branch-if-invalid.out b/tests/qapi-schema/union-branch-if-invalid.out
index 2ed43218af0ce4c153cda3b024e824fda46e7fed..e69de29bb2d1d6434b8b29ae775ad8c2e48c5391 100644 (file)
--- a/tests/qapi-schema/union-branch-if-invalid.out
+++ b/tests/qapi-schema/union-branch-if-invalid.out
@@ -1,23 +0,0 @@
-module None
-object q_empty
-enum QType
-    prefix QTYPE
-    member none
-    member qnull
-    member qnum
-    member qstring
-    member qdict
-    member qlist
-    member qbool
-module union-branch-if-invalid.json
-enum Branches
-    member branch1
-object Stru
-    member member: str optional=False
-object q_obj_Uni-base
-    member tag: Branches optional=False
-object Uni
-    base q_obj_Uni-base
-    tag tag
-    case branch1: Stru
-        if ['']