From dcca907bed4707f8fa8bbfdd9eef741fdaad29f8 Mon Sep 17 00:00:00 2001 From: Markus Armbruster Date: Fri, 13 Sep 2019 22:13:35 +0200 Subject: qapi: Drop check_type()'s redundant parameter @allow_optional 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 Reviewed-by: Eric Blake Message-Id: <20190913201349.24332-3-armbru@redhat.com> --- scripts/qapi/common.py | 17 ++++++++--------- 1 file changed, 8 insertions(+), 9 deletions(-) (limited to 'scripts/qapi/common.py') 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']) -- cgit 1.4.1 From b22e86585b296b254209cdc7011fcc74dd08717d Mon Sep 17 00:00:00 2001 From: Markus Armbruster Date: Fri, 13 Sep 2019 22:13:36 +0200 Subject: qapi: Drop support for boxed alternate arguments 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 Reviewed-by: Eric Blake 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(-) (limited to 'scripts/qapi/common.py') 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 -- cgit 1.4.1 From 56a8caff922df8d597895a49f55f2150bff3adb7 Mon Sep 17 00:00:00 2001 From: Markus Armbruster Date: Fri, 13 Sep 2019 22:13:39 +0200 Subject: qapi: Restrict strings to printable ASCII 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 Reviewed-by: Eric Blake 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 + tests/qapi-schema/string-code-point-127.exit | 1 + tests/qapi-schema/string-code-point-127.json | 2 ++ tests/qapi-schema/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.exit | 1 - tests/qapi-schema/unicode-str.json | 2 -- tests/qapi-schema/unicode-str.out | 0 14 files changed, 21 insertions(+), 22 deletions(-) create mode 100644 tests/qapi-schema/string-code-point-127.err create mode 100644 tests/qapi-schema/string-code-point-127.exit create mode 100644 tests/qapi-schema/string-code-point-127.json create mode 100644 tests/qapi-schema/string-code-point-127.out 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.exit delete mode 100644 tests/qapi-schema/unicode-str.json delete mode 100644 tests/qapi-schema/unicode-str.out (limited to 'scripts/qapi/common.py') 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/string-code-point-127.exit b/tests/qapi-schema/string-code-point-127.exit new file mode 100644 index 0000000000..d00491fd7e --- /dev/null +++ b/tests/qapi-schema/string-code-point-127.exit @@ -0,0 +1 @@ +1 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': '' } diff --git a/tests/qapi-schema/string-code-point-127.out b/tests/qapi-schema/string-code-point-127.out new file mode 100644 index 0000000000..e69de29bb2 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': '' } 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.exit b/tests/qapi-schema/unicode-str.exit deleted file mode 100644 index d00491fd7e..0000000000 --- a/tests/qapi-schema/unicode-str.exit +++ /dev/null @@ -1 +0,0 @@ -1 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': 'é' } diff --git a/tests/qapi-schema/unicode-str.out b/tests/qapi-schema/unicode-str.out deleted file mode 100644 index e69de29bb2..0000000000 -- cgit 1.4.1 From 9b4416bfc1ea5fb3398e8f78a90caa88dd301c37 Mon Sep 17 00:00:00 2001 From: Markus Armbruster Date: Fri, 13 Sep 2019 22:13:40 +0200 Subject: qapi: Drop support for escape sequences other than \\ 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 Reviewed-by: Eric Blake 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 (limited to 'scripts/qapi/common.py') 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':{} } -- cgit 1.4.1 From 675b214bc6ba2c1d8ac499e339a8cb99c7f23c7c Mon Sep 17 00:00:00 2001 From: Markus Armbruster Date: Fri, 13 Sep 2019 22:13:41 +0200 Subject: qapi: Permit 'boxed' with empty type 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 Reviewed-by: Eric Blake Message-Id: <20190913201349.24332-9-armbru@redhat.com> --- 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 ++++ tests/test-qmp-cmds.c | 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 (limited to 'scripts/qapi/common.py') 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 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, -- cgit 1.4.1 From f03255362ae3bfd6f105c0fc855c713944f99717 Mon Sep 17 00:00:00 2001 From: Markus Armbruster Date: Fri, 13 Sep 2019 22:13:42 +0200 Subject: qapi: Permit alternates with just one branch 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 Reviewed-by: Eric Blake 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(-) (limited to 'scripts/qapi/common.py') 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 -- cgit 1.4.1 From 0ced9531f17c1c28fa4f29b352729c7f40c2ae30 Mon Sep 17 00:00:00 2001 From: Markus Armbruster Date: Fri, 13 Sep 2019 22:13:43 +0200 Subject: qapi: Permit omitting all flat union branches 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 Reviewed-by: Eric Blake 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(-) (limited to 'scripts/qapi/common.py') 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': { } } -- cgit 1.4.1 From 398969fe1c6f696e19d0f317dd055c6b17e2cee6 Mon Sep 17 00:00:00 2001 From: Markus Armbruster Date: Fri, 13 Sep 2019 22:13:44 +0200 Subject: qapi: Adjust frontend errors to say enum value, not member For consistency with docs/devel/qapi-code-gen.txt. Signed-off-by: Markus Armbruster Reviewed-by: Eric Blake 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(-) (limited to 'scripts/qapi/common.py') 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 -- cgit 1.4.1 From 8d40738d2f60a70aaece8d990a8b95af04e085a2 Mon Sep 17 00:00:00 2001 From: Markus Armbruster Date: Fri, 13 Sep 2019 22:13:49 +0200 Subject: qapi: Tweak code to match docs/devel/qapi-code-gen.txt 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 Reviewed-by: Eric Blake 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 +- tests/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(-) (limited to 'scripts/qapi/common.py') 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 -- cgit 1.4.1 From 9f5e6b088a2d0b2b51e1cdf7c86f23f22d9ad493 Mon Sep 17 00:00:00 2001 From: Markus Armbruster Date: Sat, 14 Sep 2019 17:34:54 +0200 Subject: qapi: Use quotes more consistently in frontend error messages Consistently enclose error messages in double quotes. Use single quotes within, except for one case of "'". Signed-off-by: Markus Armbruster Message-Id: <20190914153506.2151-8-armbru@redhat.com> Reviewed-by: Eric Blake --- 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 (limited to 'scripts/qapi/common.py') 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 '}' -- cgit 1.4.1 From 14c32795024c815316337b019bdf88d76b429af8 Mon Sep 17 00:00:00 2001 From: Markus Armbruster Date: Sat, 14 Sep 2019 17:34:55 +0200 Subject: qapi: Improve reporting of lexical errors 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 Message-Id: <20190914153506.2151-9-armbru@redhat.com> Reviewed-by: Eric Blake --- 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(-) (limited to 'scripts/qapi/common.py') 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' -- cgit 1.4.1 From 9d55380b5aecd4ae5324e7d4ab0a7dfc510b634d Mon Sep 17 00:00:00 2001 From: Markus Armbruster Date: Sat, 14 Sep 2019 17:34:56 +0200 Subject: qapi: Remove null from schema language 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 Message-Id: <20190914153506.2151-10-armbru@redhat.com> Reviewed-by: Eric Blake --- 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 -------------- tests/qapi-schema/pragma-name-case-whitelist-crap.json | 2 +- 7 files changed, 6 insertions(+), 24 deletions(-) (limited to 'scripts/qapi/common.py') 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 } } -- cgit 1.4.1 From 887a2069f76fa99b9755467126dd171a9bad34a3 Mon Sep 17 00:00:00 2001 From: Markus Armbruster Date: Sat, 14 Sep 2019 17:34:57 +0200 Subject: qapi: Fix broken discriminator error messages 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 Message-Id: <20190914153506.2151-11-armbru@redhat.com> Reviewed-by: Eric Blake --- scripts/qapi/common.py | 9 ++++----- tests/qapi-schema/flat-union-invalid-discriminator.err | 2 +- tests/qapi-schema/flat-union-invalid-discriminator.json | 1 - tests/qapi-schema/flat-union-invalid-if-discriminator.err | 2 +- tests/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(-) (limited to 'scripts/qapi/common.py') 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' -- cgit 1.4.1 From c2c7065e1752a3be1b437d1ea359cde35d28ee3b Mon Sep 17 00:00:00 2001 From: Markus Armbruster Date: Sat, 14 Sep 2019 17:34:58 +0200 Subject: qapi: Reject blank 'if' conditions in addition to empty ones "'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 Message-Id: <20190914153506.2151-12-armbru@redhat.com> Reviewed-by: Eric Blake --- 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(-) (limited to 'scripts/qapi/common.py') 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', ' '] } -- cgit 1.4.1 From dec0012ef8d644b5dde1b68ee8dab3f8c12e0b6b Mon Sep 17 00:00:00 2001 From: Markus Armbruster Date: Sat, 14 Sep 2019 17:34:59 +0200 Subject: qapi: Fix missing 'if' checks in struct, union, alternate 'data' Commit 87adbbffd4..3e270dcacc "qapi: Add 'if' to (implicit struct|union|alternate) members" (v4.0.0) neglected test coverage, and promptly failed to check the conditions. Review fail. Recent commit "tests/qapi-schema: Demonstrate insufficient 'if' checking" added test coverage, demonstrating the bug. Fix it by add the missing check_if(). Signed-off-by: Markus Armbruster Message-Id: <20190914153506.2151-13-armbru@redhat.com> Reviewed-by: Eric Blake --- scripts/qapi/common.py | 3 +++ tests/qapi-schema/alternate-branch-if-invalid.err | 1 + tests/qapi-schema/alternate-branch-if-invalid.exit | 2 +- tests/qapi-schema/alternate-branch-if-invalid.json | 1 - tests/qapi-schema/alternate-branch-if-invalid.out | 16 --------------- tests/qapi-schema/struct-member-if-invalid.err | 1 + tests/qapi-schema/struct-member-if-invalid.exit | 2 +- tests/qapi-schema/struct-member-if-invalid.json | 1 - tests/qapi-schema/struct-member-if-invalid.out | 15 -------------- tests/qapi-schema/union-branch-if-invalid.err | 1 + tests/qapi-schema/union-branch-if-invalid.exit | 2 +- tests/qapi-schema/union-branch-if-invalid.json | 1 - tests/qapi-schema/union-branch-if-invalid.out | 23 ---------------------- 13 files changed, 9 insertions(+), 60 deletions(-) (limited to 'scripts/qapi/common.py') 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 [''] -- cgit 1.4.1 From fe9c4dcf9021f4111f37373bb71ef9e08863c349 Mon Sep 17 00:00:00 2001 From: Markus Armbruster Date: Sat, 14 Sep 2019 17:35:00 +0200 Subject: qapi: Normalize 'if' in check_exprs(), like other sugar 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 Message-Id: <20190914153506.2151-14-armbru@redhat.com> Reviewed-by: Eric Blake --- scripts/qapi/common.py | 26 +++++++++++++++----------- 1 file changed, 15 insertions(+), 11 deletions(-) (limited to 'scripts/qapi/common.py') 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): -- cgit 1.4.1 From dc234189f8d1b129785a5933c15a4b2fb89d7e1b Mon Sep 17 00:00:00 2001 From: Markus Armbruster Date: Sat, 14 Sep 2019 17:35:01 +0200 Subject: qapi: Simplify check_keys() 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 Message-Id: <20190914153506.2151-15-armbru@redhat.com> Reviewed-by: Eric Blake --- scripts/qapi/common.py | 19 ++++++++----------- 1 file changed, 8 insertions(+), 11 deletions(-) (limited to 'scripts/qapi/common.py') 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) -- cgit 1.4.1 From 6955397677859ada5a65f430edee8035fb4e4b7c Mon Sep 17 00:00:00 2001 From: Markus Armbruster Date: Sat, 14 Sep 2019 17:35:02 +0200 Subject: qapi: Clean up around check_known_keys() 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 Message-Id: <20190914153506.2151-16-armbru@redhat.com> Reviewed-by: Eric Blake --- scripts/qapi/common.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) (limited to 'scripts/qapi/common.py') 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, -- cgit 1.4.1 From e31fe1266c7daa49bd086c45efeb5af06309c0ef Mon Sep 17 00:00:00 2001 From: Markus Armbruster Date: Sat, 14 Sep 2019 17:35:03 +0200 Subject: qapi: Delete useless check_exprs() code for simple union kind 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 Message-Id: <20190914153506.2151-17-armbru@redhat.com> Reviewed-by: Eric Blake --- scripts/qapi/common.py | 39 ++------------------------------------- 1 file changed, 2 insertions(+), 37 deletions(-) (limited to 'scripts/qapi/common.py') 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'] -- cgit 1.4.1 From b1bc31f4b7db07ca7b76de604bef4088be0c18b7 Mon Sep 17 00:00:00 2001 From: Markus Armbruster Date: Sat, 14 Sep 2019 17:35:04 +0200 Subject: qapi: Fix to .check() empty structs just once 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 Message-Id: <20190914153506.2151-18-armbru@redhat.com> Reviewed-by: Eric Blake --- scripts/qapi/common.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'scripts/qapi/common.py') 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() -- cgit 1.4.1 From f9d1743b9b07a27d4f17b3316b5e491e366ddc35 Mon Sep 17 00:00:00 2001 From: Markus Armbruster Date: Sat, 14 Sep 2019 17:35:05 +0200 Subject: qapi: Fix excessive QAPISchemaEntity.check() recursion 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 Message-Id: <20190914153506.2151-19-armbru@redhat.com> Reviewed-by: Eric Blake [Commit message tidied up] --- scripts/qapi/common.py | 74 +++++++++++++++++++++++++++++++++++--------------- 1 file changed, 52 insertions(+), 22 deletions(-) (limited to 'scripts/qapi/common.py') 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'") -- cgit 1.4.1 From 56176412e7fcfae1c69e4426d94e856b75358231 Mon Sep 17 00:00:00 2001 From: Markus Armbruster Date: Sat, 14 Sep 2019 17:35:06 +0200 Subject: qapi: Assert .visit() and .check_clash() run only after .check() Easy since the previous commit provides .checked. Signed-off-by: Markus Armbruster Message-Id: <20190914153506.2151-20-armbru@redhat.com> Reviewed-by: Eric Blake --- scripts/qapi/common.py | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) (limited to 'scripts/qapi/common.py') 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) -- cgit 1.4.1