From: Markus Armbruster <armbru@redhat.com>
To: qemu-devel@nongnu.org
Cc: marcandre.lureau@redhat.com, mdroth@linux.vnet.ibm.com
Subject: [Qemu-devel] [PATCH 12/19] qapi: Fix missing 'if' checks in struct, union, alternate 'data'
Date: Sat, 14 Sep 2019 17:34:59 +0200 [thread overview]
Message-ID: <20190914153506.2151-13-armbru@redhat.com> (raw)
In-Reply-To: <20190914153506.2151-1-armbru@redhat.com>
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>
---
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 ['']
--
2.21.0
next prev parent reply other threads:[~2019-09-14 15:40 UTC|newest]
Thread overview: 43+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-09-14 15:34 [Qemu-devel] [PATCH 00/19] qapi: Frontend fixes and cleanups Markus Armbruster
2019-09-14 15:34 ` [Qemu-devel] [PATCH 01/19] tests/qapi-schema: Cover unknown pragma Markus Armbruster
2019-09-17 16:36 ` Eric Blake
2019-09-14 15:34 ` [Qemu-devel] [PATCH 02/19] tests/qapi-schema: Delete two redundant tests Markus Armbruster
2019-09-17 16:57 ` Eric Blake
2019-09-14 15:34 ` [Qemu-devel] [PATCH 03/19] tests/qapi-schema: Demonstrate misleading optional tag error Markus Armbruster
2019-09-17 17:36 ` Eric Blake
2019-09-14 15:34 ` [Qemu-devel] [PATCH 04/19] tests/qapi-schema: Demonstrate broken discriminator errors Markus Armbruster
2019-09-17 17:43 ` Eric Blake
2019-09-14 15:34 ` [Qemu-devel] [PATCH 05/19] tests/qapi-schema: Demonstrate insufficient 'if' checking Markus Armbruster
2019-09-17 17:47 ` Eric Blake
2019-09-23 11:55 ` Markus Armbruster
2019-09-23 12:55 ` Eric Blake
2019-09-14 15:34 ` [Qemu-devel] [PATCH 06/19] tests/qapi-schema: Demonstrate suboptimal lexical errors Markus Armbruster
2019-09-17 17:49 ` Eric Blake
2019-09-14 15:34 ` [Qemu-devel] [PATCH 07/19] qapi: Use quotes more consistently in frontend error messages Markus Armbruster
2019-09-17 17:52 ` Eric Blake
2019-09-14 15:34 ` [Qemu-devel] [PATCH 08/19] qapi: Improve reporting of lexical errors Markus Armbruster
2019-09-17 17:54 ` Eric Blake
2019-09-14 15:34 ` [Qemu-devel] [PATCH 09/19] qapi: Remove null from schema language Markus Armbruster
2019-09-17 18:01 ` Eric Blake
2019-09-14 15:34 ` [Qemu-devel] [PATCH 10/19] qapi: Fix broken discriminator error messages Markus Armbruster
2019-09-17 18:05 ` Eric Blake
2019-09-14 15:34 ` [Qemu-devel] [PATCH 11/19] qapi: Reject blank 'if' conditions in addition to empty ones Markus Armbruster
2019-09-17 18:06 ` Eric Blake
2019-09-14 15:34 ` Markus Armbruster [this message]
2019-09-17 18:10 ` [Qemu-devel] [PATCH 12/19] qapi: Fix missing 'if' checks in struct, union, alternate 'data' Eric Blake
2019-09-14 15:35 ` [Qemu-devel] [PATCH 13/19] qapi: Normalize 'if' in check_exprs(), like other sugar Markus Armbruster
2019-09-17 18:14 ` Eric Blake
2019-09-23 11:59 ` Markus Armbruster
2019-09-14 15:35 ` [Qemu-devel] [PATCH 14/19] qapi: Simplify check_keys() Markus Armbruster
2019-09-17 19:01 ` Eric Blake
2019-09-14 15:35 ` [Qemu-devel] [PATCH 15/19] qapi: Clean up around check_known_keys() Markus Armbruster
2019-09-17 19:03 ` Eric Blake
2019-09-14 15:35 ` [Qemu-devel] [PATCH 16/19] qapi: Delete useless check_exprs() code for simple union kind Markus Armbruster
2019-09-17 19:05 ` Eric Blake
2019-09-14 15:35 ` [Qemu-devel] [PATCH 17/19] qapi: Fix to .check() empty structs just once Markus Armbruster
2019-09-17 19:06 ` Eric Blake
2019-09-14 15:35 ` [Qemu-devel] [PATCH 18/19] qapi: Fix excessive QAPISchemaEntity.check() recursion Markus Armbruster
2019-09-17 19:26 ` Eric Blake
2019-09-23 12:01 ` Markus Armbruster
2019-09-14 15:35 ` [Qemu-devel] [PATCH 19/19] qapi: Assert .visit() and .check_clash() run only after .check() Markus Armbruster
2019-09-17 19:27 ` Eric Blake
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20190914153506.2151-13-armbru@redhat.com \
--to=armbru@redhat.com \
--cc=marcandre.lureau@redhat.com \
--cc=mdroth@linux.vnet.ibm.com \
--cc=qemu-devel@nongnu.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).