From dec0012ef8d644b5dde1b68ee8dab3f8c12e0b6b Mon Sep 17 00:00:00 2001 From: Markus Armbruster Date: Sat, 14 Sep 2019 17:34:59 +0200 Subject: [PATCH] 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 Message-Id: <20190914153506.2151-13-armbru@redhat.com> Reviewed-by: Eric Blake --- scripts/qapi/common.py | 3 +++ .../alternate-branch-if-invalid.err | 1 + .../alternate-branch-if-invalid.exit | 2 +- .../alternate-branch-if-invalid.json | 1 - .../alternate-branch-if-invalid.out | 16 ------------- .../qapi-schema/struct-member-if-invalid.err | 1 + .../qapi-schema/struct-member-if-invalid.exit | 2 +- .../qapi-schema/struct-member-if-invalid.json | 1 - .../qapi-schema/struct-member-if-invalid.out | 15 ------------ tests/qapi-schema/union-branch-if-invalid.err | 1 + .../qapi-schema/union-branch-if-invalid.exit | 2 +- .../qapi-schema/union-branch-if-invalid.json | 1 - tests/qapi-schema/union-branch-if-invalid.out | 23 ------------------- 13 files changed, 9 insertions(+), 60 deletions(-) diff --git a/scripts/qapi/common.py b/scripts/qapi/common.py index 2b46164854..cacee9b8bb 100644 --- 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 e69de29bb2..f1d6c10e00 100644 --- 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 573541ac97..d00491fd7e 100644 --- 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 6497f53475..fea6d9080c 100644 --- 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 89305d7f21..e69de29bb2 100644 --- 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 e69de29bb2..bfd65db97b 100644 --- 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 573541ac97..d00491fd7e 100644 --- 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 73987e04fc..35078bd660 100644 --- 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 8fbb97985c..e69de29bb2 100644 --- 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 e69de29bb2..607edee382 100644 --- 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 573541ac97..d00491fd7e 100644 --- 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 859b63b610..46d4239af6 100644 --- 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 2ed43218af..e69de29bb2 100644 --- 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 ['']