* [PULL 00/37] QAPI patches for 2019-09-24
@ 2019-09-24 12:32 Markus Armbruster
2019-09-24 12:32 ` [PULL 01/37] qapi: Make visit_next_list()'s comment less confusing Markus Armbruster
` (39 more replies)
0 siblings, 40 replies; 41+ messages in thread
From: Markus Armbruster @ 2019-09-24 12:32 UTC (permalink / raw)
To: qemu-devel
The following changes since commit 2f93a3ecdd3bb060bd04f698ccafe66efd98563a:
Merge remote-tracking branch 'remotes/davidhildenbrand/tags/s390x-tcg-2019-09-23' into staging (2019-09-23 15:44:52 +0100)
are available in the Git repository at:
git://repo.or.cz/qemu/armbru.git tags/pull-qapi-2019-09-24
for you to fetch changes up to 56176412e7fcfae1c69e4426d94e856b75358231:
qapi: Assert .visit() and .check_clash() run only after .check() (2019-09-24 14:07:23 +0200)
----------------------------------------------------------------
QAPI patches for 2019-09-24
----------------------------------------------------------------
Andrey Shinkevich (1):
make check-unit: use after free in test-opts-visitor
Markus Armbruster (36):
qapi: Make visit_next_list()'s comment less confusing
scripts/git.orderfile: Match QAPI schema more precisely
qapi: Drop check_type()'s redundant parameter @allow_optional
qapi: Drop support for boxed alternate arguments
docs/devel/qapi-code-gen: Minor specification fixes
tests/qapi-schema: Demonstrate bad reporting of funny characters
qapi: Restrict strings to printable ASCII
qapi: Drop support for escape sequences other than \\
qapi: Permit 'boxed' with empty type
qapi: Permit alternates with just one branch
qapi: Permit omitting all flat union branches
qapi: Adjust frontend errors to say enum value, not member
docs/devel/qapi-code-gen: Reorder sections for readability
docs/devel/qapi-code-gen: Rewrite compatibility considerations
docs/devel/qapi-code-gen: Rewrite introduction to schema
docs/devel/qapi-code-gen: Improve QAPI schema language doc
qapi: Tweak code to match docs/devel/qapi-code-gen.txt
tests/qapi-schema: Cover unknown pragma
tests/qapi-schema: Delete two redundant tests
tests/qapi-schema: Demonstrate misleading optional tag error
tests/qapi-schema: Demonstrate broken discriminator errors
tests/qapi-schema: Demonstrate insufficient 'if' checking
tests/qapi-schema: Demonstrate suboptimal lexical errors
qapi: Use quotes more consistently in frontend error messages
qapi: Improve reporting of lexical errors
qapi: Remove null from schema language
qapi: Fix broken discriminator error messages
qapi: Reject blank 'if' conditions in addition to empty ones
qapi: Fix missing 'if' checks in struct, union, alternate 'data'
qapi: Normalize 'if' in check_exprs(), like other sugar
qapi: Simplify check_keys()
qapi: Clean up around check_known_keys()
qapi: Delete useless check_exprs() code for simple union kind
qapi: Fix to .check() empty structs just once
qapi: Fix excessive QAPISchemaEntity.check() recursion
qapi: Assert .visit() and .check_clash() run only after .check()
docs/devel/qapi-code-gen.txt | 1075 +++++++++++---------
include/qapi/visitor.h | 8 +-
qapi/opts-visitor.c | 26 +-
tests/test-qmp-cmds.c | 4 +
scripts/git.orderfile | 4 +-
scripts/qapi/commands.py | 4 +-
scripts/qapi/common.py | 380 ++++---
scripts/qapi/events.py | 12 +-
tests/Makefile.include | 16 +-
tests/qapi-schema/alternate-branch-if-invalid.err | 1 +
...empty.exit => alternate-branch-if-invalid.exit} | 0
tests/qapi-schema/alternate-branch-if-invalid.json | 3 +
...d-empty.out => alternate-branch-if-invalid.out} | 0
tests/qapi-schema/alternate-empty.err | 2 +-
tests/qapi-schema/alternate-empty.json | 4 +-
tests/qapi-schema/args-boxed-empty.err | 1 -
tests/qapi-schema/args-boxed-empty.json | 3 -
tests/qapi-schema/args-invalid.err | 2 +-
tests/qapi-schema/bad-if-list.err | 2 +-
tests/qapi-schema/bad-if-list.json | 2 +-
tests/qapi-schema/bad-type-int.err | 2 +-
tests/qapi-schema/bad-type-int.json | 2 +-
tests/qapi-schema/doc-missing-colon.err | 2 +-
tests/qapi-schema/doc-missing.err | 2 +-
tests/qapi-schema/doc-no-symbol.err | 2 +-
tests/qapi-schema/double-data.err | 1 -
tests/qapi-schema/double-data.json | 2 -
tests/qapi-schema/duplicate-key.err | 2 +-
tests/qapi-schema/duplicate-key.json | 2 +-
tests/qapi-schema/enum-bad-name.err | 2 +-
tests/qapi-schema/enum-bad-name.json | 3 +-
tests/qapi-schema/enum-clash-member.err | 2 +-
tests/qapi-schema/enum-dict-member-unknown.err | 2 +-
tests/qapi-schema/enum-int-member.err | 2 +-
tests/qapi-schema/enum-member-case.err | 2 +-
tests/qapi-schema/escape-outside-string.err | 2 +-
tests/qapi-schema/escape-outside-string.json | 3 -
tests/qapi-schema/escape-too-big.err | 1 -
tests/qapi-schema/escape-too-big.json | 3 -
tests/qapi-schema/escape-too-short.err | 1 -
tests/qapi-schema/escape-too-short.json | 3 -
tests/qapi-schema/features-if-invalid.err | 1 +
.../{double-data.exit => features-if-invalid.exit} | 0
tests/qapi-schema/features-if-invalid.json | 4 +
.../{double-data.out => features-if-invalid.out} | 0
.../flat-union-discriminator-bad-name.err | 1 +
...exit => flat-union-discriminator-bad-name.exit} | 0
.../flat-union-discriminator-bad-name.json | 11 +
...g.out => flat-union-discriminator-bad-name.out} | 0
tests/qapi-schema/flat-union-empty.err | 2 +-
tests/qapi-schema/flat-union-empty.json | 2 +-
.../flat-union-invalid-discriminator.err | 2 +-
.../flat-union-invalid-discriminator.json | 5 +-
.../flat-union-invalid-if-discriminator.err | 2 +-
.../flat-union-invalid-if-discriminator.json | 5 +-
.../flat-union-optional-discriminator.err | 2 +-
.../flat-union-optional-discriminator.json | 3 +-
tests/qapi-schema/funny-char.err | 2 +-
tests/qapi-schema/funny-word.err | 1 +
.../{escape-too-big.exit => funny-word.exit} | 0
tests/qapi-schema/funny-word.json | 1 +
.../{escape-too-big.out => funny-word.out} | 0
tests/qapi-schema/ident-with-escape.err | 1 +
tests/qapi-schema/ident-with-escape.exit | 2 +-
tests/qapi-schema/ident-with-escape.json | 2 +-
tests/qapi-schema/ident-with-escape.out | 16 -
tests/qapi-schema/include-before-err.err | 2 +-
tests/qapi-schema/include-format-err.err | 1 -
tests/qapi-schema/include-format-err.json | 2 -
tests/qapi-schema/include-nested-err.err | 2 +-
tests/qapi-schema/leading-comma-list.err | 2 +-
tests/qapi-schema/leading-comma-object.err | 2 +-
tests/qapi-schema/missing-colon.err | 2 +-
tests/qapi-schema/missing-comma-list.err | 2 +-
tests/qapi-schema/missing-comma-object.err | 2 +-
tests/qapi-schema/non-objects.err | 2 +-
.../pragma-name-case-whitelist-crap.json | 2 +-
tests/qapi-schema/pragma-non-dict.err | 2 +-
tests/qapi-schema/pragma-unknown.err | 1 +
.../{escape-too-short.exit => pragma-unknown.exit} | 0
tests/qapi-schema/pragma-unknown.json | 1 +
.../{escape-too-short.out => pragma-unknown.out} | 0
tests/qapi-schema/qapi-schema-test.json | 13 +-
tests/qapi-schema/qapi-schema-test.out | 21 +-
tests/qapi-schema/quoted-structural-chars.err | 2 +-
tests/qapi-schema/string-code-point-127.err | 1 +
...-format-err.exit => string-code-point-127.exit} | 0
tests/qapi-schema/string-code-point-127.json | 2 +
...de-format-err.out => string-code-point-127.out} | 0
tests/qapi-schema/string-code-point-31.err | 1 +
...{unicode-str.exit => string-code-point-31.exit} | 0
tests/qapi-schema/string-code-point-31.json | 2 +
.../{unicode-str.out => string-code-point-31.out} | 0
tests/qapi-schema/struct-data-invalid.err | 2 +-
tests/qapi-schema/struct-member-if-invalid.err | 1 +
tests/qapi-schema/struct-member-if-invalid.exit | 1 +
tests/qapi-schema/struct-member-if-invalid.json | 3 +
tests/qapi-schema/struct-member-if-invalid.out | 0
tests/qapi-schema/trailing-comma-list.err | 2 +-
tests/qapi-schema/unclosed-list.err | 2 +-
tests/qapi-schema/unclosed-object.err | 2 +-
tests/qapi-schema/unicode-str.err | 1 -
tests/qapi-schema/unicode-str.json | 2 -
tests/qapi-schema/union-base-empty.err | 2 +-
tests/qapi-schema/union-branch-if-invalid.err | 1 +
tests/qapi-schema/union-branch-if-invalid.exit | 1 +
tests/qapi-schema/union-branch-if-invalid.json | 6 +
tests/qapi-schema/union-branch-if-invalid.out | 0
tests/qapi-schema/union-empty.err | 2 +-
tests/qapi-schema/union-empty.json | 2 +-
tests/qapi-schema/unknown-escape.json | 2 +-
111 files changed, 971 insertions(+), 785 deletions(-)
create mode 100644 tests/qapi-schema/alternate-branch-if-invalid.err
rename tests/qapi-schema/{args-boxed-empty.exit => alternate-branch-if-invalid.exit} (100%)
create mode 100644 tests/qapi-schema/alternate-branch-if-invalid.json
rename tests/qapi-schema/{args-boxed-empty.out => alternate-branch-if-invalid.out} (100%)
delete mode 100644 tests/qapi-schema/args-boxed-empty.err
delete mode 100644 tests/qapi-schema/args-boxed-empty.json
delete mode 100644 tests/qapi-schema/double-data.err
delete mode 100644 tests/qapi-schema/double-data.json
delete mode 100644 tests/qapi-schema/escape-outside-string.json
delete mode 100644 tests/qapi-schema/escape-too-big.err
delete mode 100644 tests/qapi-schema/escape-too-big.json
delete mode 100644 tests/qapi-schema/escape-too-short.err
delete mode 100644 tests/qapi-schema/escape-too-short.json
create mode 100644 tests/qapi-schema/features-if-invalid.err
rename tests/qapi-schema/{double-data.exit => features-if-invalid.exit} (100%)
create mode 100644 tests/qapi-schema/features-if-invalid.json
rename tests/qapi-schema/{double-data.out => features-if-invalid.out} (100%)
create mode 100644 tests/qapi-schema/flat-union-discriminator-bad-name.err
rename tests/qapi-schema/{escape-outside-string.exit => flat-union-discriminator-bad-name.exit} (100%)
create mode 100644 tests/qapi-schema/flat-union-discriminator-bad-name.json
rename tests/qapi-schema/{escape-outside-string.out => flat-union-discriminator-bad-name.out} (100%)
create mode 100644 tests/qapi-schema/funny-word.err
rename tests/qapi-schema/{escape-too-big.exit => funny-word.exit} (100%)
create mode 100644 tests/qapi-schema/funny-word.json
rename tests/qapi-schema/{escape-too-big.out => funny-word.out} (100%)
delete mode 100644 tests/qapi-schema/include-format-err.err
delete mode 100644 tests/qapi-schema/include-format-err.json
create mode 100644 tests/qapi-schema/pragma-unknown.err
rename tests/qapi-schema/{escape-too-short.exit => pragma-unknown.exit} (100%)
create mode 100644 tests/qapi-schema/pragma-unknown.json
rename tests/qapi-schema/{escape-too-short.out => pragma-unknown.out} (100%)
create mode 100644 tests/qapi-schema/string-code-point-127.err
rename tests/qapi-schema/{include-format-err.exit => string-code-point-127.exit} (100%)
create mode 100644 tests/qapi-schema/string-code-point-127.json
rename tests/qapi-schema/{include-format-err.out => string-code-point-127.out} (100%)
create mode 100644 tests/qapi-schema/string-code-point-31.err
rename tests/qapi-schema/{unicode-str.exit => string-code-point-31.exit} (100%)
create mode 100644 tests/qapi-schema/string-code-point-31.json
rename tests/qapi-schema/{unicode-str.out => string-code-point-31.out} (100%)
create mode 100644 tests/qapi-schema/struct-member-if-invalid.err
create mode 100644 tests/qapi-schema/struct-member-if-invalid.exit
create mode 100644 tests/qapi-schema/struct-member-if-invalid.json
create mode 100644 tests/qapi-schema/struct-member-if-invalid.out
delete mode 100644 tests/qapi-schema/unicode-str.err
delete mode 100644 tests/qapi-schema/unicode-str.json
create mode 100644 tests/qapi-schema/union-branch-if-invalid.err
create mode 100644 tests/qapi-schema/union-branch-if-invalid.exit
create mode 100644 tests/qapi-schema/union-branch-if-invalid.json
create mode 100644 tests/qapi-schema/union-branch-if-invalid.out
--
2.21.0
^ permalink raw reply [flat|nested] 41+ messages in thread
* [PULL 01/37] qapi: Make visit_next_list()'s comment less confusing
2019-09-24 12:32 [PULL 00/37] QAPI patches for 2019-09-24 Markus Armbruster
@ 2019-09-24 12:32 ` Markus Armbruster
2019-09-24 12:32 ` [PULL 02/37] make check-unit: use after free in test-opts-visitor Markus Armbruster
` (38 subsequent siblings)
39 siblings, 0 replies; 41+ messages in thread
From: Markus Armbruster @ 2019-09-24 12:32 UTC (permalink / raw)
To: qemu-devel
visit_next_list() returns non-null on success, null on failure. The
comment's phrasing "until NULL return or error occurs" is needlessly
confusing. Scratch the "or error occurs" part.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Message-Id: <20190802122325.16520-1-armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
---
include/qapi/visitor.h | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/include/qapi/visitor.h b/include/qapi/visitor.h
index 5b2ed3f202..c5b23851a1 100644
--- a/include/qapi/visitor.h
+++ b/include/qapi/visitor.h
@@ -364,10 +364,10 @@ void visit_start_list(Visitor *v, const char *name, GenericList **list,
* @tail must not be NULL; on the first call, @tail is the value of
* *list after visit_start_list(), and on subsequent calls @tail must
* be the previously returned value. Should be called in a loop until
- * a NULL return or error occurs; for each non-NULL return, the caller
- * then calls the appropriate visit_type_*() for the element type of
- * the list, with that function's name parameter set to NULL and obj
- * set to the address of @tail->value.
+ * a NULL return; for each non-NULL return, the caller then calls the
+ * appropriate visit_type_*() for the element type of the list, with
+ * that function's name parameter set to NULL and obj set to the
+ * address of @tail->value.
*/
GenericList *visit_next_list(Visitor *v, GenericList *tail, size_t size);
--
2.21.0
^ permalink raw reply related [flat|nested] 41+ messages in thread
* [PULL 02/37] make check-unit: use after free in test-opts-visitor
2019-09-24 12:32 [PULL 00/37] QAPI patches for 2019-09-24 Markus Armbruster
2019-09-24 12:32 ` [PULL 01/37] qapi: Make visit_next_list()'s comment less confusing Markus Armbruster
@ 2019-09-24 12:32 ` Markus Armbruster
2019-09-24 12:33 ` [PULL 03/37] scripts/git.orderfile: Match QAPI schema more precisely Markus Armbruster
` (37 subsequent siblings)
39 siblings, 0 replies; 41+ messages in thread
From: Markus Armbruster @ 2019-09-24 12:32 UTC (permalink / raw)
To: qemu-devel; +Cc: Andrey Shinkevich
From: Andrey Shinkevich <andrey.shinkevich@virtuozzo.com>
In the struct OptsVisitor, the 'repeated_opts' member points to a list
in the 'unprocessed_opts' hash table after the list has been destroyed.
A subsequent call to visit_type_int() references the deleted list.
It results in use-after-free issue reproduced by running the test case
under the Valgrind: valgrind tests/test-opts-visitor.
A new mode ListMode::LM_TRAVERSED is declared to mark the list
traversal completed.
Suggested-by: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Andrey Shinkevich <andrey.shinkevich@virtuozzo.com>
Message-Id: <1565024586-387112-1-git-send-email-andrey.shinkevich@virtuozzo.com>
---
qapi/opts-visitor.c | 26 ++++++++++++++++++++++----
1 file changed, 22 insertions(+), 4 deletions(-)
diff --git a/qapi/opts-visitor.c b/qapi/opts-visitor.c
index 324b197495..5fe0276c1c 100644
--- a/qapi/opts-visitor.c
+++ b/qapi/opts-visitor.c
@@ -24,7 +24,8 @@ enum ListMode
{
LM_NONE, /* not traversing a list of repeated options */
- LM_IN_PROGRESS, /* opts_next_list() ready to be called.
+ LM_IN_PROGRESS, /*
+ * opts_next_list() ready to be called.
*
* Generating the next list link will consume the most
* recently parsed QemuOpt instance of the repeated
@@ -36,7 +37,8 @@ enum ListMode
* LM_UNSIGNED_INTERVAL.
*/
- LM_SIGNED_INTERVAL, /* opts_next_list() has been called.
+ LM_SIGNED_INTERVAL, /*
+ * opts_next_list() has been called.
*
* Generating the next list link will consume the most
* recently stored element from the signed interval,
@@ -48,7 +50,14 @@ enum ListMode
* next element of the signed interval.
*/
- LM_UNSIGNED_INTERVAL /* Same as above, only for an unsigned interval. */
+ LM_UNSIGNED_INTERVAL, /* Same as above, only for an unsigned interval. */
+
+ LM_TRAVERSED /*
+ * opts_next_list() has been called.
+ *
+ * No more QemuOpt instance in the list.
+ * The traversal has been completed.
+ */
};
typedef enum ListMode ListMode;
@@ -238,6 +247,8 @@ opts_next_list(Visitor *v, GenericList *tail, size_t size)
OptsVisitor *ov = to_ov(v);
switch (ov->list_mode) {
+ case LM_TRAVERSED:
+ return NULL;
case LM_SIGNED_INTERVAL:
case LM_UNSIGNED_INTERVAL:
if (ov->list_mode == LM_SIGNED_INTERVAL) {
@@ -258,6 +269,8 @@ opts_next_list(Visitor *v, GenericList *tail, size_t size)
opt = g_queue_pop_head(ov->repeated_opts);
if (g_queue_is_empty(ov->repeated_opts)) {
g_hash_table_remove(ov->unprocessed_opts, opt->name);
+ ov->repeated_opts = NULL;
+ ov->list_mode = LM_TRAVERSED;
return NULL;
}
break;
@@ -289,7 +302,8 @@ opts_end_list(Visitor *v, void **obj)
assert(ov->list_mode == LM_IN_PROGRESS ||
ov->list_mode == LM_SIGNED_INTERVAL ||
- ov->list_mode == LM_UNSIGNED_INTERVAL);
+ ov->list_mode == LM_UNSIGNED_INTERVAL ||
+ ov->list_mode == LM_TRAVERSED);
ov->repeated_opts = NULL;
ov->list_mode = LM_NONE;
}
@@ -306,6 +320,10 @@ lookup_scalar(const OptsVisitor *ov, const char *name, Error **errp)
list = lookup_distinct(ov, name, errp);
return list ? g_queue_peek_tail(list) : NULL;
}
+ if (ov->list_mode == LM_TRAVERSED) {
+ error_setg(errp, "Fewer list elements than expected");
+ return NULL;
+ }
assert(ov->list_mode == LM_IN_PROGRESS);
return g_queue_peek_head(ov->repeated_opts);
}
--
2.21.0
^ permalink raw reply related [flat|nested] 41+ messages in thread
* [PULL 03/37] scripts/git.orderfile: Match QAPI schema more precisely
2019-09-24 12:32 [PULL 00/37] QAPI patches for 2019-09-24 Markus Armbruster
2019-09-24 12:32 ` [PULL 01/37] qapi: Make visit_next_list()'s comment less confusing Markus Armbruster
2019-09-24 12:32 ` [PULL 02/37] make check-unit: use after free in test-opts-visitor Markus Armbruster
@ 2019-09-24 12:33 ` Markus Armbruster
2019-09-24 12:33 ` [PULL 04/37] qapi: Drop check_type()'s redundant parameter @allow_optional Markus Armbruster
` (36 subsequent siblings)
39 siblings, 0 replies; 41+ messages in thread
From: Markus Armbruster @ 2019-09-24 12:33 UTC (permalink / raw)
To: qemu-devel; +Cc: Philippe Mathieu-Daudé
Pattern *.json also matches the tests/qapi-schema/*.json. Separates
them from the tests/qapi-schema/*.{err,exit,out} in diffs. I hate
that. Change the pattern to match just the "real" QAPI schemata.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Message-Id: <20190913201349.24332-2-armbru@redhat.com>
---
scripts/git.orderfile | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/scripts/git.orderfile b/scripts/git.orderfile
index ac699700b1..e89790941c 100644
--- a/scripts/git.orderfile
+++ b/scripts/git.orderfile
@@ -19,11 +19,11 @@ Makefile*
*.mak
# qapi schema
-*.json
+qapi/*.json
+qga/*.json
# headers
*.h
# code
*.c
-
--
2.21.0
^ permalink raw reply related [flat|nested] 41+ messages in thread
* [PULL 04/37] qapi: Drop check_type()'s redundant parameter @allow_optional
2019-09-24 12:32 [PULL 00/37] QAPI patches for 2019-09-24 Markus Armbruster
` (2 preceding siblings ...)
2019-09-24 12:33 ` [PULL 03/37] scripts/git.orderfile: Match QAPI schema more precisely Markus Armbruster
@ 2019-09-24 12:33 ` Markus Armbruster
2019-09-24 12:33 ` [PULL 05/37] qapi: Drop support for boxed alternate arguments Markus Armbruster
` (35 subsequent siblings)
39 siblings, 0 replies; 41+ messages in thread
From: Markus Armbruster @ 2019-09-24 12:33 UTC (permalink / raw)
To: qemu-devel
check_type() uses @allow_optional only when @value is a dictionary and
@allow_dict is True. All callers that pass allow_dict=True also pass
allow_optional=True.
Therefore, @allow_optional is always True when check_type() uses it.
Drop the redundant parameter.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Message-Id: <20190913201349.24332-3-armbru@redhat.com>
---
scripts/qapi/common.py | 17 ++++++++---------
1 file changed, 8 insertions(+), 9 deletions(-)
diff --git a/scripts/qapi/common.py b/scripts/qapi/common.py
index d61bfdc526..9aefcfe015 100644
--- a/scripts/qapi/common.py
+++ b/scripts/qapi/common.py
@@ -783,9 +783,8 @@ def check_if(expr, info):
check_if_str(ifcond, info)
-def check_type(info, source, value, allow_array=False,
- allow_dict=False, allow_optional=False,
- allow_metas=[]):
+def check_type(info, source, value,
+ allow_array=False, allow_dict=False, allow_metas=[]):
global all_names
if value is None:
@@ -821,7 +820,7 @@ def check_type(info, source, value, allow_array=False,
# value is a dictionary, check that each member is okay
for (key, arg) in value.items():
check_name(info, "Member of %s" % source, key,
- allow_optional=allow_optional)
+ allow_optional=True)
if c_name(key, False) == 'u' or c_name(key, False).startswith('has_'):
raise QAPISemError(info, "Member of %s uses reserved name '%s'"
% (source, key))
@@ -843,14 +842,14 @@ def check_command(expr, info):
if boxed:
args_meta += ['union', 'alternate']
check_type(info, "'data' for command '%s'" % name,
- expr.get('data'), allow_dict=not boxed, allow_optional=True,
+ expr.get('data'), allow_dict=not boxed,
allow_metas=args_meta)
returns_meta = ['union', 'struct']
if name in returns_whitelist:
returns_meta += ['built-in', 'alternate', 'enum']
check_type(info, "'returns' for command '%s'" % name,
expr.get('returns'), allow_array=True,
- allow_optional=True, allow_metas=returns_meta)
+ allow_metas=returns_meta)
def check_event(expr, info):
@@ -861,7 +860,7 @@ def check_event(expr, info):
if boxed:
meta += ['union', 'alternate']
check_type(info, "'data' for event '%s'" % name,
- expr.get('data'), allow_dict=not boxed, allow_optional=True,
+ expr.get('data'), allow_dict=not boxed,
allow_metas=meta)
@@ -889,7 +888,7 @@ def check_union(expr, info):
else:
# The object must have a string or dictionary 'base'.
check_type(info, "'base' for union '%s'" % name,
- base, allow_dict=True, allow_optional=True,
+ base, allow_dict=True,
allow_metas=['struct'])
if not base:
raise QAPISemError(info, "Flat union '%s' must have a base"
@@ -1012,7 +1011,7 @@ def check_struct(expr, info):
features = expr.get('features')
check_type(info, "'data' for struct '%s'" % name, members,
- allow_dict=True, allow_optional=True)
+ allow_dict=True)
check_type(info, "'base' for struct '%s'" % name, expr.get('base'),
allow_metas=['struct'])
--
2.21.0
^ permalink raw reply related [flat|nested] 41+ messages in thread
* [PULL 05/37] qapi: Drop support for boxed alternate arguments
2019-09-24 12:32 [PULL 00/37] QAPI patches for 2019-09-24 Markus Armbruster
` (3 preceding siblings ...)
2019-09-24 12:33 ` [PULL 04/37] qapi: Drop check_type()'s redundant parameter @allow_optional Markus Armbruster
@ 2019-09-24 12:33 ` Markus Armbruster
2019-09-24 12:33 ` [PULL 06/37] docs/devel/qapi-code-gen: Minor specification fixes Markus Armbruster
` (34 subsequent siblings)
39 siblings, 0 replies; 41+ messages in thread
From: Markus Armbruster @ 2019-09-24 12:33 UTC (permalink / raw)
To: qemu-devel
Commands and events can define their argument type inline (default) or
by referring to another type ('boxed': true, since commit c818408e44
"qapi: Implement boxed types for commands/events", v2.7.0). The
unboxed inline definition is an (anonymous) struct type. The boxed
type may be a struct, union, or alternate type.
The latter is problematic: docs/interop/qemu-spec.txt requires the
value of the 'data' key to be a json-object, but any non-degenerate
alternate type has at least one branch that isn't.
Fortunately, we haven't made use of alternates in this context outside
tests/. Drop support for them.
QAPISchemaAlternateType.is_empty() is now unused. Drop it, too.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Message-Id: <20190913201349.24332-4-armbru@redhat.com>
---
docs/devel/qapi-code-gen.txt | 12 ++++++------
scripts/qapi/common.py | 15 ++++-----------
tests/qapi-schema/qapi-schema-test.json | 2 +-
tests/qapi-schema/qapi-schema-test.out | 2 +-
4 files changed, 12 insertions(+), 19 deletions(-)
diff --git a/docs/devel/qapi-code-gen.txt b/docs/devel/qapi-code-gen.txt
index e8ec8ac1de..3d3931fb7a 100644
--- a/docs/devel/qapi-code-gen.txt
+++ b/docs/devel/qapi-code-gen.txt
@@ -612,9 +612,9 @@ the command. Normally, 'data' is a dictionary for an anonymous type,
or names a struct type (possibly empty, but not a union), and its
members are passed as separate arguments to this function. If the
command definition includes a key 'boxed' with the boolean value true,
-then 'data' is instead the name of any non-empty complex type
-(struct, union, or alternate), and a pointer to that QAPI type is
-passed as a single argument.
+then 'data' is instead the name of any non-empty complex type (struct
+or union), and a pointer to that QAPI type is passed as a single
+argument.
The generator also emits a marshalling function that extracts
arguments for the user's function out of an input QDict, calls the
@@ -714,9 +714,9 @@ The generator emits a function to send the event. Normally, 'data' is
a dictionary for an anonymous type, or names a struct type (possibly
empty, but not a union), and its members are passed as separate
arguments to this function. If the event definition includes a key
-'boxed' with the boolean value true, then 'data' is instead the name of
-any non-empty complex type (struct, union, or alternate), and a
-pointer to that QAPI type is passed as a single argument.
+'boxed' with the boolean value true, then 'data' is instead the name
+of any non-empty complex type (struct or union), and a pointer to that
+QAPI type is passed as a single argument.
=== Features ===
diff --git a/scripts/qapi/common.py b/scripts/qapi/common.py
index 9aefcfe015..54d02458b5 100644
--- a/scripts/qapi/common.py
+++ b/scripts/qapi/common.py
@@ -840,7 +840,7 @@ def check_command(expr, info):
args_meta = ['struct']
if boxed:
- args_meta += ['union', 'alternate']
+ args_meta += ['union']
check_type(info, "'data' for command '%s'" % name,
expr.get('data'), allow_dict=not boxed,
allow_metas=args_meta)
@@ -858,7 +858,7 @@ def check_event(expr, info):
meta = ['struct']
if boxed:
- meta += ['union', 'alternate']
+ meta += ['union']
check_type(info, "'data' for event '%s'" % name,
expr.get('data'), allow_dict=not boxed,
allow_metas=meta)
@@ -1690,9 +1690,6 @@ class QAPISchemaAlternateType(QAPISchemaType):
visitor.visit_alternate_type(self.name, self.info, self.ifcond,
self.variants)
- def is_empty(self):
- return False
-
class QAPISchemaCommand(QAPISchemaEntity):
def __init__(self, name, info, doc, ifcond, arg_type, ret_type,
@@ -1714,15 +1711,13 @@ class QAPISchemaCommand(QAPISchemaEntity):
QAPISchemaEntity.check(self, schema)
if self._arg_type_name:
self.arg_type = schema.lookup_type(self._arg_type_name)
- assert (isinstance(self.arg_type, QAPISchemaObjectType) or
- isinstance(self.arg_type, QAPISchemaAlternateType))
+ assert isinstance(self.arg_type, QAPISchemaObjectType)
self.arg_type.check(schema)
if self.boxed:
if self.arg_type.is_empty():
raise QAPISemError(self.info,
"Cannot use 'boxed' with empty type")
else:
- assert not isinstance(self.arg_type, QAPISchemaAlternateType)
assert not self.arg_type.variants
elif self.boxed:
raise QAPISemError(self.info, "Use of 'boxed' requires 'data'")
@@ -1750,15 +1745,13 @@ class QAPISchemaEvent(QAPISchemaEntity):
QAPISchemaEntity.check(self, schema)
if self._arg_type_name:
self.arg_type = schema.lookup_type(self._arg_type_name)
- assert (isinstance(self.arg_type, QAPISchemaObjectType) or
- isinstance(self.arg_type, QAPISchemaAlternateType))
+ assert isinstance(self.arg_type, QAPISchemaObjectType)
self.arg_type.check(schema)
if self.boxed:
if self.arg_type.is_empty():
raise QAPISemError(self.info,
"Cannot use 'boxed' with empty type")
else:
- assert not isinstance(self.arg_type, QAPISchemaAlternateType)
assert not self.arg_type.variants
elif self.boxed:
raise QAPISemError(self.info, "Use of 'boxed' requires 'data'")
diff --git a/tests/qapi-schema/qapi-schema-test.json b/tests/qapi-schema/qapi-schema-test.json
index c6d59acc3e..0fadb4ddd7 100644
--- a/tests/qapi-schema/qapi-schema-test.json
+++ b/tests/qapi-schema/qapi-schema-test.json
@@ -180,7 +180,7 @@
{ 'event': 'EVENT_D',
'data': { 'a' : 'EventStructOne', 'b' : 'str', '*c': 'str', '*enum3': 'EnumOne' } }
{ 'event': 'EVENT_E', 'boxed': true, 'data': 'UserDefZero' }
-{ 'event': 'EVENT_F', 'boxed': true, 'data': 'UserDefAlternate' }
+{ 'event': 'EVENT_F', 'boxed': true, 'data': 'UserDefFlatUnion' }
# test that we correctly compile downstream extensions, as well as munge
# ticklish names
diff --git a/tests/qapi-schema/qapi-schema-test.out b/tests/qapi-schema/qapi-schema-test.out
index 85d510bc00..5470a525f5 100644
--- a/tests/qapi-schema/qapi-schema-test.out
+++ b/tests/qapi-schema/qapi-schema-test.out
@@ -252,7 +252,7 @@ event EVENT_D q_obj_EVENT_D-arg
boxed=False
event EVENT_E UserDefZero
boxed=True
-event EVENT_F UserDefAlternate
+event EVENT_F UserDefFlatUnion
boxed=True
enum __org.qemu_x-Enum
member __org.qemu_x-value
--
2.21.0
^ permalink raw reply related [flat|nested] 41+ messages in thread
* [PULL 06/37] docs/devel/qapi-code-gen: Minor specification fixes
2019-09-24 12:32 [PULL 00/37] QAPI patches for 2019-09-24 Markus Armbruster
` (4 preceding siblings ...)
2019-09-24 12:33 ` [PULL 05/37] qapi: Drop support for boxed alternate arguments Markus Armbruster
@ 2019-09-24 12:33 ` Markus Armbruster
2019-09-24 12:33 ` [PULL 07/37] tests/qapi-schema: Demonstrate bad reporting of funny characters Markus Armbruster
` (33 subsequent siblings)
39 siblings, 0 replies; 41+ messages in thread
From: Markus Armbruster @ 2019-09-24 12:33 UTC (permalink / raw)
To: qemu-devel
The specification claims "Each expression that isn't an include
directive may be preceded by a documentation block", but the code also
rejects them for pragma directives. The code is correct. Fix the
specification.
The specification reserves member names starting with 'has_', but the
code also reserves name 'u'. Fix the specification.
The specification claims "The string 'max' is not allowed as an enum
value". Untrue. Fix the specification. While there, delete the
naming advice, because it's redundant with the naming rules in section
"Schema overview"
The specification claims "No branch of the union can be named 'max',
as this would collide with the implicit enum". Untrue. Fix the
specification.
The specification claims "It is not allowed to name an event 'MAX',
since the generator also produces a C enumeration of all event names
with a generated _MAX value at the end." Untrue. Fix the
specification.
The specification claims "All branches of the union must be complex
types", but the code permits only struct types. The code is correct.
Fix the specification.
The specification claims a command's return type "must be the string
name of a complex or built-in type, a one-element array containing the
name of a complex or built-in type" unless the command is in pragma
'returns-whitelist'. The code does not permit built-in types. Fix
the specification.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Message-Id: <20190913201349.24332-5-armbru@redhat.com>
---
docs/devel/qapi-code-gen.txt | 34 ++++++++++++++++------------------
1 file changed, 16 insertions(+), 18 deletions(-)
diff --git a/docs/devel/qapi-code-gen.txt b/docs/devel/qapi-code-gen.txt
index 3d3931fb7a..4ce67752a7 100644
--- a/docs/devel/qapi-code-gen.txt
+++ b/docs/devel/qapi-code-gen.txt
@@ -117,9 +117,9 @@ Example:
==== Expression documentation ====
-Each expression that isn't an include directive may be preceded by a
-documentation block. Such blocks are called expression documentation
-blocks.
+Expressions other than include and pragma directives may be preceded
+by a documentation block. Such blocks are called expression
+documentation blocks.
When documentation is required (see pragma 'doc-required'), expression
documentation blocks are mandatory.
@@ -243,8 +243,9 @@ underscore.
Event names should be ALL_CAPS with words separated by underscore.
-Member names starting with 'has-' or 'has_' are reserved for the
-generator, which uses them for tracking optional members.
+Member name 'u' and names starting with 'has-' or 'has_' are reserved
+for the generator, which uses them for unions and for tracking
+optional members.
Any name (command, event, type, member, or enum value) beginning with
"x-" is marked experimental, and may be withdrawn or changed
@@ -460,15 +461,14 @@ discriminator value, as in these examples:
The generated C code uses a struct containing a union. Additionally,
an implicit C enum 'NameKind' is created, corresponding to the union
-'Name', for accessing the various branches of the union. No branch of
-the union can be named 'max', as this would collide with the implicit
-enum. The value for each branch can be of any type.
+'Name', for accessing the various branches of the union. The value
+for each branch can be of any type.
A flat union definition avoids nesting on the wire, and specifies a
set of common members that occur in all variants of the union. The
'base' key must specify either a type name (the type must be a
struct, not a union), or a dictionary representing an anonymous type.
-All branches of the union must be complex types, and the top-level
+All branches of the union must be struct types, and the top-level
members of the union dictionary on the wire will be combination of
members from both the base type and the appropriate branch type (when
merging two dictionaries, there must be no keys in common). The
@@ -578,8 +578,8 @@ The 'returns' member describes what will appear in the "return" member
of a Client JSON Protocol reply on successful completion of a command.
The member is optional from the command declaration; if absent, the
"return" member will be an empty dictionary. If 'returns' is present,
-it must be the string name of a complex or built-in type, a
-one-element array containing the name of a complex or built-in type.
+it must be the string name of a complex type, or a
+one-element array containing the name of a complex type.
To return anything else, you have to list the command in pragma
'returns-whitelist'. If you do this, the command cannot be extended
to return additional information in the future. Use of
@@ -691,13 +691,11 @@ started with --preconfig.
Usage: { 'event': STRING, '*data': COMPLEX-TYPE-NAME-OR-DICT,
'*boxed': true }
-Events are defined with the keyword 'event'. It is not allowed to
-name an event 'MAX', since the generator also produces a C enumeration
-of all event names with a generated _MAX value at the end. When
-'data' is also specified, additional info will be included in the
-event, with similar semantics to a 'struct' expression. Finally there
-will be C API generated in qapi-events.h; when called by QEMU code, a
-message with timestamp will be emitted on the wire.
+Events are defined with the keyword 'event'. When 'data' is also
+specified, additional info will be included in the event, with similar
+semantics to a 'struct' expression. Finally there will be C API
+generated in qapi-events.h; when called by QEMU code, a message with
+timestamp will be emitted on the wire.
An example event is:
--
2.21.0
^ permalink raw reply related [flat|nested] 41+ messages in thread
* [PULL 07/37] tests/qapi-schema: Demonstrate bad reporting of funny characters
2019-09-24 12:32 [PULL 00/37] QAPI patches for 2019-09-24 Markus Armbruster
` (5 preceding siblings ...)
2019-09-24 12:33 ` [PULL 06/37] docs/devel/qapi-code-gen: Minor specification fixes Markus Armbruster
@ 2019-09-24 12:33 ` Markus Armbruster
2019-09-24 12:33 ` [PULL 08/37] qapi: Restrict strings to printable ASCII Markus Armbruster
` (32 subsequent siblings)
39 siblings, 0 replies; 41+ messages in thread
From: Markus Armbruster @ 2019-09-24 12:33 UTC (permalink / raw)
To: qemu-devel
Invalid name 'not\\possible' is reported as 'not\possible'. Control
characters (quoted or not) are even more confusing. Mark FIXME.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Message-Id: <20190913201349.24332-6-armbru@redhat.com>
---
tests/qapi-schema/enum-bad-name.err | 2 +-
tests/qapi-schema/enum-bad-name.json | 3 ++-
2 files changed, 3 insertions(+), 2 deletions(-)
diff --git a/tests/qapi-schema/enum-bad-name.err b/tests/qapi-schema/enum-bad-name.err
index 9c3c1002b7..26a09f84ad 100644
--- a/tests/qapi-schema/enum-bad-name.err
+++ b/tests/qapi-schema/enum-bad-name.err
@@ -1 +1 @@
-tests/qapi-schema/enum-bad-name.json:2: Member of enum 'MyEnum' uses invalid name 'not^possible'
+tests/qapi-schema/enum-bad-name.json:3: Member of enum 'MyEnum' uses invalid name 'not\possible'
diff --git a/tests/qapi-schema/enum-bad-name.json b/tests/qapi-schema/enum-bad-name.json
index 8506562b31..1c4620edda 100644
--- a/tests/qapi-schema/enum-bad-name.json
+++ b/tests/qapi-schema/enum-bad-name.json
@@ -1,2 +1,3 @@
# we ensure all enum names can map to C
-{ 'enum': 'MyEnum', 'data': [ 'not^possible' ] }
+# FIXME reports 'not\possible' instead of 'not\\possible'
+{ 'enum': 'MyEnum', 'data': [ 'not\\possible' ] }
--
2.21.0
^ permalink raw reply related [flat|nested] 41+ messages in thread
* [PULL 08/37] qapi: Restrict strings to printable ASCII
2019-09-24 12:32 [PULL 00/37] QAPI patches for 2019-09-24 Markus Armbruster
` (6 preceding siblings ...)
2019-09-24 12:33 ` [PULL 07/37] tests/qapi-schema: Demonstrate bad reporting of funny characters Markus Armbruster
@ 2019-09-24 12:33 ` Markus Armbruster
2019-09-24 12:33 ` [PULL 09/37] qapi: Drop support for escape sequences other than \\ Markus Armbruster
` (31 subsequent siblings)
39 siblings, 0 replies; 41+ messages in thread
From: Markus Armbruster @ 2019-09-24 12:33 UTC (permalink / raw)
To: qemu-devel
RFC 8259 on string contents:
All Unicode characters may be placed within the quotation marks,
except for the characters that MUST be escaped: quotation mark,
reverse solidus, and the control characters (U+0000 through
U+001F).
The QAPI schema parser accepts both less and more than JSON: it
accepts only ASCII with \u (less), and accepts control characters
other than LF (new line) unescaped. How it treats unescaped non-ASCII
input differs between Python 2 and Python 3.
Make it accept strictly less: require printable ASCII. Drop support
for \b, \f, \n, \r, \t.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Message-Id: <20190913201349.24332-7-armbru@redhat.com>
---
scripts/qapi/common.py | 28 ++++++++-----------
tests/Makefile.include | 3 +-
tests/qapi-schema/string-code-point-127.err | 1 +
...de-str.exit => string-code-point-127.exit} | 0
tests/qapi-schema/string-code-point-127.json | 2 ++
...code-str.out => string-code-point-127.out} | 0
tests/qapi-schema/string-code-point-31.err | 1 +
tests/qapi-schema/string-code-point-31.exit | 1 +
tests/qapi-schema/string-code-point-31.json | 2 ++
tests/qapi-schema/string-code-point-31.out | 0
tests/qapi-schema/unicode-str.err | 1 -
tests/qapi-schema/unicode-str.json | 2 --
12 files changed, 20 insertions(+), 21 deletions(-)
create mode 100644 tests/qapi-schema/string-code-point-127.err
rename tests/qapi-schema/{unicode-str.exit => string-code-point-127.exit} (100%)
create mode 100644 tests/qapi-schema/string-code-point-127.json
rename tests/qapi-schema/{unicode-str.out => string-code-point-127.out} (100%)
create mode 100644 tests/qapi-schema/string-code-point-31.err
create mode 100644 tests/qapi-schema/string-code-point-31.exit
create mode 100644 tests/qapi-schema/string-code-point-31.json
create mode 100644 tests/qapi-schema/string-code-point-31.out
delete mode 100644 tests/qapi-schema/unicode-str.err
delete mode 100644 tests/qapi-schema/unicode-str.json
diff --git a/scripts/qapi/common.py b/scripts/qapi/common.py
index 54d02458b5..539b50f9ac 100644
--- a/scripts/qapi/common.py
+++ b/scripts/qapi/common.py
@@ -515,6 +515,7 @@ class QAPISchemaParser(object):
elif self.tok in '{}:,[]':
return
elif self.tok == "'":
+ # Note: we accept only printable ASCII
string = ''
esc = False
while True:
@@ -523,17 +524,9 @@ class QAPISchemaParser(object):
if ch == '\n':
raise QAPIParseError(self, 'Missing terminating "\'"')
if esc:
- if ch == 'b':
- string += '\b'
- elif ch == 'f':
- string += '\f'
- elif ch == 'n':
- string += '\n'
- elif ch == 'r':
- string += '\r'
- elif ch == 't':
- string += '\t'
- elif ch == 'u':
+ # Note: we don't recognize escape sequences
+ # for control characters
+ if ch == 'u':
value = 0
for _ in range(0, 4):
ch = self.src[self.cursor]
@@ -552,20 +545,21 @@ class QAPISchemaParser(object):
'For now, \\u escape '
'only supports non-zero '
'values up to \\u007f')
- string += chr(value)
- elif ch in '\\/\'"':
- string += ch
- else:
+ ch = chr(value)
+ elif ch not in '\\/\'"':
raise QAPIParseError(self,
"Unknown escape \\%s" % ch)
esc = False
elif ch == '\\':
esc = True
+ continue
elif ch == "'":
self.val = string
return
- else:
- string += ch
+ if ord(ch) < 32 or ord(ch) >= 127:
+ raise QAPIParseError(
+ self, "Funny character in string")
+ string += ch
elif self.src.startswith('true', self.pos):
self.val = True
self.cursor += 3
diff --git a/tests/Makefile.include b/tests/Makefile.include
index 479664f899..393cfd78f0 100644
--- a/tests/Makefile.include
+++ b/tests/Makefile.include
@@ -451,6 +451,8 @@ qapi-schema += returns-array-bad.json
qapi-schema += returns-dict.json
qapi-schema += returns-unknown.json
qapi-schema += returns-whitelist.json
+qapi-schema += string-code-point-31.json
+qapi-schema += string-code-point-127.json
qapi-schema += struct-base-clash-deep.json
qapi-schema += struct-base-clash.json
qapi-schema += struct-data-invalid.json
@@ -462,7 +464,6 @@ qapi-schema += type-bypass-bad-gen.json
qapi-schema += unclosed-list.json
qapi-schema += unclosed-object.json
qapi-schema += unclosed-string.json
-qapi-schema += unicode-str.json
qapi-schema += union-base-empty.json
qapi-schema += union-base-no-discriminator.json
qapi-schema += union-branch-case.json
diff --git a/tests/qapi-schema/string-code-point-127.err b/tests/qapi-schema/string-code-point-127.err
new file mode 100644
index 0000000000..c310910c23
--- /dev/null
+++ b/tests/qapi-schema/string-code-point-127.err
@@ -0,0 +1 @@
+tests/qapi-schema/string-code-point-127.json:2:14: Funny character in string
diff --git a/tests/qapi-schema/unicode-str.exit b/tests/qapi-schema/string-code-point-127.exit
similarity index 100%
rename from tests/qapi-schema/unicode-str.exit
rename to tests/qapi-schema/string-code-point-127.exit
diff --git a/tests/qapi-schema/string-code-point-127.json b/tests/qapi-schema/string-code-point-127.json
new file mode 100644
index 0000000000..480318a69f
--- /dev/null
+++ b/tests/qapi-schema/string-code-point-127.json
@@ -0,0 +1,2 @@
+# We accept printable ASCII: code points 32..126. Test code point 127:
+{ 'command': '\x7f' }
diff --git a/tests/qapi-schema/unicode-str.out b/tests/qapi-schema/string-code-point-127.out
similarity index 100%
rename from tests/qapi-schema/unicode-str.out
rename to tests/qapi-schema/string-code-point-127.out
diff --git a/tests/qapi-schema/string-code-point-31.err b/tests/qapi-schema/string-code-point-31.err
new file mode 100644
index 0000000000..45797928d9
--- /dev/null
+++ b/tests/qapi-schema/string-code-point-31.err
@@ -0,0 +1 @@
+tests/qapi-schema/string-code-point-31.json:2:14: Funny character in string
diff --git a/tests/qapi-schema/string-code-point-31.exit b/tests/qapi-schema/string-code-point-31.exit
new file mode 100644
index 0000000000..d00491fd7e
--- /dev/null
+++ b/tests/qapi-schema/string-code-point-31.exit
@@ -0,0 +1 @@
+1
diff --git a/tests/qapi-schema/string-code-point-31.json b/tests/qapi-schema/string-code-point-31.json
new file mode 100644
index 0000000000..f186cbd720
--- /dev/null
+++ b/tests/qapi-schema/string-code-point-31.json
@@ -0,0 +1,2 @@
+# We accept printable ASCII: code points 32..126. Test code point 127:
+{ 'command': '\x1f' }
diff --git a/tests/qapi-schema/string-code-point-31.out b/tests/qapi-schema/string-code-point-31.out
new file mode 100644
index 0000000000..e69de29bb2
diff --git a/tests/qapi-schema/unicode-str.err b/tests/qapi-schema/unicode-str.err
deleted file mode 100644
index f621cd6448..0000000000
--- a/tests/qapi-schema/unicode-str.err
+++ /dev/null
@@ -1 +0,0 @@
-tests/qapi-schema/unicode-str.json:2: 'command' uses invalid name 'é'
diff --git a/tests/qapi-schema/unicode-str.json b/tests/qapi-schema/unicode-str.json
deleted file mode 100644
index 5253a1b9f3..0000000000
--- a/tests/qapi-schema/unicode-str.json
+++ /dev/null
@@ -1,2 +0,0 @@
-# we don't support full Unicode strings, yet
-{ 'command': 'é' }
--
2.21.0
^ permalink raw reply related [flat|nested] 41+ messages in thread
* [PULL 09/37] qapi: Drop support for escape sequences other than \\
2019-09-24 12:32 [PULL 00/37] QAPI patches for 2019-09-24 Markus Armbruster
` (7 preceding siblings ...)
2019-09-24 12:33 ` [PULL 08/37] qapi: Restrict strings to printable ASCII Markus Armbruster
@ 2019-09-24 12:33 ` Markus Armbruster
2019-09-24 12:33 ` [PULL 10/37] qapi: Permit 'boxed' with empty type Markus Armbruster
` (30 subsequent siblings)
39 siblings, 0 replies; 41+ messages in thread
From: Markus Armbruster @ 2019-09-24 12:33 UTC (permalink / raw)
To: qemu-devel
Since the previous commit restricted strings to printable ASCII,
\uXXXX's only use is obfuscation. Drop it.
This leaves \\, \/, \', and \". Since QAPI schema strings are all
names, and names are restricted to ASCII letters, digits, hyphen, and
underscore, none of them is useful.
The latter three have no test coverage. Drop them.
Keep \\ to avoid (more) gratuitous incompatibility with JSON.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Message-Id: <20190913201349.24332-8-armbru@redhat.com>
---
scripts/qapi/common.py | 26 +++-----------------
tests/Makefile.include | 3 ---
tests/qapi-schema/escape-outside-string.err | 1 -
tests/qapi-schema/escape-outside-string.exit | 1 -
tests/qapi-schema/escape-outside-string.json | 3 ---
tests/qapi-schema/escape-outside-string.out | 0
tests/qapi-schema/escape-too-big.err | 1 -
tests/qapi-schema/escape-too-big.exit | 1 -
tests/qapi-schema/escape-too-big.json | 3 ---
tests/qapi-schema/escape-too-big.out | 0
tests/qapi-schema/escape-too-short.err | 1 -
tests/qapi-schema/escape-too-short.exit | 1 -
tests/qapi-schema/escape-too-short.json | 3 ---
tests/qapi-schema/escape-too-short.out | 0
tests/qapi-schema/ident-with-escape.err | 1 +
tests/qapi-schema/ident-with-escape.exit | 2 +-
tests/qapi-schema/ident-with-escape.json | 2 +-
tests/qapi-schema/ident-with-escape.out | 16 ------------
tests/qapi-schema/unknown-escape.json | 2 +-
19 files changed, 7 insertions(+), 60 deletions(-)
delete mode 100644 tests/qapi-schema/escape-outside-string.err
delete mode 100644 tests/qapi-schema/escape-outside-string.exit
delete mode 100644 tests/qapi-schema/escape-outside-string.json
delete mode 100644 tests/qapi-schema/escape-outside-string.out
delete mode 100644 tests/qapi-schema/escape-too-big.err
delete mode 100644 tests/qapi-schema/escape-too-big.exit
delete mode 100644 tests/qapi-schema/escape-too-big.json
delete mode 100644 tests/qapi-schema/escape-too-big.out
delete mode 100644 tests/qapi-schema/escape-too-short.err
delete mode 100644 tests/qapi-schema/escape-too-short.exit
delete mode 100644 tests/qapi-schema/escape-too-short.json
delete mode 100644 tests/qapi-schema/escape-too-short.out
diff --git a/scripts/qapi/common.py b/scripts/qapi/common.py
index 539b50f9ac..0fb1d1956a 100644
--- a/scripts/qapi/common.py
+++ b/scripts/qapi/common.py
@@ -524,29 +524,9 @@ class QAPISchemaParser(object):
if ch == '\n':
raise QAPIParseError(self, 'Missing terminating "\'"')
if esc:
- # Note: we don't recognize escape sequences
- # for control characters
- if ch == 'u':
- value = 0
- for _ in range(0, 4):
- ch = self.src[self.cursor]
- self.cursor += 1
- if ch not in '0123456789abcdefABCDEF':
- raise QAPIParseError(self,
- '\\u escape needs 4 '
- 'hex digits')
- value = (value << 4) + int(ch, 16)
- # If Python 2 and 3 didn't disagree so much on
- # how to handle Unicode, then we could allow
- # Unicode string defaults. But most of QAPI is
- # ASCII-only, so we aren't losing much for now.
- if not value or value > 0x7f:
- raise QAPIParseError(self,
- 'For now, \\u escape '
- 'only supports non-zero '
- 'values up to \\u007f')
- ch = chr(value)
- elif ch not in '\\/\'"':
+ # Note: we recognize only \\ because we have
+ # no use for funny characters in strings
+ if ch != '\\':
raise QAPIParseError(self,
"Unknown escape \\%s" % ch)
esc = False
diff --git a/tests/Makefile.include b/tests/Makefile.include
index 393cfd78f0..8585e7ed26 100644
--- a/tests/Makefile.include
+++ b/tests/Makefile.include
@@ -374,9 +374,6 @@ qapi-schema += enum-int-member.json
qapi-schema += enum-member-case.json
qapi-schema += enum-missing-data.json
qapi-schema += enum-wrong-data.json
-qapi-schema += escape-outside-string.json
-qapi-schema += escape-too-big.json
-qapi-schema += escape-too-short.json
qapi-schema += event-boxed-empty.json
qapi-schema += event-case.json
qapi-schema += event-member-invalid-dict.json
diff --git a/tests/qapi-schema/escape-outside-string.err b/tests/qapi-schema/escape-outside-string.err
deleted file mode 100644
index b9b8837fd2..0000000000
--- a/tests/qapi-schema/escape-outside-string.err
+++ /dev/null
@@ -1 +0,0 @@
-tests/qapi-schema/escape-outside-string.json:3:27: Stray "\"
diff --git a/tests/qapi-schema/escape-outside-string.exit b/tests/qapi-schema/escape-outside-string.exit
deleted file mode 100644
index d00491fd7e..0000000000
--- a/tests/qapi-schema/escape-outside-string.exit
+++ /dev/null
@@ -1 +0,0 @@
-1
diff --git a/tests/qapi-schema/escape-outside-string.json b/tests/qapi-schema/escape-outside-string.json
deleted file mode 100644
index 482f79554b..0000000000
--- a/tests/qapi-schema/escape-outside-string.json
+++ /dev/null
@@ -1,3 +0,0 @@
-# escape sequences are permitted only inside strings
-# { 'command': 'foo', 'data': {} }
-{ 'command': 'foo', 'data'\u003a{} }
diff --git a/tests/qapi-schema/escape-outside-string.out b/tests/qapi-schema/escape-outside-string.out
deleted file mode 100644
index e69de29bb2..0000000000
diff --git a/tests/qapi-schema/escape-too-big.err b/tests/qapi-schema/escape-too-big.err
deleted file mode 100644
index d9aeb5dc38..0000000000
--- a/tests/qapi-schema/escape-too-big.err
+++ /dev/null
@@ -1 +0,0 @@
-tests/qapi-schema/escape-too-big.json:3:14: For now, \u escape only supports non-zero values up to \u007f
diff --git a/tests/qapi-schema/escape-too-big.exit b/tests/qapi-schema/escape-too-big.exit
deleted file mode 100644
index d00491fd7e..0000000000
--- a/tests/qapi-schema/escape-too-big.exit
+++ /dev/null
@@ -1 +0,0 @@
-1
diff --git a/tests/qapi-schema/escape-too-big.json b/tests/qapi-schema/escape-too-big.json
deleted file mode 100644
index 62bcecd557..0000000000
--- a/tests/qapi-schema/escape-too-big.json
+++ /dev/null
@@ -1,3 +0,0 @@
-# we don't support full Unicode strings, yet
-# { 'command': 'é' }
-{ 'command': '\u00e9' }
diff --git a/tests/qapi-schema/escape-too-big.out b/tests/qapi-schema/escape-too-big.out
deleted file mode 100644
index e69de29bb2..0000000000
diff --git a/tests/qapi-schema/escape-too-short.err b/tests/qapi-schema/escape-too-short.err
deleted file mode 100644
index 934de598ee..0000000000
--- a/tests/qapi-schema/escape-too-short.err
+++ /dev/null
@@ -1 +0,0 @@
-tests/qapi-schema/escape-too-short.json:3:14: \u escape needs 4 hex digits
diff --git a/tests/qapi-schema/escape-too-short.exit b/tests/qapi-schema/escape-too-short.exit
deleted file mode 100644
index d00491fd7e..0000000000
--- a/tests/qapi-schema/escape-too-short.exit
+++ /dev/null
@@ -1 +0,0 @@
-1
diff --git a/tests/qapi-schema/escape-too-short.json b/tests/qapi-schema/escape-too-short.json
deleted file mode 100644
index 6cb1dec8f7..0000000000
--- a/tests/qapi-schema/escape-too-short.json
+++ /dev/null
@@ -1,3 +0,0 @@
-# the \u escape requires 4 hex digits
-# { 'command': 'a' }
-{ 'command': '\u61' }
diff --git a/tests/qapi-schema/escape-too-short.out b/tests/qapi-schema/escape-too-short.out
deleted file mode 100644
index e69de29bb2..0000000000
diff --git a/tests/qapi-schema/ident-with-escape.err b/tests/qapi-schema/ident-with-escape.err
index e69de29bb2..5517dcb4b1 100644
--- a/tests/qapi-schema/ident-with-escape.err
+++ b/tests/qapi-schema/ident-with-escape.err
@@ -0,0 +1 @@
+tests/qapi-schema/ident-with-escape.json:3:3: Unknown escape \u
diff --git a/tests/qapi-schema/ident-with-escape.exit b/tests/qapi-schema/ident-with-escape.exit
index 573541ac97..d00491fd7e 100644
--- a/tests/qapi-schema/ident-with-escape.exit
+++ b/tests/qapi-schema/ident-with-escape.exit
@@ -1 +1 @@
-0
+1
diff --git a/tests/qapi-schema/ident-with-escape.json b/tests/qapi-schema/ident-with-escape.json
index 56617501e7..76b4503d95 100644
--- a/tests/qapi-schema/ident-with-escape.json
+++ b/tests/qapi-schema/ident-with-escape.json
@@ -1,4 +1,4 @@
-# we allow escape sequences in strings, if they map back to ASCII
+# we don't recognize any \ escapes other than \\ (tested elsewhere)
# { 'command': 'fooA', 'data': { 'bar1': 'str' } }
{ 'c\u006fmmand': '\u0066\u006f\u006FA',
'd\u0061ta': { '\u0062\u0061\u00721': '\u0073\u0074\u0072' } }
diff --git a/tests/qapi-schema/ident-with-escape.out b/tests/qapi-schema/ident-with-escape.out
index 39754eba8c..e69de29bb2 100644
--- a/tests/qapi-schema/ident-with-escape.out
+++ b/tests/qapi-schema/ident-with-escape.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 ident-with-escape.json
-object q_obj_fooA-arg
- member bar1: str optional=False
-command fooA q_obj_fooA-arg -> None
- gen=True success_response=True boxed=False oob=False preconfig=False
diff --git a/tests/qapi-schema/unknown-escape.json b/tests/qapi-schema/unknown-escape.json
index 8e6891e52a..8372e8024f 100644
--- a/tests/qapi-schema/unknown-escape.json
+++ b/tests/qapi-schema/unknown-escape.json
@@ -1,3 +1,3 @@
-# we only recognize JSON escape sequences, plus our \' extension (no \x)
+# we only recognize \\
# { 'command': 'foo', 'data': {} }
{ 'command': 'foo', 'dat\x61':{} }
--
2.21.0
^ permalink raw reply related [flat|nested] 41+ messages in thread
* [PULL 10/37] qapi: Permit 'boxed' with empty type
2019-09-24 12:32 [PULL 00/37] QAPI patches for 2019-09-24 Markus Armbruster
` (8 preceding siblings ...)
2019-09-24 12:33 ` [PULL 09/37] qapi: Drop support for escape sequences other than \\ Markus Armbruster
@ 2019-09-24 12:33 ` Markus Armbruster
2019-09-24 12:33 ` [PULL 11/37] qapi: Permit alternates with just one branch Markus Armbruster
` (29 subsequent siblings)
39 siblings, 0 replies; 41+ messages in thread
From: Markus Armbruster @ 2019-09-24 12:33 UTC (permalink / raw)
To: qemu-devel
We reject empty types with 'boxed': true. We don't really need that
to work, but making it work is actually simpler than rejecting it, so
do that.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Message-Id: <20190913201349.24332-9-armbru@redhat.com>
---
tests/test-qmp-cmds.c | 4 ++++
scripts/qapi/commands.py | 4 ++--
scripts/qapi/common.py | 14 ++------------
scripts/qapi/events.py | 10 +++++-----
tests/Makefile.include | 1 -
tests/qapi-schema/args-boxed-empty.err | 1 -
tests/qapi-schema/args-boxed-empty.exit | 1 -
tests/qapi-schema/args-boxed-empty.json | 3 ---
tests/qapi-schema/args-boxed-empty.out | 0
tests/qapi-schema/qapi-schema-test.json | 2 ++
tests/qapi-schema/qapi-schema-test.out | 4 ++++
11 files changed, 19 insertions(+), 25 deletions(-)
delete mode 100644 tests/qapi-schema/args-boxed-empty.err
delete mode 100644 tests/qapi-schema/args-boxed-empty.exit
delete mode 100644 tests/qapi-schema/args-boxed-empty.json
delete mode 100644 tests/qapi-schema/args-boxed-empty.out
diff --git a/tests/test-qmp-cmds.c b/tests/test-qmp-cmds.c
index ab389f42da..36fdf5b115 100644
--- a/tests/test-qmp-cmds.c
+++ b/tests/test-qmp-cmds.c
@@ -97,6 +97,10 @@ void qmp_boxed_union(UserDefListUnion *arg, Error **errp)
{
}
+void qmp_boxed_empty(Empty1 *arg, Error **errp)
+{
+}
+
__org_qemu_x_Union1 *qmp___org_qemu_x_command(__org_qemu_x_EnumList *a,
__org_qemu_x_StructList *b,
__org_qemu_x_Union2 *c,
diff --git a/scripts/qapi/commands.py b/scripts/qapi/commands.py
index b929e07be4..7e3dd1068a 100644
--- a/scripts/qapi/commands.py
+++ b/scripts/qapi/commands.py
@@ -30,7 +30,7 @@ def gen_call(name, arg_type, boxed, ret_type):
argstr = ''
if boxed:
- assert arg_type and not arg_type.is_empty()
+ assert arg_type
argstr = '&arg, '
elif arg_type:
assert not arg_type.variants
@@ -96,7 +96,7 @@ def gen_marshal_decl(name):
def gen_marshal(name, arg_type, boxed, ret_type):
- have_args = arg_type and not arg_type.is_empty()
+ have_args = boxed or (arg_type and not arg_type.is_empty())
ret = mcgen('''
diff --git a/scripts/qapi/common.py b/scripts/qapi/common.py
index 0fb1d1956a..c5c71287c3 100644
--- a/scripts/qapi/common.py
+++ b/scripts/qapi/common.py
@@ -1687,12 +1687,7 @@ class QAPISchemaCommand(QAPISchemaEntity):
self.arg_type = schema.lookup_type(self._arg_type_name)
assert isinstance(self.arg_type, QAPISchemaObjectType)
self.arg_type.check(schema)
- if self.boxed:
- if self.arg_type.is_empty():
- raise QAPISemError(self.info,
- "Cannot use 'boxed' with empty type")
- else:
- assert not self.arg_type.variants
+ assert not self.arg_type.variants or self.boxed
elif self.boxed:
raise QAPISemError(self.info, "Use of 'boxed' requires 'data'")
if self._ret_type_name:
@@ -1721,12 +1716,7 @@ class QAPISchemaEvent(QAPISchemaEntity):
self.arg_type = schema.lookup_type(self._arg_type_name)
assert isinstance(self.arg_type, QAPISchemaObjectType)
self.arg_type.check(schema)
- if self.boxed:
- if self.arg_type.is_empty():
- raise QAPISemError(self.info,
- "Cannot use 'boxed' with empty type")
- else:
- assert not self.arg_type.variants
+ assert not self.arg_type.variants or self.boxed
elif self.boxed:
raise QAPISemError(self.info, "Use of 'boxed' requires 'data'")
diff --git a/scripts/qapi/events.py b/scripts/qapi/events.py
index b732581046..e0abfef7b0 100644
--- a/scripts/qapi/events.py
+++ b/scripts/qapi/events.py
@@ -65,6 +65,8 @@ def gen_event_send(name, arg_type, boxed, event_enum_name, event_emit):
# practice, we can rename our local variables with a leading _ prefix,
# or split the code into a wrapper function that creates a boxed
# 'param' object then calls another to do the real work.
+ have_args = boxed or (arg_type and not arg_type.is_empty())
+
ret = mcgen('''
%(proto)s
@@ -73,15 +75,13 @@ def gen_event_send(name, arg_type, boxed, event_enum_name, event_emit):
''',
proto=build_event_send_proto(name, arg_type, boxed))
- if arg_type and not arg_type.is_empty():
+ if have_args:
ret += mcgen('''
QObject *obj;
Visitor *v;
''')
if not boxed:
ret += gen_param_var(arg_type)
- else:
- assert not boxed
ret += mcgen('''
@@ -90,7 +90,7 @@ def gen_event_send(name, arg_type, boxed, event_enum_name, event_emit):
''',
name=name)
- if arg_type and not arg_type.is_empty():
+ if have_args:
ret += mcgen('''
v = qobject_output_visitor_new(&obj);
''')
@@ -121,7 +121,7 @@ def gen_event_send(name, arg_type, boxed, event_enum_name, event_emit):
event_emit=event_emit,
c_enum=c_enum_const(event_enum_name, name))
- if arg_type and not arg_type.is_empty():
+ if have_args:
ret += mcgen('''
visit_free(v);
''')
diff --git a/tests/Makefile.include b/tests/Makefile.include
index 8585e7ed26..2c3adb1530 100644
--- a/tests/Makefile.include
+++ b/tests/Makefile.include
@@ -311,7 +311,6 @@ qapi-schema += args-array-empty.json
qapi-schema += args-array-unknown.json
qapi-schema += args-bad-boxed.json
qapi-schema += args-boxed-anon.json
-qapi-schema += args-boxed-empty.json
qapi-schema += args-boxed-string.json
qapi-schema += args-int.json
qapi-schema += args-invalid.json
diff --git a/tests/qapi-schema/args-boxed-empty.err b/tests/qapi-schema/args-boxed-empty.err
deleted file mode 100644
index 039603e85c..0000000000
--- a/tests/qapi-schema/args-boxed-empty.err
+++ /dev/null
@@ -1 +0,0 @@
-tests/qapi-schema/args-boxed-empty.json:3: Cannot use 'boxed' with empty type
diff --git a/tests/qapi-schema/args-boxed-empty.exit b/tests/qapi-schema/args-boxed-empty.exit
deleted file mode 100644
index d00491fd7e..0000000000
--- a/tests/qapi-schema/args-boxed-empty.exit
+++ /dev/null
@@ -1 +0,0 @@
-1
diff --git a/tests/qapi-schema/args-boxed-empty.json b/tests/qapi-schema/args-boxed-empty.json
deleted file mode 100644
index 52717e065f..0000000000
--- a/tests/qapi-schema/args-boxed-empty.json
+++ /dev/null
@@ -1,3 +0,0 @@
-# 'boxed' requires a non-empty type
-{ 'struct': 'Empty', 'data': {} }
-{ 'command': 'foo', 'boxed': true, 'data': 'Empty' }
diff --git a/tests/qapi-schema/args-boxed-empty.out b/tests/qapi-schema/args-boxed-empty.out
deleted file mode 100644
index e69de29bb2..0000000000
diff --git a/tests/qapi-schema/qapi-schema-test.json b/tests/qapi-schema/qapi-schema-test.json
index 0fadb4ddd7..e6dbbbd328 100644
--- a/tests/qapi-schema/qapi-schema-test.json
+++ b/tests/qapi-schema/qapi-schema-test.json
@@ -149,6 +149,7 @@
{ 'command': 'guest-sync', 'data': { 'arg': 'any' }, 'returns': 'any' }
{ 'command': 'boxed-struct', 'boxed': true, 'data': 'UserDefZero' }
{ 'command': 'boxed-union', 'data': 'UserDefListUnion', 'boxed': true }
+{ 'command': 'boxed-empty', 'boxed': true, 'data': 'Empty1' }
# Smoke test on out-of-band and allow-preconfig-test
{ 'command': 'test-flags-command', 'allow-oob': true, 'allow-preconfig': true }
@@ -181,6 +182,7 @@
'data': { 'a' : 'EventStructOne', 'b' : 'str', '*c': 'str', '*enum3': 'EnumOne' } }
{ 'event': 'EVENT_E', 'boxed': true, 'data': 'UserDefZero' }
{ 'event': 'EVENT_F', 'boxed': true, 'data': 'UserDefFlatUnion' }
+{ 'event': 'EVENT_G', 'boxed': true, 'data': 'Empty1' }
# test that we correctly compile downstream extensions, as well as munge
# ticklish names
diff --git a/tests/qapi-schema/qapi-schema-test.out b/tests/qapi-schema/qapi-schema-test.out
index 5470a525f5..fb00a21996 100644
--- a/tests/qapi-schema/qapi-schema-test.out
+++ b/tests/qapi-schema/qapi-schema-test.out
@@ -221,6 +221,8 @@ command boxed-struct UserDefZero -> None
gen=True success_response=True boxed=True oob=False preconfig=False
command boxed-union UserDefListUnion -> None
gen=True success_response=True boxed=True oob=False preconfig=False
+command boxed-empty Empty1 -> None
+ gen=True success_response=True boxed=True oob=False preconfig=False
command test-flags-command None -> None
gen=True success_response=True boxed=False oob=True preconfig=True
object UserDefOptions
@@ -254,6 +256,8 @@ event EVENT_E UserDefZero
boxed=True
event EVENT_F UserDefFlatUnion
boxed=True
+event EVENT_G Empty1
+ boxed=True
enum __org.qemu_x-Enum
member __org.qemu_x-value
object __org.qemu_x-Base
--
2.21.0
^ permalink raw reply related [flat|nested] 41+ messages in thread
* [PULL 11/37] qapi: Permit alternates with just one branch
2019-09-24 12:32 [PULL 00/37] QAPI patches for 2019-09-24 Markus Armbruster
` (9 preceding siblings ...)
2019-09-24 12:33 ` [PULL 10/37] qapi: Permit 'boxed' with empty type Markus Armbruster
@ 2019-09-24 12:33 ` Markus Armbruster
2019-09-24 12:33 ` [PULL 12/37] qapi: Permit omitting all flat union branches Markus Armbruster
` (28 subsequent siblings)
39 siblings, 0 replies; 41+ messages in thread
From: Markus Armbruster @ 2019-09-24 12:33 UTC (permalink / raw)
To: qemu-devel
A union or alternate without branches makes no sense and doesn't work:
it can't be instantiated. A union or alternate with just one branch
works, but is degenerate. We accept the former, but reject the
latter. Weird. docs/devel/qapi-code-gen.txt doesn't mention the
difference. It claims an alternate definition is "is similar to a
simple union type".
Permit degenerate alternates to make them consistent with unions.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Message-Id: <20190913201349.24332-10-armbru@redhat.com>
---
scripts/qapi/common.py | 6 ++----
tests/qapi-schema/alternate-empty.err | 2 +-
tests/qapi-schema/alternate-empty.json | 4 ++--
tests/qapi-schema/qapi-schema-test.json | 4 +++-
tests/qapi-schema/qapi-schema-test.out | 6 ++++--
5 files changed, 12 insertions(+), 10 deletions(-)
diff --git a/scripts/qapi/common.py b/scripts/qapi/common.py
index c5c71287c3..99db18f3d6 100644
--- a/scripts/qapi/common.py
+++ b/scripts/qapi/common.py
@@ -920,11 +920,9 @@ def check_alternate(expr, info):
members = expr['data']
types_seen = {}
- # Check every branch; require at least two branches
- if len(members) < 2:
+ if len(members) == 0:
raise QAPISemError(info,
- "Alternate '%s' should have at least two branches "
- "in 'data'" % name)
+ "Alternate '%s' cannot have empty 'data'" % name)
for (key, value) in members.items():
check_name(info, "Member of alternate '%s'" % name, key)
check_known_keys(info,
diff --git a/tests/qapi-schema/alternate-empty.err b/tests/qapi-schema/alternate-empty.err
index bb06c5bfec..86dbc666eb 100644
--- a/tests/qapi-schema/alternate-empty.err
+++ b/tests/qapi-schema/alternate-empty.err
@@ -1 +1 @@
-tests/qapi-schema/alternate-empty.json:2: Alternate 'Alt' should have at least two branches in 'data'
+tests/qapi-schema/alternate-empty.json:2: Alternate 'Alt' cannot have empty 'data'
diff --git a/tests/qapi-schema/alternate-empty.json b/tests/qapi-schema/alternate-empty.json
index fff15baf16..9f445474e6 100644
--- a/tests/qapi-schema/alternate-empty.json
+++ b/tests/qapi-schema/alternate-empty.json
@@ -1,2 +1,2 @@
-# alternates must list at least two types to be useful
-{ 'alternate': 'Alt', 'data': { 'i': 'int' } }
+# alternates cannot be empty
+{ 'alternate': 'Alt', 'data': { } }
diff --git a/tests/qapi-schema/qapi-schema-test.json b/tests/qapi-schema/qapi-schema-test.json
index e6dbbbd328..8b0d47c4ab 100644
--- a/tests/qapi-schema/qapi-schema-test.json
+++ b/tests/qapi-schema/qapi-schema-test.json
@@ -186,19 +186,21 @@
# test that we correctly compile downstream extensions, as well as munge
# ticklish names
+# also test union and alternate with just one branch
{ 'enum': '__org.qemu_x-Enum', 'data': [ '__org.qemu_x-value' ] }
{ 'struct': '__org.qemu_x-Base',
'data': { '__org.qemu_x-member1': '__org.qemu_x-Enum' } }
{ 'struct': '__org.qemu_x-Struct', 'base': '__org.qemu_x-Base',
'data': { '__org.qemu_x-member2': 'str', '*wchar-t': 'int' } }
{ 'union': '__org.qemu_x-Union1', 'data': { '__org.qemu_x-branch': 'str' } }
+{ 'alternate': '__org.qemu_x-Alt1', 'data': { '__org.qemu_x-branch': 'str' } }
{ 'struct': '__org.qemu_x-Struct2',
'data': { 'array': ['__org.qemu_x-Union1'] } }
{ 'union': '__org.qemu_x-Union2', 'base': '__org.qemu_x-Base',
'discriminator': '__org.qemu_x-member1',
'data': { '__org.qemu_x-value': '__org.qemu_x-Struct2' } }
{ 'alternate': '__org.qemu_x-Alt',
- 'data': { '__org.qemu_x-branch': 'str', 'b': '__org.qemu_x-Base' } }
+ 'data': { '__org.qemu_x-branch': '__org.qemu_x-Base' } }
{ 'event': '__ORG.QEMU_X-EVENT', 'data': '__org.qemu_x-Struct' }
{ 'command': '__org.qemu_x-command',
'data': { 'a': ['__org.qemu_x-Enum'], 'b': ['__org.qemu_x-Struct'],
diff --git a/tests/qapi-schema/qapi-schema-test.out b/tests/qapi-schema/qapi-schema-test.out
index fb00a21996..bea7976bbb 100644
--- a/tests/qapi-schema/qapi-schema-test.out
+++ b/tests/qapi-schema/qapi-schema-test.out
@@ -274,6 +274,9 @@ object __org.qemu_x-Union1
member type: __org.qemu_x-Union1Kind optional=False
tag type
case __org.qemu_x-branch: q_obj_str-wrapper
+alternate __org.qemu_x-Alt1
+ tag type
+ case __org.qemu_x-branch: str
array __org.qemu_x-Union1List __org.qemu_x-Union1
object __org.qemu_x-Struct2
member array: __org.qemu_x-Union1List optional=False
@@ -283,8 +286,7 @@ object __org.qemu_x-Union2
case __org.qemu_x-value: __org.qemu_x-Struct2
alternate __org.qemu_x-Alt
tag type
- case __org.qemu_x-branch: str
- case b: __org.qemu_x-Base
+ case __org.qemu_x-branch: __org.qemu_x-Base
event __ORG.QEMU_X-EVENT __org.qemu_x-Struct
boxed=False
array __org.qemu_x-EnumList __org.qemu_x-Enum
--
2.21.0
^ permalink raw reply related [flat|nested] 41+ messages in thread
* [PULL 12/37] qapi: Permit omitting all flat union branches
2019-09-24 12:32 [PULL 00/37] QAPI patches for 2019-09-24 Markus Armbruster
` (10 preceding siblings ...)
2019-09-24 12:33 ` [PULL 11/37] qapi: Permit alternates with just one branch Markus Armbruster
@ 2019-09-24 12:33 ` Markus Armbruster
2019-09-24 12:33 ` [PULL 13/37] qapi: Adjust frontend errors to say enum value, not member Markus Armbruster
` (27 subsequent siblings)
39 siblings, 0 replies; 41+ messages in thread
From: Markus Armbruster @ 2019-09-24 12:33 UTC (permalink / raw)
To: qemu-devel
Absent flat union branches default to the empty struct (since commit
800877bb16 "qapi: allow empty branches in flat unions"). But an
attempt to omit all of them is rejected with "Union 'FOO' has no
branches". Harmless oddity, but it's easy to avoid, so do that.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Message-Id: <20190913201349.24332-11-armbru@redhat.com>
[Commit message typo fixed]
---
docs/devel/qapi-code-gen.txt | 3 +--
scripts/qapi/common.py | 16 ++++++++--------
tests/qapi-schema/flat-union-empty.err | 2 +-
tests/qapi-schema/flat-union-empty.json | 2 +-
tests/qapi-schema/qapi-schema-test.json | 5 +++++
tests/qapi-schema/qapi-schema-test.out | 9 +++++++++
tests/qapi-schema/union-empty.err | 2 +-
tests/qapi-schema/union-empty.json | 2 +-
8 files changed, 27 insertions(+), 14 deletions(-)
diff --git a/docs/devel/qapi-code-gen.txt b/docs/devel/qapi-code-gen.txt
index 4ce67752a7..ec2d374483 100644
--- a/docs/devel/qapi-code-gen.txt
+++ b/docs/devel/qapi-code-gen.txt
@@ -436,8 +436,7 @@ 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), and flat (both discriminator and base). A union
type is defined using a data dictionary as explained in the following
-paragraphs. The data dictionary for either type of union must not
-be empty.
+paragraphs. Unions must have at least one branch.
A simple union type defines a mapping from automatic discriminator
values to data types like in this example:
diff --git a/scripts/qapi/common.py b/scripts/qapi/common.py
index 99db18f3d6..3393a049cc 100644
--- a/scripts/qapi/common.py
+++ b/scripts/qapi/common.py
@@ -852,7 +852,7 @@ def check_union(expr, info):
# With no discriminator it is a simple union.
if discriminator is None:
- enum_define = None
+ enum_values = members.keys()
allow_metas = ['built-in', 'union', 'alternate', 'struct', 'enum']
if base is not None:
raise QAPISemError(info, "Simple union '%s' must not have a base" %
@@ -885,16 +885,17 @@ def check_union(expr, info):
'must not be conditional' %
(base, discriminator, name))
enum_define = enum_types.get(discriminator_value['type'])
- allow_metas = ['struct']
# Do not allow string discriminator
if not enum_define:
raise QAPISemError(info,
"Discriminator '%s' must be of enumeration "
"type" % discriminator)
+ enum_values = enum_get_names(enum_define)
+ allow_metas = ['struct']
+
+ if (len(enum_values) == 0):
+ raise QAPISemError(info, "Union '%s' has no branches" % name)
- # Check every branch; don't allow an empty union
- if len(members) == 0:
- raise QAPISemError(info, "Union '%s' cannot have empty 'data'" % name)
for (key, value) in members.items():
check_name(info, "Member of union '%s'" % name, key)
@@ -907,8 +908,8 @@ def check_union(expr, info):
# If the discriminator names an enum type, then all members
# of 'data' must also be members of the enum type.
- if enum_define:
- if key not in enum_get_names(enum_define):
+ if discriminator is not None:
+ if key not in enum_values:
raise QAPISemError(info,
"Discriminator value '%s' is not found in "
"enum '%s'"
@@ -1578,7 +1579,6 @@ 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/flat-union-empty.err b/tests/qapi-schema/flat-union-empty.err
index 15754f54eb..fedbc0d1cf 100644
--- a/tests/qapi-schema/flat-union-empty.err
+++ b/tests/qapi-schema/flat-union-empty.err
@@ -1 +1 @@
-tests/qapi-schema/flat-union-empty.json:4: Union 'Union' cannot have empty 'data'
+tests/qapi-schema/flat-union-empty.json:4: Union 'Union' has no branches
diff --git a/tests/qapi-schema/flat-union-empty.json b/tests/qapi-schema/flat-union-empty.json
index 77f1d9abfb..83e1cc7b96 100644
--- a/tests/qapi-schema/flat-union-empty.json
+++ b/tests/qapi-schema/flat-union-empty.json
@@ -1,4 +1,4 @@
-# flat unions cannot be empty
+# flat union discriminator cannot be empty
{ 'enum': 'Empty', 'data': [ ] }
{ 'struct': 'Base', 'data': { 'type': 'Empty' } }
{ 'union': 'Union', 'base': 'Base', 'discriminator': 'type', 'data': { } }
diff --git a/tests/qapi-schema/qapi-schema-test.json b/tests/qapi-schema/qapi-schema-test.json
index 8b0d47c4ab..75c42eb0e3 100644
--- a/tests/qapi-schema/qapi-schema-test.json
+++ b/tests/qapi-schema/qapi-schema-test.json
@@ -25,6 +25,11 @@
{ 'struct': 'Empty1', 'data': { } }
{ 'struct': 'Empty2', 'base': 'Empty1', 'data': { } }
+# Likewise for an empty flat union
+{ 'union': 'Union',
+ 'base': { 'type': 'EnumOne' }, 'discriminator': 'type',
+ 'data': { } }
+
{ 'command': 'user_def_cmd0', 'data': 'Empty2', 'returns': 'Empty2' }
# for testing override of default naming heuristic
diff --git a/tests/qapi-schema/qapi-schema-test.out b/tests/qapi-schema/qapi-schema-test.out
index bea7976bbb..98031da96f 100644
--- a/tests/qapi-schema/qapi-schema-test.out
+++ b/tests/qapi-schema/qapi-schema-test.out
@@ -23,6 +23,15 @@ enum MyEnum
object Empty1
object Empty2
base Empty1
+object q_obj_Union-base
+ member type: EnumOne optional=False
+object Union
+ base q_obj_Union-base
+ tag type
+ case value1: q_empty
+ case value2: q_empty
+ case value3: q_empty
+ case value4: q_empty
command user_def_cmd0 Empty2 -> Empty2
gen=True success_response=True boxed=False oob=False preconfig=False
enum QEnumTwo
diff --git a/tests/qapi-schema/union-empty.err b/tests/qapi-schema/union-empty.err
index 12c20221bd..d4241a38a2 100644
--- a/tests/qapi-schema/union-empty.err
+++ b/tests/qapi-schema/union-empty.err
@@ -1 +1 @@
-tests/qapi-schema/union-empty.json:2: Union 'Union' cannot have empty 'data'
+tests/qapi-schema/union-empty.json:2: Union 'Union' has no branches
diff --git a/tests/qapi-schema/union-empty.json b/tests/qapi-schema/union-empty.json
index 1f0b13ca21..df3e5e639a 100644
--- a/tests/qapi-schema/union-empty.json
+++ b/tests/qapi-schema/union-empty.json
@@ -1,2 +1,2 @@
-# unions cannot be empty
+# simple unions cannot be empty
{ 'union': 'Union', 'data': { } }
--
2.21.0
^ permalink raw reply related [flat|nested] 41+ messages in thread
* [PULL 13/37] qapi: Adjust frontend errors to say enum value, not member
2019-09-24 12:32 [PULL 00/37] QAPI patches for 2019-09-24 Markus Armbruster
` (11 preceding siblings ...)
2019-09-24 12:33 ` [PULL 12/37] qapi: Permit omitting all flat union branches Markus Armbruster
@ 2019-09-24 12:33 ` Markus Armbruster
2019-09-24 12:33 ` [PULL 14/37] docs/devel/qapi-code-gen: Reorder sections for readability Markus Armbruster
` (26 subsequent siblings)
39 siblings, 0 replies; 41+ messages in thread
From: Markus Armbruster @ 2019-09-24 12:33 UTC (permalink / raw)
To: qemu-devel
For consistency with docs/devel/qapi-code-gen.txt.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Message-Id: <20190913201349.24332-12-armbru@redhat.com>
---
scripts/qapi/common.py | 11 ++++++++---
scripts/qapi/events.py | 2 +-
tests/qapi-schema/enum-clash-member.err | 2 +-
tests/qapi-schema/enum-member-case.err | 2 +-
4 files changed, 11 insertions(+), 6 deletions(-)
diff --git a/scripts/qapi/common.py b/scripts/qapi/common.py
index 3393a049cc..a538d2f37c 100644
--- a/scripts/qapi/common.py
+++ b/scripts/qapi/common.py
@@ -1340,7 +1340,7 @@ class QAPISchemaEnumType(QAPISchemaType):
def __init__(self, name, info, doc, ifcond, members, prefix):
QAPISchemaType.__init__(self, name, info, doc, ifcond)
for m in members:
- assert isinstance(m, QAPISchemaMember)
+ assert isinstance(m, QAPISchemaEnumMember)
m.set_owner(name)
assert prefix is None or isinstance(prefix, str)
self.members = members
@@ -1551,6 +1551,10 @@ class QAPISchemaMember(object):
return "'%s' %s" % (self.name, self._pretty_owner())
+class QAPISchemaEnumMember(QAPISchemaMember):
+ role = 'value'
+
+
class QAPISchemaFeature(QAPISchemaMember):
role = 'feature'
@@ -1807,7 +1811,8 @@ class QAPISchema(object):
return [QAPISchemaFeature(f['name'], f.get('if')) for f in features]
def _make_enum_members(self, values):
- return [QAPISchemaMember(v['name'], v.get('if')) for v in values]
+ return [QAPISchemaEnumMember(v['name'], v.get('if'))
+ for v in values]
def _make_implicit_enum_type(self, name, info, ifcond, values):
# See also QAPISchemaObjectTypeMember._pretty_owner()
@@ -2223,7 +2228,7 @@ const QEnumLookup %(c_name)s_lookup = {
def gen_enum(name, members, prefix=None):
# append automatically generated _MAX value
- enum_members = members + [QAPISchemaMember('_MAX')]
+ enum_members = members + [QAPISchemaEnumMember('_MAX')]
ret = mcgen('''
diff --git a/scripts/qapi/events.py b/scripts/qapi/events.py
index e0abfef7b0..7062553cf3 100644
--- a/scripts/qapi/events.py
+++ b/scripts/qapi/events.py
@@ -194,7 +194,7 @@ void %(event_emit)s(%(event_enum)s event, QDict *qdict);
self._event_emit_name))
# Note: we generate the enum member regardless of @ifcond, to
# keep the enumeration usable in target-independent code.
- self._event_enum_members.append(QAPISchemaMember(name))
+ self._event_enum_members.append(QAPISchemaEnumMember(name))
def gen_events(schema, output_dir, prefix):
diff --git a/tests/qapi-schema/enum-clash-member.err b/tests/qapi-schema/enum-clash-member.err
index 5403c78507..8238d2e807 100644
--- a/tests/qapi-schema/enum-clash-member.err
+++ b/tests/qapi-schema/enum-clash-member.err
@@ -1 +1 @@
-tests/qapi-schema/enum-clash-member.json:2: 'one_two' (member of MyEnum) collides with 'one-two' (member of MyEnum)
+tests/qapi-schema/enum-clash-member.json:2: 'one_two' (value of MyEnum) collides with 'one-two' (value of MyEnum)
diff --git a/tests/qapi-schema/enum-member-case.err b/tests/qapi-schema/enum-member-case.err
index 3c67a3a067..5d689e92d5 100644
--- a/tests/qapi-schema/enum-member-case.err
+++ b/tests/qapi-schema/enum-member-case.err
@@ -1 +1 @@
-tests/qapi-schema/enum-member-case.json:4: 'Value' (member of NoWayThisWillGetWhitelisted) should not use uppercase
+tests/qapi-schema/enum-member-case.json:4: 'Value' (value of NoWayThisWillGetWhitelisted) should not use uppercase
--
2.21.0
^ permalink raw reply related [flat|nested] 41+ messages in thread
* [PULL 14/37] docs/devel/qapi-code-gen: Reorder sections for readability
2019-09-24 12:32 [PULL 00/37] QAPI patches for 2019-09-24 Markus Armbruster
` (12 preceding siblings ...)
2019-09-24 12:33 ` [PULL 13/37] qapi: Adjust frontend errors to say enum value, not member Markus Armbruster
@ 2019-09-24 12:33 ` Markus Armbruster
2019-09-24 12:33 ` [PULL 15/37] docs/devel/qapi-code-gen: Rewrite compatibility considerations Markus Armbruster
` (25 subsequent siblings)
39 siblings, 0 replies; 41+ messages in thread
From: Markus Armbruster @ 2019-09-24 12:33 UTC (permalink / raw)
To: qemu-devel
Section "QMP/Guest agent schema" starts with a brief introduction,
then subsection "Comments", then subsection "Schema overview" (more
elaborate introduction), and only then talks about schema entities
like types, commands, and so forth.
Subsection "Comments" is long and tiring: almost 500 words, mostly
about doc comments. Move the doc comment part to its own subsection
"Documentation comments" at the very end of "QMP/Guest agent schema".
Subsection "Schema overview" explains naming rules at considerable
length: 250 words. Move this part to its own subsection "Naming rules
and reserved names" right after the subsections on schema entities.
Subsection "Enumeration types" is wedged between "Struct types" and
"Union types". Move it before "Struct types".
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Message-Id: <20190913201349.24332-13-armbru@redhat.com>
---
docs/devel/qapi-code-gen.txt | 436 ++++++++++++++++++-----------------
1 file changed, 221 insertions(+), 215 deletions(-)
diff --git a/docs/devel/qapi-code-gen.txt b/docs/devel/qapi-code-gen.txt
index ec2d374483..663ef10a56 100644
--- a/docs/devel/qapi-code-gen.txt
+++ b/docs/devel/qapi-code-gen.txt
@@ -50,147 +50,6 @@ schema requires the use of JSON numbers or null.
Comments are allowed; anything between an unquoted # and the following
newline is ignored.
-A multi-line comment that starts and ends with a '##' line is a
-documentation comment. These are parsed by the documentation
-generator, which recognizes certain markup detailed below.
-
-
-==== Documentation markup ====
-
-Comment text starting with '=' is a section title:
-
- # = Section title
-
-Double the '=' for a subsection title:
-
- # == Subsection title
-
-'|' denotes examples:
-
- # | Text of the example, may span
- # | multiple lines
-
-'*' starts an itemized list:
-
- # * First item, may span
- # multiple lines
- # * Second item
-
-You can also use '-' instead of '*'.
-
-A decimal number followed by '.' starts a numbered list:
-
- # 1. First item, may span
- # multiple lines
- # 2. Second item
-
-The actual number doesn't matter. You could even use '*' instead of
-'2.' for the second item.
-
-Lists can't be nested. Blank lines are currently not supported within
-lists.
-
-Additional whitespace between the initial '#' and the comment text is
-permitted.
-
-*foo* and _foo_ are for strong and emphasis styles respectively (they
-do not work over multiple lines). @foo is used to reference a name in
-the schema.
-
-Example:
-
-##
-# = Section
-# == Subsection
-#
-# Some text foo with *strong* and _emphasis_
-# 1. with a list
-# 2. like that
-#
-# And some code:
-# | $ echo foo
-# | -> do this
-# | <- get that
-#
-##
-
-
-==== Expression documentation ====
-
-Expressions other than include and pragma directives may be preceded
-by a documentation block. Such blocks are called expression
-documentation blocks.
-
-When documentation is required (see pragma 'doc-required'), expression
-documentation blocks are mandatory.
-
-The documentation block consists of a first line naming the
-expression, an optional overview, a description of each argument (for
-commands and events) or member (for structs, unions and alternates),
-and optional tagged sections.
-
-FIXME: the parser accepts these things in almost any order.
-
-Extensions added after the expression was first released carry a
-'(since x.y.z)' comment.
-
-A tagged section starts with one of the following words:
-"Note:"/"Notes:", "Since:", "Example"/"Examples", "Returns:", "TODO:".
-The section ends with the start of a new section.
-
-A 'Since: x.y.z' tagged section lists the release that introduced the
-expression.
-
-For example:
-
-##
-# @BlockStats:
-#
-# Statistics of a virtual block device or a block backing device.
-#
-# @device: If the stats are for a virtual block device, the name
-# corresponding to the virtual block device.
-#
-# @node-name: The node name of the device. (since 2.3)
-#
-# ... more members ...
-#
-# Since: 0.14.0
-##
-{ 'struct': 'BlockStats',
- 'data': {'*device': 'str', '*node-name': 'str',
- ... more members ... } }
-
-##
-# @query-blockstats:
-#
-# Query the @BlockStats for all virtual block devices.
-#
-# @query-nodes: If true, the command will query all the
-# block nodes ... explain, explain ... (since 2.3)
-#
-# Returns: A list of @BlockStats for each virtual block devices.
-#
-# Since: 0.14.0
-#
-# Example:
-#
-# -> { "execute": "query-blockstats" }
-# <- {
-# ... lots of output ...
-# }
-#
-##
-{ 'command': 'query-blockstats',
- 'data': { '*query-nodes': 'bool' },
- 'returns': ['BlockStats'] }
-
-==== Free-form documentation ====
-
-A documentation block that isn't an expression documentation block is
-a free-form documentation block. These may be used to provide
-additional text and structuring content.
-
=== Schema overview ===
@@ -217,43 +76,6 @@ refers to a single-dimension array of that type; multi-dimension
arrays are not directly supported (although an array of a complex
struct that contains an array member is possible).
-All names must begin with a letter, and contain only ASCII letters,
-digits, hyphen, and underscore. There are two exceptions: enum values
-may start with a digit, and names that are downstream extensions (see
-section Downstream extensions) start with underscore.
-
-Names beginning with 'q_' are reserved for the generator, which uses
-them for munging QMP names that resemble C keywords or other
-problematic strings. For example, a member named "default" in qapi
-becomes "q_default" in the generated C code.
-
-Types, commands, and events share a common namespace. Therefore,
-generally speaking, type definitions should always use CamelCase for
-user-defined type names, while built-in types are lowercase.
-
-Type names ending with 'Kind' or 'List' are reserved for the
-generator, which uses them for implicit union enums and array types,
-respectively.
-
-Command names, and member names within a type, should be all lower
-case with words separated by a hyphen. However, some existing older
-commands and complex types use underscore; when extending such
-expressions, consistency is preferred over blindly avoiding
-underscore.
-
-Event names should be ALL_CAPS with words separated by underscore.
-
-Member name 'u' and names starting with 'has-' or 'has_' are reserved
-for the generator, which uses them for unions and for tracking
-optional members.
-
-Any name (command, event, type, member, or enum value) beginning with
-"x-" is marked experimental, and may be withdrawn or changed
-incompatibly in a future release.
-
-Pragma 'name-case-whitelist' lets you violate the rules on use of
-upper and lower case. Use for new code is strongly discouraged.
-
In the rest of this document, usage lines are given for each
expression type, with literal strings written in lower case and
placeholders written in capitals. If a literal string includes a
@@ -328,6 +150,43 @@ Pragma 'name-case-whitelist' takes a list of names that may violate
rules on use of upper- vs. lower-case letters. Default is none.
+=== Enumeration types ===
+
+Usage: { 'enum': STRING, 'data': ARRAY-OF-STRING }
+ { 'enum': STRING, '*prefix': STRING, 'data': ARRAY-OF-STRING }
+
+An enumeration type is a dictionary containing a single 'data' key
+whose value is a list of strings. An example enumeration is:
+
+ { 'enum': 'MyEnum', 'data': [ 'value1', 'value2', 'value3' ] }
+
+Nothing prevents an empty enumeration, although it is probably not
+useful. The list of strings should be lower case; if an enum name
+represents multiple words, use '-' between words. The string 'max' is
+not allowed as an enum value, and values should not be repeated.
+
+The enum constants will be named by using a heuristic to turn the
+type name into a set of underscore separated words. For the example
+above, 'MyEnum' will turn into 'MY_ENUM' giving a constant name
+of 'MY_ENUM_VALUE1' for the first value. If the default heuristic
+does not result in a desirable name, the optional 'prefix' member
+can be used when defining the enum.
+
+The enumeration values are passed as strings over the Client JSON
+Protocol, but are encoded as C enum integral values in generated code.
+While the C code starts numbering at 0, it is better to use explicit
+comparisons to enum values than implicit comparisons to 0; the C code
+will also include a generated enum member ending in _MAX for tracking
+the size of the enum, useful when using common functions for
+converting between strings and enum values. Since the wire format
+always passes by name, it is acceptable to reorder or add new
+enumeration members in any location without breaking clients of Client
+JSON Protocol; however, removing enum values would break
+compatibility. For any struct that has a member that will only contain
+a finite set of string values, using an enum type for that member is
+better than open-coding the member to be type 'str'.
+
+
=== Struct types ===
Usage: { 'struct': STRING, 'data': DICT, '*base': STRUCT-NAME }
@@ -389,43 +248,6 @@ both members like this:
"backing": "/some/place/my-backing-file" }
-=== Enumeration types ===
-
-Usage: { 'enum': STRING, 'data': ARRAY-OF-STRING }
- { 'enum': STRING, '*prefix': STRING, 'data': ARRAY-OF-STRING }
-
-An enumeration type is a dictionary containing a single 'data' key
-whose value is a list of strings. An example enumeration is:
-
- { 'enum': 'MyEnum', 'data': [ 'value1', 'value2', 'value3' ] }
-
-Nothing prevents an empty enumeration, although it is probably not
-useful. The list of strings should be lower case; if an enum name
-represents multiple words, use '-' between words. The string 'max' is
-not allowed as an enum value, and values should not be repeated.
-
-The enum constants will be named by using a heuristic to turn the
-type name into a set of underscore separated words. For the example
-above, 'MyEnum' will turn into 'MY_ENUM' giving a constant name
-of 'MY_ENUM_VALUE1' for the first value. If the default heuristic
-does not result in a desirable name, the optional 'prefix' member
-can be used when defining the enum.
-
-The enumeration values are passed as strings over the Client JSON
-Protocol, but are encoded as C enum integral values in generated code.
-While the C code starts numbering at 0, it is better to use explicit
-comparisons to enum values than implicit comparisons to 0; the C code
-will also include a generated enum member ending in _MAX for tracking
-the size of the enum, useful when using common functions for
-converting between strings and enum values. Since the wire format
-always passes by name, it is acceptable to reorder or add new
-enumeration members in any location without breaking clients of Client
-JSON Protocol; however, removing enum values would break
-compatibility. For any struct that has a member that will only contain
-a finite set of string values, using an enum type for that member is
-better than open-coding the member to be type 'str'.
-
-
=== Union types ===
Usage: { 'union': STRING, 'data': DICT }
@@ -744,6 +566,46 @@ This expanded form is necessary if you want to make the feature
conditional (see below in "Configuring the schema").
+=== Naming rules and reserved names ===
+
+All names must begin with a letter, and contain only ASCII letters,
+digits, hyphen, and underscore. There are two exceptions: enum values
+may start with a digit, and names that are downstream extensions (see
+section Downstream extensions) start with underscore.
+
+Names beginning with 'q_' are reserved for the generator, which uses
+them for munging QMP names that resemble C keywords or other
+problematic strings. For example, a member named "default" in qapi
+becomes "q_default" in the generated C code.
+
+Types, commands, and events share a common namespace. Therefore,
+generally speaking, type definitions should always use CamelCase for
+user-defined type names, while built-in types are lowercase.
+
+Type names ending with 'Kind' or 'List' are reserved for the
+generator, which uses them for implicit union enums and array types,
+respectively.
+
+Command names, and member names within a type, should be all lower
+case with words separated by a hyphen. However, some existing older
+commands and complex types use underscore; when extending such
+expressions, consistency is preferred over blindly avoiding
+underscore.
+
+Event names should be ALL_CAPS with words separated by underscore.
+
+Member name 'u' and names starting with 'has-' or 'has_' are reserved
+for the generator, which uses them for unions and for tracking
+optional members.
+
+Any name (command, event, type, member, or enum value) beginning with
+"x-" is marked experimental, and may be withdrawn or changed
+incompatibly in a future release.
+
+Pragma 'name-case-whitelist' lets you violate the rules on use of
+upper and lower case. Use for new code is strongly discouraged.
+
+
=== Downstream extensions ===
QAPI schema names that are externally visible, say in the Client JSON
@@ -814,6 +676,150 @@ The presence of 'if' keys in the schema is reflected through to the
introspection output depending on the build configuration.
+=== Documentation comments ===
+
+A multi-line comment that starts and ends with a '##' line is a
+documentation comment. These are parsed by the documentation
+generator, which recognizes certain markup detailed below.
+
+
+==== Documentation markup ====
+
+Comment text starting with '=' is a section title:
+
+ # = Section title
+
+Double the '=' for a subsection title:
+
+ # == Subsection title
+
+'|' denotes examples:
+
+ # | Text of the example, may span
+ # | multiple lines
+
+'*' starts an itemized list:
+
+ # * First item, may span
+ # multiple lines
+ # * Second item
+
+You can also use '-' instead of '*'.
+
+A decimal number followed by '.' starts a numbered list:
+
+ # 1. First item, may span
+ # multiple lines
+ # 2. Second item
+
+The actual number doesn't matter. You could even use '*' instead of
+'2.' for the second item.
+
+Lists can't be nested. Blank lines are currently not supported within
+lists.
+
+Additional whitespace between the initial '#' and the comment text is
+permitted.
+
+*foo* and _foo_ are for strong and emphasis styles respectively (they
+do not work over multiple lines). @foo is used to reference a name in
+the schema.
+
+Example:
+
+##
+# = Section
+# == Subsection
+#
+# Some text foo with *strong* and _emphasis_
+# 1. with a list
+# 2. like that
+#
+# And some code:
+# | $ echo foo
+# | -> do this
+# | <- get that
+#
+##
+
+
+==== Expression documentation ====
+
+Expressions other than include and pragma directives may be preceded
+by a documentation block. Such blocks are called expression
+documentation blocks.
+
+When documentation is required (see pragma 'doc-required'), expression
+documentation blocks are mandatory.
+
+The documentation block consists of a first line naming the
+expression, an optional overview, a description of each argument (for
+commands and events) or member (for structs, unions and alternates),
+and optional tagged sections.
+
+FIXME: the parser accepts these things in almost any order.
+
+Extensions added after the expression was first released carry a
+'(since x.y.z)' comment.
+
+A tagged section starts with one of the following words:
+"Note:"/"Notes:", "Since:", "Example"/"Examples", "Returns:", "TODO:".
+The section ends with the start of a new section.
+
+A 'Since: x.y.z' tagged section lists the release that introduced the
+expression.
+
+For example:
+
+##
+# @BlockStats:
+#
+# Statistics of a virtual block device or a block backing device.
+#
+# @device: If the stats are for a virtual block device, the name
+# corresponding to the virtual block device.
+#
+# @node-name: The node name of the device. (since 2.3)
+#
+# ... more members ...
+#
+# Since: 0.14.0
+##
+{ 'struct': 'BlockStats',
+ 'data': {'*device': 'str', '*node-name': 'str',
+ ... more members ... } }
+
+##
+# @query-blockstats:
+#
+# Query the @BlockStats for all virtual block devices.
+#
+# @query-nodes: If true, the command will query all the
+# block nodes ... explain, explain ... (since 2.3)
+#
+# Returns: A list of @BlockStats for each virtual block devices.
+#
+# Since: 0.14.0
+#
+# Example:
+#
+# -> { "execute": "query-blockstats" }
+# <- {
+# ... lots of output ...
+# }
+#
+##
+{ 'command': 'query-blockstats',
+ 'data': { '*query-nodes': 'bool' },
+ 'returns': ['BlockStats'] }
+
+==== Free-form documentation ====
+
+A documentation block that isn't an expression documentation block is
+a free-form documentation block. These may be used to provide
+additional text and structuring content.
+
+
== Client JSON Protocol introspection ==
Clients of a Client JSON Protocol commonly need to figure out what
--
2.21.0
^ permalink raw reply related [flat|nested] 41+ messages in thread
* [PULL 15/37] docs/devel/qapi-code-gen: Rewrite compatibility considerations
2019-09-24 12:32 [PULL 00/37] QAPI patches for 2019-09-24 Markus Armbruster
` (13 preceding siblings ...)
2019-09-24 12:33 ` [PULL 14/37] docs/devel/qapi-code-gen: Reorder sections for readability Markus Armbruster
@ 2019-09-24 12:33 ` Markus Armbruster
2019-09-24 12:33 ` [PULL 16/37] docs/devel/qapi-code-gen: Rewrite introduction to schema Markus Armbruster
` (24 subsequent siblings)
39 siblings, 0 replies; 41+ messages in thread
From: Markus Armbruster @ 2019-09-24 12:33 UTC (permalink / raw)
To: qemu-devel
We have some compatibility advice buried in sections "Enumeration
types" and "Struct types". Compatibility is actually about commands
and events. It devolves to the types used there. All kinds of types,
not just enumerations and structs.
Replace the existing advice by a new section "Compatibility
considerations".
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Message-Id: <20190913201349.24332-14-armbru@redhat.com>
[Squash in paragraph on invisible schema changes, as per Eric's review]
---
docs/devel/qapi-code-gen.txt | 99 +++++++++++++++++++++++-------------
1 file changed, 64 insertions(+), 35 deletions(-)
diff --git a/docs/devel/qapi-code-gen.txt b/docs/devel/qapi-code-gen.txt
index 663ef10a56..ac36ed34e3 100644
--- a/docs/devel/qapi-code-gen.txt
+++ b/docs/devel/qapi-code-gen.txt
@@ -178,14 +178,11 @@ While the C code starts numbering at 0, it is better to use explicit
comparisons to enum values than implicit comparisons to 0; the C code
will also include a generated enum member ending in _MAX for tracking
the size of the enum, useful when using common functions for
-converting between strings and enum values. Since the wire format
-always passes by name, it is acceptable to reorder or add new
-enumeration members in any location without breaking clients of Client
-JSON Protocol; however, removing enum values would break
-compatibility. For any struct that has a member that will only contain
-a finite set of string values, using an enum type for that member is
-better than open-coding the member to be type 'str'.
+converting between strings and enum values.
+For any struct that has a member that will only contain a finite set
+of string values, using an enum type for that member is better than
+open-coding the member to be type 'str'.
=== Struct types ===
@@ -203,34 +200,6 @@ name. An example of a struct is:
The use of '*' as a prefix to the name means the member is optional in
the corresponding JSON protocol usage.
-The default initialization value of an optional argument should not be changed
-between versions of QEMU unless the new default maintains backward
-compatibility to the user-visible behavior of the old default.
-
-With proper documentation, this policy still allows some flexibility; for
-example, documenting that a default of 0 picks an optimal buffer size allows
-one release to declare the optimal size at 512 while another release declares
-the optimal size at 4096 - the user-visible behavior is not the bytes used by
-the buffer, but the fact that the buffer was optimal size.
-
-On input structures (only mentioned in the 'data' side of a command), changing
-from mandatory to optional is safe (older clients will supply the option, and
-newer clients can benefit from the default); changing from optional to
-mandatory is backwards incompatible (older clients may be omitting the option,
-and must continue to work).
-
-On output structures (only mentioned in the 'returns' side of a command),
-changing from mandatory to optional is in general unsafe (older clients may be
-expecting the member, and could crash if it is missing), although it
-can be done if the only way that the optional argument will be omitted
-is when it is triggered by the presence of a new input flag to the
-command that older clients don't know to send. Changing from optional
-to mandatory is safe.
-
-A structure that is used in both input and output of various commands
-must consider the backwards compatibility constraints of both directions
-of use.
-
A struct definition can specify another struct as its base.
In this case, the members of the base type are included as top-level members
of the new struct's dictionary in the Client JSON Protocol wire
@@ -1037,6 +1006,66 @@ the names of built-in types. Clients should examine member
"json-type" instead of hard-coding names of built-in types.
+== Compatibility considerations ==
+
+Maintaining backward compatibility at the Client JSON Protocol level
+while evolving the schema requires some care. This section is about
+syntactic compatibility, which is necessary, but not sufficient, for
+actual compatibility.
+
+Clients send commands with argument data, and receive command
+responses with return data and events with event data.
+
+Adding opt-in functionality to the send direction is backwards
+compatible: adding commands, optional arguments, enumeration values,
+union and alternate branches; turning an argument type into an
+alternate of that type; making mandatory arguments optional. Clients
+oblivious of the new functionality continue to work.
+
+Incompatible changes include removing commands, command arguments,
+enumeration values, union and alternate branches, adding mandatory
+command arguments, and making optional arguments mandatory.
+
+The specified behavior of an absent optional argument should remain
+the same. With proper documentation, this policy still allows some
+flexibility; for example, when an optional 'buffer-size' argument is
+specified to default to a sensible buffer size, the actual default
+value can still be changed. The specified default behavior is not the
+exact size of the buffer, only that the default size is sensible.
+
+Adding functionality to the receive direction is generally backwards
+compatible: adding events, adding return and event data members.
+Clients are expected to ignore the ones they don't know.
+
+Removing "unreachable" stuff like events that can't be triggered
+anymore, optional return or event data members that can't be sent
+anymore, and return or event data member (enumeration) values that
+can't be sent anymore makes no difference to clients, except for
+introspection. The latter can conceivably confuse clients, so tread
+carefully.
+
+Incompatible changes include removing return and event data members.
+
+Any change to a command definition's 'data' or one of the types used
+there (recursively) needs to consider send direction compatibility.
+
+Any change to a command definition's 'return', an event definition's
+'data', or one of the types used there (recursively) needs to consider
+receive direction compatibility.
+
+Any change to types used in both contexts need to consider both.
+
+Members of enumeration types, complex types and alternate types may be
+reordered freely. For enumerations and alternate types, this doesn't
+affect the wire encoding. For complex types, this might make the
+implementation emit JSON object members in a different order, which
+the Client JSON Protocol permits.
+
+Since type names are not visible in the Client JSON Protocol, types
+may be freely renamed. Even certain refactorings are invisible, such
+as splitting members from one type into a common base type.
+
+
== Code generation ==
The QAPI code generator qapi-gen.py generates code and documentation
--
2.21.0
^ permalink raw reply related [flat|nested] 41+ messages in thread
* [PULL 16/37] docs/devel/qapi-code-gen: Rewrite introduction to schema
2019-09-24 12:32 [PULL 00/37] QAPI patches for 2019-09-24 Markus Armbruster
` (14 preceding siblings ...)
2019-09-24 12:33 ` [PULL 15/37] docs/devel/qapi-code-gen: Rewrite compatibility considerations Markus Armbruster
@ 2019-09-24 12:33 ` Markus Armbruster
2019-09-24 12:33 ` [PULL 17/37] docs/devel/qapi-code-gen: Improve QAPI schema language doc Markus Armbruster
` (23 subsequent siblings)
39 siblings, 0 replies; 41+ messages in thread
From: Markus Armbruster @ 2019-09-24 12:33 UTC (permalink / raw)
To: qemu-devel
The introduction to the QAPI schema is somewhat rambling. Rewrite for
clarity.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Message-Id: <20190913201349.24332-15-armbru@redhat.com>
---
docs/devel/qapi-code-gen.txt | 107 ++++++++++++++++-------------------
1 file changed, 48 insertions(+), 59 deletions(-)
diff --git a/docs/devel/qapi-code-gen.txt b/docs/devel/qapi-code-gen.txt
index ac36ed34e3..d7bc79b48b 100644
--- a/docs/devel/qapi-code-gen.txt
+++ b/docs/devel/qapi-code-gen.txt
@@ -16,65 +16,54 @@ well as the QEMU Guest Agent (QGA) for communicating with the guest.
The remainder of this document uses "Client JSON Protocol" when
referring to the wire contents of a QMP or QGA connection.
-To map Client JSON Protocol interfaces to the native C QAPI
-implementations, a JSON-based schema is used to define types and
-function signatures, and a set of scripts is used to generate types,
-signatures, and marshaling/dispatch code. This document will describe
-how the schemas, scripts, and resulting code are used.
-
-
-== QMP/Guest agent schema ==
-
-A QAPI schema file is designed to be loosely based on JSON
-(http://www.ietf.org/rfc/rfc8259.txt) with changes for quoting style
-and the use of comments; a QAPI schema file is then parsed by a python
-code generation program. A valid QAPI schema consists of a series of
-top-level expressions, with no commas between them. Where
-dictionaries (JSON objects) are used, they are parsed as python
-OrderedDicts so that ordering is preserved (for predictable layout of
-generated C structs and parameter lists). Ordering doesn't matter
-between top-level expressions or the keys within an expression, but
-does matter within dictionary values for 'data' and 'returns' members
-of a single expression. QAPI schema input is written using 'single
-quotes' instead of JSON's "double quotes" (in contrast, Client JSON
-Protocol uses no comments, and while input accepts 'single quotes' as
-an extension, output is strict JSON using only "double quotes"). As
-in JSON, trailing commas are not permitted in arrays or dictionaries.
-Input must be ASCII (although QMP supports full Unicode strings, the
-QAPI parser does not). At present, there is no place where a QAPI
-schema requires the use of JSON numbers or null.
-
-
-=== Comments ===
-
-Comments are allowed; anything between an unquoted # and the following
-newline is ignored.
-
-
-=== Schema overview ===
-
-The schema sets up a series of types, as well as commands and events
-that will use those types. Forward references are allowed: the parser
-scans in two passes, where the first pass learns all type names, and
-the second validates the schema and generates the code. This allows
-the definition of complex structs that can have mutually recursive
-types, and allows for indefinite nesting of Client JSON Protocol that
-satisfies the schema. A type name should not be defined more than
-once. It is permissible for the schema to contain additional types
-not used by any commands or events in the Client JSON Protocol, for
-the side effect of generated C code used internally.
-
-There are eight top-level expressions recognized by the parser:
-'include', 'pragma', 'command', 'struct', 'enum', 'union',
-'alternate', and 'event'. There are several groups of types: simple
-types (a number of built-in types, such as 'int' and 'str'; as well as
-enumerations), complex types (structs and two flavors of unions), and
-alternate types (a choice between other types). The 'command' and
-'event' expressions can refer to existing types by name, or list an
-anonymous type as a dictionary. Listing a type name inside an array
-refers to a single-dimension array of that type; multi-dimension
-arrays are not directly supported (although an array of a complex
-struct that contains an array member is possible).
+To map between Client JSON Protocol interfaces and the native C API,
+we generate C code from a QAPI schema. This document describes the
+QAPI schema language, and how it gets mapped to the Client JSON
+Protocol and to C. It additionally provides guidance on maintaining
+Client JSON Protocol compatibility.
+
+
+== The QAPI schema language ==
+
+The QAPI schema defines the Client JSON Protocol's commands and
+events, as well as types used by them. Forward references are
+allowed.
+
+It is permissible for the schema to contain additional types not used
+by any commands or events, for the side effect of generated C code
+used internally.
+
+There are several kinds of types: simple types (a number of built-in
+types, such as 'int' and 'str'; as well as enumerations), arrays,
+complex types (structs and two flavors of unions), and alternate types
+(a choice between other types).
+
+
+=== Schema syntax ===
+
+Syntax is loosely based on JSON (http://www.ietf.org/rfc/rfc8259.txt).
+Differences:
+
+* Comments: start with a hash character (#) that is not part of a
+ string, and extend to the end of the line.
+
+* Strings are enclosed in 'single quotes', not "double quotes".
+
+* Strings are restricted to printable ASCII, and escape sequences to
+ just '\\'.
+
+* Numbers are not supported.
+
+A QAPI schema consists of a series of top-level expressions (JSON
+objects). Code and documentation is generated in schema definition
+order. Code order should not matter.
+
+The order of keys within JSON objects does not matter unless
+explicitly noted.
+
+There are eight kinds of top-level expressions: 'include', 'pragma',
+'command', 'struct', 'enum', 'union', 'alternate', and 'event'. These
+are discussed in detail below.
In the rest of this document, usage lines are given for each
expression type, with literal strings written in lower case and
--
2.21.0
^ permalink raw reply related [flat|nested] 41+ messages in thread
* [PULL 17/37] docs/devel/qapi-code-gen: Improve QAPI schema language doc
2019-09-24 12:32 [PULL 00/37] QAPI patches for 2019-09-24 Markus Armbruster
` (15 preceding siblings ...)
2019-09-24 12:33 ` [PULL 16/37] docs/devel/qapi-code-gen: Rewrite introduction to schema Markus Armbruster
@ 2019-09-24 12:33 ` Markus Armbruster
2019-09-24 12:33 ` [PULL 18/37] qapi: Tweak code to match docs/devel/qapi-code-gen.txt Markus Armbruster
` (22 subsequent siblings)
39 siblings, 0 replies; 41+ messages in thread
From: Markus Armbruster @ 2019-09-24 12:33 UTC (permalink / raw)
To: qemu-devel
We document the language by giving patterns of valid JSON objects.
The patterns contain placeholders we don't define anywhere; their
names have to speak for themselves. I guess they do, but I'd prefer a
bit more rigor. Provide a grammar instead, and rework the text
accordingly.
Documentation for QAPI schema conditionals (commit 967c885108,
6cc32b0e14, 87adbbffd4..3e270dcacc) and feature flags (commit
6a8c0b5102) was bolted on. The sections documenting types, commands
and events don't mention them. Section "Features" and "Configuring
the schema" then provide additional syntax for types, commands and
events. I hate that. Fix the sections documenting types, commands
and events to provide accurate syntax, and point to "Features" and
"Configuring the schema" for details.
We talk about "(top-level) expressions other than include and pragma".
Adopt more convenient terminology: a (top-level) expression is either
a directive (include or pragma) or a definition (anything else).
Avoid the terms "dictionary" and "key". Stick to JSON terminology
"object" and "member name" instead.
While there, make spacing more consistent.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Message-Id: <20190913201349.24332-16-armbru@redhat.com>
---
docs/devel/qapi-code-gen.txt | 596 ++++++++++++++++++++++-------------
1 file changed, 372 insertions(+), 224 deletions(-)
diff --git a/docs/devel/qapi-code-gen.txt b/docs/devel/qapi-code-gen.txt
index d7bc79b48b..b12a5b4de8 100644
--- a/docs/devel/qapi-code-gen.txt
+++ b/docs/devel/qapi-code-gen.txt
@@ -4,12 +4,12 @@ Copyright IBM Corp. 2011
Copyright (C) 2012-2016 Red Hat, Inc.
This work is licensed under the terms of the GNU GPL, version 2 or
-later. See the COPYING file in the top-level directory.
+later. See the COPYING file in the top-level directory.
== Introduction ==
QAPI is a native C API within QEMU which provides management-level
-functionality to internal and external users. For external
+functionality to internal and external users. For external
users/processes, this interface is made available by a JSON-based wire
format for the QEMU Monitor Protocol (QMP) for controlling qemu, as
well as the QEMU Guest Agent (QGA) for communicating with the guest.
@@ -54,24 +54,49 @@ Differences:
* Numbers are not supported.
-A QAPI schema consists of a series of top-level expressions (JSON
-objects). Code and documentation is generated in schema definition
-order. Code order should not matter.
+A second layer of syntax defines the sequences of JSON texts that are
+a correctly structured QAPI schema. We provide a grammar for this
+syntax in an EBNF-like notation:
-The order of keys within JSON objects does not matter unless
+* Production rules look like non-terminal = expression
+* Concatenation: expression A B matches expression A, then B
+* Alternation: expression A | B matches expression A or B
+* Repetition: expression A... matches zero or more occurrences of
+ expression A
+* Repetition: expression A, ... matches zero or more occurrences of
+ expression A separated by ,
+* Grouping: expression ( A ) matches expression A
+* JSON's structural characters are terminals: { } [ ] : ,
+* JSON's literal names are terminals: false true null
+* String literals enclosed in 'single quotes' are terminal, and match
+ this JSON string, with a leading '*' stripped off
+* When JSON object member's name starts with '*', the member is
+ optional.
+* The symbol STRING is a terminal, and matches any JSON string
+* The symbol BOOL is a terminal, and matches JSON false or true
+* ALL-CAPS words other than STRING are non-terminals
+
+The order of members within JSON objects does not matter unless
explicitly noted.
-There are eight kinds of top-level expressions: 'include', 'pragma',
-'command', 'struct', 'enum', 'union', 'alternate', and 'event'. These
-are discussed in detail below.
-
-In the rest of this document, usage lines are given for each
-expression type, with literal strings written in lower case and
-placeholders written in capitals. If a literal string includes a
-prefix of '*', that key/value pair can be omitted from the expression.
-For example, a usage statement that includes '*base':STRUCT-NAME
-means that an expression has an optional key 'base', which if present
-must have a value that forms a struct name.
+A QAPI schema consists of a series of top-level expressions:
+
+ SCHEMA = TOP-LEVEL-EXPR...
+
+The top-level expressions are all JSON objects. Code and
+documentation is generated in schema definition order. Code order
+should not matter.
+
+A top-level expressions is either a directive or a definition:
+
+ TOP-LEVEL-EXPR = DIRECTIVE | DEFINITION
+
+There are two kinds of directives and six kinds of definitions:
+
+ DIRECTIVE = INCLUDE | PRAGMA
+ DEFINITION = ENUM | STRUCT | UNION | ALTERNATE | COMMAND | EVENT
+
+These are discussed in detail below.
=== Built-in Types ===
@@ -101,16 +126,16 @@ The following types are predefined, and map to C as follows:
=== Include directives ===
-Usage: { 'include': STRING }
+Syntax:
+ INCLUDE = { 'include': STRING }
The QAPI schema definitions can be modularized using the 'include' directive:
{ 'include': 'path/to/file.json' }
-The directive is evaluated recursively, and include paths are relative to the
-file using the directive. Multiple includes of the same file are
-idempotent. No other keys should appear in the expression, and the include
-value should be a string.
+The directive is evaluated recursively, and include paths are relative
+to the file using the directive. Multiple includes of the same file
+are idempotent.
As a matter of style, it is a good idea to have all files be
self-contained, but at the moment, nothing prevents an included file
@@ -121,10 +146,12 @@ prevent incomplete include files.
=== Pragma directives ===
-Usage: { 'pragma': DICT }
+Syntax:
+ PRAGMA = { 'pragma': { '*doc-required': BOOL,
+ '*returns-whitelist': [ STRING, ... ],
+ '*name-case-whitelist': [ STRING, ... ] } }
The pragma directive lets you control optional generator behavior.
-The dictionary's entries are pragma names and values.
Pragma's scope is currently the complete schema. Setting the same
pragma to different values in parts of the schema doesn't work.
@@ -141,60 +168,95 @@ rules on use of upper- vs. lower-case letters. Default is none.
=== Enumeration types ===
-Usage: { 'enum': STRING, 'data': ARRAY-OF-STRING }
- { 'enum': STRING, '*prefix': STRING, 'data': ARRAY-OF-STRING }
+Syntax:
+ ENUM = { 'enum': STRING,
+ 'data': [ ENUM-VALUE, ... ],
+ '*prefix': STRING,
+ '*if': COND }
+ ENUM-VALUE = STRING
+ | { 'name': STRING, '*if': COND }
-An enumeration type is a dictionary containing a single 'data' key
-whose value is a list of strings. An example enumeration is:
+Member 'enum' names the enum type.
+
+Each member of the 'data' array defines a value of the enumeration
+type. The form STRING is shorthand for { 'name': STRING }. The
+'name' values must be be distinct.
+
+Example:
{ 'enum': 'MyEnum', 'data': [ 'value1', 'value2', 'value3' ] }
Nothing prevents an empty enumeration, although it is probably not
-useful. The list of strings should be lower case; if an enum name
-represents multiple words, use '-' between words. The string 'max' is
-not allowed as an enum value, and values should not be repeated.
-
-The enum constants will be named by using a heuristic to turn the
-type name into a set of underscore separated words. For the example
-above, 'MyEnum' will turn into 'MY_ENUM' giving a constant name
-of 'MY_ENUM_VALUE1' for the first value. If the default heuristic
-does not result in a desirable name, the optional 'prefix' member
-can be used when defining the enum.
-
-The enumeration values are passed as strings over the Client JSON
-Protocol, but are encoded as C enum integral values in generated code.
-While the C code starts numbering at 0, it is better to use explicit
-comparisons to enum values than implicit comparisons to 0; the C code
-will also include a generated enum member ending in _MAX for tracking
-the size of the enum, useful when using common functions for
-converting between strings and enum values.
-
-For any struct that has a member that will only contain a finite set
-of string values, using an enum type for that member is better than
-open-coding the member to be type 'str'.
+useful.
+
+On the wire, an enumeration type's value is represented by its
+(string) name. In C, it's represented by an enumeration constant.
+These are of the form PREFIX_NAME, where PREFIX is derived from the
+enumeration type's name, and NAME from the value's name. For the
+example above, the generator maps 'MyEnum' to MY_ENUM and 'value1' to
+VALUE1, resulting in the enumeration constant MY_ENUM_VALUE1. The
+optional 'prefix' member overrides PREFIX.
+
+The generated C enumeration constants have values 0, 1, ..., N-1 (in
+QAPI schema order), where N is the number of values. There is an
+additional enumeration constant PREFIX__MAX with value N.
+
+Do not use string or an integer type when an enumeration type can do
+the job satisfactorily.
+
+The optional 'if' member specifies a conditional. See "Configuring
+the schema" below for more on this.
+
+
+=== Type references and array types ===
+
+Syntax:
+ TYPE-REF = STRING | ARRAY-TYPE
+ ARRAY-TYPE = [ STRING ]
+
+A string denotes the type named by the string.
+
+A one-element array containing a string denotes an array of the type
+named by the string. Example: ['int'] denotes an array of 'int'.
+
=== Struct types ===
-Usage: { 'struct': STRING, 'data': DICT, '*base': STRUCT-NAME }
+Syntax:
+ STRUCT = { 'struct': STRING,
+ 'data': MEMBERS,
+ '*base': STRING,
+ '*if': COND,
+ '*features': FEATURES }
+ MEMBERS = { MEMBER, ... }
+ MEMBER = STRING : TYPE-REF
+ | STRING : { 'type': TYPE-REF, '*if': COND }
-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:
+Member 'struct' names the struct type.
+
+Each MEMBER of the 'data' object defines a member of the struct type.
+
+The MEMBER's STRING name consists of an optional '*' prefix and the
+struct member name. If '*' is present, the member is optional.
+
+The MEMBER's value defines its properties, in particular its type.
+The form TYPE-REF is shorthand for { 'type': TYPE-REF }.
+
+Example:
{ 'struct': 'MyType',
- 'data': { 'member1': 'str', 'member2': 'int', '*member3': 'str' } }
+ 'data': { 'member1': 'str', 'member2': ['int'], '*member3': 'str' } }
-The use of '*' as a prefix to the name means the member is optional in
-the corresponding JSON protocol usage.
+A struct type corresponds to a struct in C, and an object in JSON.
+The C struct's members are generated in QAPI schema order.
-A struct definition can specify another struct as its base.
-In this case, the members of the base type are included as top-level members
-of the new struct's dictionary in the Client JSON Protocol wire
-format. An example definition is:
+The optional 'base' member names a struct type whose members are to be
+included in this type. They go first in the C struct.
- { 'struct': 'BlockdevOptionsGenericFormat', 'data': { 'file': 'str' } }
+Example:
+
+ { 'struct': 'BlockdevOptionsGenericFormat',
+ 'data': { 'file': 'str' } }
{ 'struct': 'BlockdevOptionsGenericCOWFormat',
'base': 'BlockdevOptionsGenericFormat',
'data': { '*backing': 'str' } }
@@ -205,18 +267,40 @@ both members like this:
{ "file": "/some/place/my-image",
"backing": "/some/place/my-backing-file" }
+The optional 'if' member specifies a conditional. See "Configuring
+the schema" below for more on this.
+
+The optional 'features' member specifies features. See "Features"
+below for more on this.
+
=== Union types ===
-Usage: { 'union': STRING, 'data': DICT }
-or: { 'union': STRING, 'data': DICT, 'base': STRUCT-NAME-OR-DICT,
- 'discriminator': ENUM-MEMBER-OF-BASE }
-
-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), and flat (both discriminator and base). A union
-type is defined using a data dictionary as explained in the following
-paragraphs. Unions must have at least one branch.
+Syntax:
+ UNION = { 'union': STRING,
+ 'data': BRANCHES,
+ '*if': COND }
+ | { 'union': STRING,
+ 'data': BRANCHES,
+ 'base': ( MEMBERS | STRING ),
+ 'discriminator': STRING,
+ '*if': COND }
+ BRANCHES = { BRANCH, ... }
+ BRANCH = STRING : TYPE-REF
+ | STRING : { 'type': TYPE-REF, '*if': COND }
+
+Member 'union' names the union type.
+
+There are two flavors of union types: simple (no discriminator or
+base), and flat (both discriminator and base).
+
+Each BRANCH of the 'data' object defines a branch of the union. A
+union must have at least one branch.
+
+The BRANCH's STRING name is the branch name.
+
+The BRANCH's value defines the branch's properties, in particular its
+type. The form TYPE-REF is shorthand for { 'type': TYPE-REF }.
A simple union type defines a mapping from automatic discriminator
values to data types like in this example:
@@ -229,8 +313,8 @@ values to data types like in this example:
'data': { 'file': 'BlockdevOptionsFile',
'qcow2': 'BlockdevOptionsQcow2' } }
-In the Client JSON Protocol, a simple union is represented by a
-dictionary that contains the 'type' member as a discriminator, and a
+In the Client JSON Protocol, a simple union is represented by an
+object that contains the 'type' member as a discriminator, and a
'data' member that is of the specified data type corresponding to the
discriminator value, as in these examples:
@@ -238,21 +322,27 @@ discriminator value, as in these examples:
{ "type": "qcow2", "data": { "backing": "/some/place/my-image",
"lazy-refcounts": true } }
-The generated C code uses a struct containing a union. Additionally,
+The generated C code uses a struct containing a union. Additionally,
an implicit C enum 'NameKind' is created, corresponding to the union
'Name', for accessing the various branches of the union. The value
for each branch can be of any type.
-A flat union definition avoids nesting on the wire, and specifies a
-set of common members that occur in all variants of the union. The
-'base' key must specify either a type name (the type must be a
-struct, not a union), or a dictionary representing an anonymous type.
-All branches of the union must be struct types, and the top-level
-members of the union dictionary on the wire will be combination of
-members from both the base type and the appropriate branch type (when
-merging two dictionaries, there must be no keys in common). The
-'discriminator' member must be the name of a non-optional enum-typed
-member of the base struct.
+Flat unions permit arbitrary common members that occur in all variants
+of the union, not just a discriminator. Their discriminators need not
+be named 'type'. They also avoid nesting on the wire.
+
+The 'base' member defines the common members. If it is a MEMBERS
+object, it defines common members just like a struct type's 'data'
+member defines struct type members. If it is a STRING, it names a
+struct type whose members are the common members.
+
+All flat union branches must be of struct type.
+
+In the Client JSON Protocol, a flat union is represented by an object
+with the common members (from the base type) and the selected branch's
+members. The two sets of member names must be disjoint. Member
+'discriminator' must name a non-optional enum-typed member of the base
+struct.
The following example enhances the above simple union example by
adding an optional common member 'read-only', renaming the
@@ -276,12 +366,13 @@ Resulting in these JSON objects:
Notice that in a flat union, the discriminator name is controlled by
the user, but because it must map to a base member with enum type, the
code generator ensures that branches match the existing values of the
-enum. The order of the keys need not match the declaration of the enum.
-The keys need not cover all possible enum values. Omitted enum values
-are still valid branches that add no additional members to the data type.
-In the resulting generated C data types, a flat union is
-represented as a struct with the base members included directly, and
-then a union of structures for each branch of the struct.
+enum. The order of branches need not match the order of the enum
+values. The branches need not cover all possible enum values.
+Omitted enum values are still valid branches that add no additional
+members to the data type. In the resulting generated C data types, a
+flat union is represented as a struct with the base members in QAPI
+schema order, and then a union of structures for each branch of the
+struct.
A simple union can always be re-written as a flat union where the base
class has a single member named 'type', and where each branch of the
@@ -297,32 +388,47 @@ is identical on the wire to:
{ 'union': 'Flat': 'base': { 'type': 'Enum' }, 'discriminator': 'type',
'data': { 'one': 'Branch1', 'two': 'Branch2' } }
+The optional 'if' member specifies a conditional. See "Configuring
+the schema" below for more on this.
+
=== Alternate types ===
-Usage: { 'alternate': STRING, 'data': DICT }
+Syntax:
+ ALTERNATE = { 'alternate': STRING,
+ 'data': ALTERNATIVES,
+ '*if': COND }
+ ALTERNATIVES = { ALTERNATIVE, ... }
+ ALTERNATIVE = STRING : TYPE-REF
+ | STRING : { 'type': STRING, '*if': COND }
-An alternate type is one that allows a choice between two or more JSON
-data types (string, integer, number, or object, but currently not
-array) on the wire. The definition is similar to a simple union type,
-where each branch of the union names a QAPI type. For example:
+Member 'alternate' names the alternate type.
+
+Each ALTERNATIVE of the 'data' object defines a branch of the
+alternate. An alternate must have at least one branch.
+
+The ALTERNATIVE's STRING name is the branch name.
+
+The ALTERNATIVE's value defines the branch's properties, in particular
+its type. The form STRING is shorthand for { 'type': STRING }.
+
+Example:
{ 'alternate': 'BlockdevRef',
'data': { 'definition': 'BlockdevOptions',
'reference': 'str' } }
-Unlike a union, the discriminator string is never passed on the wire
-for the Client JSON Protocol. Instead, the value's JSON type serves
-as an implicit discriminator, which in turn means that an alternate
-can only express a choice between types represented differently in
-JSON. If a branch is typed as the 'bool' built-in, the alternate
-accepts true and false; if it is typed as any of the various numeric
+An alternate type is like a union type, except there is no
+discriminator on the wire. Instead, the branch to use is inferred
+from the value. An alternate can only express a choice between types
+represented differently on the wire.
+
+If a branch is typed as the 'bool' built-in, the alternate accepts
+true and false; if it is typed as any of the various numeric
built-ins, it accepts a JSON number; if it is typed as a 'str'
built-in or named enum type, it accepts a JSON string; if it is typed
as the 'null' built-in, it accepts JSON null; and if it is typed as a
-complex type (struct or union), it accepts a JSON object. Two
-different complex types, for instance, aren't permitted, because both
-are represented as a JSON object.
+complex type (struct or union), it accepts a JSON object.
The example alternate declaration above allows using both of the
following example objects:
@@ -332,43 +438,52 @@ following example objects:
"read-only": false,
"filename": "/tmp/mydisk.qcow2" } }
+The optional 'if' member specifies a conditional. See "Configuring
+the schema" below for more on this.
+
=== Commands ===
---- General Command Layout ---
+Syntax:
+ COMMAND = { 'command': STRING,
+ (
+ '*data': ( MEMBERS | STRING ),
+ |
+ 'data': STRING,
+ 'boxed': true,
+ )
+ '*returns': TYPE-REF,
+ '*success-response': false,
+ '*gen': false,
+ '*allow-oob': true,
+ '*allow-preconfig': true,
+ '*if': COND }
-Usage: { 'command': STRING, '*data': COMPLEX-TYPE-NAME-OR-DICT,
- '*returns': TYPE-NAME, '*boxed': true,
- '*gen': false, '*success-response': false,
- '*allow-oob': true, '*allow-preconfig': true }
+Member 'command' names the command.
-Commands are defined by using a dictionary containing several members,
-where three members are most common. The 'command' member is a
-mandatory string, and determines the "execute" value passed in a
-Client JSON Protocol command exchange.
+Member 'data' defines the arguments. It defaults to an empty MEMBERS
+object.
-The 'data' argument maps to the "arguments" dictionary passed in as
-part of a Client JSON Protocol command. The 'data' member is optional
-and defaults to {} (an empty dictionary). If present, it must be the
-string name of a complex type, or a dictionary that declares an
-anonymous type with the same semantics as a 'struct' expression.
+If 'data' is a MEMBERS object, then MEMBERS defines arguments just
+like a struct type's 'data' defines struct type members.
-The 'returns' member describes what will appear in the "return" member
-of a Client JSON Protocol reply on successful completion of a command.
-The member is optional from the command declaration; if absent, the
-"return" member will be an empty dictionary. If 'returns' is present,
-it must be the string name of a complex type, or a
-one-element array containing the name of a complex type.
-To return anything else, you have to list the command in pragma
-'returns-whitelist'. If you do this, the command cannot be extended
-to return additional information in the future. Use of
+If 'data' is a STRING, then STRING names a complex type whose members
+are the arguments. A union type requires 'boxed': true.
+
+Member 'returns' defines the command's return type. It defaults to an
+empty struct type. It must normally be a complex type or an array of
+a complex type. To return anything else, the command must be listed
+in pragma 'returns-whitelist'. If you do this, extending the command
+to return additional information will be harder. Use of
'returns-whitelist' for new commands is strongly discouraged.
-All commands in Client JSON Protocol use a dictionary to report
-failure, with no way to specify that in QAPI. Where the error return
-is different than the usual GenericError class in order to help the
-client react differently to certain error conditions, it is worth
-documenting this in the comments before the command declaration.
+A command's error responses are not specified in the QAPI schema.
+Error conditions should be documented in comments.
+
+In the Client JSON Protocol, the value of the "execute" or "exec-oob"
+member is the command name. The value of the "arguments" member then
+has to conform to the arguments, and the value of the success
+response's "return" member will conform to the return type.
Some example commands:
@@ -386,23 +501,24 @@ which would validate this Client JSON Protocol transaction:
=> { "execute": "my-second-command" }
<= { "return": [ { "value": "one" }, { } ] }
-The generator emits a prototype for the user's function implementing
-the command. Normally, 'data' is a dictionary for an anonymous type,
-or names a struct type (possibly empty, but not a union), and its
-members are passed as separate arguments to this function. If the
-command definition includes a key 'boxed' with the boolean value true,
-then 'data' is instead the name of any non-empty complex type (struct
-or union), and a pointer to that QAPI type is passed as a single
-argument.
+The generator emits a prototype for the C function implementing the
+command. The function itself needs to be written by hand. See
+section "Code generated for commands" for examples.
+
+The function returns the return type. When member 'boxed' is absent,
+it takes the command arguments as arguments one by one, in QAPI schema
+order. Else it takes them wrapped in the C struct generated for the
+complex argument type. It takes an additional Error ** argument in
+either case.
The generator also emits a marshalling function that extracts
arguments for the user's function out of an input QDict, calls the
user's function, and if it succeeded, builds an output QObject from
-its return value.
+its return value. This is for use by the QMP monitor core.
In rare cases, QAPI cannot express a type-safe representation of a
corresponding Client JSON Protocol command. You then have to suppress
-generation of a marshalling function by including a key 'gen' with
+generation of a marshalling function by including a member 'gen' with
boolean value false, and instead write your own function. For
example:
@@ -416,13 +532,12 @@ use type-safe unions.
Normally, the QAPI schema is used to describe synchronous exchanges,
where a response is expected. But in some cases, the action of a
command is expected to change state in a way that a successful
-response is not possible (although the command will still return a
-normal dictionary error on failure). When a successful reply is not
-possible, the command expression includes the optional key
-'success-response' with boolean value false. So far, only QGA makes
-use of this member.
+response is not possible (although the command will still return an
+error object on failure). When a successful reply is not possible,
+the command definition includes the optional member 'success-response'
+with boolean value false. So far, only QGA makes use of this member.
-Key 'allow-oob' declares whether the command supports out-of-band
+Member 'allow-oob' declares whether the command supports out-of-band
(OOB) execution. It defaults to false. For example:
{ 'command': 'migrate_recover',
@@ -455,8 +570,8 @@ other "slow" lock.
When in doubt, do not implement OOB execution support.
-Key 'allow-preconfig' declares whether the command is available before
-the machine is built. It defaults to false. For example:
+Member 'allow-preconfig' declares whether the command is available
+before the machine is built. It defaults to false. For example:
{ 'command': 'qmp_capabilities',
'data': { '*enable': [ 'QMPCapability' ] },
@@ -465,16 +580,33 @@ the machine is built. It defaults to false. For example:
QMP is available before the machine is built only when QEMU was
started with --preconfig.
+The optional 'if' member specifies a conditional. See "Configuring
+the schema" below for more on this.
+
+
=== Events ===
-Usage: { 'event': STRING, '*data': COMPLEX-TYPE-NAME-OR-DICT,
- '*boxed': true }
-
-Events are defined with the keyword 'event'. When 'data' is also
-specified, additional info will be included in the event, with similar
-semantics to a 'struct' expression. Finally there will be C API
-generated in qapi-events.h; when called by QEMU code, a message with
-timestamp will be emitted on the wire.
+Syntax:
+ EVENT = { 'event': STRING,
+ (
+ '*data': ( MEMBERS | STRING ),
+ |
+ 'data': STRING,
+ 'boxed': true,
+ )
+ '*if': COND }
+
+Member 'event' names the event. This is the event name used in the
+Client JSON Protocol.
+
+Member 'data' defines the event-specific data. It defaults to an
+empty MEMBERS object.
+
+If 'data' is a MEMBERS object, then MEMBERS defines event-specific
+data just like a struct type's 'data' defines struct type members.
+
+If 'data' is a STRING, then STRING names a complex type whose members
+are the event-specific data. A union type requires 'boxed': true.
An example event is:
@@ -487,42 +619,44 @@ Resulting in this JSON object:
"data": { "b": "test string" },
"timestamp": { "seconds": 1267020223, "microseconds": 435656 } }
-The generator emits a function to send the event. Normally, 'data' is
-a dictionary for an anonymous type, or names a struct type (possibly
-empty, but not a union), and its members are passed as separate
-arguments to this function. If the event definition includes a key
-'boxed' with the boolean value true, then 'data' is instead the name
-of any non-empty complex type (struct or union), and a pointer to that
-QAPI type is passed as a single argument.
+The generator emits a function to send the event. When member 'boxed'
+is absent, it takes event-specific data one by one, in QAPI schema
+order. Else it takes them wrapped in the C struct generated for the
+complex type. See section "Code generated for events" for examples.
+
+The optional 'if' member specifies a conditional. See "Configuring
+the schema" below for more on this.
=== Features ===
+Syntax:
+ FEATURES = [ FEATURE, ... ]
+ FEATURE = STRING
+ | { 'name': STRING, '*if': COND }
+
Sometimes, the behaviour of QEMU changes compatibly, but without a
-change in the QMP syntax (usually by allowing values or operations that
-previously resulted in an error). QMP clients may still need to know
-whether the extension is available.
+change in the QMP syntax (usually by allowing values or operations
+that previously resulted in an error). QMP clients may still need to
+know whether the extension is available.
For this purpose, a list of features can be specified for a struct type.
This is exposed to the client as a list of string, where each string
signals that this build of QEMU shows a certain behaviour.
-In the schema, features can be specified as simple strings, for example:
+Each member of the 'features' array defines a feature. It can either
+be { 'name': STRING, '*if': COND }, or STRING, which is shorthand for
+{ 'name': STRING }.
+
+The optional 'if' member specifies a conditional. See "Configuring
+the schema" below for more on this.
+
+Example:
{ 'struct': 'TestType',
'data': { 'number': 'int' },
'features': [ 'allow-negative-numbers' ] }
-Another option is to specify features as dictionaries, where the key
-'name' specifies the feature string to be exposed to clients:
-
-{ 'struct': 'TestType',
- 'data': { 'number': 'int' },
- 'features': [ { 'name': 'allow-negative-numbers' } ] }
-
-This expanded form is necessary if you want to make the feature
-conditional (see below in "Configuring the schema").
-
=== Naming rules and reserved names ===
@@ -546,9 +680,8 @@ respectively.
Command names, and member names within a type, should be all lower
case with words separated by a hyphen. However, some existing older
-commands and complex types use underscore; when extending such
-expressions, consistency is preferred over blindly avoiding
-underscore.
+commands and complex types use underscore; when extending them,
+consistency is preferred over blindly avoiding underscore.
Event names should be ALL_CAPS with words separated by underscore.
@@ -578,11 +711,14 @@ downstream command __com.redhat_drive-mirror.
=== Configuring the schema ===
-The 'struct', 'enum', 'union', 'alternate', 'command' and 'event'
-top-level expressions can take an 'if' key. Its value must be a string
-or a list of strings. A string is shorthand for a list containing just
-that string. The code generated for the top-level expression will then
-be guarded by #if COND for each COND in the list.
+Syntax:
+ COND = STRING
+ | [ STRING, ... ]
+
+All definitions take an optional 'if' member. Its value must be a
+string or a list of strings. A string is shorthand for a list
+containing just that string. The code generated for the definition
+will then be guarded by #if STRING for each STRING in the COND list.
Example: a conditional struct
@@ -597,29 +733,33 @@ gets its generated code guarded like this:
#endif /* defined(HAVE_BAR) */
#endif /* defined(CONFIG_FOO) */
-Where a member can be defined with a single string value for its type,
-it is also possible to supply a dictionary instead with both 'type'
-and 'if' keys.
+Individual members of complex types, commands arguments, and
+event-specific data can also be made conditional. This requires the
+longhand form of MEMBER.
-Example: a conditional 'bar' member
+Example: a struct type with unconditional member 'foo' and conditional
+member 'bar'
{ 'struct': 'IfStruct', 'data':
{ 'foo': 'int',
'bar': { 'type': 'int', 'if': 'defined(IFCOND)'} } }
-An enum value can be replaced by a dictionary with a 'name' and a 'if'
-key.
+A union's discriminator may not be conditional.
-Example: a conditional 'bar' enum member.
+Likewise, individual enumeration values be conditional. This requires
+the longhand form of ENUM-VALUE.
+
+Example: an enum type with unconditional value 'foo' and conditional
+value 'bar'
{ 'enum': 'IfEnum', 'data':
[ 'foo',
{ 'name' : 'bar', 'if': 'defined(IFCOND)' } ] }
-Similarly, features can be specified as a dictionary with a 'name' and
-an 'if' key.
+Likewise, features can be conditional. This requires the longhand
+form of FEATURE.
-Example: a conditional 'allow-negative-numbers' feature
+Example: a struct with conditional feature 'allow-negative-numbers'
{ 'struct': 'TestType',
'data': { 'number': 'int' },
@@ -628,17 +768,30 @@ Example: a conditional 'allow-negative-numbers' feature
Please note that you are responsible to ensure that the C code will
compile with an arbitrary combination of conditions, since the
-generators are unable to check it at this point.
+generator is unable to check it at this point.
-The presence of 'if' keys in the schema is reflected through to the
-introspection output depending on the build configuration.
+The conditions apply to introspection as well, i.e. introspection
+shows a conditional entity only when the condition is satisfied in
+this particular build.
=== Documentation comments ===
A multi-line comment that starts and ends with a '##' line is a
-documentation comment. These are parsed by the documentation
-generator, which recognizes certain markup detailed below.
+documentation comment.
+
+If the documentation comment starts like
+
+ ##
+ # @SYMBOL:
+
+it documents the definition if SYMBOL, else it's free-form
+documentation.
+
+See below for more on definition documentation.
+
+Free-form documentation may be used to provide additional text and
+structuring content.
==== Documentation markup ====
@@ -701,23 +854,24 @@ Example:
##
-==== Expression documentation ====
+==== Definition documentation ====
-Expressions other than include and pragma directives may be preceded
-by a documentation block. Such blocks are called expression
-documentation blocks.
+Definition documentation, if present, must immediately precede the
+definition it documents.
-When documentation is required (see pragma 'doc-required'), expression
-documentation blocks are mandatory.
+When documentation is required (see pragma 'doc-required'), every
+definition must have documentation.
-The documentation block consists of a first line naming the
-expression, an optional overview, a description of each argument (for
-commands and events) or member (for structs, unions and alternates),
-and optional tagged sections.
+Definition documentation starts with a line naming the definition,
+followed by an optional overview, a description of each argument (for
+commands and events), member (for structs and unions), branch (for
+alternates), or value (for enums), and finally optional tagged
+sections.
FIXME: the parser accepts these things in almost any order.
+FIXME: union branches should be described, too.
-Extensions added after the expression was first released carry a
+Extensions added after the definition was first released carry a
'(since x.y.z)' comment.
A tagged section starts with one of the following words:
@@ -725,7 +879,7 @@ A tagged section starts with one of the following words:
The section ends with the start of a new section.
A 'Since: x.y.z' tagged section lists the release that introduced the
-expression.
+definition.
For example:
@@ -771,12 +925,6 @@ For example:
'data': { '*query-nodes': 'bool' },
'returns': ['BlockStats'] }
-==== Free-form documentation ====
-
-A documentation block that isn't an expression documentation block is
-a free-form documentation block. These may be used to provide
-additional text and structuring content.
-
== Client JSON Protocol introspection ==
@@ -862,7 +1010,7 @@ If the event carries no additional information, "arg-type" names an
object type without members. The event may not have a data member on
the wire then.
-Each command or event defined with dictionary-valued 'data' in the
+Each command or event defined with 'data' as MEMBERS object in the
QAPI schema implicitly defines an object type.
Example: the SchemaInfo for EVENT_C from section Events
@@ -1044,7 +1192,7 @@ receive direction compatibility.
Any change to types used in both contexts need to consider both.
-Members of enumeration types, complex types and alternate types may be
+Enumeration type values and complex and alternate type members may be
reordered freely. For enumerations and alternate types, this doesn't
affect the wire encoding. For complex types, this might make the
implementation emit JSON object members in a different order, which
--
2.21.0
^ permalink raw reply related [flat|nested] 41+ messages in thread
* [PULL 18/37] qapi: Tweak code to match docs/devel/qapi-code-gen.txt
2019-09-24 12:32 [PULL 00/37] QAPI patches for 2019-09-24 Markus Armbruster
` (16 preceding siblings ...)
2019-09-24 12:33 ` [PULL 17/37] docs/devel/qapi-code-gen: Improve QAPI schema language doc Markus Armbruster
@ 2019-09-24 12:33 ` Markus Armbruster
2019-09-24 12:33 ` [PULL 19/37] tests/qapi-schema: Cover unknown pragma Markus Armbruster
` (21 subsequent siblings)
39 siblings, 0 replies; 41+ messages in thread
From: Markus Armbruster @ 2019-09-24 12:33 UTC (permalink / raw)
To: qemu-devel
The previous commit made qapi-code-gen.txt define "(top-level)
expression" as either "directive" or "definition". The code still
uses "expression" when it really means "definition". Tidy up.
The previous commit made qapi-code-gen.txt use "object" rather than
"dictionary". The code still uses "dictionary". Tidy up.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Message-Id: <20190913201349.24332-17-armbru@redhat.com>
---
scripts/qapi/common.py | 24 +++++++++----------
tests/qapi-schema/args-invalid.err | 2 +-
tests/qapi-schema/doc-missing.err | 2 +-
tests/qapi-schema/doc-no-symbol.err | 2 +-
.../qapi-schema/enum-dict-member-unknown.err | 2 +-
tests/qapi-schema/pragma-non-dict.err | 2 +-
tests/qapi-schema/struct-data-invalid.err | 2 +-
7 files changed, 18 insertions(+), 18 deletions(-)
diff --git a/scripts/qapi/common.py b/scripts/qapi/common.py
index a538d2f37c..f27860540b 100644
--- a/scripts/qapi/common.py
+++ b/scripts/qapi/common.py
@@ -103,11 +103,11 @@ class QAPISemError(QAPIError):
class QAPIDoc(object):
"""
- A documentation comment block, either expression or free-form
+ A documentation comment block, either definition or free-form
- Expression documentation blocks consist of
+ Definition documentation blocks consist of
- * a body section: one line naming the expression, followed by an
+ * a body section: one line naming the definition, followed by an
overview (any number of lines)
* argument sections: a description of each argument (for commands
@@ -200,9 +200,9 @@ class QAPIDoc(object):
Process a line of documentation text in the body section.
If this a symbol line and it is the section's first line, this
- is an expression documentation block for that symbol.
+ is a definition documentation block for that symbol.
- If it's an expression documentation block, another symbol line
+ If it's a definition documentation block, another symbol line
begins the argument section for the argument named by it, and
a section tag begins an additional section. Start that
section and append the line to it.
@@ -220,7 +220,7 @@ class QAPIDoc(object):
if not self.symbol:
raise QAPIParseError(self._parser, "Invalid name")
elif self.symbol:
- # This is an expression documentation block
+ # This is a definition documentation block
if name.startswith('@') and name.endswith(':'):
self._append_line = self._append_args_line
self._append_args_line(line)
@@ -428,7 +428,7 @@ class QAPISchemaParser(object):
pragma = expr['pragma']
if not isinstance(pragma, dict):
raise QAPISemError(
- info, "Value of 'pragma' must be a dictionary")
+ info, "Value of 'pragma' must be an object")
for name, value in pragma.items():
self._pragma(name, value, info)
else:
@@ -437,7 +437,7 @@ class QAPISchemaParser(object):
if cur_doc:
if not cur_doc.symbol:
raise QAPISemError(
- cur_doc.info, "Expression documentation required")
+ cur_doc.info, "Definition documentation required")
expr_elem['doc'] = cur_doc
self.exprs.append(expr_elem)
cur_doc = None
@@ -789,7 +789,7 @@ def check_type(info, source, value,
if not isinstance(value, OrderedDict):
raise QAPISemError(info,
- "%s should be a dictionary or type name" % source)
+ "%s should be an object or type name" % source)
# value is a dictionary, check that each member is okay
for (key, arg) in value.items():
@@ -971,8 +971,8 @@ def check_enum(expr, info):
"Enum '%s' requires a string for 'prefix'" % name)
for member in members:
- source = "dictionary member of enum '%s'" % name
- check_known_keys(info, source, member, ['name'], ['if'])
+ check_known_keys(info, "member of enum '%s'" % name, member,
+ ['name'], ['if'])
check_if(member, info)
check_name(info, "Member of enum '%s'" % name, member['name'],
enum_member=True)
@@ -1081,7 +1081,7 @@ def check_exprs(exprs):
if not doc and doc_required:
raise QAPISemError(info,
- "Expression missing documentation comment")
+ "Definition missing documentation comment")
if 'enum' in expr:
meta = 'enum'
diff --git a/tests/qapi-schema/args-invalid.err b/tests/qapi-schema/args-invalid.err
index fe1e94975b..bfb2e4133e 100644
--- a/tests/qapi-schema/args-invalid.err
+++ b/tests/qapi-schema/args-invalid.err
@@ -1 +1 @@
-tests/qapi-schema/args-invalid.json:1: 'data' for command 'foo' should be a dictionary or type name
+tests/qapi-schema/args-invalid.json:1: 'data' for command 'foo' should be an object or type name
diff --git a/tests/qapi-schema/doc-missing.err b/tests/qapi-schema/doc-missing.err
index 7f2f326b30..3a377ddc57 100644
--- a/tests/qapi-schema/doc-missing.err
+++ b/tests/qapi-schema/doc-missing.err
@@ -1 +1 @@
-tests/qapi-schema/doc-missing.json:5: Expression missing documentation comment
+tests/qapi-schema/doc-missing.json:5: Definition missing documentation comment
diff --git a/tests/qapi-schema/doc-no-symbol.err b/tests/qapi-schema/doc-no-symbol.err
index 75f032a942..212984ff20 100644
--- a/tests/qapi-schema/doc-no-symbol.err
+++ b/tests/qapi-schema/doc-no-symbol.err
@@ -1 +1 @@
-tests/qapi-schema/doc-no-symbol.json:3: Expression documentation required
+tests/qapi-schema/doc-no-symbol.json:3: Definition documentation required
diff --git a/tests/qapi-schema/enum-dict-member-unknown.err b/tests/qapi-schema/enum-dict-member-unknown.err
index 2aae618be0..7fd9c032bf 100644
--- a/tests/qapi-schema/enum-dict-member-unknown.err
+++ b/tests/qapi-schema/enum-dict-member-unknown.err
@@ -1,2 +1,2 @@
-tests/qapi-schema/enum-dict-member-unknown.json:2: Unknown key 'bad-key' in dictionary member of enum 'MyEnum'
+tests/qapi-schema/enum-dict-member-unknown.json:2: Unknown key 'bad-key' in member of enum 'MyEnum'
Valid keys are 'if', 'name'.
diff --git a/tests/qapi-schema/pragma-non-dict.err b/tests/qapi-schema/pragma-non-dict.err
index 75bc335aea..b358261050 100644
--- a/tests/qapi-schema/pragma-non-dict.err
+++ b/tests/qapi-schema/pragma-non-dict.err
@@ -1 +1 @@
-tests/qapi-schema/pragma-non-dict.json:3: Value of 'pragma' must be a dictionary
+tests/qapi-schema/pragma-non-dict.json:3: Value of 'pragma' must be an object
diff --git a/tests/qapi-schema/struct-data-invalid.err b/tests/qapi-schema/struct-data-invalid.err
index 6644f4c2ad..4bf5bcc255 100644
--- a/tests/qapi-schema/struct-data-invalid.err
+++ b/tests/qapi-schema/struct-data-invalid.err
@@ -1 +1 @@
-tests/qapi-schema/struct-data-invalid.json:1: 'data' for struct 'foo' should be a dictionary or type name
+tests/qapi-schema/struct-data-invalid.json:1: 'data' for struct 'foo' should be an object or type name
--
2.21.0
^ permalink raw reply related [flat|nested] 41+ messages in thread
* [PULL 19/37] tests/qapi-schema: Cover unknown pragma
2019-09-24 12:32 [PULL 00/37] QAPI patches for 2019-09-24 Markus Armbruster
` (17 preceding siblings ...)
2019-09-24 12:33 ` [PULL 18/37] qapi: Tweak code to match docs/devel/qapi-code-gen.txt Markus Armbruster
@ 2019-09-24 12:33 ` Markus Armbruster
2019-09-24 12:33 ` [PULL 20/37] tests/qapi-schema: Delete two redundant tests Markus Armbruster
` (20 subsequent siblings)
39 siblings, 0 replies; 41+ messages in thread
From: Markus Armbruster @ 2019-09-24 12:33 UTC (permalink / raw)
To: qemu-devel
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Message-Id: <20190914153506.2151-2-armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
---
tests/Makefile.include | 1 +
tests/qapi-schema/pragma-unknown.err | 1 +
tests/qapi-schema/pragma-unknown.exit | 1 +
tests/qapi-schema/pragma-unknown.json | 1 +
tests/qapi-schema/pragma-unknown.out | 0
5 files changed, 4 insertions(+)
create mode 100644 tests/qapi-schema/pragma-unknown.err
create mode 100644 tests/qapi-schema/pragma-unknown.exit
create mode 100644 tests/qapi-schema/pragma-unknown.json
create mode 100644 tests/qapi-schema/pragma-unknown.out
diff --git a/tests/Makefile.include b/tests/Makefile.include
index 2c3adb1530..089cc3ea95 100644
--- a/tests/Makefile.include
+++ b/tests/Makefile.include
@@ -427,6 +427,7 @@ qapi-schema += pragma-doc-required-crap.json
qapi-schema += pragma-extra-junk.json
qapi-schema += pragma-name-case-whitelist-crap.json
qapi-schema += pragma-non-dict.json
+qapi-schema += pragma-unknown.json
qapi-schema += pragma-returns-whitelist-crap.json
qapi-schema += qapi-schema-test.json
qapi-schema += quoted-structural-chars.json
diff --git a/tests/qapi-schema/pragma-unknown.err b/tests/qapi-schema/pragma-unknown.err
new file mode 100644
index 0000000000..6ef2058316
--- /dev/null
+++ b/tests/qapi-schema/pragma-unknown.err
@@ -0,0 +1 @@
+tests/qapi-schema/pragma-unknown.json:1: Unknown pragma 'no-such-pragma'
diff --git a/tests/qapi-schema/pragma-unknown.exit b/tests/qapi-schema/pragma-unknown.exit
new file mode 100644
index 0000000000..d00491fd7e
--- /dev/null
+++ b/tests/qapi-schema/pragma-unknown.exit
@@ -0,0 +1 @@
+1
diff --git a/tests/qapi-schema/pragma-unknown.json b/tests/qapi-schema/pragma-unknown.json
new file mode 100644
index 0000000000..c51bbbb53f
--- /dev/null
+++ b/tests/qapi-schema/pragma-unknown.json
@@ -0,0 +1 @@
+{ 'pragma': { 'no-such-pragma': false } }
diff --git a/tests/qapi-schema/pragma-unknown.out b/tests/qapi-schema/pragma-unknown.out
new file mode 100644
index 0000000000..e69de29bb2
--
2.21.0
^ permalink raw reply related [flat|nested] 41+ messages in thread
* [PULL 20/37] tests/qapi-schema: Delete two redundant tests
2019-09-24 12:32 [PULL 00/37] QAPI patches for 2019-09-24 Markus Armbruster
` (18 preceding siblings ...)
2019-09-24 12:33 ` [PULL 19/37] tests/qapi-schema: Cover unknown pragma Markus Armbruster
@ 2019-09-24 12:33 ` Markus Armbruster
2019-09-24 12:33 ` [PULL 21/37] tests/qapi-schema: Demonstrate misleading optional tag error Markus Armbruster
` (19 subsequent siblings)
39 siblings, 0 replies; 41+ messages in thread
From: Markus Armbruster @ 2019-09-24 12:33 UTC (permalink / raw)
To: qemu-devel
Tests duplicate-key and double-data test the same thing. The former
predates the latter, and it has a better name. Delete the latter, and
tweak the former's comment.
Tests include-format-err and include-extra-junk test the same thing.
The former predates the latter, but the latter has a better name and a
comment. Delete the former.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Message-Id: <20190914153506.2151-3-armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
---
tests/Makefile.include | 2 --
tests/qapi-schema/double-data.err | 1 -
tests/qapi-schema/double-data.exit | 1 -
tests/qapi-schema/double-data.json | 2 --
tests/qapi-schema/double-data.out | 0
tests/qapi-schema/duplicate-key.json | 2 +-
tests/qapi-schema/include-format-err.err | 1 -
tests/qapi-schema/include-format-err.exit | 1 -
tests/qapi-schema/include-format-err.json | 2 --
tests/qapi-schema/include-format-err.out | 0
10 files changed, 1 insertion(+), 11 deletions(-)
delete mode 100644 tests/qapi-schema/double-data.err
delete mode 100644 tests/qapi-schema/double-data.exit
delete mode 100644 tests/qapi-schema/double-data.json
delete mode 100644 tests/qapi-schema/double-data.out
delete mode 100644 tests/qapi-schema/include-format-err.err
delete mode 100644 tests/qapi-schema/include-format-err.exit
delete mode 100644 tests/qapi-schema/include-format-err.json
delete mode 100644 tests/qapi-schema/include-format-err.out
diff --git a/tests/Makefile.include b/tests/Makefile.include
index 089cc3ea95..0c862dbc42 100644
--- a/tests/Makefile.include
+++ b/tests/Makefile.include
@@ -359,7 +359,6 @@ qapi-schema += doc-missing-expr.json
qapi-schema += doc-missing-space.json
qapi-schema += doc-missing.json
qapi-schema += doc-no-symbol.json
-qapi-schema += double-data.json
qapi-schema += double-type.json
qapi-schema += duplicate-key.json
qapi-schema += empty.json
@@ -404,7 +403,6 @@ qapi-schema += ident-with-escape.json
qapi-schema += include-before-err.json
qapi-schema += include-cycle.json
qapi-schema += include-extra-junk.json
-qapi-schema += include-format-err.json
qapi-schema += include-nested-err.json
qapi-schema += include-no-file.json
qapi-schema += include-non-file.json
diff --git a/tests/qapi-schema/double-data.err b/tests/qapi-schema/double-data.err
deleted file mode 100644
index cc765c4ff2..0000000000
--- a/tests/qapi-schema/double-data.err
+++ /dev/null
@@ -1 +0,0 @@
-tests/qapi-schema/double-data.json:2:41: Duplicate key "data"
diff --git a/tests/qapi-schema/double-data.exit b/tests/qapi-schema/double-data.exit
deleted file mode 100644
index d00491fd7e..0000000000
--- a/tests/qapi-schema/double-data.exit
+++ /dev/null
@@ -1 +0,0 @@
-1
diff --git a/tests/qapi-schema/double-data.json b/tests/qapi-schema/double-data.json
deleted file mode 100644
index e76b519538..0000000000
--- a/tests/qapi-schema/double-data.json
+++ /dev/null
@@ -1,2 +0,0 @@
-# we reject an expression with duplicate top-level keys
-{ 'struct': 'bar', 'data': { }, 'data': { 'string': 'str'} }
diff --git a/tests/qapi-schema/double-data.out b/tests/qapi-schema/double-data.out
deleted file mode 100644
index e69de29bb2..0000000000
diff --git a/tests/qapi-schema/duplicate-key.json b/tests/qapi-schema/duplicate-key.json
index 14ac0e8a40..06b55840c9 100644
--- a/tests/qapi-schema/duplicate-key.json
+++ b/tests/qapi-schema/duplicate-key.json
@@ -1,3 +1,3 @@
-# QAPI cannot include the same key more than once in any {}
+# Cannot include the same key more than once in any {}
{ 'key': 'value',
'key': 'value' }
diff --git a/tests/qapi-schema/include-format-err.err b/tests/qapi-schema/include-format-err.err
deleted file mode 100644
index 721ff4eccc..0000000000
--- a/tests/qapi-schema/include-format-err.err
+++ /dev/null
@@ -1 +0,0 @@
-tests/qapi-schema/include-format-err.json:1: Invalid 'include' directive
diff --git a/tests/qapi-schema/include-format-err.exit b/tests/qapi-schema/include-format-err.exit
deleted file mode 100644
index d00491fd7e..0000000000
--- a/tests/qapi-schema/include-format-err.exit
+++ /dev/null
@@ -1 +0,0 @@
-1
diff --git a/tests/qapi-schema/include-format-err.json b/tests/qapi-schema/include-format-err.json
deleted file mode 100644
index 44980f026f..0000000000
--- a/tests/qapi-schema/include-format-err.json
+++ /dev/null
@@ -1,2 +0,0 @@
-{ 'include': 'include-simple-sub.json',
- 'foo': 'bar' }
diff --git a/tests/qapi-schema/include-format-err.out b/tests/qapi-schema/include-format-err.out
deleted file mode 100644
index e69de29bb2..0000000000
--
2.21.0
^ permalink raw reply related [flat|nested] 41+ messages in thread
* [PULL 21/37] tests/qapi-schema: Demonstrate misleading optional tag error
2019-09-24 12:32 [PULL 00/37] QAPI patches for 2019-09-24 Markus Armbruster
` (19 preceding siblings ...)
2019-09-24 12:33 ` [PULL 20/37] tests/qapi-schema: Delete two redundant tests Markus Armbruster
@ 2019-09-24 12:33 ` Markus Armbruster
2019-09-24 12:33 ` [PULL 22/37] tests/qapi-schema: Demonstrate broken discriminator errors Markus Armbruster
` (18 subsequent siblings)
39 siblings, 0 replies; 41+ messages in thread
From: Markus Armbruster @ 2019-09-24 12:33 UTC (permalink / raw)
To: qemu-devel
Test flat-union-optional-discriminator declares its union tag as
'*switch': 'Enum', and points to it with 'discriminator': '*switch'.
This gets rejected as "discriminator of flat union 'MyUnion' uses
invalid name '*switch'". Correct; member 'discriminator' doesn't
accept a '*' prefix.
However, this merely tests name validity checking, which we already
cover elsewhere. More interesting is testing the valid name 'switch'.
This reports "discriminator 'switch' is not a member of base struct
'Base'", which is misleading.
Copy the existing 'discriminator': '*switch' test to
flat-union-discriminator-bad-name, and rewrite its comment. Change
flat-union-optional-discriminator to test 'discriminator': 'switch',
and mark it FIXME.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Message-Id: <20190914153506.2151-4-armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
---
tests/Makefile.include | 1 +
.../qapi-schema/flat-union-discriminator-bad-name.err | 1 +
.../flat-union-discriminator-bad-name.exit | 1 +
.../flat-union-discriminator-bad-name.json | 11 +++++++++++
.../qapi-schema/flat-union-discriminator-bad-name.out | 0
.../qapi-schema/flat-union-optional-discriminator.err | 2 +-
.../flat-union-optional-discriminator.json | 3 ++-
7 files changed, 17 insertions(+), 2 deletions(-)
create mode 100644 tests/qapi-schema/flat-union-discriminator-bad-name.err
create mode 100644 tests/qapi-schema/flat-union-discriminator-bad-name.exit
create mode 100644 tests/qapi-schema/flat-union-discriminator-bad-name.json
create mode 100644 tests/qapi-schema/flat-union-discriminator-bad-name.out
diff --git a/tests/Makefile.include b/tests/Makefile.include
index 0c862dbc42..7553616069 100644
--- a/tests/Makefile.include
+++ b/tests/Makefile.include
@@ -388,6 +388,7 @@ qapi-schema += flat-union-bad-discriminator.json
qapi-schema += flat-union-base-any.json
qapi-schema += flat-union-base-union.json
qapi-schema += flat-union-clash-member.json
+qapi-schema += flat-union-discriminator-bad-name.json
qapi-schema += flat-union-empty.json
qapi-schema += flat-union-inline.json
qapi-schema += flat-union-inline-invalid-dict.json
diff --git a/tests/qapi-schema/flat-union-discriminator-bad-name.err b/tests/qapi-schema/flat-union-discriminator-bad-name.err
new file mode 100644
index 0000000000..7238d126ca
--- /dev/null
+++ b/tests/qapi-schema/flat-union-discriminator-bad-name.err
@@ -0,0 +1 @@
+tests/qapi-schema/flat-union-discriminator-bad-name.json:7: Discriminator of flat union 'MyUnion' does not allow optional name '*switch'
diff --git a/tests/qapi-schema/flat-union-discriminator-bad-name.exit b/tests/qapi-schema/flat-union-discriminator-bad-name.exit
new file mode 100644
index 0000000000..d00491fd7e
--- /dev/null
+++ b/tests/qapi-schema/flat-union-discriminator-bad-name.exit
@@ -0,0 +1 @@
+1
diff --git a/tests/qapi-schema/flat-union-discriminator-bad-name.json b/tests/qapi-schema/flat-union-discriminator-bad-name.json
new file mode 100644
index 0000000000..66376084fc
--- /dev/null
+++ b/tests/qapi-schema/flat-union-discriminator-bad-name.json
@@ -0,0 +1,11 @@
+# discriminator '*switch' isn't a member of base, 'switch' is
+# reports "does not allow optional name", which is good enough
+{ 'enum': 'Enum', 'data': [ 'one', 'two' ] }
+{ 'struct': 'Base',
+ 'data': { '*switch': 'Enum' } }
+{ 'struct': 'Branch', 'data': { 'name': 'str' } }
+{ 'union': 'MyUnion',
+ 'base': 'Base',
+ 'discriminator': '*switch',
+ 'data': { 'one': 'Branch',
+ 'two': 'Branch' } }
diff --git a/tests/qapi-schema/flat-union-discriminator-bad-name.out b/tests/qapi-schema/flat-union-discriminator-bad-name.out
new file mode 100644
index 0000000000..e69de29bb2
diff --git a/tests/qapi-schema/flat-union-optional-discriminator.err b/tests/qapi-schema/flat-union-optional-discriminator.err
index aaabedb3bd..8b4a4ba847 100644
--- a/tests/qapi-schema/flat-union-optional-discriminator.err
+++ b/tests/qapi-schema/flat-union-optional-discriminator.err
@@ -1 +1 @@
-tests/qapi-schema/flat-union-optional-discriminator.json:6: Discriminator of flat union 'MyUnion' does not allow optional name '*switch'
+tests/qapi-schema/flat-union-optional-discriminator.json:7: Discriminator 'switch' is not a member of base struct 'Base'
diff --git a/tests/qapi-schema/flat-union-optional-discriminator.json b/tests/qapi-schema/flat-union-optional-discriminator.json
index 08a8f7ef8b..143ab23a0d 100644
--- a/tests/qapi-schema/flat-union-optional-discriminator.json
+++ b/tests/qapi-schema/flat-union-optional-discriminator.json
@@ -1,10 +1,11 @@
# we require the discriminator to be non-optional
+# FIXME reports "discriminator 'switch' is not a member of base struct 'Base'"
{ 'enum': 'Enum', 'data': [ 'one', 'two' ] }
{ 'struct': 'Base',
'data': { '*switch': 'Enum' } }
{ 'struct': 'Branch', 'data': { 'name': 'str' } }
{ 'union': 'MyUnion',
'base': 'Base',
- 'discriminator': '*switch',
+ 'discriminator': 'switch',
'data': { 'one': 'Branch',
'two': 'Branch' } }
--
2.21.0
^ permalink raw reply related [flat|nested] 41+ messages in thread
* [PULL 22/37] tests/qapi-schema: Demonstrate broken discriminator errors
2019-09-24 12:32 [PULL 00/37] QAPI patches for 2019-09-24 Markus Armbruster
` (20 preceding siblings ...)
2019-09-24 12:33 ` [PULL 21/37] tests/qapi-schema: Demonstrate misleading optional tag error Markus Armbruster
@ 2019-09-24 12:33 ` Markus Armbruster
2019-09-24 12:33 ` [PULL 23/37] tests/qapi-schema: Demonstrate insufficient 'if' checking Markus Armbruster
` (17 subsequent siblings)
39 siblings, 0 replies; 41+ messages in thread
From: Markus Armbruster @ 2019-09-24 12:33 UTC (permalink / raw)
To: qemu-devel
When the union definition's base is an object, some error messages
show it as an OrderedDict. Oops. Mark FIXME.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Message-Id: <20190914153506.2151-5-armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
---
tests/qapi-schema/flat-union-invalid-discriminator.err | 2 +-
tests/qapi-schema/flat-union-invalid-discriminator.json | 6 ++----
tests/qapi-schema/flat-union-invalid-if-discriminator.err | 2 +-
tests/qapi-schema/flat-union-invalid-if-discriminator.json | 6 ++----
4 files changed, 6 insertions(+), 10 deletions(-)
diff --git a/tests/qapi-schema/flat-union-invalid-discriminator.err b/tests/qapi-schema/flat-union-invalid-discriminator.err
index 5f4055614e..947a6b73aa 100644
--- a/tests/qapi-schema/flat-union-invalid-discriminator.err
+++ b/tests/qapi-schema/flat-union-invalid-discriminator.err
@@ -1 +1 @@
-tests/qapi-schema/flat-union-invalid-discriminator.json:13: Discriminator 'enum_wrong' is not a member of base struct 'TestBase'
+tests/qapi-schema/flat-union-invalid-discriminator.json:11: Discriminator 'enum_wrong' is not a member of base struct 'OrderedDict([('enum1', {'type': 'TestEnum'})])'
diff --git a/tests/qapi-schema/flat-union-invalid-discriminator.json b/tests/qapi-schema/flat-union-invalid-discriminator.json
index 48b94c3a4d..de86cf0760 100644
--- a/tests/qapi-schema/flat-union-invalid-discriminator.json
+++ b/tests/qapi-schema/flat-union-invalid-discriminator.json
@@ -1,9 +1,7 @@
+# FIXME error message shows base as OrderedDict
{ 'enum': 'TestEnum',
'data': [ 'value1', 'value2' ] }
-{ 'struct': 'TestBase',
- 'data': { 'enum1': 'TestEnum' } }
-
{ 'struct': 'TestTypeA',
'data': { 'string': 'str' } }
@@ -11,7 +9,7 @@
'data': { 'integer': 'int' } }
{ 'union': 'TestUnion',
- 'base': 'TestBase',
+ 'base': { 'enum1': 'TestEnum' },
'discriminator': 'enum_wrong',
'data': { 'value1': 'TestTypeA',
'value2': 'TestTypeB' } }
diff --git a/tests/qapi-schema/flat-union-invalid-if-discriminator.err b/tests/qapi-schema/flat-union-invalid-if-discriminator.err
index 0c94c9860d..ec04c4840c 100644
--- a/tests/qapi-schema/flat-union-invalid-if-discriminator.err
+++ b/tests/qapi-schema/flat-union-invalid-if-discriminator.err
@@ -1 +1 @@
-tests/qapi-schema/flat-union-invalid-if-discriminator.json:13: The discriminator TestBase.enum1 for union TestUnion must not be conditional
+tests/qapi-schema/flat-union-invalid-if-discriminator.json:11: The discriminator OrderedDict([('enum1', OrderedDict([('type', 'TestEnum'), ('if', 'FOO')]))]).enum1 for union TestUnion must not be conditional
diff --git a/tests/qapi-schema/flat-union-invalid-if-discriminator.json b/tests/qapi-schema/flat-union-invalid-if-discriminator.json
index 618ec36396..bbaa9a3f82 100644
--- a/tests/qapi-schema/flat-union-invalid-if-discriminator.json
+++ b/tests/qapi-schema/flat-union-invalid-if-discriminator.json
@@ -1,9 +1,7 @@
+# FIXME error message shows base as OrderedDict
{ 'enum': 'TestEnum',
'data': [ 'value1', 'value2' ] }
-{ 'struct': 'TestBase',
- 'data': { 'enum1': { 'type': 'TestEnum', 'if': 'FOO' } } }
-
{ 'struct': 'TestTypeA',
'data': { 'string': 'str' } }
@@ -11,7 +9,7 @@
'data': { 'integer': 'int' } }
{ 'union': 'TestUnion',
- 'base': 'TestBase',
+ 'base': { 'enum1': { 'type': 'TestEnum', 'if': 'FOO' } },
'discriminator': 'enum1',
'data': { 'value1': 'TestTypeA',
'value2': 'TestTypeB' } }
--
2.21.0
^ permalink raw reply related [flat|nested] 41+ messages in thread
* [PULL 23/37] tests/qapi-schema: Demonstrate insufficient 'if' checking
2019-09-24 12:32 [PULL 00/37] QAPI patches for 2019-09-24 Markus Armbruster
` (21 preceding siblings ...)
2019-09-24 12:33 ` [PULL 22/37] tests/qapi-schema: Demonstrate broken discriminator errors Markus Armbruster
@ 2019-09-24 12:33 ` Markus Armbruster
2019-09-24 12:33 ` [PULL 24/37] tests/qapi-schema: Demonstrate suboptimal lexical errors Markus Armbruster
` (16 subsequent siblings)
39 siblings, 0 replies; 41+ messages in thread
From: Markus Armbruster @ 2019-09-24 12:33 UTC (permalink / raw)
To: qemu-devel
Cover invalid 'if' in struct members, features, union and alternate
branches. Four out of four are broken. Mark FIXME.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Message-Id: <20190914153506.2151-6-armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
[Comment typo fixed]
---
tests/Makefile.include | 4 ++++
.../alternate-branch-if-invalid.err | 0
.../alternate-branch-if-invalid.exit | 1 +
.../alternate-branch-if-invalid.json | 4 ++++
.../alternate-branch-if-invalid.out | 16 +++++++++++++
tests/qapi-schema/features-if-invalid.err | 0
tests/qapi-schema/features-if-invalid.exit | 1 +
tests/qapi-schema/features-if-invalid.json | 5 ++++
tests/qapi-schema/features-if-invalid.out | 14 +++++++++++
.../qapi-schema/struct-member-if-invalid.err | 0
.../qapi-schema/struct-member-if-invalid.exit | 1 +
.../qapi-schema/struct-member-if-invalid.json | 4 ++++
.../qapi-schema/struct-member-if-invalid.out | 15 ++++++++++++
tests/qapi-schema/union-branch-if-invalid.err | 0
.../qapi-schema/union-branch-if-invalid.exit | 1 +
.../qapi-schema/union-branch-if-invalid.json | 7 ++++++
tests/qapi-schema/union-branch-if-invalid.out | 23 +++++++++++++++++++
17 files changed, 96 insertions(+)
create mode 100644 tests/qapi-schema/alternate-branch-if-invalid.err
create mode 100644 tests/qapi-schema/alternate-branch-if-invalid.exit
create mode 100644 tests/qapi-schema/alternate-branch-if-invalid.json
create mode 100644 tests/qapi-schema/alternate-branch-if-invalid.out
create mode 100644 tests/qapi-schema/features-if-invalid.err
create mode 100644 tests/qapi-schema/features-if-invalid.exit
create mode 100644 tests/qapi-schema/features-if-invalid.json
create mode 100644 tests/qapi-schema/features-if-invalid.out
create mode 100644 tests/qapi-schema/struct-member-if-invalid.err
create mode 100644 tests/qapi-schema/struct-member-if-invalid.exit
create mode 100644 tests/qapi-schema/struct-member-if-invalid.json
create mode 100644 tests/qapi-schema/struct-member-if-invalid.out
create mode 100644 tests/qapi-schema/union-branch-if-invalid.err
create mode 100644 tests/qapi-schema/union-branch-if-invalid.exit
create mode 100644 tests/qapi-schema/union-branch-if-invalid.json
create mode 100644 tests/qapi-schema/union-branch-if-invalid.out
diff --git a/tests/Makefile.include b/tests/Makefile.include
index 7553616069..799fd4af02 100644
--- a/tests/Makefile.include
+++ b/tests/Makefile.include
@@ -294,6 +294,7 @@ check-qtest-generic-y += tests/test-hmp$(EXESUF)
qapi-schema += alternate-any.json
qapi-schema += alternate-array.json
qapi-schema += alternate-base.json
+qapi-schema += alternate-branch-if-invalid.json
qapi-schema += alternate-clash.json
qapi-schema += alternate-conflict-dict.json
qapi-schema += alternate-conflict-enum-bool.json
@@ -378,6 +379,7 @@ qapi-schema += event-member-invalid-dict.json
qapi-schema += event-nest-struct.json
qapi-schema += features-bad-type.json
qapi-schema += features-duplicate-name.json
+qapi-schema += features-if-invalid.json
qapi-schema += features-missing-name.json
qapi-schema += features-name-bad-type.json
qapi-schema += features-no-list.json
@@ -452,6 +454,7 @@ qapi-schema += string-code-point-127.json
qapi-schema += struct-base-clash-deep.json
qapi-schema += struct-base-clash.json
qapi-schema += struct-data-invalid.json
+qapi-schema += struct-member-if-invalid.json
qapi-schema += struct-member-invalid-dict.json
qapi-schema += struct-member-invalid.json
qapi-schema += trailing-comma-list.json
@@ -463,6 +466,7 @@ qapi-schema += unclosed-string.json
qapi-schema += union-base-empty.json
qapi-schema += union-base-no-discriminator.json
qapi-schema += union-branch-case.json
+qapi-schema += union-branch-if-invalid.json
qapi-schema += union-branch-invalid-dict.json
qapi-schema += union-clash-branches.json
qapi-schema += union-empty.json
diff --git a/tests/qapi-schema/alternate-branch-if-invalid.err b/tests/qapi-schema/alternate-branch-if-invalid.err
new file mode 100644
index 0000000000..e69de29bb2
diff --git a/tests/qapi-schema/alternate-branch-if-invalid.exit b/tests/qapi-schema/alternate-branch-if-invalid.exit
new file mode 100644
index 0000000000..573541ac97
--- /dev/null
+++ b/tests/qapi-schema/alternate-branch-if-invalid.exit
@@ -0,0 +1 @@
+0
diff --git a/tests/qapi-schema/alternate-branch-if-invalid.json b/tests/qapi-schema/alternate-branch-if-invalid.json
new file mode 100644
index 0000000000..6497f53475
--- /dev/null
+++ b/tests/qapi-schema/alternate-branch-if-invalid.json
@@ -0,0 +1,4 @@
+# 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
new file mode 100644
index 0000000000..89305d7f21
--- /dev/null
+++ b/tests/qapi-schema/alternate-branch-if-invalid.out
@@ -0,0 +1,16 @@
+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/features-if-invalid.err b/tests/qapi-schema/features-if-invalid.err
new file mode 100644
index 0000000000..e69de29bb2
diff --git a/tests/qapi-schema/features-if-invalid.exit b/tests/qapi-schema/features-if-invalid.exit
new file mode 100644
index 0000000000..573541ac97
--- /dev/null
+++ b/tests/qapi-schema/features-if-invalid.exit
@@ -0,0 +1 @@
+0
diff --git a/tests/qapi-schema/features-if-invalid.json b/tests/qapi-schema/features-if-invalid.json
new file mode 100644
index 0000000000..7e4c1ad720
--- /dev/null
+++ b/tests/qapi-schema/features-if-invalid.json
@@ -0,0 +1,5 @@
+# Cover feature with invalid 'if'
+# FIXME not rejected, misinterpreted as unconditional
+{ 'struct': 'Stru',
+ 'data': {},
+ 'features': [{'name': 'f', 'if': null }] }
diff --git a/tests/qapi-schema/features-if-invalid.out b/tests/qapi-schema/features-if-invalid.out
new file mode 100644
index 0000000000..9c2637baa3
--- /dev/null
+++ b/tests/qapi-schema/features-if-invalid.out
@@ -0,0 +1,14 @@
+module None
+object q_empty
+enum QType
+ prefix QTYPE
+ member none
+ member qnull
+ member qnum
+ member qstring
+ member qdict
+ member qlist
+ member qbool
+module features-if-invalid.json
+object Stru
+ feature f
diff --git a/tests/qapi-schema/struct-member-if-invalid.err b/tests/qapi-schema/struct-member-if-invalid.err
new file mode 100644
index 0000000000..e69de29bb2
diff --git a/tests/qapi-schema/struct-member-if-invalid.exit b/tests/qapi-schema/struct-member-if-invalid.exit
new file mode 100644
index 0000000000..573541ac97
--- /dev/null
+++ b/tests/qapi-schema/struct-member-if-invalid.exit
@@ -0,0 +1 @@
+0
diff --git a/tests/qapi-schema/struct-member-if-invalid.json b/tests/qapi-schema/struct-member-if-invalid.json
new file mode 100644
index 0000000000..73987e04fc
--- /dev/null
+++ b/tests/qapi-schema/struct-member-if-invalid.json
@@ -0,0 +1,4 @@
+# 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
new file mode 100644
index 0000000000..8fbb97985c
--- /dev/null
+++ b/tests/qapi-schema/struct-member-if-invalid.out
@@ -0,0 +1,15 @@
+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
new file mode 100644
index 0000000000..e69de29bb2
diff --git a/tests/qapi-schema/union-branch-if-invalid.exit b/tests/qapi-schema/union-branch-if-invalid.exit
new file mode 100644
index 0000000000..573541ac97
--- /dev/null
+++ b/tests/qapi-schema/union-branch-if-invalid.exit
@@ -0,0 +1 @@
+0
diff --git a/tests/qapi-schema/union-branch-if-invalid.json b/tests/qapi-schema/union-branch-if-invalid.json
new file mode 100644
index 0000000000..859b63b610
--- /dev/null
+++ b/tests/qapi-schema/union-branch-if-invalid.json
@@ -0,0 +1,7 @@
+# Cover branch with invalid 'if'
+# FIXME not rejected, would generate '#if \n'
+{ 'enum': 'Branches', 'data': ['branch1'] }
+{ 'struct': 'Stru', 'data': { 'member': 'str' } }
+{ 'union': 'Uni',
+ 'base': { 'tag': 'Branches' }, 'discriminator': 'tag',
+ 'data': { 'branch1': { 'type': 'Stru', 'if': [''] } } }
diff --git a/tests/qapi-schema/union-branch-if-invalid.out b/tests/qapi-schema/union-branch-if-invalid.out
new file mode 100644
index 0000000000..2ed43218af
--- /dev/null
+++ b/tests/qapi-schema/union-branch-if-invalid.out
@@ -0,0 +1,23 @@
+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
^ permalink raw reply related [flat|nested] 41+ messages in thread
* [PULL 24/37] tests/qapi-schema: Demonstrate suboptimal lexical errors
2019-09-24 12:32 [PULL 00/37] QAPI patches for 2019-09-24 Markus Armbruster
` (22 preceding siblings ...)
2019-09-24 12:33 ` [PULL 23/37] tests/qapi-schema: Demonstrate insufficient 'if' checking Markus Armbruster
@ 2019-09-24 12:33 ` Markus Armbruster
2019-09-24 12:33 ` [PULL 25/37] qapi: Use quotes more consistently in frontend error messages Markus Armbruster
` (15 subsequent siblings)
39 siblings, 0 replies; 41+ messages in thread
From: Markus Armbruster @ 2019-09-24 12:33 UTC (permalink / raw)
To: qemu-devel
The error message for forgotten quotes around a name shows just the
name's first character, which isn't as nice as it could be. Same for
attempting to use a number.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Message-Id: <20190914153506.2151-7-armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
---
tests/Makefile.include | 1 +
tests/qapi-schema/bad-type-int.json | 2 +-
tests/qapi-schema/funny-word.err | 1 +
tests/qapi-schema/funny-word.exit | 1 +
tests/qapi-schema/funny-word.json | 1 +
tests/qapi-schema/funny-word.out | 0
6 files changed, 5 insertions(+), 1 deletion(-)
create mode 100644 tests/qapi-schema/funny-word.err
create mode 100644 tests/qapi-schema/funny-word.exit
create mode 100644 tests/qapi-schema/funny-word.json
create mode 100644 tests/qapi-schema/funny-word.out
diff --git a/tests/Makefile.include b/tests/Makefile.include
index 799fd4af02..0595914526 100644
--- a/tests/Makefile.include
+++ b/tests/Makefile.include
@@ -402,6 +402,7 @@ qapi-schema += flat-union-no-base.json
qapi-schema += flat-union-optional-discriminator.json
qapi-schema += flat-union-string-discriminator.json
qapi-schema += funny-char.json
+qapi-schema += funny-word.json
qapi-schema += ident-with-escape.json
qapi-schema += include-before-err.json
qapi-schema += include-cycle.json
diff --git a/tests/qapi-schema/bad-type-int.json b/tests/qapi-schema/bad-type-int.json
index 56fc6f8126..f3ad803cb6 100644
--- a/tests/qapi-schema/bad-type-int.json
+++ b/tests/qapi-schema/bad-type-int.json
@@ -1,3 +1,3 @@
# we reject an expression with a metatype that is not a string
# FIXME: once the parser understands integer inputs, improve the error message
-{ 'struct': 1, 'data': { } }
+{ 'struct': 123, 'data': { } }
diff --git a/tests/qapi-schema/funny-word.err b/tests/qapi-schema/funny-word.err
new file mode 100644
index 0000000000..0a440574bd
--- /dev/null
+++ b/tests/qapi-schema/funny-word.err
@@ -0,0 +1 @@
+tests/qapi-schema/funny-word.json:1:3: Stray "c"
diff --git a/tests/qapi-schema/funny-word.exit b/tests/qapi-schema/funny-word.exit
new file mode 100644
index 0000000000..d00491fd7e
--- /dev/null
+++ b/tests/qapi-schema/funny-word.exit
@@ -0,0 +1 @@
+1
diff --git a/tests/qapi-schema/funny-word.json b/tests/qapi-schema/funny-word.json
new file mode 100644
index 0000000000..1153b9f12f
--- /dev/null
+++ b/tests/qapi-schema/funny-word.json
@@ -0,0 +1 @@
+{ command: 'foo' }
diff --git a/tests/qapi-schema/funny-word.out b/tests/qapi-schema/funny-word.out
new file mode 100644
index 0000000000..e69de29bb2
--
2.21.0
^ permalink raw reply related [flat|nested] 41+ messages in thread
* [PULL 25/37] qapi: Use quotes more consistently in frontend error messages
2019-09-24 12:32 [PULL 00/37] QAPI patches for 2019-09-24 Markus Armbruster
` (23 preceding siblings ...)
2019-09-24 12:33 ` [PULL 24/37] tests/qapi-schema: Demonstrate suboptimal lexical errors Markus Armbruster
@ 2019-09-24 12:33 ` Markus Armbruster
2019-09-24 12:33 ` [PULL 26/37] qapi: Improve reporting of lexical errors Markus Armbruster
` (14 subsequent siblings)
39 siblings, 0 replies; 41+ messages in thread
From: Markus Armbruster @ 2019-09-24 12:33 UTC (permalink / raw)
To: qemu-devel
Consistently enclose error messages in double quotes. Use single
quotes within, except for one case of "'".
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Message-Id: <20190914153506.2151-8-armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
---
scripts/qapi/common.py | 37 ++++++++++---------
tests/qapi-schema/bad-type-int.err | 2 +-
tests/qapi-schema/doc-missing-colon.err | 2 +-
tests/qapi-schema/duplicate-key.err | 2 +-
tests/qapi-schema/enum-int-member.err | 2 +-
tests/qapi-schema/escape-outside-string.err | 1 +
tests/qapi-schema/funny-char.err | 2 +-
tests/qapi-schema/funny-word.err | 2 +-
tests/qapi-schema/include-before-err.err | 2 +-
tests/qapi-schema/include-nested-err.err | 2 +-
tests/qapi-schema/leading-comma-list.err | 2 +-
tests/qapi-schema/leading-comma-object.err | 2 +-
tests/qapi-schema/missing-colon.err | 2 +-
tests/qapi-schema/missing-comma-list.err | 2 +-
tests/qapi-schema/missing-comma-object.err | 2 +-
tests/qapi-schema/non-objects.err | 2 +-
tests/qapi-schema/quoted-structural-chars.err | 2 +-
tests/qapi-schema/trailing-comma-list.err | 2 +-
tests/qapi-schema/unclosed-list.err | 2 +-
tests/qapi-schema/unclosed-object.err | 2 +-
20 files changed, 38 insertions(+), 36 deletions(-)
create mode 100644 tests/qapi-schema/escape-outside-string.err
diff --git a/scripts/qapi/common.py b/scripts/qapi/common.py
index f27860540b..142ab276ff 100644
--- a/scripts/qapi/common.py
+++ b/scripts/qapi/common.py
@@ -214,7 +214,7 @@ class QAPIDoc(object):
# recognized, and get silently treated as ordinary text
if not self.symbol and not self.body.text and line.startswith('@'):
if not line.endswith(':'):
- raise QAPIParseError(self._parser, "Line should end with :")
+ raise QAPIParseError(self._parser, "Line should end with ':'")
self.symbol = line[1:-1]
# FIXME invalid names other than the empty string aren't flagged
if not self.symbol:
@@ -470,7 +470,7 @@ class QAPISchemaParser(object):
else:
fobj = open(incl_fname, 'r')
except IOError as e:
- raise QAPISemError(info, '%s: %s' % (e.strerror, incl_fname))
+ raise QAPISemError(info, "%s: %s" % (e.strerror, incl_fname))
return QAPISchemaParser(fobj, previously_included, info)
def _pragma(self, name, value, info):
@@ -522,7 +522,7 @@ class QAPISchemaParser(object):
ch = self.src[self.cursor]
self.cursor += 1
if ch == '\n':
- raise QAPIParseError(self, 'Missing terminating "\'"')
+ raise QAPIParseError(self, "Missing terminating \"'\"")
if esc:
# Note: we recognize only \\ because we have
# no use for funny characters in strings
@@ -559,7 +559,7 @@ class QAPISchemaParser(object):
self.line += 1
self.line_pos = self.cursor
elif not self.tok.isspace():
- raise QAPIParseError(self, 'Stray "%s"' % self.tok)
+ raise QAPIParseError(self, "Stray '%s'" % self.tok)
def get_members(self):
expr = OrderedDict()
@@ -567,24 +567,24 @@ class QAPISchemaParser(object):
self.accept()
return expr
if self.tok != "'":
- raise QAPIParseError(self, 'Expected string or "}"')
+ raise QAPIParseError(self, "Expected string or '}'")
while True:
key = self.val
self.accept()
if self.tok != ':':
- raise QAPIParseError(self, 'Expected ":"')
+ raise QAPIParseError(self, "Expected ':'")
self.accept()
if key in expr:
- raise QAPIParseError(self, 'Duplicate key "%s"' % key)
+ raise QAPIParseError(self, "Duplicate key '%s'" % key)
expr[key] = self.get_expr(True)
if self.tok == '}':
self.accept()
return expr
if self.tok != ',':
- raise QAPIParseError(self, 'Expected "," or "}"')
+ raise QAPIParseError(self, "Expected ',' or '}'")
self.accept()
if self.tok != "'":
- raise QAPIParseError(self, 'Expected string')
+ raise QAPIParseError(self, "Expected string")
def get_values(self):
expr = []
@@ -592,20 +592,20 @@ class QAPISchemaParser(object):
self.accept()
return expr
if self.tok not in "{['tfn":
- raise QAPIParseError(self, 'Expected "{", "[", "]", string, '
- 'boolean or "null"')
+ raise QAPIParseError(
+ self, "Expected '{', '[', ']', string, boolean or 'null'")
while True:
expr.append(self.get_expr(True))
if self.tok == ']':
self.accept()
return expr
if self.tok != ',':
- raise QAPIParseError(self, 'Expected "," or "]"')
+ raise QAPIParseError(self, "Expected ',' or ']'")
self.accept()
def get_expr(self, nested):
if self.tok != '{' and not nested:
- raise QAPIParseError(self, 'Expected "{"')
+ raise QAPIParseError(self, "Expected '{'")
if self.tok == '{':
self.accept()
expr = self.get_members()
@@ -616,8 +616,8 @@ class QAPISchemaParser(object):
expr = self.val
self.accept()
else:
- raise QAPIParseError(self, 'Expected "{", "[", string, '
- 'boolean or "null"')
+ raise QAPIParseError(
+ self, "Expected '{', '[', string, boolean or 'null'")
return expr
def get_doc(self, info):
@@ -881,9 +881,10 @@ def check_union(expr, info):
"struct '%s'"
% (discriminator, base))
if discriminator_value.get('if'):
- raise QAPISemError(info, 'The discriminator %s.%s for union %s '
- 'must not be conditional' %
- (base, discriminator, name))
+ raise QAPISemError(
+ info,
+ "The discriminator %s.%s for union %s must not be conditional"
+ % (base, discriminator, name))
enum_define = enum_types.get(discriminator_value['type'])
# Do not allow string discriminator
if not enum_define:
diff --git a/tests/qapi-schema/bad-type-int.err b/tests/qapi-schema/bad-type-int.err
index da89895404..2021fda5d1 100644
--- a/tests/qapi-schema/bad-type-int.err
+++ b/tests/qapi-schema/bad-type-int.err
@@ -1 +1 @@
-tests/qapi-schema/bad-type-int.json:3:13: Stray "1"
+tests/qapi-schema/bad-type-int.json:3:13: Stray '1'
diff --git a/tests/qapi-schema/doc-missing-colon.err b/tests/qapi-schema/doc-missing-colon.err
index 817398b8e4..b41d5078b0 100644
--- a/tests/qapi-schema/doc-missing-colon.err
+++ b/tests/qapi-schema/doc-missing-colon.err
@@ -1 +1 @@
-tests/qapi-schema/doc-missing-colon.json:4:1: Line should end with :
+tests/qapi-schema/doc-missing-colon.json:4:1: Line should end with ':'
diff --git a/tests/qapi-schema/duplicate-key.err b/tests/qapi-schema/duplicate-key.err
index 6d02f83538..3af2f55174 100644
--- a/tests/qapi-schema/duplicate-key.err
+++ b/tests/qapi-schema/duplicate-key.err
@@ -1 +1 @@
-tests/qapi-schema/duplicate-key.json:3:10: Duplicate key "key"
+tests/qapi-schema/duplicate-key.json:3:10: Duplicate key 'key'
diff --git a/tests/qapi-schema/enum-int-member.err b/tests/qapi-schema/enum-int-member.err
index 071c5213d8..3f8d7b5b71 100644
--- a/tests/qapi-schema/enum-int-member.err
+++ b/tests/qapi-schema/enum-int-member.err
@@ -1 +1 @@
-tests/qapi-schema/enum-int-member.json:3:31: Stray "1"
+tests/qapi-schema/enum-int-member.json:3:31: Stray '1'
diff --git a/tests/qapi-schema/escape-outside-string.err b/tests/qapi-schema/escape-outside-string.err
new file mode 100644
index 0000000000..efee335ba0
--- /dev/null
+++ b/tests/qapi-schema/escape-outside-string.err
@@ -0,0 +1 @@
+tests/qapi-schema/escape-outside-string.json:3:27: Stray '\'
diff --git a/tests/qapi-schema/funny-char.err b/tests/qapi-schema/funny-char.err
index bfc890cd9f..139ecf4eb8 100644
--- a/tests/qapi-schema/funny-char.err
+++ b/tests/qapi-schema/funny-char.err
@@ -1 +1 @@
-tests/qapi-schema/funny-char.json:2:36: Stray ";"
+tests/qapi-schema/funny-char.json:2:36: Stray ';'
diff --git a/tests/qapi-schema/funny-word.err b/tests/qapi-schema/funny-word.err
index 0a440574bd..18aedb4a99 100644
--- a/tests/qapi-schema/funny-word.err
+++ b/tests/qapi-schema/funny-word.err
@@ -1 +1 @@
-tests/qapi-schema/funny-word.json:1:3: Stray "c"
+tests/qapi-schema/funny-word.json:1:3: Stray 'c'
diff --git a/tests/qapi-schema/include-before-err.err b/tests/qapi-schema/include-before-err.err
index 55652751e1..2b26322170 100644
--- a/tests/qapi-schema/include-before-err.err
+++ b/tests/qapi-schema/include-before-err.err
@@ -1 +1 @@
-tests/qapi-schema/include-before-err.json:2:13: Expected ":"
+tests/qapi-schema/include-before-err.json:2:13: Expected ':'
diff --git a/tests/qapi-schema/include-nested-err.err b/tests/qapi-schema/include-nested-err.err
index 1b7b22706b..aec6207eb0 100644
--- a/tests/qapi-schema/include-nested-err.err
+++ b/tests/qapi-schema/include-nested-err.err
@@ -1,2 +1,2 @@
In file included from tests/qapi-schema/include-nested-err.json:1:
-tests/qapi-schema/missing-colon.json:1:10: Expected ":"
+tests/qapi-schema/missing-colon.json:1:10: Expected ':'
diff --git a/tests/qapi-schema/leading-comma-list.err b/tests/qapi-schema/leading-comma-list.err
index f5c870bb9c..e021e42ad9 100644
--- a/tests/qapi-schema/leading-comma-list.err
+++ b/tests/qapi-schema/leading-comma-list.err
@@ -1 +1 @@
-tests/qapi-schema/leading-comma-list.json:2:13: Expected "{", "[", "]", string, boolean or "null"
+tests/qapi-schema/leading-comma-list.json:2:13: Expected '{', '[', ']', string, boolean or 'null'
diff --git a/tests/qapi-schema/leading-comma-object.err b/tests/qapi-schema/leading-comma-object.err
index f767b95544..3852f123d2 100644
--- a/tests/qapi-schema/leading-comma-object.err
+++ b/tests/qapi-schema/leading-comma-object.err
@@ -1 +1 @@
-tests/qapi-schema/leading-comma-object.json:1:3: Expected string or "}"
+tests/qapi-schema/leading-comma-object.json:1:3: Expected string or '}'
diff --git a/tests/qapi-schema/missing-colon.err b/tests/qapi-schema/missing-colon.err
index d9d66b377a..a255e51918 100644
--- a/tests/qapi-schema/missing-colon.err
+++ b/tests/qapi-schema/missing-colon.err
@@ -1 +1 @@
-tests/qapi-schema/missing-colon.json:1:10: Expected ":"
+tests/qapi-schema/missing-colon.json:1:10: Expected ':'
diff --git a/tests/qapi-schema/missing-comma-list.err b/tests/qapi-schema/missing-comma-list.err
index e73d2770d6..df3f553f39 100644
--- a/tests/qapi-schema/missing-comma-list.err
+++ b/tests/qapi-schema/missing-comma-list.err
@@ -1 +1 @@
-tests/qapi-schema/missing-comma-list.json:2:20: Expected "," or "]"
+tests/qapi-schema/missing-comma-list.json:2:20: Expected ',' or ']'
diff --git a/tests/qapi-schema/missing-comma-object.err b/tests/qapi-schema/missing-comma-object.err
index 52b3a8a1ec..0f691b8ddd 100644
--- a/tests/qapi-schema/missing-comma-object.err
+++ b/tests/qapi-schema/missing-comma-object.err
@@ -1 +1 @@
-tests/qapi-schema/missing-comma-object.json:2:3: Expected "," or "}"
+tests/qapi-schema/missing-comma-object.json:2:3: Expected ',' or '}'
diff --git a/tests/qapi-schema/non-objects.err b/tests/qapi-schema/non-objects.err
index 334f0c91ae..a972abd937 100644
--- a/tests/qapi-schema/non-objects.err
+++ b/tests/qapi-schema/non-objects.err
@@ -1 +1 @@
-tests/qapi-schema/non-objects.json:1:1: Expected "{"
+tests/qapi-schema/non-objects.json:1:1: Expected '{'
diff --git a/tests/qapi-schema/quoted-structural-chars.err b/tests/qapi-schema/quoted-structural-chars.err
index 9b183841dd..6e036c8044 100644
--- a/tests/qapi-schema/quoted-structural-chars.err
+++ b/tests/qapi-schema/quoted-structural-chars.err
@@ -1 +1 @@
-tests/qapi-schema/quoted-structural-chars.json:1:1: Expected "{"
+tests/qapi-schema/quoted-structural-chars.json:1:1: Expected '{'
diff --git a/tests/qapi-schema/trailing-comma-list.err b/tests/qapi-schema/trailing-comma-list.err
index 212e14ae28..601c4537c3 100644
--- a/tests/qapi-schema/trailing-comma-list.err
+++ b/tests/qapi-schema/trailing-comma-list.err
@@ -1 +1 @@
-tests/qapi-schema/trailing-comma-list.json:2:36: Expected "{", "[", string, boolean or "null"
+tests/qapi-schema/trailing-comma-list.json:2:36: Expected '{', '[', string, boolean or 'null'
diff --git a/tests/qapi-schema/unclosed-list.err b/tests/qapi-schema/unclosed-list.err
index fb41a86abd..1cc3a094fe 100644
--- a/tests/qapi-schema/unclosed-list.err
+++ b/tests/qapi-schema/unclosed-list.err
@@ -1 +1 @@
-tests/qapi-schema/unclosed-list.json:1:20: Expected "," or "]"
+tests/qapi-schema/unclosed-list.json:1:20: Expected ',' or ']'
diff --git a/tests/qapi-schema/unclosed-object.err b/tests/qapi-schema/unclosed-object.err
index db3deedd63..fd1a86b704 100644
--- a/tests/qapi-schema/unclosed-object.err
+++ b/tests/qapi-schema/unclosed-object.err
@@ -1 +1 @@
-tests/qapi-schema/unclosed-object.json:1:21: Expected "," or "}"
+tests/qapi-schema/unclosed-object.json:1:21: Expected ',' or '}'
--
2.21.0
^ permalink raw reply related [flat|nested] 41+ messages in thread
* [PULL 26/37] qapi: Improve reporting of lexical errors
2019-09-24 12:32 [PULL 00/37] QAPI patches for 2019-09-24 Markus Armbruster
` (24 preceding siblings ...)
2019-09-24 12:33 ` [PULL 25/37] qapi: Use quotes more consistently in frontend error messages Markus Armbruster
@ 2019-09-24 12:33 ` Markus Armbruster
2019-09-24 12:33 ` [PULL 27/37] qapi: Remove null from schema language Markus Armbruster
` (13 subsequent siblings)
39 siblings, 0 replies; 41+ messages in thread
From: Markus Armbruster @ 2019-09-24 12:33 UTC (permalink / raw)
To: qemu-devel
Show text up to next structural character, whitespace, or quote
character instead of just the first character.
Forgotten quotes now get reported like "Stray 'command'" instead of
"Stray 'c'".
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Message-Id: <20190914153506.2151-9-armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
---
scripts/qapi/common.py | 6 +++++-
tests/qapi-schema/bad-type-int.err | 2 +-
tests/qapi-schema/funny-word.err | 2 +-
3 files changed, 7 insertions(+), 3 deletions(-)
diff --git a/scripts/qapi/common.py b/scripts/qapi/common.py
index 142ab276ff..b3383b17ef 100644
--- a/scripts/qapi/common.py
+++ b/scripts/qapi/common.py
@@ -559,7 +559,11 @@ class QAPISchemaParser(object):
self.line += 1
self.line_pos = self.cursor
elif not self.tok.isspace():
- raise QAPIParseError(self, "Stray '%s'" % self.tok)
+ # Show up to next structural, whitespace or quote
+ # character
+ match = re.match('[^[\\]{}:,\\s\'"]+',
+ self.src[self.cursor-1:])
+ raise QAPIParseError(self, "Stray '%s'" % match.group(0))
def get_members(self):
expr = OrderedDict()
diff --git a/tests/qapi-schema/bad-type-int.err b/tests/qapi-schema/bad-type-int.err
index 2021fda5d1..9b2c12c1eb 100644
--- a/tests/qapi-schema/bad-type-int.err
+++ b/tests/qapi-schema/bad-type-int.err
@@ -1 +1 @@
-tests/qapi-schema/bad-type-int.json:3:13: Stray '1'
+tests/qapi-schema/bad-type-int.json:3:13: Stray '123'
diff --git a/tests/qapi-schema/funny-word.err b/tests/qapi-schema/funny-word.err
index 18aedb4a99..af92fe2551 100644
--- a/tests/qapi-schema/funny-word.err
+++ b/tests/qapi-schema/funny-word.err
@@ -1 +1 @@
-tests/qapi-schema/funny-word.json:1:3: Stray 'c'
+tests/qapi-schema/funny-word.json:1:3: Stray 'command'
--
2.21.0
^ permalink raw reply related [flat|nested] 41+ messages in thread
* [PULL 27/37] qapi: Remove null from schema language
2019-09-24 12:32 [PULL 00/37] QAPI patches for 2019-09-24 Markus Armbruster
` (25 preceding siblings ...)
2019-09-24 12:33 ` [PULL 26/37] qapi: Improve reporting of lexical errors Markus Armbruster
@ 2019-09-24 12:33 ` Markus Armbruster
2019-09-24 12:33 ` [PULL 28/37] qapi: Fix broken discriminator error messages Markus Armbruster
` (12 subsequent siblings)
39 siblings, 0 replies; 41+ messages in thread
From: Markus Armbruster @ 2019-09-24 12:33 UTC (permalink / raw)
To: qemu-devel
We represent the parse tree as OrderedDict. We fetch optional dict
members with .get(). So far, so good.
We represent null literals as None. .get() returns None both for
"absent" and for "present, value is the null literal". Uh-oh.
Test features-if-invalid exposes this bug: "'if': null" is
misinterpreted as absent "if".
We added null to the schema language to "allow [...] an explicit
default value" (commit e53188ada5 "qapi: Allow true, false and null in
schema json", v2.4.0). Hasn't happened; null is still unused except
as generic invalid value in tests/.
To fix, we'd have to replace .get() by something more careful, or
represent null differently. Feasible, but we got more and bigger fish
to fry right now. Remove the null literal from the schema language.
Replace null in tests by another invalid value.
Test features-if-invalid now behaves as it should.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Message-Id: <20190914153506.2151-10-armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
---
docs/devel/qapi-code-gen.txt | 4 ++--
scripts/qapi/common.py | 4 ----
tests/qapi-schema/features-if-invalid.err | 1 +
tests/qapi-schema/features-if-invalid.exit | 2 +-
tests/qapi-schema/features-if-invalid.json | 3 +--
tests/qapi-schema/features-if-invalid.out | 14 --------------
.../pragma-name-case-whitelist-crap.json | 2 +-
7 files changed, 6 insertions(+), 24 deletions(-)
diff --git a/docs/devel/qapi-code-gen.txt b/docs/devel/qapi-code-gen.txt
index b12a5b4de8..64d9e4c6a9 100644
--- a/docs/devel/qapi-code-gen.txt
+++ b/docs/devel/qapi-code-gen.txt
@@ -52,7 +52,7 @@ Differences:
* Strings are restricted to printable ASCII, and escape sequences to
just '\\'.
-* Numbers are not supported.
+* Numbers and null are not supported.
A second layer of syntax defines the sequences of JSON texts that are
a correctly structured QAPI schema. We provide a grammar for this
@@ -67,7 +67,7 @@ syntax in an EBNF-like notation:
expression A separated by ,
* Grouping: expression ( A ) matches expression A
* JSON's structural characters are terminals: { } [ ] : ,
-* JSON's literal names are terminals: false true null
+* JSON's literal names are terminals: false true
* String literals enclosed in 'single quotes' are terminal, and match
this JSON string, with a leading '*' stripped off
* When JSON object member's name starts with '*', the member is
diff --git a/scripts/qapi/common.py b/scripts/qapi/common.py
index b3383b17ef..ef7c7be4fd 100644
--- a/scripts/qapi/common.py
+++ b/scripts/qapi/common.py
@@ -548,10 +548,6 @@ class QAPISchemaParser(object):
self.val = False
self.cursor += 4
return
- elif self.src.startswith('null', self.pos):
- self.val = None
- self.cursor += 3
- return
elif self.tok == '\n':
if self.cursor == len(self.src):
self.tok = None
diff --git a/tests/qapi-schema/features-if-invalid.err b/tests/qapi-schema/features-if-invalid.err
index e69de29bb2..295800b4fc 100644
--- a/tests/qapi-schema/features-if-invalid.err
+++ b/tests/qapi-schema/features-if-invalid.err
@@ -0,0 +1 @@
+tests/qapi-schema/features-if-invalid.json:2: 'if' condition must be a string or a list of strings
diff --git a/tests/qapi-schema/features-if-invalid.exit b/tests/qapi-schema/features-if-invalid.exit
index 573541ac97..d00491fd7e 100644
--- a/tests/qapi-schema/features-if-invalid.exit
+++ b/tests/qapi-schema/features-if-invalid.exit
@@ -1 +1 @@
-0
+1
diff --git a/tests/qapi-schema/features-if-invalid.json b/tests/qapi-schema/features-if-invalid.json
index 7e4c1ad720..89c2a6c234 100644
--- a/tests/qapi-schema/features-if-invalid.json
+++ b/tests/qapi-schema/features-if-invalid.json
@@ -1,5 +1,4 @@
# Cover feature with invalid 'if'
-# FIXME not rejected, misinterpreted as unconditional
{ 'struct': 'Stru',
'data': {},
- 'features': [{'name': 'f', 'if': null }] }
+ 'features': [{'name': 'f', 'if': false }] }
diff --git a/tests/qapi-schema/features-if-invalid.out b/tests/qapi-schema/features-if-invalid.out
index 9c2637baa3..e69de29bb2 100644
--- a/tests/qapi-schema/features-if-invalid.out
+++ b/tests/qapi-schema/features-if-invalid.out
@@ -1,14 +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 features-if-invalid.json
-object Stru
- feature f
diff --git a/tests/qapi-schema/pragma-name-case-whitelist-crap.json b/tests/qapi-schema/pragma-name-case-whitelist-crap.json
index 58382bf4e4..734e1c617b 100644
--- a/tests/qapi-schema/pragma-name-case-whitelist-crap.json
+++ b/tests/qapi-schema/pragma-name-case-whitelist-crap.json
@@ -1,3 +1,3 @@
# 'name-case-whitelist' must be list of strings
-{ 'pragma': { 'name-case-whitelist': null } }
+{ 'pragma': { 'name-case-whitelist': false } }
--
2.21.0
^ permalink raw reply related [flat|nested] 41+ messages in thread
* [PULL 28/37] qapi: Fix broken discriminator error messages
2019-09-24 12:32 [PULL 00/37] QAPI patches for 2019-09-24 Markus Armbruster
` (26 preceding siblings ...)
2019-09-24 12:33 ` [PULL 27/37] qapi: Remove null from schema language Markus Armbruster
@ 2019-09-24 12:33 ` Markus Armbruster
2019-09-24 12:33 ` [PULL 29/37] qapi: Reject blank 'if' conditions in addition to empty ones Markus Armbruster
` (11 subsequent siblings)
39 siblings, 0 replies; 41+ messages in thread
From: Markus Armbruster @ 2019-09-24 12:33 UTC (permalink / raw)
To: qemu-devel
check_union() checks the discriminator exists in base and makes sense.
Two error messages mention the base. These are broken for anonymous
bases, as demonstrated by tests flat-union-invalid-discriminator and
flat-union-invalid-if-discriminator.err. The third one doesn't
bother.
First broken when commit ac4338f8eb "qapi: Allow anonymous base for
flat union" (v2.6.0) neglected to adjust the "not a member of base"
error message. Commit ccadd6bcba "qapi: Add 'if' to implicit struct
members" (v4.0.0) then cloned the flawed error message.
Dumb them down not to mention the base.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Message-Id: <20190914153506.2151-11-armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
---
scripts/qapi/common.py | 9 ++++-----
tests/qapi-schema/flat-union-invalid-discriminator.err | 2 +-
tests/qapi-schema/flat-union-invalid-discriminator.json | 1 -
.../qapi-schema/flat-union-invalid-if-discriminator.err | 2 +-
.../qapi-schema/flat-union-invalid-if-discriminator.json | 1 -
tests/qapi-schema/flat-union-optional-discriminator.err | 2 +-
tests/qapi-schema/union-base-empty.err | 2 +-
7 files changed, 8 insertions(+), 11 deletions(-)
diff --git a/scripts/qapi/common.py b/scripts/qapi/common.py
index ef7c7be4fd..a58e904978 100644
--- a/scripts/qapi/common.py
+++ b/scripts/qapi/common.py
@@ -877,14 +877,13 @@ def check_union(expr, info):
discriminator_value = base_members.get(discriminator)
if not discriminator_value:
raise QAPISemError(info,
- "Discriminator '%s' is not a member of base "
- "struct '%s'"
- % (discriminator, base))
+ "Discriminator '%s' is not a member of 'base'"
+ % discriminator)
if discriminator_value.get('if'):
raise QAPISemError(
info,
- "The discriminator %s.%s for union %s must not be conditional"
- % (base, discriminator, name))
+ "The discriminator '%s' for union %s must not be conditional"
+ % (discriminator, name))
enum_define = enum_types.get(discriminator_value['type'])
# Do not allow string discriminator
if not enum_define:
diff --git a/tests/qapi-schema/flat-union-invalid-discriminator.err b/tests/qapi-schema/flat-union-invalid-discriminator.err
index 947a6b73aa..495d5a520e 100644
--- a/tests/qapi-schema/flat-union-invalid-discriminator.err
+++ b/tests/qapi-schema/flat-union-invalid-discriminator.err
@@ -1 +1 @@
-tests/qapi-schema/flat-union-invalid-discriminator.json:11: Discriminator 'enum_wrong' is not a member of base struct 'OrderedDict([('enum1', {'type': 'TestEnum'})])'
+tests/qapi-schema/flat-union-invalid-discriminator.json:10: Discriminator 'enum_wrong' is not a member of 'base'
diff --git a/tests/qapi-schema/flat-union-invalid-discriminator.json b/tests/qapi-schema/flat-union-invalid-discriminator.json
index de86cf0760..c4fce97362 100644
--- a/tests/qapi-schema/flat-union-invalid-discriminator.json
+++ b/tests/qapi-schema/flat-union-invalid-discriminator.json
@@ -1,4 +1,3 @@
-# FIXME error message shows base as OrderedDict
{ 'enum': 'TestEnum',
'data': [ 'value1', 'value2' ] }
diff --git a/tests/qapi-schema/flat-union-invalid-if-discriminator.err b/tests/qapi-schema/flat-union-invalid-if-discriminator.err
index ec04c4840c..cc5c3fb80b 100644
--- a/tests/qapi-schema/flat-union-invalid-if-discriminator.err
+++ b/tests/qapi-schema/flat-union-invalid-if-discriminator.err
@@ -1 +1 @@
-tests/qapi-schema/flat-union-invalid-if-discriminator.json:11: The discriminator OrderedDict([('enum1', OrderedDict([('type', 'TestEnum'), ('if', 'FOO')]))]).enum1 for union TestUnion must not be conditional
+tests/qapi-schema/flat-union-invalid-if-discriminator.json:10: The discriminator 'enum1' for union TestUnion must not be conditional
diff --git a/tests/qapi-schema/flat-union-invalid-if-discriminator.json b/tests/qapi-schema/flat-union-invalid-if-discriminator.json
index bbaa9a3f82..e49992b798 100644
--- a/tests/qapi-schema/flat-union-invalid-if-discriminator.json
+++ b/tests/qapi-schema/flat-union-invalid-if-discriminator.json
@@ -1,4 +1,3 @@
-# FIXME error message shows base as OrderedDict
{ 'enum': 'TestEnum',
'data': [ 'value1', 'value2' ] }
diff --git a/tests/qapi-schema/flat-union-optional-discriminator.err b/tests/qapi-schema/flat-union-optional-discriminator.err
index 8b4a4ba847..45f5407c34 100644
--- a/tests/qapi-schema/flat-union-optional-discriminator.err
+++ b/tests/qapi-schema/flat-union-optional-discriminator.err
@@ -1 +1 @@
-tests/qapi-schema/flat-union-optional-discriminator.json:7: Discriminator 'switch' is not a member of base struct 'Base'
+tests/qapi-schema/flat-union-optional-discriminator.json:7: Discriminator 'switch' is not a member of 'base'
diff --git a/tests/qapi-schema/union-base-empty.err b/tests/qapi-schema/union-base-empty.err
index 7695806d81..9453720ede 100644
--- a/tests/qapi-schema/union-base-empty.err
+++ b/tests/qapi-schema/union-base-empty.err
@@ -1 +1 @@
-tests/qapi-schema/union-base-empty.json:5: Discriminator 'type' is not a member of base struct 'Empty'
+tests/qapi-schema/union-base-empty.json:5: Discriminator 'type' is not a member of 'base'
--
2.21.0
^ permalink raw reply related [flat|nested] 41+ messages in thread
* [PULL 29/37] qapi: Reject blank 'if' conditions in addition to empty ones
2019-09-24 12:32 [PULL 00/37] QAPI patches for 2019-09-24 Markus Armbruster
` (27 preceding siblings ...)
2019-09-24 12:33 ` [PULL 28/37] qapi: Fix broken discriminator error messages Markus Armbruster
@ 2019-09-24 12:33 ` Markus Armbruster
2019-09-24 12:33 ` [PULL 30/37] qapi: Fix missing 'if' checks in struct, union, alternate 'data' Markus Armbruster
` (10 subsequent siblings)
39 siblings, 0 replies; 41+ messages in thread
From: Markus Armbruster @ 2019-09-24 12:33 UTC (permalink / raw)
To: qemu-devel
"'if': 'COND'" generates "#if COND". We reject empty COND because it
won't compile. Blank COND won't compile any better, so reject that,
too.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Message-Id: <20190914153506.2151-12-armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
---
scripts/qapi/common.py | 5 +++--
tests/qapi-schema/bad-if-list.err | 2 +-
tests/qapi-schema/bad-if-list.json | 2 +-
3 files changed, 5 insertions(+), 4 deletions(-)
diff --git a/scripts/qapi/common.py b/scripts/qapi/common.py
index a58e904978..2b46164854 100644
--- a/scripts/qapi/common.py
+++ b/scripts/qapi/common.py
@@ -742,8 +742,9 @@ def check_if(expr, info):
if not isinstance(ifcond, str):
raise QAPISemError(
info, "'if' condition must be a string or a list of strings")
- if ifcond == '':
- raise QAPISemError(info, "'if' condition '' makes no sense")
+ if ifcond.strip() == '':
+ raise QAPISemError(info, "'if' condition '%s' makes no sense"
+ % ifcond)
ifcond = expr.get('if')
if ifcond is None:
diff --git a/tests/qapi-schema/bad-if-list.err b/tests/qapi-schema/bad-if-list.err
index 0af6316f78..53af099083 100644
--- a/tests/qapi-schema/bad-if-list.err
+++ b/tests/qapi-schema/bad-if-list.err
@@ -1 +1 @@
-tests/qapi-schema/bad-if-list.json:2: 'if' condition '' makes no sense
+tests/qapi-schema/bad-if-list.json:2: 'if' condition ' ' makes no sense
diff --git a/tests/qapi-schema/bad-if-list.json b/tests/qapi-schema/bad-if-list.json
index 49ced9b9ca..ea3d95bb6b 100644
--- a/tests/qapi-schema/bad-if-list.json
+++ b/tests/qapi-schema/bad-if-list.json
@@ -1,3 +1,3 @@
# check invalid 'if' content
{ 'struct': 'TestIfStruct', 'data': { 'foo': 'int' },
- 'if': ['foo', ''] }
+ 'if': ['foo', ' '] }
--
2.21.0
^ permalink raw reply related [flat|nested] 41+ messages in thread
* [PULL 30/37] qapi: Fix missing 'if' checks in struct, union, alternate 'data'
2019-09-24 12:32 [PULL 00/37] QAPI patches for 2019-09-24 Markus Armbruster
` (28 preceding siblings ...)
2019-09-24 12:33 ` [PULL 29/37] qapi: Reject blank 'if' conditions in addition to empty ones Markus Armbruster
@ 2019-09-24 12:33 ` Markus Armbruster
2019-09-24 12:33 ` [PULL 31/37] qapi: Normalize 'if' in check_exprs(), like other sugar Markus Armbruster
` (9 subsequent siblings)
39 siblings, 0 replies; 41+ messages in thread
From: Markus Armbruster @ 2019-09-24 12:33 UTC (permalink / raw)
To: qemu-devel
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>
---
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
^ permalink raw reply related [flat|nested] 41+ messages in thread
* [PULL 31/37] qapi: Normalize 'if' in check_exprs(), like other sugar
2019-09-24 12:32 [PULL 00/37] QAPI patches for 2019-09-24 Markus Armbruster
` (29 preceding siblings ...)
2019-09-24 12:33 ` [PULL 30/37] qapi: Fix missing 'if' checks in struct, union, alternate 'data' Markus Armbruster
@ 2019-09-24 12:33 ` Markus Armbruster
2019-09-24 12:33 ` [PULL 32/37] qapi: Simplify check_keys() Markus Armbruster
` (8 subsequent siblings)
39 siblings, 0 replies; 41+ messages in thread
From: Markus Armbruster @ 2019-09-24 12:33 UTC (permalink / raw)
To: qemu-devel
We normalize shorthand to longhand forms in check_expr(): enumeration
values with normalize_enum(), feature values with
normalize_features(), struct members, union branches and alternate
branches with normalize_members(). If conditions are an exception: we
normalize them in QAPISchemaEntity.check() and
QAPISchemaMember.__init(), with listify_cond(). The idea goes back to
commit 2cbc94376e "qapi: pass 'if' condition into QAPISchemaEntity
objects", v3.0.0.
Normalize in check_expr() instead, with new helper normalize_if().
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Message-Id: <20190914153506.2151-14-armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
---
scripts/qapi/common.py | 26 +++++++++++++++-----------
1 file changed, 15 insertions(+), 11 deletions(-)
diff --git a/scripts/qapi/common.py b/scripts/qapi/common.py
index cacee9b8bb..4d1f62e808 100644
--- a/scripts/qapi/common.py
+++ b/scripts/qapi/common.py
@@ -804,6 +804,7 @@ def check_type(info, source, value,
check_known_keys(info, "member '%s' of %s" % (key, source),
arg, ['type'], ['if'])
check_if(arg, info)
+ normalize_if(arg)
check_type(info, "Member '%s' of %s" % (key, source),
arg['type'], allow_array=True,
allow_metas=['built-in', 'union', 'alternate', 'struct',
@@ -904,6 +905,7 @@ def check_union(expr, info):
check_known_keys(info, "member '%s' of union '%s'" % (key, name),
value, ['type'], ['if'])
check_if(value, info)
+ normalize_if(value)
# Each value must name a known type
check_type(info, "Member '%s' of union '%s'" % (key, name),
value['type'],
@@ -933,6 +935,7 @@ def check_alternate(expr, info):
"member '%s' of alternate '%s'" % (key, name),
value, ['type'], ['if'])
check_if(value, info)
+ normalize_if(value)
typ = value['type']
# Ensure alternates have no type conflicts.
@@ -978,6 +981,7 @@ def check_enum(expr, info):
check_known_keys(info, "member of enum '%s'" % name, member,
['name'], ['if'])
check_if(member, info)
+ normalize_if(member)
check_name(info, "Member of enum '%s'" % name, member['name'],
enum_member=True)
@@ -1003,6 +1007,7 @@ def check_struct(expr, info):
['name'], ['if'])
check_if(f, info)
+ normalize_if(f)
check_name(info, "Feature of struct %s" % name, f['name'])
@@ -1067,6 +1072,12 @@ def normalize_features(features):
for f in features]
+def normalize_if(expr):
+ ifcond = expr.get('if')
+ if isinstance(ifcond, str):
+ expr['if'] = [ifcond]
+
+
def check_exprs(exprs):
global all_names
@@ -1123,6 +1134,7 @@ def check_exprs(exprs):
else:
raise QAPISemError(expr_elem['info'],
"Expression is missing metatype")
+ normalize_if(expr)
name = expr[meta]
add_name(name, info, meta)
if doc and doc.symbol != name:
@@ -1177,14 +1189,6 @@ def check_exprs(exprs):
# Schema compiler frontend
#
-def listify_cond(ifcond):
- if not ifcond:
- return []
- if not isinstance(ifcond, list):
- return [ifcond]
- return ifcond
-
-
class QAPISchemaEntity(object):
def __init__(self, name, info, doc, ifcond=None):
assert name is None or isinstance(name, str)
@@ -1197,7 +1201,7 @@ class QAPISchemaEntity(object):
# such place).
self.info = info
self.doc = doc
- self._ifcond = ifcond # self.ifcond is set only after .check()
+ self._ifcond = ifcond or []
def c_name(self):
return c_name(self.name)
@@ -1209,7 +1213,7 @@ class QAPISchemaEntity(object):
typ.check(schema)
self.ifcond = typ.ifcond
else:
- self.ifcond = listify_cond(self._ifcond)
+ self.ifcond = self._ifcond
if self.info:
self.module = os.path.relpath(self.info['file'],
os.path.dirname(schema.fname))
@@ -1515,7 +1519,7 @@ class QAPISchemaMember(object):
def __init__(self, name, ifcond=None):
assert isinstance(name, str)
self.name = name
- self.ifcond = listify_cond(ifcond)
+ self.ifcond = ifcond or []
self.owner = None
def set_owner(self, name):
--
2.21.0
^ permalink raw reply related [flat|nested] 41+ messages in thread
* [PULL 32/37] qapi: Simplify check_keys()
2019-09-24 12:32 [PULL 00/37] QAPI patches for 2019-09-24 Markus Armbruster
` (30 preceding siblings ...)
2019-09-24 12:33 ` [PULL 31/37] qapi: Normalize 'if' in check_exprs(), like other sugar Markus Armbruster
@ 2019-09-24 12:33 ` Markus Armbruster
2019-09-24 12:33 ` [PULL 33/37] qapi: Clean up around check_known_keys() Markus Armbruster
` (7 subsequent siblings)
39 siblings, 0 replies; 41+ messages in thread
From: Markus Armbruster @ 2019-09-24 12:33 UTC (permalink / raw)
To: qemu-devel
check_keys() parameter expr_elem expects a dictionary with keys 'expr'
and 'info'. Passing the two values separately is simpler, so do that.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Message-Id: <20190914153506.2151-15-armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
---
scripts/qapi/common.py | 19 ++++++++-----------
1 file changed, 8 insertions(+), 11 deletions(-)
diff --git a/scripts/qapi/common.py b/scripts/qapi/common.py
index 4d1f62e808..4d4e0be770 100644
--- a/scripts/qapi/common.py
+++ b/scripts/qapi/common.py
@@ -1029,9 +1029,7 @@ def check_known_keys(info, source, keys, required, optional):
source, pprint(allowed)))
-def check_keys(expr_elem, meta, required, optional=[]):
- expr = expr_elem['expr']
- info = expr_elem['info']
+def check_keys(expr, info, meta, required, optional=[]):
name = expr[meta]
if not isinstance(name, str):
raise QAPISemError(info, "'%s' key must have a string value" % meta)
@@ -1100,40 +1098,39 @@ def check_exprs(exprs):
if 'enum' in expr:
meta = 'enum'
- check_keys(expr_elem, 'enum', ['data'], ['if', 'prefix'])
+ check_keys(expr, info, 'enum', ['data'], ['if', 'prefix'])
normalize_enum(expr)
enum_types[expr[meta]] = expr
elif 'union' in expr:
meta = 'union'
- check_keys(expr_elem, 'union', ['data'],
+ check_keys(expr, info, 'union', ['data'],
['base', 'discriminator', 'if'])
normalize_members(expr.get('base'))
normalize_members(expr['data'])
union_types[expr[meta]] = expr
elif 'alternate' in expr:
meta = 'alternate'
- check_keys(expr_elem, 'alternate', ['data'], ['if'])
+ check_keys(expr, info, 'alternate', ['data'], ['if'])
normalize_members(expr['data'])
elif 'struct' in expr:
meta = 'struct'
- check_keys(expr_elem, 'struct', ['data'],
+ check_keys(expr, info, 'struct', ['data'],
['base', 'if', 'features'])
normalize_members(expr['data'])
normalize_features(expr.get('features'))
struct_types[expr[meta]] = expr
elif 'command' in expr:
meta = 'command'
- check_keys(expr_elem, 'command', [],
+ check_keys(expr, info, 'command', [],
['data', 'returns', 'gen', 'success-response',
'boxed', 'allow-oob', 'allow-preconfig', 'if'])
normalize_members(expr.get('data'))
elif 'event' in expr:
meta = 'event'
- check_keys(expr_elem, 'event', [], ['data', 'boxed', 'if'])
+ check_keys(expr, info, 'event', [], ['data', 'boxed', 'if'])
normalize_members(expr.get('data'))
else:
- raise QAPISemError(expr_elem['info'],
- "Expression is missing metatype")
+ raise QAPISemError(info, "Expression is missing metatype")
normalize_if(expr)
name = expr[meta]
add_name(name, info, meta)
--
2.21.0
^ permalink raw reply related [flat|nested] 41+ messages in thread
* [PULL 33/37] qapi: Clean up around check_known_keys()
2019-09-24 12:32 [PULL 00/37] QAPI patches for 2019-09-24 Markus Armbruster
` (31 preceding siblings ...)
2019-09-24 12:33 ` [PULL 32/37] qapi: Simplify check_keys() Markus Armbruster
@ 2019-09-24 12:33 ` Markus Armbruster
2019-09-24 12:33 ` [PULL 34/37] qapi: Delete useless check_exprs() code for simple union kind Markus Armbruster
` (6 subsequent siblings)
39 siblings, 0 replies; 41+ messages in thread
From: Markus Armbruster @ 2019-09-24 12:33 UTC (permalink / raw)
To: qemu-devel
All callers pass a dict argument to @keys, except check_keys() passes
a dict's .keys(). Drop .keys() there, and rename parameter @keys to
@value.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Message-Id: <20190914153506.2151-16-armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
---
scripts/qapi/common.py | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/scripts/qapi/common.py b/scripts/qapi/common.py
index 4d4e0be770..3c3154a039 100644
--- a/scripts/qapi/common.py
+++ b/scripts/qapi/common.py
@@ -1011,18 +1011,18 @@ def check_struct(expr, info):
check_name(info, "Feature of struct %s" % name, f['name'])
-def check_known_keys(info, source, keys, required, optional):
+def check_known_keys(info, source, value, required, optional):
def pprint(elems):
return ', '.join("'" + e + "'" for e in sorted(elems))
- missing = set(required) - set(keys)
+ missing = set(required) - set(value)
if missing:
raise QAPISemError(info, "Key%s %s %s missing from %s"
% ('s' if len(missing) > 1 else '', pprint(missing),
'are' if len(missing) > 1 else 'is', source))
allowed = set(required + optional)
- unknown = set(keys) - allowed
+ unknown = set(value) - allowed
if unknown:
raise QAPISemError(info, "Unknown key%s %s in %s\nValid keys are %s."
% ('s' if len(unknown) > 1 else '', pprint(unknown),
@@ -1035,7 +1035,7 @@ def check_keys(expr, info, meta, required, optional=[]):
raise QAPISemError(info, "'%s' key must have a string value" % meta)
required = required + [meta]
source = "%s '%s'" % (meta, name)
- check_known_keys(info, source, expr.keys(), required, optional)
+ check_known_keys(info, source, expr, required, optional)
for (key, value) in expr.items():
if key in ['gen', 'success-response'] and value is not False:
raise QAPISemError(info,
--
2.21.0
^ permalink raw reply related [flat|nested] 41+ messages in thread
* [PULL 34/37] qapi: Delete useless check_exprs() code for simple union kind
2019-09-24 12:32 [PULL 00/37] QAPI patches for 2019-09-24 Markus Armbruster
` (32 preceding siblings ...)
2019-09-24 12:33 ` [PULL 33/37] qapi: Clean up around check_known_keys() Markus Armbruster
@ 2019-09-24 12:33 ` Markus Armbruster
2019-09-24 12:33 ` [PULL 35/37] qapi: Fix to .check() empty structs just once Markus Armbruster
` (5 subsequent siblings)
39 siblings, 0 replies; 41+ messages in thread
From: Markus Armbruster @ 2019-09-24 12:33 UTC (permalink / raw)
To: qemu-devel
Commit bceae7697f "qapi script: support enum type as discriminator in
union" made check_exprs() add the implicit enum types of simple unions
to global @enum_types. I'm not sure it was needed even then. It's
certainly not needed now. Delete it.
discriminator_find_enum_define() and add_name() parameter @implicit
are now dead. Bury them.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Message-Id: <20190914153506.2151-17-armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
---
scripts/qapi/common.py | 39 ++-------------------------------------
1 file changed, 2 insertions(+), 37 deletions(-)
diff --git a/scripts/qapi/common.py b/scripts/qapi/common.py
index 3c3154a039..7e79c42b6a 100644
--- a/scripts/qapi/common.py
+++ b/scripts/qapi/common.py
@@ -672,26 +672,6 @@ def find_alternate_member_qtype(qapi_type):
return None
-# Return the discriminator enum define if discriminator is specified as an
-# enum type, otherwise return None.
-def discriminator_find_enum_define(expr):
- base = expr.get('base')
- discriminator = expr.get('discriminator')
-
- if not (discriminator and base):
- return None
-
- base_members = find_base_members(base)
- if not base_members:
- return None
-
- discriminator_value = base_members.get(discriminator)
- if not discriminator_value:
- return None
-
- return enum_types.get(discriminator_value['type'])
-
-
# Names must be letters, numbers, -, and _. They must start with letter,
# except for downstream extensions which must start with __RFQDN_.
# Dots are only valid in the downstream extension prefix.
@@ -722,7 +702,7 @@ def check_name(info, source, name, allow_optional=False,
raise QAPISemError(info, "%s uses invalid name '%s'" % (source, name))
-def add_name(name, info, meta, implicit=False):
+def add_name(name, info, meta):
global all_names
check_name(info, "'%s'" % meta, name)
# FIXME should reject names that differ only in '_' vs. '.'
@@ -730,7 +710,7 @@ def add_name(name, info, meta, implicit=False):
if name in all_names:
raise QAPISemError(info, "%s '%s' is already defined"
% (all_names[name], name))
- if not implicit and (name.endswith('Kind') or name.endswith('List')):
+ if name.endswith('Kind') or name.endswith('List'):
raise QAPISemError(info, "%s '%s' should not end in '%s'"
% (meta, name, name[-4:]))
all_names[name] = meta
@@ -1138,21 +1118,6 @@ def check_exprs(exprs):
raise QAPISemError(info, "Definition of '%s' follows documentation"
" for '%s'" % (name, doc.symbol))
- # Try again for hidden UnionKind enum
- for expr_elem in exprs:
- expr = expr_elem['expr']
-
- if 'include' in expr:
- continue
- if 'union' in expr and not discriminator_find_enum_define(expr):
- name = '%sKind' % expr['union']
- elif 'alternate' in expr:
- name = '%sKind' % expr['alternate']
- else:
- continue
- enum_types[name] = {'enum': name}
- add_name(name, info, 'enum', implicit=True)
-
# Validate that exprs make sense
for expr_elem in exprs:
expr = expr_elem['expr']
--
2.21.0
^ permalink raw reply related [flat|nested] 41+ messages in thread
* [PULL 35/37] qapi: Fix to .check() empty structs just once
2019-09-24 12:32 [PULL 00/37] QAPI patches for 2019-09-24 Markus Armbruster
` (33 preceding siblings ...)
2019-09-24 12:33 ` [PULL 34/37] qapi: Delete useless check_exprs() code for simple union kind Markus Armbruster
@ 2019-09-24 12:33 ` Markus Armbruster
2019-09-24 12:33 ` [PULL 36/37] qapi: Fix excessive QAPISchemaEntity.check() recursion Markus Armbruster
` (4 subsequent siblings)
39 siblings, 0 replies; 41+ messages in thread
From: Markus Armbruster @ 2019-09-24 12:33 UTC (permalink / raw)
To: qemu-devel
QAPISchemaObjectType.check() does nothing for types that have been
checked already. Except the "has been checked" predicate is broken
for empty types: self.members is [] then, which isn't true. The bug
is harmless, but fix it anyway: use self.member is not None instead.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Message-Id: <20190914153506.2151-18-armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
---
scripts/qapi/common.py | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/scripts/qapi/common.py b/scripts/qapi/common.py
index 7e79c42b6a..e2c87d1349 100644
--- a/scripts/qapi/common.py
+++ b/scripts/qapi/common.py
@@ -1406,7 +1406,7 @@ class QAPISchemaObjectType(QAPISchemaType):
if self.members is False: # check for cycles
raise QAPISemError(self.info,
"Object %s contains itself" % self.name)
- if self.members:
+ if self.members is not None: # already checked
return
self.members = False # mark as being checked
seen = OrderedDict()
--
2.21.0
^ permalink raw reply related [flat|nested] 41+ messages in thread
* [PULL 36/37] qapi: Fix excessive QAPISchemaEntity.check() recursion
2019-09-24 12:32 [PULL 00/37] QAPI patches for 2019-09-24 Markus Armbruster
` (34 preceding siblings ...)
2019-09-24 12:33 ` [PULL 35/37] qapi: Fix to .check() empty structs just once Markus Armbruster
@ 2019-09-24 12:33 ` Markus Armbruster
2019-09-24 12:33 ` [PULL 37/37] qapi: Assert .visit() and .check_clash() run only after .check() Markus Armbruster
` (3 subsequent siblings)
39 siblings, 0 replies; 41+ messages in thread
From: Markus Armbruster @ 2019-09-24 12:33 UTC (permalink / raw)
To: qemu-devel
Entity checking goes back to commit ac88219a6c "qapi: New QAPISchema
intermediate representation", v2.5.0. It's designed to work as
follows: QAPISchema.check() calls .check() for all the schema's
entities. An entity's .check() recurses into another entity's
.check() only if the C struct generated for the former contains the C
struct generated for the latter (pointers don't count). This is used
to detect "object contains itself".
There are two instances of this:
* An object's C struct contains its base's C struct
QAPISchemaObjectType.check() calls self.base.check()
* An object's C struct contains its variants' C structs
QAPISchemaObjectTypeVariants().check calls v.type.check(). Since
commit b807a1e1e3 "qapi: Check for QAPI collisions involving variant
members", v2.6.0.
Thus, only object types can participate in recursion.
QAPISchemaObjectType.check() is made for that: it checks @self when
called the first time, recursing into base and variants, it reports an
"contains itself" error when this recursion reaches an object being
checked, and does nothing it reaches an object that has been checked
already.
The other .check() may safely assume they get called exactly once.
Sadly, this design has since eroded:
* QAPISchemaCommand.check() and QAPISchemaEvent.check() call
.args_type.check(). Since commit c818408e44 "qapi: Implement boxed
types for commands/events", v2.7.0. Harmless, since args_type can
only be an object type.
* QAPISchemaEntity.check() calls ._ifcond.check() when inheriting the
condition from another type. Since commit 4fca21c1b0 qapi: leave
the ifcond attribute undefined until check(), v3.0.0. This makes
simple union wrapper types recurse into the wrapped type (nothing
else uses this condition inheritance). The .check() of types used
as simple union branch type get called multiple times.
* QAPISchemaObjectType.check() calls its super type's .check()
*before* the conditional handling multiple calls. Also since commit
4fca21c1b0. QAPISchemaObjectType.check()'s guard against multiple
checking doesn't protect QAPISchemaEntity.check().
* QAPISchemaArrayType.check() calls .element_type.check(). Also since
commit 4fca21c1b0. The .check() of types used as array element
types get called multiple times.
Commit 56a4689582 "qapi: Fix array first used in a different module"
(v4.0.0) added more code relying on this .element_type.check().
The absence of explosions suggests the .check() involved happen to be
effectively idempotent.
Fix the unwanted recursion anyway:
* QAPISchemaCommand.check() and QAPISchemaEvent.check() calling
.args_type.check() is unnecessary. Delete the calls.
* Fix QAPISchemaObjectType.check() to call its super type's .check()
after the conditional handling multiple calls.
* A QAPISchemaEntity's .ifcond becomes valid at .check(). This is due
to arrays and simple unions.
Most types get ifcond and info passed to their constructor.
Array types don't: they get it from their element type, which
becomes known only in .element_type.check().
The implicit wrapper object types for simple union branches don't:
they get it from the wrapped type, which might be an array.
Ditch the idea to set .ifcond in .check(). Instead, turn it into a
property and compute it on demand. Safe because it's only used
after the schema has been checked.
Most types simply return the ifcond passed to their constructor.
Array types forward to their .element_type instead, and the wrapper
types forward to the wrapped type.
* A QAPISchemaEntity's .module becomes valid at .check(). This is
because computing it needs info and schema.fname, and because array
types get it from their element type instead.
Make it a property just like .ifcond.
Additionally, have QAPISchemaEntity.check() assert it gets called at
most once, so the design invariant will stick this time. Neglecting
that was entirely my fault.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Message-Id: <20190914153506.2151-19-armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
[Commit message tidied up]
---
scripts/qapi/common.py | 74 +++++++++++++++++++++++++++++-------------
1 file changed, 52 insertions(+), 22 deletions(-)
diff --git a/scripts/qapi/common.py b/scripts/qapi/common.py
index e2c87d1349..c199a2b58c 100644
--- a/scripts/qapi/common.py
+++ b/scripts/qapi/common.py
@@ -1155,7 +1155,7 @@ class QAPISchemaEntity(object):
def __init__(self, name, info, doc, ifcond=None):
assert name is None or isinstance(name, str)
self.name = name
- self.module = None
+ self._module = None
# For explicitly defined entities, info points to the (explicit)
# definition. For builtins (and their arrays), info is None.
# For implicitly defined entities, info points to a place that
@@ -1164,21 +1164,27 @@ class QAPISchemaEntity(object):
self.info = info
self.doc = doc
self._ifcond = ifcond or []
+ self._checked = False
def c_name(self):
return c_name(self.name)
def check(self, schema):
- if isinstance(self._ifcond, QAPISchemaType):
- # inherit the condition from a type
- typ = self._ifcond
- typ.check(schema)
- self.ifcond = typ.ifcond
- else:
- self.ifcond = self._ifcond
+ assert not self._checked
if self.info:
- self.module = os.path.relpath(self.info['file'],
- os.path.dirname(schema.fname))
+ self._module = os.path.relpath(self.info['file'],
+ os.path.dirname(schema.fname))
+ self._checked = True
+
+ @property
+ def ifcond(self):
+ assert self._checked
+ return self._ifcond
+
+ @property
+ def module(self):
+ assert self._checked
+ return self._module
def is_implicit(self):
return not self.info
@@ -1353,9 +1359,16 @@ class QAPISchemaArrayType(QAPISchemaType):
QAPISchemaType.check(self, schema)
self.element_type = schema.lookup_type(self._element_type_name)
assert self.element_type
- self.element_type.check(schema)
- self.module = self.element_type.module
- self.ifcond = self.element_type.ifcond
+
+ @property
+ def ifcond(self):
+ assert self._checked
+ return self.element_type.ifcond
+
+ @property
+ def module(self):
+ assert self._checked
+ return self.element_type.module
def is_implicit(self):
return True
@@ -1402,13 +1415,20 @@ class QAPISchemaObjectType(QAPISchemaType):
self.features = features
def check(self, schema):
- QAPISchemaType.check(self, schema)
- if self.members is False: # check for cycles
+ # This calls another type T's .check() exactly when the C
+ # struct emitted by gen_object() contains that T's C struct
+ # (pointers don't count).
+ if self.members is not None:
+ # A previous .check() completed: nothing to do
+ return
+ if self._checked:
+ # Recursed: C struct contains itself
raise QAPISemError(self.info,
"Object %s contains itself" % self.name)
- if self.members is not None: # already checked
- return
- self.members = False # mark as being checked
+
+ QAPISchemaType.check(self, schema)
+ assert self._checked and self.members is None
+
seen = OrderedDict()
if self._base_name:
self.base = schema.lookup_type(self._base_name)
@@ -1420,10 +1440,11 @@ class QAPISchemaObjectType(QAPISchemaType):
m.check_clash(self.info, seen)
if self.doc:
self.doc.connect_member(m)
- self.members = seen.values()
+ members = seen.values()
+
if self.variants:
self.variants.check(schema, seen)
- assert self.variants.tag_member in self.members
+ assert self.variants.tag_member in members
self.variants.check_clash(self.info, seen)
# Features are in a name space separate from members
@@ -1434,6 +1455,8 @@ class QAPISchemaObjectType(QAPISchemaType):
if self.doc:
self.doc.check()
+ self.members = members # mark completed
+
# Check that the members of this type do not cause duplicate JSON members,
# and update seen to track the members seen so far. Report any errors
# on behalf of info, which is not necessarily self.info
@@ -1442,6 +1465,15 @@ class QAPISchemaObjectType(QAPISchemaType):
for m in self.members:
m.check_clash(info, seen)
+ @property
+ def ifcond(self):
+ assert self._checked
+ if isinstance(self._ifcond, QAPISchemaType):
+ # Simple union wrapper type inherits from wrapped type;
+ # see _make_implicit_object_type()
+ return self._ifcond.ifcond
+ return self._ifcond
+
def is_implicit(self):
# See QAPISchema._make_implicit_object_type(), as well as
# _def_predefineds()
@@ -1658,7 +1690,6 @@ class QAPISchemaCommand(QAPISchemaEntity):
if self._arg_type_name:
self.arg_type = schema.lookup_type(self._arg_type_name)
assert isinstance(self.arg_type, QAPISchemaObjectType)
- self.arg_type.check(schema)
assert not self.arg_type.variants or self.boxed
elif self.boxed:
raise QAPISemError(self.info, "Use of 'boxed' requires 'data'")
@@ -1687,7 +1718,6 @@ class QAPISchemaEvent(QAPISchemaEntity):
if self._arg_type_name:
self.arg_type = schema.lookup_type(self._arg_type_name)
assert isinstance(self.arg_type, QAPISchemaObjectType)
- self.arg_type.check(schema)
assert not self.arg_type.variants or self.boxed
elif self.boxed:
raise QAPISemError(self.info, "Use of 'boxed' requires 'data'")
--
2.21.0
^ permalink raw reply related [flat|nested] 41+ messages in thread
* [PULL 37/37] qapi: Assert .visit() and .check_clash() run only after .check()
2019-09-24 12:32 [PULL 00/37] QAPI patches for 2019-09-24 Markus Armbruster
` (35 preceding siblings ...)
2019-09-24 12:33 ` [PULL 36/37] qapi: Fix excessive QAPISchemaEntity.check() recursion Markus Armbruster
@ 2019-09-24 12:33 ` Markus Armbruster
2019-09-25 1:21 ` [PULL 00/37] QAPI patches for 2019-09-24 no-reply
` (2 subsequent siblings)
39 siblings, 0 replies; 41+ messages in thread
From: Markus Armbruster @ 2019-09-24 12:33 UTC (permalink / raw)
To: qemu-devel
Easy since the previous commit provides .checked.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Message-Id: <20190914153506.2151-20-armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
---
scripts/qapi/common.py | 11 ++++++++++-
1 file changed, 10 insertions(+), 1 deletion(-)
diff --git a/scripts/qapi/common.py b/scripts/qapi/common.py
index c199a2b58c..b00caacca3 100644
--- a/scripts/qapi/common.py
+++ b/scripts/qapi/common.py
@@ -1190,7 +1190,7 @@ class QAPISchemaEntity(object):
return not self.info
def visit(self, visitor):
- pass
+ assert self._checked
class QAPISchemaVisitor(object):
@@ -1245,6 +1245,7 @@ class QAPISchemaInclude(QAPISchemaEntity):
self.fname = fname
def visit(self, visitor):
+ QAPISchemaEntity.visit(self, visitor)
visitor.visit_include(self.fname, self.info)
@@ -1309,6 +1310,7 @@ class QAPISchemaBuiltinType(QAPISchemaType):
return self.json_type()
def visit(self, visitor):
+ QAPISchemaType.visit(self, visitor)
visitor.visit_builtin_type(self.name, self.info, self.json_type())
@@ -1344,6 +1346,7 @@ class QAPISchemaEnumType(QAPISchemaType):
return 'string'
def visit(self, visitor):
+ QAPISchemaType.visit(self, visitor)
visitor.visit_enum_type(self.name, self.info, self.ifcond,
self.members, self.prefix)
@@ -1386,6 +1389,7 @@ class QAPISchemaArrayType(QAPISchemaType):
return 'array of ' + elt_doc_type
def visit(self, visitor):
+ QAPISchemaType.visit(self, visitor)
visitor.visit_array_type(self.name, self.info, self.ifcond,
self.element_type)
@@ -1461,6 +1465,7 @@ class QAPISchemaObjectType(QAPISchemaType):
# and update seen to track the members seen so far. Report any errors
# on behalf of info, which is not necessarily self.info
def check_clash(self, info, seen):
+ assert self._checked
assert not self.variants # not implemented
for m in self.members:
m.check_clash(info, seen)
@@ -1498,6 +1503,7 @@ class QAPISchemaObjectType(QAPISchemaType):
return 'object'
def visit(self, visitor):
+ QAPISchemaType.visit(self, visitor)
visitor.visit_object_type(self.name, self.info, self.ifcond,
self.base, self.local_members, self.variants,
self.features)
@@ -1665,6 +1671,7 @@ class QAPISchemaAlternateType(QAPISchemaType):
return 'value'
def visit(self, visitor):
+ QAPISchemaType.visit(self, visitor)
visitor.visit_alternate_type(self.name, self.info, self.ifcond,
self.variants)
@@ -1698,6 +1705,7 @@ class QAPISchemaCommand(QAPISchemaEntity):
assert isinstance(self.ret_type, QAPISchemaType)
def visit(self, visitor):
+ QAPISchemaEntity.visit(self, visitor)
visitor.visit_command(self.name, self.info, self.ifcond,
self.arg_type, self.ret_type,
self.gen, self.success_response,
@@ -1723,6 +1731,7 @@ class QAPISchemaEvent(QAPISchemaEntity):
raise QAPISemError(self.info, "Use of 'boxed' requires 'data'")
def visit(self, visitor):
+ QAPISchemaEntity.visit(self, visitor)
visitor.visit_event(self.name, self.info, self.ifcond,
self.arg_type, self.boxed)
--
2.21.0
^ permalink raw reply related [flat|nested] 41+ messages in thread
* Re: [PULL 00/37] QAPI patches for 2019-09-24
2019-09-24 12:32 [PULL 00/37] QAPI patches for 2019-09-24 Markus Armbruster
` (36 preceding siblings ...)
2019-09-24 12:33 ` [PULL 37/37] qapi: Assert .visit() and .check_clash() run only after .check() Markus Armbruster
@ 2019-09-25 1:21 ` no-reply
2019-09-25 15:28 ` no-reply
2019-09-26 9:13 ` Peter Maydell
39 siblings, 0 replies; 41+ messages in thread
From: no-reply @ 2019-09-25 1:21 UTC (permalink / raw)
To: armbru; +Cc: qemu-devel
Patchew URL: https://patchew.org/QEMU/20190924123334.30645-1-armbru@redhat.com/
Hi,
This series seems to have some coding style problems. See output below for
more information:
Type: series
Message-id: 20190924123334.30645-1-armbru@redhat.com
Subject: [PULL 00/37] QAPI patches for 2019-09-24
=== TEST SCRIPT BEGIN ===
#!/bin/bash
git rev-parse base > /dev/null || exit 0
git config --local diff.renamelimit 0
git config --local diff.renames True
git config --local diff.algorithm histogram
./scripts/checkpatch.pl --mailback base..
=== TEST SCRIPT END ===
Switched to a new branch 'test'
94bb71a qapi: Assert .visit() and .check_clash() run only after .check()
e235213 qapi: Fix excessive QAPISchemaEntity.check() recursion
6abca0e qapi: Fix to .check() empty structs just once
474e373 qapi: Delete useless check_exprs() code for simple union kind
e8046dc qapi: Clean up around check_known_keys()
e04348f qapi: Simplify check_keys()
88e075d qapi: Normalize 'if' in check_exprs(), like other sugar
7aa7ae2 qapi: Fix missing 'if' checks in struct, union, alternate 'data'
85793e7 qapi: Reject blank 'if' conditions in addition to empty ones
a0f2b57 qapi: Fix broken discriminator error messages
300d20c qapi: Remove null from schema language
d7b3044 qapi: Improve reporting of lexical errors
ae7d3b9 qapi: Use quotes more consistently in frontend error messages
c390bd2 tests/qapi-schema: Demonstrate suboptimal lexical errors
fc20fdc tests/qapi-schema: Demonstrate insufficient 'if' checking
06b681b tests/qapi-schema: Demonstrate broken discriminator errors
d042260 tests/qapi-schema: Demonstrate misleading optional tag error
897d892 tests/qapi-schema: Delete two redundant tests
110d38a tests/qapi-schema: Cover unknown pragma
d43d15e qapi: Tweak code to match docs/devel/qapi-code-gen.txt
128b472 docs/devel/qapi-code-gen: Improve QAPI schema language doc
0564225 docs/devel/qapi-code-gen: Rewrite introduction to schema
7596e24 docs/devel/qapi-code-gen: Rewrite compatibility considerations
2dfb8e7 docs/devel/qapi-code-gen: Reorder sections for readability
02f7e54 qapi: Adjust frontend errors to say enum value, not member
432d8bd qapi: Permit omitting all flat union branches
0dcc5f7 qapi: Permit alternates with just one branch
d6e3fbe qapi: Permit 'boxed' with empty type
888703f qapi: Drop support for escape sequences other than \\
0446d61 qapi: Restrict strings to printable ASCII
b1b7662 tests/qapi-schema: Demonstrate bad reporting of funny characters
c106539 docs/devel/qapi-code-gen: Minor specification fixes
9d585b9 qapi: Drop support for boxed alternate arguments
93e25ba qapi: Drop check_type()'s redundant parameter @allow_optional
052b3a4 scripts/git.orderfile: Match QAPI schema more precisely
e44ecb0 make check-unit: use after free in test-opts-visitor
5cf38c1 qapi: Make visit_next_list()'s comment less confusing
=== OUTPUT BEGIN ===
1/37 Checking commit 5cf38c18d9b8 (qapi: Make visit_next_list()'s comment less confusing)
2/37 Checking commit e44ecb08c63d (make check-unit: use after free in test-opts-visitor)
3/37 Checking commit 052b3a49a2f3 (scripts/git.orderfile: Match QAPI schema more precisely)
4/37 Checking commit 93e25bab7a23 (qapi: Drop check_type()'s redundant parameter @allow_optional)
5/37 Checking commit 9d585b933636 (qapi: Drop support for boxed alternate arguments)
6/37 Checking commit c1065396940f (docs/devel/qapi-code-gen: Minor specification fixes)
7/37 Checking commit b1b7662708bf (tests/qapi-schema: Demonstrate bad reporting of funny characters)
8/37 Checking commit 0446d6114ed7 (qapi: Restrict strings to printable ASCII)
WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
#110:
new file mode 100644
ERROR: Invalid UTF-8, patch and commit message should be encoded in UTF-8
#127: FILE: tests/qapi-schema/string-code-point-127.json:2:
+{ 'command': '⌦' }
^
ERROR: Invalid UTF-8, patch and commit message should be encoded in UTF-8
#153: FILE: tests/qapi-schema/string-code-point-31.json:2:
+{ 'command': '␟' }
^
total: 2 errors, 1 warnings, 76 lines checked
Patch 8/37 has style problems, please review. If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
9/37 Checking commit 888703f31123 (qapi: Drop support for escape sequences other than \\)
WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
#75:
deleted file mode 100644
total: 0 errors, 1 warnings, 53 lines checked
Patch 9/37 has style problems, please review. If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
10/37 Checking commit d6e3fbe4fa99 (qapi: Permit 'boxed' with empty type)
WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
#131:
deleted file mode 100644
total: 0 errors, 1 warnings, 129 lines checked
Patch 10/37 has style problems, please review. If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
11/37 Checking commit 0dcc5f73a05f (qapi: Permit alternates with just one branch)
12/37 Checking commit 432d8bd595d7 (qapi: Permit omitting all flat union branches)
13/37 Checking commit 02f7e541fd74 (qapi: Adjust frontend errors to say enum value, not member)
14/37 Checking commit 2dfb8e7c344b (docs/devel/qapi-code-gen: Reorder sections for readability)
15/37 Checking commit 7596e24f1bec (docs/devel/qapi-code-gen: Rewrite compatibility considerations)
16/37 Checking commit 05642257c0db (docs/devel/qapi-code-gen: Rewrite introduction to schema)
17/37 Checking commit 128b4726448e (docs/devel/qapi-code-gen: Improve QAPI schema language doc)
18/37 Checking commit d43d15e7d540 (qapi: Tweak code to match docs/devel/qapi-code-gen.txt)
19/37 Checking commit 110d38a4a19f (tests/qapi-schema: Cover unknown pragma)
WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
#25:
new file mode 100644
total: 0 errors, 1 warnings, 10 lines checked
Patch 19/37 has style problems, please review. If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
20/37 Checking commit 897d892dedcf (tests/qapi-schema: Delete two redundant tests)
WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
#41:
deleted file mode 100644
total: 0 errors, 1 warnings, 18 lines checked
Patch 20/37 has style problems, please review. If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
21/37 Checking commit d042260f5405 (tests/qapi-schema: Demonstrate misleading optional tag error)
WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
#41:
new file mode 100644
total: 0 errors, 1 warnings, 34 lines checked
Patch 21/37 has style problems, please review. If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
22/37 Checking commit 06b681bc7ab7 (tests/qapi-schema: Demonstrate broken discriminator errors)
23/37 Checking commit fc20fdc04c5b (tests/qapi-schema: Demonstrate insufficient 'if' checking)
WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
#53:
new file mode 100644
total: 0 errors, 1 warnings, 120 lines checked
Patch 23/37 has style problems, please review. If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
24/37 Checking commit c390bd203f6a (tests/qapi-schema: Demonstrate suboptimal lexical errors)
WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
#38:
new file mode 100644
total: 0 errors, 1 warnings, 14 lines checked
Patch 24/37 has style problems, please review. If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
25/37 Checking commit ae7d3b9c8421 (qapi: Use quotes more consistently in frontend error messages)
WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
#164:
new file mode 100644
total: 0 errors, 1 warnings, 146 lines checked
Patch 25/37 has style problems, please review. If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
26/37 Checking commit d7b3044af92a (qapi: Improve reporting of lexical errors)
27/37 Checking commit 300d20c4ce3c (qapi: Remove null from schema language)
28/37 Checking commit a0f2b570a4b9 (qapi: Fix broken discriminator error messages)
29/37 Checking commit 85793e79ac65 (qapi: Reject blank 'if' conditions in addition to empty ones)
30/37 Checking commit 7aa7ae279141 (qapi: Fix missing 'if' checks in struct, union, alternate 'data')
31/37 Checking commit 88e075d1fccb (qapi: Normalize 'if' in check_exprs(), like other sugar)
32/37 Checking commit e04348f20e28 (qapi: Simplify check_keys())
33/37 Checking commit e8046dc24358 (qapi: Clean up around check_known_keys())
34/37 Checking commit 474e37349da0 (qapi: Delete useless check_exprs() code for simple union kind)
35/37 Checking commit 6abca0e2ed33 (qapi: Fix to .check() empty structs just once)
36/37 Checking commit e235213891c9 (qapi: Fix excessive QAPISchemaEntity.check() recursion)
37/37 Checking commit 94bb71ada7d7 (qapi: Assert .visit() and .check_clash() run only after .check())
=== OUTPUT END ===
Test command exited with code: 1
The full log is available at
http://patchew.org/logs/20190924123334.30645-1-armbru@redhat.com/testing.checkpatch/?type=message.
---
Email generated automatically by Patchew [https://patchew.org/].
Please send your feedback to patchew-devel@redhat.com
^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: [PULL 00/37] QAPI patches for 2019-09-24
2019-09-24 12:32 [PULL 00/37] QAPI patches for 2019-09-24 Markus Armbruster
` (37 preceding siblings ...)
2019-09-25 1:21 ` [PULL 00/37] QAPI patches for 2019-09-24 no-reply
@ 2019-09-25 15:28 ` no-reply
2019-09-26 9:13 ` Peter Maydell
39 siblings, 0 replies; 41+ messages in thread
From: no-reply @ 2019-09-25 15:28 UTC (permalink / raw)
To: armbru; +Cc: qemu-devel
Patchew URL: https://patchew.org/QEMU/20190924123334.30645-1-armbru@redhat.com/
Hi,
This series seems to have some coding style problems. See output below for
more information:
Type: series
Message-id: 20190924123334.30645-1-armbru@redhat.com
Subject: [PULL 00/37] QAPI patches for 2019-09-24
=== TEST SCRIPT BEGIN ===
#!/bin/bash
git rev-parse base > /dev/null || exit 0
git config --local diff.renamelimit 0
git config --local diff.renames True
git config --local diff.algorithm histogram
./scripts/checkpatch.pl --mailback base..
=== TEST SCRIPT END ===
Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
From https://github.com/patchew-project/qemu
- [tag update] patchew/20190918130633.4872-1-shameerali.kolothum.thodi@huawei.com -> patchew/20190918130633.4872-1-shameerali.kolothum.thodi@huawei.com
Auto packing the repository for optimum performance. You may also
run "git gc" manually. See "git help gc" for more information.
Switched to a new branch 'test'
fca7965 qapi: Assert .visit() and .check_clash() run only after .check()
1dee36f qapi: Fix excessive QAPISchemaEntity.check() recursion
67296b6 qapi: Fix to .check() empty structs just once
5df7aa4 qapi: Delete useless check_exprs() code for simple union kind
9bfd443 qapi: Clean up around check_known_keys()
6f36de8 qapi: Simplify check_keys()
f4fdcc8 qapi: Normalize 'if' in check_exprs(), like other sugar
886f5a9 qapi: Fix missing 'if' checks in struct, union, alternate 'data'
1646435 qapi: Reject blank 'if' conditions in addition to empty ones
46fb782 qapi: Fix broken discriminator error messages
66f3e2f qapi: Remove null from schema language
109b4b8 qapi: Improve reporting of lexical errors
780e43a qapi: Use quotes more consistently in frontend error messages
3befc8b tests/qapi-schema: Demonstrate suboptimal lexical errors
93a6e44 tests/qapi-schema: Demonstrate insufficient 'if' checking
1ed8709 tests/qapi-schema: Demonstrate broken discriminator errors
9b5253f tests/qapi-schema: Demonstrate misleading optional tag error
e478cdf tests/qapi-schema: Delete two redundant tests
e440f86 tests/qapi-schema: Cover unknown pragma
41391c5 qapi: Tweak code to match docs/devel/qapi-code-gen.txt
8e35fe1 docs/devel/qapi-code-gen: Improve QAPI schema language doc
886b3d4 docs/devel/qapi-code-gen: Rewrite introduction to schema
d1e96d8 docs/devel/qapi-code-gen: Rewrite compatibility considerations
d5959c2 docs/devel/qapi-code-gen: Reorder sections for readability
915a37a qapi: Adjust frontend errors to say enum value, not member
6538fc9 qapi: Permit omitting all flat union branches
417683b qapi: Permit alternates with just one branch
35d1a1a qapi: Permit 'boxed' with empty type
ad5a002 qapi: Drop support for escape sequences other than \\
37e2210 qapi: Restrict strings to printable ASCII
e95c677 tests/qapi-schema: Demonstrate bad reporting of funny characters
172dafd docs/devel/qapi-code-gen: Minor specification fixes
c33926d qapi: Drop support for boxed alternate arguments
25f22f3 qapi: Drop check_type()'s redundant parameter @allow_optional
6a914ec scripts/git.orderfile: Match QAPI schema more precisely
017b60e make check-unit: use after free in test-opts-visitor
a9711ba qapi: Make visit_next_list()'s comment less confusing
=== OUTPUT BEGIN ===
1/37 Checking commit a9711ba87218 (qapi: Make visit_next_list()'s comment less confusing)
2/37 Checking commit 017b60eb5cb9 (make check-unit: use after free in test-opts-visitor)
3/37 Checking commit 6a914ec905eb (scripts/git.orderfile: Match QAPI schema more precisely)
4/37 Checking commit 25f22f3a0fa1 (qapi: Drop check_type()'s redundant parameter @allow_optional)
5/37 Checking commit c33926d14c78 (qapi: Drop support for boxed alternate arguments)
6/37 Checking commit 172dafd2d40d (docs/devel/qapi-code-gen: Minor specification fixes)
7/37 Checking commit e95c677acdb7 (tests/qapi-schema: Demonstrate bad reporting of funny characters)
8/37 Checking commit 37e22103c527 (qapi: Restrict strings to printable ASCII)
WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
#110:
new file mode 100644
ERROR: Invalid UTF-8, patch and commit message should be encoded in UTF-8
#127: FILE: tests/qapi-schema/string-code-point-127.json:2:
+{ 'command': '⌦' }
^
ERROR: Invalid UTF-8, patch and commit message should be encoded in UTF-8
#153: FILE: tests/qapi-schema/string-code-point-31.json:2:
+{ 'command': '␟' }
^
total: 2 errors, 1 warnings, 76 lines checked
Patch 8/37 has style problems, please review. If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
9/37 Checking commit ad5a0020e820 (qapi: Drop support for escape sequences other than \\)
WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
#75:
deleted file mode 100644
total: 0 errors, 1 warnings, 53 lines checked
Patch 9/37 has style problems, please review. If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
10/37 Checking commit 35d1a1a20963 (qapi: Permit 'boxed' with empty type)
WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
#131:
deleted file mode 100644
total: 0 errors, 1 warnings, 129 lines checked
Patch 10/37 has style problems, please review. If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
11/37 Checking commit 417683ba43fa (qapi: Permit alternates with just one branch)
12/37 Checking commit 6538fc9fa42b (qapi: Permit omitting all flat union branches)
13/37 Checking commit 915a37a4eecd (qapi: Adjust frontend errors to say enum value, not member)
14/37 Checking commit d5959c29193e (docs/devel/qapi-code-gen: Reorder sections for readability)
15/37 Checking commit d1e96d83a97f (docs/devel/qapi-code-gen: Rewrite compatibility considerations)
16/37 Checking commit 886b3d4a9b69 (docs/devel/qapi-code-gen: Rewrite introduction to schema)
17/37 Checking commit 8e35fe16b761 (docs/devel/qapi-code-gen: Improve QAPI schema language doc)
18/37 Checking commit 41391c527ffc (qapi: Tweak code to match docs/devel/qapi-code-gen.txt)
19/37 Checking commit e440f8624eba (tests/qapi-schema: Cover unknown pragma)
WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
#25:
new file mode 100644
total: 0 errors, 1 warnings, 10 lines checked
Patch 19/37 has style problems, please review. If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
20/37 Checking commit e478cdf5f30f (tests/qapi-schema: Delete two redundant tests)
WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
#41:
deleted file mode 100644
total: 0 errors, 1 warnings, 18 lines checked
Patch 20/37 has style problems, please review. If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
21/37 Checking commit 9b5253f1cce0 (tests/qapi-schema: Demonstrate misleading optional tag error)
WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
#41:
new file mode 100644
total: 0 errors, 1 warnings, 34 lines checked
Patch 21/37 has style problems, please review. If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
22/37 Checking commit 1ed8709cff6f (tests/qapi-schema: Demonstrate broken discriminator errors)
23/37 Checking commit 93a6e4434ee7 (tests/qapi-schema: Demonstrate insufficient 'if' checking)
WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
#53:
new file mode 100644
total: 0 errors, 1 warnings, 120 lines checked
Patch 23/37 has style problems, please review. If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
24/37 Checking commit 3befc8b02156 (tests/qapi-schema: Demonstrate suboptimal lexical errors)
WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
#38:
new file mode 100644
total: 0 errors, 1 warnings, 14 lines checked
Patch 24/37 has style problems, please review. If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
25/37 Checking commit 780e43ac4ae8 (qapi: Use quotes more consistently in frontend error messages)
WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
#164:
new file mode 100644
total: 0 errors, 1 warnings, 146 lines checked
Patch 25/37 has style problems, please review. If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
26/37 Checking commit 109b4b89f1f3 (qapi: Improve reporting of lexical errors)
27/37 Checking commit 66f3e2fdfea8 (qapi: Remove null from schema language)
28/37 Checking commit 46fb7827e670 (qapi: Fix broken discriminator error messages)
29/37 Checking commit 16464352bdb8 (qapi: Reject blank 'if' conditions in addition to empty ones)
30/37 Checking commit 886f5a9e9a15 (qapi: Fix missing 'if' checks in struct, union, alternate 'data')
31/37 Checking commit f4fdcc853a48 (qapi: Normalize 'if' in check_exprs(), like other sugar)
32/37 Checking commit 6f36de8c07ee (qapi: Simplify check_keys())
33/37 Checking commit 9bfd443bcb0a (qapi: Clean up around check_known_keys())
34/37 Checking commit 5df7aa47a9ba (qapi: Delete useless check_exprs() code for simple union kind)
35/37 Checking commit 67296b6e3775 (qapi: Fix to .check() empty structs just once)
36/37 Checking commit 1dee36ff6bf8 (qapi: Fix excessive QAPISchemaEntity.check() recursion)
37/37 Checking commit fca7965dc1e6 (qapi: Assert .visit() and .check_clash() run only after .check())
=== OUTPUT END ===
Test command exited with code: 1
The full log is available at
http://patchew.org/logs/20190924123334.30645-1-armbru@redhat.com/testing.checkpatch/?type=message.
---
Email generated automatically by Patchew [https://patchew.org/].
Please send your feedback to patchew-devel@redhat.com
^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: [PULL 00/37] QAPI patches for 2019-09-24
2019-09-24 12:32 [PULL 00/37] QAPI patches for 2019-09-24 Markus Armbruster
` (38 preceding siblings ...)
2019-09-25 15:28 ` no-reply
@ 2019-09-26 9:13 ` Peter Maydell
39 siblings, 0 replies; 41+ messages in thread
From: Peter Maydell @ 2019-09-26 9:13 UTC (permalink / raw)
To: Markus Armbruster; +Cc: QEMU Developers
On Tue, 24 Sep 2019 at 13:42, Markus Armbruster <armbru@redhat.com> wrote:
>
> The following changes since commit 2f93a3ecdd3bb060bd04f698ccafe66efd98563a:
>
> Merge remote-tracking branch 'remotes/davidhildenbrand/tags/s390x-tcg-2019-09-23' into staging (2019-09-23 15:44:52 +0100)
>
> are available in the Git repository at:
>
> git://repo.or.cz/qemu/armbru.git tags/pull-qapi-2019-09-24
>
> for you to fetch changes up to 56176412e7fcfae1c69e4426d94e856b75358231:
>
> qapi: Assert .visit() and .check_clash() run only after .check() (2019-09-24 14:07:23 +0200)
>
> ----------------------------------------------------------------
> QAPI patches for 2019-09-24
>
Applied, thanks.
Please update the changelog at https://wiki.qemu.org/ChangeLog/4.2
for any user-visible changes.
-- PMM
^ permalink raw reply [flat|nested] 41+ messages in thread
end of thread, other threads:[~2019-09-26 9:15 UTC | newest]
Thread overview: 41+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-09-24 12:32 [PULL 00/37] QAPI patches for 2019-09-24 Markus Armbruster
2019-09-24 12:32 ` [PULL 01/37] qapi: Make visit_next_list()'s comment less confusing Markus Armbruster
2019-09-24 12:32 ` [PULL 02/37] make check-unit: use after free in test-opts-visitor Markus Armbruster
2019-09-24 12:33 ` [PULL 03/37] scripts/git.orderfile: Match QAPI schema more precisely Markus Armbruster
2019-09-24 12:33 ` [PULL 04/37] qapi: Drop check_type()'s redundant parameter @allow_optional Markus Armbruster
2019-09-24 12:33 ` [PULL 05/37] qapi: Drop support for boxed alternate arguments Markus Armbruster
2019-09-24 12:33 ` [PULL 06/37] docs/devel/qapi-code-gen: Minor specification fixes Markus Armbruster
2019-09-24 12:33 ` [PULL 07/37] tests/qapi-schema: Demonstrate bad reporting of funny characters Markus Armbruster
2019-09-24 12:33 ` [PULL 08/37] qapi: Restrict strings to printable ASCII Markus Armbruster
2019-09-24 12:33 ` [PULL 09/37] qapi: Drop support for escape sequences other than \\ Markus Armbruster
2019-09-24 12:33 ` [PULL 10/37] qapi: Permit 'boxed' with empty type Markus Armbruster
2019-09-24 12:33 ` [PULL 11/37] qapi: Permit alternates with just one branch Markus Armbruster
2019-09-24 12:33 ` [PULL 12/37] qapi: Permit omitting all flat union branches Markus Armbruster
2019-09-24 12:33 ` [PULL 13/37] qapi: Adjust frontend errors to say enum value, not member Markus Armbruster
2019-09-24 12:33 ` [PULL 14/37] docs/devel/qapi-code-gen: Reorder sections for readability Markus Armbruster
2019-09-24 12:33 ` [PULL 15/37] docs/devel/qapi-code-gen: Rewrite compatibility considerations Markus Armbruster
2019-09-24 12:33 ` [PULL 16/37] docs/devel/qapi-code-gen: Rewrite introduction to schema Markus Armbruster
2019-09-24 12:33 ` [PULL 17/37] docs/devel/qapi-code-gen: Improve QAPI schema language doc Markus Armbruster
2019-09-24 12:33 ` [PULL 18/37] qapi: Tweak code to match docs/devel/qapi-code-gen.txt Markus Armbruster
2019-09-24 12:33 ` [PULL 19/37] tests/qapi-schema: Cover unknown pragma Markus Armbruster
2019-09-24 12:33 ` [PULL 20/37] tests/qapi-schema: Delete two redundant tests Markus Armbruster
2019-09-24 12:33 ` [PULL 21/37] tests/qapi-schema: Demonstrate misleading optional tag error Markus Armbruster
2019-09-24 12:33 ` [PULL 22/37] tests/qapi-schema: Demonstrate broken discriminator errors Markus Armbruster
2019-09-24 12:33 ` [PULL 23/37] tests/qapi-schema: Demonstrate insufficient 'if' checking Markus Armbruster
2019-09-24 12:33 ` [PULL 24/37] tests/qapi-schema: Demonstrate suboptimal lexical errors Markus Armbruster
2019-09-24 12:33 ` [PULL 25/37] qapi: Use quotes more consistently in frontend error messages Markus Armbruster
2019-09-24 12:33 ` [PULL 26/37] qapi: Improve reporting of lexical errors Markus Armbruster
2019-09-24 12:33 ` [PULL 27/37] qapi: Remove null from schema language Markus Armbruster
2019-09-24 12:33 ` [PULL 28/37] qapi: Fix broken discriminator error messages Markus Armbruster
2019-09-24 12:33 ` [PULL 29/37] qapi: Reject blank 'if' conditions in addition to empty ones Markus Armbruster
2019-09-24 12:33 ` [PULL 30/37] qapi: Fix missing 'if' checks in struct, union, alternate 'data' Markus Armbruster
2019-09-24 12:33 ` [PULL 31/37] qapi: Normalize 'if' in check_exprs(), like other sugar Markus Armbruster
2019-09-24 12:33 ` [PULL 32/37] qapi: Simplify check_keys() Markus Armbruster
2019-09-24 12:33 ` [PULL 33/37] qapi: Clean up around check_known_keys() Markus Armbruster
2019-09-24 12:33 ` [PULL 34/37] qapi: Delete useless check_exprs() code for simple union kind Markus Armbruster
2019-09-24 12:33 ` [PULL 35/37] qapi: Fix to .check() empty structs just once Markus Armbruster
2019-09-24 12:33 ` [PULL 36/37] qapi: Fix excessive QAPISchemaEntity.check() recursion Markus Armbruster
2019-09-24 12:33 ` [PULL 37/37] qapi: Assert .visit() and .check_clash() run only after .check() Markus Armbruster
2019-09-25 1:21 ` [PULL 00/37] QAPI patches for 2019-09-24 no-reply
2019-09-25 15:28 ` no-reply
2019-09-26 9:13 ` Peter Maydell
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).