The following changes since commit 9950da284fa5e2ea9ab57d87e05b693fb60c79ce: Merge remote-tracking branch 'remotes/alistair/tags/pull-riscv-to-apply-20210322-2' into staging (2021-03-23 15:30:46 +0000) are available in the Git repository at: git://repo.or.cz/qemu/armbru.git tags/pull-qapi-2021-03-23 for you to fetch changes up to bdabafc6836edc0f34732cac473899cb4e77a296: block: Remove monitor command block_passwd (2021-03-23 22:31:56 +0100) ---------------------------------------------------------------- QAPI patches patches for 2021-03-23 ---------------------------------------------------------------- Markus Armbruster (29): qapi/pragma: Tidy up after removal of deprecated commands tests/qapi-schema: Drop redundant flat-union-inline test tests/qapi-schema: Rework comments on longhand member definitions tests/qapi-schema: Belatedly update comment on alternate clash tests/qapi-schema: Drop TODO comment on simple unions tests/qapi-schema: Tweak to demonstrate buggy member name check qapi: Fix to reject optional members with reserved names qapi: Permit flat union members for any tag value qapi: Lift enum-specific code out of check_name_str() qapi: Rework name checking in preparation of stricter checking qapi: Move uppercase rejection to check_name_lower() qapi: Consistently permit any case in downstream prefixes qapi: Enforce event naming rules qapi: Enforce type naming rules tests/qapi-schema: Rename redefined-builtin to redefined-predefined qapi: Factor out QAPISchemaParser._check_pragma_list_of_str() tests/qapi-schema: Rename pragma-*-crap to pragma-value-not-* tests/qapi-schema: Rename returns-whitelist to returns-bad-type qapi: Rename pragma *-whitelist to *-exceptions qapi/pragma: Streamline comments on member-name-exceptions tests-qmp-cmds: Drop unused and incorrect qmp_TestIfCmd() qapi: Prepare for rejecting underscore in command and member names qapi: Enforce feature naming rules qapi: Enforce command naming rules tests/qapi-schema: Switch member name clash test to struct qapi: Enforce struct member naming rules qapi: Enforce enum member naming rules qapi: Enforce union and alternate branch naming rules block: Remove monitor command block_passwd docs/devel/qapi-code-gen.txt | 28 ++++--- qapi/block-core.json | 14 ---- qapi/pragma.json | 64 ++++++++++++--- qga/qapi-schema.json | 6 +- block/monitor/block-hmp-cmds.c | 10 --- blockdev.c | 8 -- tests/unit/test-qmp-cmds.c | 17 ++-- tests/unit/test-qmp-event.c | 6 +- hmp-commands.hx | 15 ---- scripts/qapi/common.py | 8 +- scripts/qapi/expr.py | 94 ++++++++++++++-------- scripts/qapi/parser.py | 30 +++---- scripts/qapi/schema.py | 2 +- scripts/qapi/source.py | 10 ++- tests/qapi-schema/alternate-clash.err | 2 +- tests/qapi-schema/alternate-clash.json | 9 +-- tests/qapi-schema/args-member-case.err | 2 +- tests/qapi-schema/args-name-clash.err | 2 - tests/qapi-schema/args-name-clash.json | 4 - tests/qapi-schema/doc-bad-union-member.json | 4 +- tests/qapi-schema/doc-good.json | 4 +- tests/qapi-schema/doc-good.out | 4 +- tests/qapi-schema/doc-good.txt | 2 +- tests/qapi-schema/doc-invalid-return.json | 4 +- tests/qapi-schema/double-type.err | 2 +- tests/qapi-schema/double-type.json | 2 +- tests/qapi-schema/enum-clash-member.err | 2 +- tests/qapi-schema/enum-clash-member.json | 1 + tests/qapi-schema/enum-member-case.err | 2 +- tests/qapi-schema/enum-member-case.json | 2 +- tests/qapi-schema/event-case.err | 2 + tests/qapi-schema/event-case.json | 2 - tests/qapi-schema/event-case.out | 14 ---- tests/qapi-schema/event-member-invalid-dict.err | 2 +- tests/qapi-schema/event-member-invalid-dict.json | 2 + tests/qapi-schema/features-deprecated-type.err | 2 +- tests/qapi-schema/features-deprecated-type.json | 2 +- .../flat-union-inline-invalid-dict.json | 4 +- tests/qapi-schema/flat-union-inline.err | 2 - tests/qapi-schema/flat-union-inline.json | 11 --- tests/qapi-schema/flat-union-no-base.err | 2 +- tests/qapi-schema/flat-union-no-base.json | 1 - tests/qapi-schema/meson.build | 14 ++-- .../nested-struct-data-invalid-dict.err | 2 +- .../nested-struct-data-invalid-dict.json | 3 +- tests/qapi-schema/nested-struct-data.json | 2 +- tests/qapi-schema/pragma-doc-required-crap.err | 1 - .../pragma-name-case-whitelist-crap.err | 1 - .../pragma-name-case-whitelist-crap.json | 3 - .../qapi-schema/pragma-returns-whitelist-crap.err | 1 - .../qapi-schema/pragma-returns-whitelist-crap.json | 3 - tests/qapi-schema/pragma-value-not-bool.err | 1 + ...quired-crap.json => pragma-value-not-bool.json} | 2 +- ...gs-name-clash.out => pragma-value-not-bool.out} | 0 tests/qapi-schema/pragma-value-not-list-of-str.err | 1 + .../qapi-schema/pragma-value-not-list-of-str.json | 3 + ...inline.out => pragma-value-not-list-of-str.out} | 0 tests/qapi-schema/pragma-value-not-list.err | 1 + tests/qapi-schema/pragma-value-not-list.json | 2 + ...required-crap.out => pragma-value-not-list.out} | 0 tests/qapi-schema/qapi-schema-test.json | 35 ++++---- tests/qapi-schema/qapi-schema-test.out | 42 +++++----- tests/qapi-schema/redefined-builtin.err | 2 - tests/qapi-schema/redefined-builtin.json | 2 - tests/qapi-schema/redefined-predefined.err | 2 + tests/qapi-schema/redefined-predefined.json | 2 + ...whitelist-crap.out => redefined-predefined.out} | 0 tests/qapi-schema/redefined-type.err | 6 +- tests/qapi-schema/redefined-type.json | 4 +- tests/qapi-schema/reserved-member-u.err | 2 +- tests/qapi-schema/reserved-member-u.json | 2 +- tests/qapi-schema/returns-bad-type.err | 2 + ...eturns-whitelist.json => returns-bad-type.json} | 2 +- ...rns-whitelist-crap.out => returns-bad-type.out} | 0 tests/qapi-schema/returns-whitelist.err | 2 - tests/qapi-schema/struct-data-invalid.err | 2 +- tests/qapi-schema/struct-data-invalid.json | 2 +- tests/qapi-schema/struct-member-invalid-dict.err | 4 +- tests/qapi-schema/struct-member-invalid-dict.json | 5 +- tests/qapi-schema/struct-member-invalid.err | 2 +- tests/qapi-schema/struct-member-invalid.json | 2 +- tests/qapi-schema/struct-member-name-clash.err | 2 + tests/qapi-schema/struct-member-name-clash.json | 5 ++ ...ed-builtin.out => struct-member-name-clash.out} | 0 tests/qapi-schema/type-case.err | 2 + tests/qapi-schema/type-case.json | 2 + .../{returns-whitelist.out => type-case.out} | 0 tests/qapi-schema/union-branch-case.err | 2 +- tests/qapi-schema/union-clash-branches.err | 2 +- tests/qapi-schema/union-clash-branches.json | 6 +- tests/qapi-schema/unknown-expr-key.err | 2 +- tests/qapi-schema/unknown-expr-key.json | 2 +- 92 files changed, 305 insertions(+), 288 deletions(-) delete mode 100644 tests/qapi-schema/args-name-clash.err delete mode 100644 tests/qapi-schema/args-name-clash.json delete mode 100644 tests/qapi-schema/flat-union-inline.err delete mode 100644 tests/qapi-schema/flat-union-inline.json delete mode 100644 tests/qapi-schema/pragma-doc-required-crap.err delete mode 100644 tests/qapi-schema/pragma-name-case-whitelist-crap.err delete mode 100644 tests/qapi-schema/pragma-name-case-whitelist-crap.json delete mode 100644 tests/qapi-schema/pragma-returns-whitelist-crap.err delete mode 100644 tests/qapi-schema/pragma-returns-whitelist-crap.json create mode 100644 tests/qapi-schema/pragma-value-not-bool.err rename tests/qapi-schema/{pragma-doc-required-crap.json => pragma-value-not-bool.json} (55%) rename tests/qapi-schema/{args-name-clash.out => pragma-value-not-bool.out} (100%) create mode 100644 tests/qapi-schema/pragma-value-not-list-of-str.err create mode 100644 tests/qapi-schema/pragma-value-not-list-of-str.json rename tests/qapi-schema/{flat-union-inline.out => pragma-value-not-list-of-str.out} (100%) create mode 100644 tests/qapi-schema/pragma-value-not-list.err create mode 100644 tests/qapi-schema/pragma-value-not-list.json rename tests/qapi-schema/{pragma-doc-required-crap.out => pragma-value-not-list.out} (100%) delete mode 100644 tests/qapi-schema/redefined-builtin.err delete mode 100644 tests/qapi-schema/redefined-builtin.json create mode 100644 tests/qapi-schema/redefined-predefined.err create mode 100644 tests/qapi-schema/redefined-predefined.json rename tests/qapi-schema/{pragma-name-case-whitelist-crap.out => redefined-predefined.out} (100%) create mode 100644 tests/qapi-schema/returns-bad-type.err rename tests/qapi-schema/{returns-whitelist.json => returns-bad-type.json} (91%) rename tests/qapi-schema/{pragma-returns-whitelist-crap.out => returns-bad-type.out} (100%) delete mode 100644 tests/qapi-schema/returns-whitelist.err create mode 100644 tests/qapi-schema/struct-member-name-clash.err create mode 100644 tests/qapi-schema/struct-member-name-clash.json rename tests/qapi-schema/{redefined-builtin.out => struct-member-name-clash.out} (100%) create mode 100644 tests/qapi-schema/type-case.err create mode 100644 tests/qapi-schema/type-case.json rename tests/qapi-schema/{returns-whitelist.out => type-case.out} (100%) -- 2.26.3
Commit cbde7be900 "migrate: remove QMP/HMP commands for speed, downtime and cache size" neglected to remove query-migrate-cache-size from pragma returns-whitelist. Commit 8af54b9172 "machine: remove 'query-cpus' QMP command" neglected to remove CpuInfo & friends from pragma name-case-exceptions. Remove these now. Signed-off-by: Markus Armbruster <armbru@redhat.com> Message-Id: <20210323094025.3569441-2-armbru@redhat.com> Reviewed-by: John Snow <jsnow@redhat.com> --- qapi/pragma.json | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/qapi/pragma.json b/qapi/pragma.json index cffae27666..7f158e183d 100644 --- a/qapi/pragma.json +++ b/qapi/pragma.json @@ -7,18 +7,14 @@ 'returns-whitelist': [ 'human-monitor-command', 'qom-get', - 'query-migrate-cache-size', 'query-tpm-models', 'query-tpm-types', 'ringbuf-read' ], 'name-case-whitelist': [ 'ACPISlotType', # DIMM, visible through query-acpi-ospm-status - 'CpuInfoMIPS', # PC, visible through query-cpu - 'CpuInfoTricore', # PC, visible through query-cpu 'BlockdevVmdkSubformat', # all members, to match VMDK spec spellings 'BlockdevVmdkAdapterType', # legacyESX, to match VMDK spec spellings 'QapiErrorClass', # all members, visible through errors 'UuidInfo', # UUID, visible through query-uuid - 'X86CPURegister32', # all members, visible indirectly through qom-get - 'CpuInfo' # CPU, visible through query-cpu + 'X86CPURegister32' # all members, visible indirectly through qom-get ] } } -- 2.26.3
flat-union-inline.json covers longhand branch definition with an invalid type value. It's redundant: longhand branch definition is covered by flat-union-inline-invalid-dict.json, and invalid type value is covered by nested-struct-data.json. Drop the test. Signed-off-by: Markus Armbruster <armbru@redhat.com> Message-Id: <20210323094025.3569441-3-armbru@redhat.com> Reviewed-by: John Snow <jsnow@redhat.com> --- tests/qapi-schema/flat-union-inline.err | 2 -- tests/qapi-schema/flat-union-inline.json | 11 ----------- tests/qapi-schema/flat-union-inline.out | 0 tests/qapi-schema/meson.build | 1 - 4 files changed, 14 deletions(-) delete mode 100644 tests/qapi-schema/flat-union-inline.err delete mode 100644 tests/qapi-schema/flat-union-inline.json delete mode 100644 tests/qapi-schema/flat-union-inline.out diff --git a/tests/qapi-schema/flat-union-inline.err b/tests/qapi-schema/flat-union-inline.err deleted file mode 100644 index 538283f5db..0000000000 --- a/tests/qapi-schema/flat-union-inline.err +++ /dev/null @@ -1,2 +0,0 @@ -flat-union-inline.json: In union 'TestUnion': -flat-union-inline.json:7: 'data' member 'value1' should be a type name diff --git a/tests/qapi-schema/flat-union-inline.json b/tests/qapi-schema/flat-union-inline.json deleted file mode 100644 index a9b3ce3f0d..0000000000 --- a/tests/qapi-schema/flat-union-inline.json +++ /dev/null @@ -1,11 +0,0 @@ -# we require branches to be a struct name -# TODO: should we allow anonymous inline branch types? -{ 'enum': 'TestEnum', - 'data': [ 'value1', 'value2' ] } -{ 'struct': 'Base', - 'data': { 'enum1': 'TestEnum', 'kind': 'str' } } -{ 'union': 'TestUnion', - 'base': 'Base', - 'discriminator': 'enum1', - 'data': { 'value1': { 'type': {} }, - 'value2': { 'integer': 'int' } } } diff --git a/tests/qapi-schema/flat-union-inline.out b/tests/qapi-schema/flat-union-inline.out deleted file mode 100644 index e69de29bb2..0000000000 diff --git a/tests/qapi-schema/meson.build b/tests/qapi-schema/meson.build index 304ef939bd..d5fa035507 100644 --- a/tests/qapi-schema/meson.build +++ b/tests/qapi-schema/meson.build @@ -111,7 +111,6 @@ schemas = [ 'flat-union-clash-member.json', 'flat-union-discriminator-bad-name.json', 'flat-union-empty.json', - 'flat-union-inline.json', 'flat-union-inline-invalid-dict.json', 'flat-union-int-branch.json', 'flat-union-invalid-branch-key.json', -- 2.26.3
A few old comments talk about "desired future use of defaults" and "anonymous inline branch types". Kind of misleading since commit 87adbbffd4 "qapi: add a dictionary form for TYPE" added longhand member definitions. Talk about that instead. Signed-off-by: Markus Armbruster <armbru@redhat.com> Message-Id: <20210323094025.3569441-4-armbru@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com> Reviewed-by: John Snow <jsnow@redhat.com> --- tests/qapi-schema/event-member-invalid-dict.err | 2 +- tests/qapi-schema/event-member-invalid-dict.json | 2 ++ tests/qapi-schema/flat-union-inline-invalid-dict.json | 4 ++-- tests/qapi-schema/nested-struct-data-invalid-dict.err | 2 +- tests/qapi-schema/nested-struct-data-invalid-dict.json | 3 ++- tests/qapi-schema/nested-struct-data.json | 2 +- tests/qapi-schema/struct-member-invalid-dict.err | 2 +- tests/qapi-schema/struct-member-invalid-dict.json | 3 ++- 8 files changed, 12 insertions(+), 8 deletions(-) diff --git a/tests/qapi-schema/event-member-invalid-dict.err b/tests/qapi-schema/event-member-invalid-dict.err index c7a6a24305..82f8989344 100644 --- a/tests/qapi-schema/event-member-invalid-dict.err +++ b/tests/qapi-schema/event-member-invalid-dict.err @@ -1,2 +1,2 @@ event-member-invalid-dict.json: In event 'EVENT_A': -event-member-invalid-dict.json:1: 'data' member 'a' misses key 'type' +event-member-invalid-dict.json:3: 'data' member 'a' misses key 'type' diff --git a/tests/qapi-schema/event-member-invalid-dict.json b/tests/qapi-schema/event-member-invalid-dict.json index ee6f3ecb6f..e58560abca 100644 --- a/tests/qapi-schema/event-member-invalid-dict.json +++ b/tests/qapi-schema/event-member-invalid-dict.json @@ -1,2 +1,4 @@ +# event 'data' member with dict value is (longhand) argument +# definition, not inline complex type { 'event': 'EVENT_A', 'data': { 'a' : { 'string' : 'str', 'integer': 'int' }, 'b' : 'str' } } diff --git a/tests/qapi-schema/flat-union-inline-invalid-dict.json b/tests/qapi-schema/flat-union-inline-invalid-dict.json index 62c7cda617..1779712795 100644 --- a/tests/qapi-schema/flat-union-inline-invalid-dict.json +++ b/tests/qapi-schema/flat-union-inline-invalid-dict.json @@ -1,5 +1,5 @@ -# we require branches to be a struct name -# TODO: should we allow anonymous inline branch types? +# union 'data' member with dict value is (longhand) branch +# definition, not inline complex type { 'enum': 'TestEnum', 'data': [ 'value1', 'value2' ] } { 'struct': 'Base', diff --git a/tests/qapi-schema/nested-struct-data-invalid-dict.err b/tests/qapi-schema/nested-struct-data-invalid-dict.err index c044b2b17a..375e155fe6 100644 --- a/tests/qapi-schema/nested-struct-data-invalid-dict.err +++ b/tests/qapi-schema/nested-struct-data-invalid-dict.err @@ -1,2 +1,2 @@ nested-struct-data-invalid-dict.json: In command 'foo': -nested-struct-data-invalid-dict.json:2: 'data' member 'a' misses key 'type' +nested-struct-data-invalid-dict.json:3: 'data' member 'a' misses key 'type' diff --git a/tests/qapi-schema/nested-struct-data-invalid-dict.json b/tests/qapi-schema/nested-struct-data-invalid-dict.json index efbe773ded..aa37b85e19 100644 --- a/tests/qapi-schema/nested-struct-data-invalid-dict.json +++ b/tests/qapi-schema/nested-struct-data-invalid-dict.json @@ -1,3 +1,4 @@ -# inline subtypes collide with our desired future use of defaults +# command 'data' member with dict value is (longhand) argument +# definition, not inline complex type { 'command': 'foo', 'data': { 'a' : { 'string' : 'str', 'integer': 'int' }, 'b' : 'str' } } diff --git a/tests/qapi-schema/nested-struct-data.json b/tests/qapi-schema/nested-struct-data.json index 5b8a40cca3..2980d45d05 100644 --- a/tests/qapi-schema/nested-struct-data.json +++ b/tests/qapi-schema/nested-struct-data.json @@ -1,3 +1,3 @@ -# inline subtypes collide with our desired future use of defaults +# {} is not a valid type reference { 'command': 'foo', 'data': { 'a' : { 'type': {} }, 'b' : 'str' } } diff --git a/tests/qapi-schema/struct-member-invalid-dict.err b/tests/qapi-schema/struct-member-invalid-dict.err index 0621aecfbd..f9b3f33551 100644 --- a/tests/qapi-schema/struct-member-invalid-dict.err +++ b/tests/qapi-schema/struct-member-invalid-dict.err @@ -1,2 +1,2 @@ struct-member-invalid-dict.json: In struct 'foo': -struct-member-invalid-dict.json:2: 'data' member '*a' misses key 'type' +struct-member-invalid-dict.json:3: 'data' member '*a' misses key 'type' diff --git a/tests/qapi-schema/struct-member-invalid-dict.json b/tests/qapi-schema/struct-member-invalid-dict.json index 9fe0d455a9..bc3d62ae63 100644 --- a/tests/qapi-schema/struct-member-invalid-dict.json +++ b/tests/qapi-schema/struct-member-invalid-dict.json @@ -1,3 +1,4 @@ -# Long form of member must have a value member 'type' +# struct 'data' member with dict value is (longhand) member +# definition, not inline complex type { 'struct': 'foo', 'data': { '*a': { 'case': 'foo' } } } -- 2.26.3
Commit 0426d53c65 "qapi: Simplify visiting of alternate types" eliminated the implicit alternate enum, but neglected to update a comment about it in a test. Do that now. Signed-off-by: Markus Armbruster <armbru@redhat.com> Message-Id: <20210323094025.3569441-5-armbru@redhat.com> Reviewed-by: John Snow <jsnow@redhat.com> --- tests/qapi-schema/alternate-clash.err | 2 +- tests/qapi-schema/alternate-clash.json | 5 +---- 2 files changed, 2 insertions(+), 5 deletions(-) diff --git a/tests/qapi-schema/alternate-clash.err b/tests/qapi-schema/alternate-clash.err index f58b977f7b..0fe02f2c99 100644 --- a/tests/qapi-schema/alternate-clash.err +++ b/tests/qapi-schema/alternate-clash.err @@ -1,2 +1,2 @@ alternate-clash.json: In alternate 'Alt1': -alternate-clash.json:7: branch 'a_b' collides with branch 'a-b' +alternate-clash.json:4: branch 'a_b' collides with branch 'a-b' diff --git a/tests/qapi-schema/alternate-clash.json b/tests/qapi-schema/alternate-clash.json index 9a59b88ced..039c4be658 100644 --- a/tests/qapi-schema/alternate-clash.json +++ b/tests/qapi-schema/alternate-clash.json @@ -1,8 +1,5 @@ # Alternate branch name collision # Reject an alternate that would result in a collision in generated C -# names (this would try to generate two enum values 'ALT1_KIND_A_B'). -# TODO: In the future, if alternates are simplified to not generate -# the implicit Alt1Kind enum, we would still have a collision with the -# resulting C union trying to have two members named 'a_b'. +# names (this would try to generate two union members named 'a_b'). { 'alternate': 'Alt1', 'data': { 'a-b': 'bool', 'a_b': 'int' } } -- 2.26.3
Simple unions don't need more features, they need to die. Signed-off-by: Markus Armbruster <armbru@redhat.com> Message-Id: <20210323094025.3569441-6-armbru@redhat.com> Reviewed-by: John Snow <jsnow@redhat.com> --- tests/qapi-schema/flat-union-no-base.err | 2 +- tests/qapi-schema/flat-union-no-base.json | 1 - 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/tests/qapi-schema/flat-union-no-base.err b/tests/qapi-schema/flat-union-no-base.err index 9bd595bcfb..5167565b00 100644 --- a/tests/qapi-schema/flat-union-no-base.err +++ b/tests/qapi-schema/flat-union-no-base.err @@ -1,2 +1,2 @@ flat-union-no-base.json: In union 'TestUnion': -flat-union-no-base.json:9: 'discriminator' requires 'base' +flat-union-no-base.json:8: 'discriminator' requires 'base' diff --git a/tests/qapi-schema/flat-union-no-base.json b/tests/qapi-schema/flat-union-no-base.json index ffc4c6f0e6..327877b563 100644 --- a/tests/qapi-schema/flat-union-no-base.json +++ b/tests/qapi-schema/flat-union-no-base.json @@ -1,5 +1,4 @@ # flat unions require a base -# TODO: simple unions should be able to use an enum discriminator { 'struct': 'TestTypeA', 'data': { 'string': 'str' } } { 'struct': 'TestTypeB', -- 2.26.3
Member name 'u' and names starting with 'has-' or 'has_' are reserved for the generator. check_type() enforces this, covered by tests reserved-member-u and reserved-member-has. These tests neglect to cover optional members, where the name starts with '*'. Tweak reserved-member-u to fix that. Test reserved-member-has still covers non-optional members. This demonstrates the reserved member name check is broken for optional members. The next commit will fix it. Signed-off-by: Markus Armbruster <armbru@redhat.com> Message-Id: <20210323094025.3569441-7-armbru@redhat.com> Reviewed-by: John Snow <jsnow@redhat.com> [Commit message improved slightly] --- tests/qapi-schema/reserved-member-u.err | 2 -- tests/qapi-schema/reserved-member-u.json | 3 ++- tests/qapi-schema/reserved-member-u.out | 14 ++++++++++++++ 3 files changed, 16 insertions(+), 3 deletions(-) diff --git a/tests/qapi-schema/reserved-member-u.err b/tests/qapi-schema/reserved-member-u.err index 231d552494..e69de29bb2 100644 --- a/tests/qapi-schema/reserved-member-u.err +++ b/tests/qapi-schema/reserved-member-u.err @@ -1,2 +0,0 @@ -reserved-member-u.json: In struct 'Oops': -reserved-member-u.json:7: 'data' member 'u' uses reserved name diff --git a/tests/qapi-schema/reserved-member-u.json b/tests/qapi-schema/reserved-member-u.json index 1eaf0f301c..15005abb09 100644 --- a/tests/qapi-schema/reserved-member-u.json +++ b/tests/qapi-schema/reserved-member-u.json @@ -4,4 +4,5 @@ # This is true even for non-unions, because it is possible to convert a # struct to flat union while remaining backwards compatible in QMP. # TODO - we could munge the member name to 'q_u' to avoid the collision -{ 'struct': 'Oops', 'data': { 'u': 'str' } } +# BUG: not rejected +{ 'struct': 'Oops', 'data': { '*u': 'str' } } diff --git a/tests/qapi-schema/reserved-member-u.out b/tests/qapi-schema/reserved-member-u.out index e69de29bb2..6a3705518b 100644 --- a/tests/qapi-schema/reserved-member-u.out +++ b/tests/qapi-schema/reserved-member-u.out @@ -0,0 +1,14 @@ +module ./builtin +object q_empty +enum QType + prefix QTYPE + member none + member qnull + member qnum + member qstring + member qdict + member qlist + member qbool +module reserved-member-u.json +object Oops + member u: str optional=True -- 2.26.3
check_type() fails to reject optional members with reserved names, because it neglects to strip off the leading '*'. Fix that. The stripping in check_name_str() is now useless. Drop. Also drop the "no leading '*'" assertion, because valid_name.match() ensures it can't fail. Fixes: 9fb081e0b98409556d023c7193eeb68947cd1211 Signed-off-by: Markus Armbruster <armbru@redhat.com> Message-Id: <20210323094025.3569441-8-armbru@redhat.com> Reviewed-by: John Snow <jsnow@redhat.com> --- scripts/qapi/expr.py | 9 ++++----- tests/qapi-schema/reserved-member-u.err | 2 ++ tests/qapi-schema/reserved-member-u.json | 1 - tests/qapi-schema/reserved-member-u.out | 14 -------------- 4 files changed, 6 insertions(+), 20 deletions(-) diff --git a/scripts/qapi/expr.py b/scripts/qapi/expr.py index 2fcaaa2497..cf09fa9fd3 100644 --- a/scripts/qapi/expr.py +++ b/scripts/qapi/expr.py @@ -34,12 +34,10 @@ def check_name_is_str(name, info, source): def check_name_str(name, info, source, - allow_optional=False, enum_member=False, + enum_member=False, permit_upper=False): membername = name - if allow_optional and name.startswith('*'): - membername = name[1:] # Enum members can start with a digit, because the generated C # code always prefixes it with the enum name if enum_member and membername[0].isdigit(): @@ -52,7 +50,6 @@ def check_name_str(name, info, source, if not permit_upper and name.lower() != name: raise QAPISemError( info, "%s uses uppercase in name" % source) - assert not membername.startswith('*') def check_defn_name_str(name, info, meta): @@ -171,8 +168,10 @@ def check_type(value, info, source, # value is a dictionary, check that each member is okay for (key, arg) in value.items(): key_source = "%s member '%s'" % (source, key) + if key.startswith('*'): + key = key[1:] check_name_str(key, info, key_source, - allow_optional=True, permit_upper=permit_upper) + permit_upper=permit_upper) if c_name(key, False) == 'u' or c_name(key, False).startswith('has_'): raise QAPISemError(info, "%s uses reserved name" % key_source) check_keys(arg, info, key_source, ['type'], ['if', 'features']) diff --git a/tests/qapi-schema/reserved-member-u.err b/tests/qapi-schema/reserved-member-u.err index e69de29bb2..b58e599a00 100644 --- a/tests/qapi-schema/reserved-member-u.err +++ b/tests/qapi-schema/reserved-member-u.err @@ -0,0 +1,2 @@ +reserved-member-u.json: In struct 'Oops': +reserved-member-u.json:7: 'data' member '*u' uses reserved name diff --git a/tests/qapi-schema/reserved-member-u.json b/tests/qapi-schema/reserved-member-u.json index 15005abb09..2bfb8f59b6 100644 --- a/tests/qapi-schema/reserved-member-u.json +++ b/tests/qapi-schema/reserved-member-u.json @@ -4,5 +4,4 @@ # This is true even for non-unions, because it is possible to convert a # struct to flat union while remaining backwards compatible in QMP. # TODO - we could munge the member name to 'q_u' to avoid the collision -# BUG: not rejected { 'struct': 'Oops', 'data': { '*u': 'str' } } diff --git a/tests/qapi-schema/reserved-member-u.out b/tests/qapi-schema/reserved-member-u.out index 6a3705518b..e69de29bb2 100644 --- a/tests/qapi-schema/reserved-member-u.out +++ b/tests/qapi-schema/reserved-member-u.out @@ -1,14 +0,0 @@ -module ./builtin -object q_empty -enum QType - prefix QTYPE - member none - member qnull - member qnum - member qstring - member qdict - member qlist - member qbool -module reserved-member-u.json -object Oops - member u: str optional=True -- 2.26.3
Flat union branch names match the tag enum's member names. Omitted branches default to "no members for this tag value". Branch names starting with a digit get rejected like "'data' member '0' has an invalid name". However, omitting the branch works. This is because flat union tag values get checked twice: as enum member name, and as union branch name. The former accepts leading digits, the latter doesn't. Branches whose names start with a digit therefore cannot have members. Feels wrong. Get rid of the restriction by skipping the latter check. This can expose c_name() to input it can't handle: a name starting with a digit. Improve it to return a valid C identifier for any input. Signed-off-by: Markus Armbruster <armbru@redhat.com> Message-Id: <20210323094025.3569441-9-armbru@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com> [Commit message rewritten] --- scripts/qapi/common.py | 8 ++++---- scripts/qapi/expr.py | 4 +++- 2 files changed, 7 insertions(+), 5 deletions(-) diff --git a/scripts/qapi/common.py b/scripts/qapi/common.py index 11b86beeab..cbd3fd81d3 100644 --- a/scripts/qapi/common.py +++ b/scripts/qapi/common.py @@ -18,7 +18,6 @@ #: Magic string that gets removed along with all space to its right. EATSPACE = '\033EATSPACE.' POINTER_SUFFIX = ' *' + EATSPACE -_C_NAME_TRANS = str.maketrans('.-', '__') def camel_to_upper(value: str) -> str: @@ -109,9 +108,10 @@ def c_name(name: str, protect: bool = True) -> str: 'not_eq', 'or', 'or_eq', 'xor', 'xor_eq']) # namespace pollution: polluted_words = set(['unix', 'errno', 'mips', 'sparc', 'i386']) - name = name.translate(_C_NAME_TRANS) - if protect and (name in c89_words | c99_words | c11_words | gcc_words - | cpp_words | polluted_words): + name = re.sub(r'[^A-Za-z0-9_]', '_', name) + if protect and (name in (c89_words | c99_words | c11_words | gcc_words + | cpp_words | polluted_words) + or name[0].isdigit()): return 'q_' + name return name diff --git a/scripts/qapi/expr.py b/scripts/qapi/expr.py index cf09fa9fd3..507550c340 100644 --- a/scripts/qapi/expr.py +++ b/scripts/qapi/expr.py @@ -246,7 +246,9 @@ def check_union(expr, info): for (key, value) in members.items(): source = "'data' member '%s'" % key - check_name_str(key, info, source) + if discriminator is None: + check_name_str(key, info, source) + # else: name is in discriminator enum, which gets checked check_keys(value, info, source, ['type'], ['if']) check_if(value, info, source) check_type(value['type'], info, source, allow_array=not base) -- 2.26.3
check_name_str() masks leading digits when passed enum_member=True. Only check_enum() does. Lift the masking into check_enum(). Signed-off-by: Markus Armbruster <armbru@redhat.com> Message-Id: <20210323094025.3569441-10-armbru@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com> --- scripts/qapi/expr.py | 23 ++++++++++------------- 1 file changed, 10 insertions(+), 13 deletions(-) diff --git a/scripts/qapi/expr.py b/scripts/qapi/expr.py index 507550c340..e00467636c 100644 --- a/scripts/qapi/expr.py +++ b/scripts/qapi/expr.py @@ -34,18 +34,11 @@ def check_name_is_str(name, info, source): def check_name_str(name, info, source, - enum_member=False, permit_upper=False): - membername = name - - # Enum members can start with a digit, because the generated C - # code always prefixes it with the enum name - if enum_member and membername[0].isdigit(): - membername = 'D' + membername # Reserve the entire 'q_' namespace for c_name(), and for 'q_empty' # and 'q_obj_*' implicit type names. - if not valid_name.match(membername) or \ - c_name(membername, False).startswith('q_'): + if not valid_name.match(name) or \ + c_name(name, False).startswith('q_'): raise QAPISemError(info, "%s has an invalid name" % source) if not permit_upper and name.lower() != name: raise QAPISemError( @@ -213,11 +206,15 @@ def check_enum(expr, info): for m in members] for member in members: source = "'data' member" + member_name = member['name'] check_keys(member, info, source, ['name'], ['if']) - check_name_is_str(member['name'], info, source) - source = "%s '%s'" % (source, member['name']) - check_name_str(member['name'], info, source, - enum_member=True, permit_upper=permit_upper) + check_name_is_str(member_name, info, source) + source = "%s '%s'" % (source, member_name) + # Enum members may start with a digit + if member_name[0].isdigit(): + member_name = 'd' + member_name # Hack: hide the digit + check_name_str(member_name, info, source, + permit_upper=permit_upper) check_if(member, info, source) -- 2.26.3
Naming rules differ for the various kinds of names. To prepare enforcing them, define functions to check them: check_name_upper(), check_name_lower(), and check_name_camel(). For now, these merely wrap around check_name_str(), but that will change shortly. Replace the other uses of check_name_str() by appropriate uses of the wrappers. No change in behavior just yet. check_name_str() now returns the name without downstream and x- prefix, for use by the wrappers in later patches. Requires tweaking regexp @valid_name. It accepts the same strings as before. Signed-off-by: Markus Armbruster <armbru@redhat.com> Message-Id: <20210323094025.3569441-11-armbru@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com> [Commit message improved] --- scripts/qapi/expr.py | 51 +++++++++++++++++++++++++++++++------------- 1 file changed, 36 insertions(+), 15 deletions(-) diff --git a/scripts/qapi/expr.py b/scripts/qapi/expr.py index e00467636c..30285fe334 100644 --- a/scripts/qapi/expr.py +++ b/scripts/qapi/expr.py @@ -21,11 +21,12 @@ from .error import QAPISemError -# 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. -valid_name = re.compile(r'^(__[a-zA-Z0-9.-]+_)?' - '[a-zA-Z][a-zA-Z0-9_-]*$') +# Names consist of letters, digits, -, and _, starting with a letter. +# An experimental name is prefixed with x-. A name of a downstream +# extension is prefixed with __RFQDN_. The latter prefix goes first. +valid_name = re.compile(r'(__[a-z0-9.-]+_)?' + r'(x-)?' + r'([a-z][a-z0-9_-]*)$', re.IGNORECASE) def check_name_is_str(name, info, source): @@ -37,16 +38,38 @@ def check_name_str(name, info, source, permit_upper=False): # Reserve the entire 'q_' namespace for c_name(), and for 'q_empty' # and 'q_obj_*' implicit type names. - if not valid_name.match(name) or \ - c_name(name, False).startswith('q_'): + match = valid_name.match(name) + if not match or c_name(name, False).startswith('q_'): raise QAPISemError(info, "%s has an invalid name" % source) if not permit_upper and name.lower() != name: raise QAPISemError( info, "%s uses uppercase in name" % source) + return match.group(3) + + +def check_name_upper(name, info, source): + stem = check_name_str(name, info, source, permit_upper=True) + # TODO reject '[a-z-]' in @stem + + +def check_name_lower(name, info, source, + permit_upper=False): + stem = check_name_str(name, info, source, permit_upper) + # TODO reject '_' in stem + + +def check_name_camel(name, info, source): + stem = check_name_str(name, info, source, permit_upper=True) + # TODO reject '[_-]' in stem, require CamelCase def check_defn_name_str(name, info, meta): - check_name_str(name, info, meta, permit_upper=True) + if meta == 'event': + check_name_upper(name, info, meta) + elif meta == 'command': + check_name_lower(name, info, meta, permit_upper=True) + else: + check_name_camel(name, info, meta) if name.endswith('Kind') or name.endswith('List'): raise QAPISemError( info, "%s name should not end in '%s'" % (meta, name[-4:])) @@ -163,8 +186,7 @@ def check_type(value, info, source, key_source = "%s member '%s'" % (source, key) if key.startswith('*'): key = key[1:] - check_name_str(key, info, key_source, - permit_upper=permit_upper) + check_name_lower(key, info, key_source, permit_upper) if c_name(key, False) == 'u' or c_name(key, False).startswith('has_'): raise QAPISemError(info, "%s uses reserved name" % key_source) check_keys(arg, info, key_source, ['type'], ['if', 'features']) @@ -186,7 +208,7 @@ def check_features(features, info): check_keys(f, info, source, ['name'], ['if']) check_name_is_str(f['name'], info, source) source = "%s '%s'" % (source, f['name']) - check_name_str(f['name'], info, source) + check_name_lower(f['name'], info, source) check_if(f, info, source) @@ -213,8 +235,7 @@ def check_enum(expr, info): # Enum members may start with a digit if member_name[0].isdigit(): member_name = 'd' + member_name # Hack: hide the digit - check_name_str(member_name, info, source, - permit_upper=permit_upper) + check_name_lower(member_name, info, source, permit_upper) check_if(member, info, source) @@ -244,7 +265,7 @@ def check_union(expr, info): for (key, value) in members.items(): source = "'data' member '%s'" % key if discriminator is None: - check_name_str(key, info, source) + check_name_lower(key, info, source) # else: name is in discriminator enum, which gets checked check_keys(value, info, source, ['type'], ['if']) check_if(value, info, source) @@ -258,7 +279,7 @@ def check_alternate(expr, info): raise QAPISemError(info, "'data' must not be empty") for (key, value) in members.items(): source = "'data' member '%s'" % key - check_name_str(key, info, source) + check_name_lower(key, info, source) check_keys(value, info, source, ['type'], ['if']) check_if(value, info, source) check_type(value['type'], info, source) -- 2.26.3
check_name_lower() is the only user of check_name_str() using permit_upper=False. Move the associated code from check_name_str() to check_name_lower(), and drop the parameter. Signed-off-by: Markus Armbruster <armbru@redhat.com> Message-Id: <20210323094025.3569441-12-armbru@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com> --- scripts/qapi/expr.py | 15 +++++++-------- 1 file changed, 7 insertions(+), 8 deletions(-) diff --git a/scripts/qapi/expr.py b/scripts/qapi/expr.py index 30285fe334..a815060ee2 100644 --- a/scripts/qapi/expr.py +++ b/scripts/qapi/expr.py @@ -34,32 +34,31 @@ def check_name_is_str(name, info, source): raise QAPISemError(info, "%s requires a string name" % source) -def check_name_str(name, info, source, - permit_upper=False): +def check_name_str(name, info, source): # Reserve the entire 'q_' namespace for c_name(), and for 'q_empty' # and 'q_obj_*' implicit type names. match = valid_name.match(name) if not match or c_name(name, False).startswith('q_'): raise QAPISemError(info, "%s has an invalid name" % source) - if not permit_upper and name.lower() != name: - raise QAPISemError( - info, "%s uses uppercase in name" % source) return match.group(3) def check_name_upper(name, info, source): - stem = check_name_str(name, info, source, permit_upper=True) + stem = check_name_str(name, info, source) # TODO reject '[a-z-]' in @stem def check_name_lower(name, info, source, permit_upper=False): - stem = check_name_str(name, info, source, permit_upper) + stem = check_name_str(name, info, source) + if not permit_upper and name.lower() != name: + raise QAPISemError( + info, "%s uses uppercase in name" % source) # TODO reject '_' in stem def check_name_camel(name, info, source): - stem = check_name_str(name, info, source, permit_upper=True) + stem = check_name_str(name, info, source) # TODO reject '[_-]' in stem, require CamelCase -- 2.26.3
We require lowercase __RFQDN_ downstream prefixes only where we require the prefixed name to be lowercase. Don't; permit any case in __RFQDN_ prefixes anywhere. Signed-off-by: Markus Armbruster <armbru@redhat.com> Message-Id: <20210323094025.3569441-13-armbru@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com> --- scripts/qapi/expr.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/scripts/qapi/expr.py b/scripts/qapi/expr.py index a815060ee2..b5fb0be48b 100644 --- a/scripts/qapi/expr.py +++ b/scripts/qapi/expr.py @@ -51,7 +51,7 @@ def check_name_upper(name, info, source): def check_name_lower(name, info, source, permit_upper=False): stem = check_name_str(name, info, source) - if not permit_upper and name.lower() != name: + if not permit_upper and re.search(r'[A-Z]', stem): raise QAPISemError( info, "%s uses uppercase in name" % source) # TODO reject '_' in stem -- 2.26.3
Event names should be ALL_CAPS with words separated by underscore. Enforce this. The only offenders are in tests/. Fix them. Existing test event-case covers the new error. Signed-off-by: Markus Armbruster <armbru@redhat.com> Message-Id: <20210323094025.3569441-14-armbru@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com> --- tests/unit/test-qmp-event.c | 6 +++--- scripts/qapi/expr.py | 4 +++- tests/qapi-schema/doc-good.json | 4 ++-- tests/qapi-schema/doc-good.out | 4 ++-- tests/qapi-schema/doc-good.txt | 2 +- tests/qapi-schema/doc-invalid-return.json | 4 ++-- tests/qapi-schema/event-case.err | 2 ++ tests/qapi-schema/event-case.json | 2 -- tests/qapi-schema/event-case.out | 14 -------------- tests/qapi-schema/qapi-schema-test.json | 6 +++--- tests/qapi-schema/qapi-schema-test.out | 8 ++++---- 11 files changed, 22 insertions(+), 34 deletions(-) diff --git a/tests/unit/test-qmp-event.c b/tests/unit/test-qmp-event.c index 047f44ff9a..d58c3b78f2 100644 --- a/tests/unit/test-qmp-event.c +++ b/tests/unit/test-qmp-event.c @@ -143,7 +143,7 @@ static void test_event_d(TestEventData *data, static void test_event_deprecated(TestEventData *data, const void *unused) { - data->expect = qdict_from_jsonf_nofail("{ 'event': 'TEST-EVENT-FEATURES1' }"); + data->expect = qdict_from_jsonf_nofail("{ 'event': 'TEST_EVENT_FEATURES1' }"); memset(&compat_policy, 0, sizeof(compat_policy)); @@ -163,7 +163,7 @@ static void test_event_deprecated_data(TestEventData *data, const void *unused) { memset(&compat_policy, 0, sizeof(compat_policy)); - data->expect = qdict_from_jsonf_nofail("{ 'event': 'TEST-EVENT-FEATURES0'," + data->expect = qdict_from_jsonf_nofail("{ 'event': 'TEST_EVENT_FEATURES0'," " 'data': { 'foo': 42 } }"); qapi_event_send_test_event_features0(42); g_assert(data->emitted); @@ -172,7 +172,7 @@ static void test_event_deprecated_data(TestEventData *data, const void *unused) compat_policy.has_deprecated_output = true; compat_policy.deprecated_output = COMPAT_POLICY_OUTPUT_HIDE; - data->expect = qdict_from_jsonf_nofail("{ 'event': 'TEST-EVENT-FEATURES0' }"); + data->expect = qdict_from_jsonf_nofail("{ 'event': 'TEST_EVENT_FEATURES0' }"); qapi_event_send_test_event_features0(42); g_assert(data->emitted); diff --git a/scripts/qapi/expr.py b/scripts/qapi/expr.py index b5fb0be48b..c065505b27 100644 --- a/scripts/qapi/expr.py +++ b/scripts/qapi/expr.py @@ -45,7 +45,9 @@ def check_name_str(name, info, source): def check_name_upper(name, info, source): stem = check_name_str(name, info, source) - # TODO reject '[a-z-]' in @stem + if re.search(r'[a-z-]', stem): + raise QAPISemError( + info, "name of %s must not use lowercase or '-'" % source) def check_name_lower(name, info, source, diff --git a/tests/qapi-schema/doc-good.json b/tests/qapi-schema/doc-good.json index e9af0857db..423ea23e07 100644 --- a/tests/qapi-schema/doc-good.json +++ b/tests/qapi-schema/doc-good.json @@ -179,10 +179,10 @@ 'features': [ 'cmd-feat1', 'cmd-feat2' ] } ## -# @EVT-BOXED: +# @EVT_BOXED: # Features: # @feat3: a feature ## -{ 'event': 'EVT-BOXED', 'boxed': true, +{ 'event': 'EVT_BOXED', 'boxed': true, 'features': [ 'feat3' ], 'data': 'Object' } diff --git a/tests/qapi-schema/doc-good.out b/tests/qapi-schema/doc-good.out index 715b0bbc1a..8f54ceff2e 100644 --- a/tests/qapi-schema/doc-good.out +++ b/tests/qapi-schema/doc-good.out @@ -63,7 +63,7 @@ command cmd-boxed Object -> None gen=True success_response=True boxed=True oob=False preconfig=False feature cmd-feat1 feature cmd-feat2 -event EVT-BOXED Object +event EVT_BOXED Object boxed=True feature feat3 doc freeform @@ -211,7 +211,7 @@ another feature -> in <- out -doc symbol=EVT-BOXED +doc symbol=EVT_BOXED body= feature=feat3 diff --git a/tests/qapi-schema/doc-good.txt b/tests/qapi-schema/doc-good.txt index 6ca03d49d0..726727af74 100644 --- a/tests/qapi-schema/doc-good.txt +++ b/tests/qapi-schema/doc-good.txt @@ -272,7 +272,7 @@ Example <- out -"EVT-BOXED" (Event) +"EVT_BOXED" (Event) ------------------- diff --git a/tests/qapi-schema/doc-invalid-return.json b/tests/qapi-schema/doc-invalid-return.json index 1ba45de414..95e7583930 100644 --- a/tests/qapi-schema/doc-invalid-return.json +++ b/tests/qapi-schema/doc-invalid-return.json @@ -1,7 +1,7 @@ # Events can't have 'Returns' section ## -# @foo: +# @FOO: # Returns: blah ## -{ 'event': 'foo' } +{ 'event': 'FOO' } diff --git a/tests/qapi-schema/event-case.err b/tests/qapi-schema/event-case.err index e69de29bb2..d3007cfa63 100644 --- a/tests/qapi-schema/event-case.err +++ b/tests/qapi-schema/event-case.err @@ -0,0 +1,2 @@ +event-case.json: In event 'oops': +event-case.json:1: name of event must not use lowercase or '-' diff --git a/tests/qapi-schema/event-case.json b/tests/qapi-schema/event-case.json index 3a92d8b610..4d8a5d8a71 100644 --- a/tests/qapi-schema/event-case.json +++ b/tests/qapi-schema/event-case.json @@ -1,3 +1 @@ -# TODO: might be nice to enforce naming conventions; but until then this works -# even though events should usually be ALL_CAPS { 'event': 'oops' } diff --git a/tests/qapi-schema/event-case.out b/tests/qapi-schema/event-case.out index 9ae44052ac..e69de29bb2 100644 --- a/tests/qapi-schema/event-case.out +++ b/tests/qapi-schema/event-case.out @@ -1,14 +0,0 @@ -module ./builtin -object q_empty -enum QType - prefix QTYPE - member none - member qnull - member qnum - member qstring - member qdict - member qlist - member qbool -module event-case.json -event oops None - boxed=False diff --git a/tests/qapi-schema/qapi-schema-test.json b/tests/qapi-schema/qapi-schema-test.json index 12ec588b52..a355321258 100644 --- a/tests/qapi-schema/qapi-schema-test.json +++ b/tests/qapi-schema/qapi-schema-test.json @@ -249,7 +249,7 @@ { 'command': 'TestCmdReturnDefThree', 'returns': 'UserDefThree' } -{ 'event': 'TestIfEvent', 'data': +{ 'event': 'TEST_IF_EVENT', 'data': { 'foo': 'TestIfStruct', 'bar': { 'type': ['TestIfEnum'], 'if': 'defined(TEST_IF_EVT_BAR)' } }, 'if': 'defined(TEST_IF_EVT) && defined(TEST_IF_STRUCT)' } @@ -324,8 +324,8 @@ 'features': [ { 'name': 'feature1', 'if': [ 'defined(TEST_IF_COND_1)', 'defined(TEST_IF_COND_2)'] } ] } -{ 'event': 'TEST-EVENT-FEATURES0', +{ 'event': 'TEST_EVENT_FEATURES0', 'data': 'FeatureStruct1' } -{ 'event': 'TEST-EVENT-FEATURES1', +{ 'event': 'TEST_EVENT_FEATURES1', 'features': [ 'deprecated' ] } diff --git a/tests/qapi-schema/qapi-schema-test.out b/tests/qapi-schema/qapi-schema-test.out index f5741df97f..882d0e7c56 100644 --- a/tests/qapi-schema/qapi-schema-test.out +++ b/tests/qapi-schema/qapi-schema-test.out @@ -349,12 +349,12 @@ command TestCmdReturnDefThree None -> UserDefThree gen=True success_response=True boxed=False oob=False preconfig=False array TestIfEnumList TestIfEnum if ['defined(TEST_IF_ENUM)'] -object q_obj_TestIfEvent-arg +object q_obj_TEST_IF_EVENT-arg member foo: TestIfStruct optional=False member bar: TestIfEnumList optional=False if ['defined(TEST_IF_EVT_BAR)'] if ['defined(TEST_IF_EVT) && defined(TEST_IF_STRUCT)'] -event TestIfEvent q_obj_TestIfEvent-arg +event TEST_IF_EVENT q_obj_TEST_IF_EVENT-arg boxed=False if ['defined(TEST_IF_EVT) && defined(TEST_IF_STRUCT)'] object FeatureStruct0 @@ -440,9 +440,9 @@ command test-command-cond-features3 None -> None gen=True success_response=True boxed=False oob=False preconfig=False feature feature1 if ['defined(TEST_IF_COND_1)', 'defined(TEST_IF_COND_2)'] -event TEST-EVENT-FEATURES0 FeatureStruct1 +event TEST_EVENT_FEATURES0 FeatureStruct1 boxed=False -event TEST-EVENT-FEATURES1 None +event TEST_EVENT_FEATURES1 None boxed=False feature deprecated module include/sub-module.json -- 2.26.3
Type names should be CamelCase. Enforce this. The only offenders are in tests/. Fix them. Add test type-case to cover the new error. Signed-off-by: Markus Armbruster <armbru@redhat.com> Message-Id: <20210323094025.3569441-15-armbru@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com> [Regexp simplified, new test made more robust] --- scripts/qapi/expr.py | 3 ++- tests/qapi-schema/doc-bad-union-member.json | 4 ++-- tests/qapi-schema/double-type.err | 2 +- tests/qapi-schema/double-type.json | 2 +- tests/qapi-schema/features-deprecated-type.err | 2 +- tests/qapi-schema/features-deprecated-type.json | 2 +- tests/qapi-schema/meson.build | 1 + tests/qapi-schema/redefined-builtin.err | 4 ++-- tests/qapi-schema/redefined-builtin.json | 4 ++-- tests/qapi-schema/redefined-type.err | 6 +++--- tests/qapi-schema/redefined-type.json | 4 ++-- tests/qapi-schema/struct-data-invalid.err | 2 +- tests/qapi-schema/struct-data-invalid.json | 2 +- tests/qapi-schema/struct-member-invalid-dict.err | 2 +- tests/qapi-schema/struct-member-invalid-dict.json | 2 +- tests/qapi-schema/struct-member-invalid.err | 2 +- tests/qapi-schema/struct-member-invalid.json | 2 +- tests/qapi-schema/type-case.err | 2 ++ tests/qapi-schema/type-case.json | 2 ++ tests/qapi-schema/type-case.out | 0 tests/qapi-schema/unknown-expr-key.err | 2 +- tests/qapi-schema/unknown-expr-key.json | 2 +- 22 files changed, 30 insertions(+), 24 deletions(-) create mode 100644 tests/qapi-schema/type-case.err create mode 100644 tests/qapi-schema/type-case.json create mode 100644 tests/qapi-schema/type-case.out diff --git a/scripts/qapi/expr.py b/scripts/qapi/expr.py index c065505b27..7bd15559de 100644 --- a/scripts/qapi/expr.py +++ b/scripts/qapi/expr.py @@ -61,7 +61,8 @@ def check_name_lower(name, info, source, def check_name_camel(name, info, source): stem = check_name_str(name, info, source) - # TODO reject '[_-]' in stem, require CamelCase + if not re.match(r'[A-Z][A-Za-z0-9]*[a-z][A-Za-z0-9]*$', stem): + raise QAPISemError(info, "name of %s must use CamelCase" % source) def check_defn_name_str(name, info, meta): diff --git a/tests/qapi-schema/doc-bad-union-member.json b/tests/qapi-schema/doc-bad-union-member.json index d611435f6a..bd231a0109 100644 --- a/tests/qapi-schema/doc-bad-union-member.json +++ b/tests/qapi-schema/doc-bad-union-member.json @@ -11,9 +11,9 @@ 'data': { 'nothing': 'Empty' } } { 'struct': 'Base', - 'data': { 'type': 'T' } } + 'data': { 'type': 'FrobType' } } { 'struct': 'Empty', 'data': { } } -{ 'enum': 'T', 'data': ['nothing'] } +{ 'enum': 'FrobType', 'data': ['nothing'] } diff --git a/tests/qapi-schema/double-type.err b/tests/qapi-schema/double-type.err index 71fc4dbb52..576e716197 100644 --- a/tests/qapi-schema/double-type.err +++ b/tests/qapi-schema/double-type.err @@ -1,3 +1,3 @@ -double-type.json: In struct 'bar': +double-type.json: In struct 'Bar': double-type.json:2: struct has unknown key 'command' Valid keys are 'base', 'data', 'features', 'if', 'struct'. diff --git a/tests/qapi-schema/double-type.json b/tests/qapi-schema/double-type.json index 911fa7af50..2c0809f38d 100644 --- a/tests/qapi-schema/double-type.json +++ b/tests/qapi-schema/double-type.json @@ -1,2 +1,2 @@ # we reject an expression with ambiguous metatype -{ 'command': 'foo', 'struct': 'bar', 'data': { } } +{ 'command': 'foo', 'struct': 'Bar', 'data': { } } diff --git a/tests/qapi-schema/features-deprecated-type.err b/tests/qapi-schema/features-deprecated-type.err index af4ffe20aa..ddaedf604e 100644 --- a/tests/qapi-schema/features-deprecated-type.err +++ b/tests/qapi-schema/features-deprecated-type.err @@ -1,2 +1,2 @@ -features-deprecated-type.json: In struct 'S': +features-deprecated-type.json: In struct 'Foo': features-deprecated-type.json:2: feature 'deprecated' is not supported for types diff --git a/tests/qapi-schema/features-deprecated-type.json b/tests/qapi-schema/features-deprecated-type.json index 4b5bf5b86e..265849b1f7 100644 --- a/tests/qapi-schema/features-deprecated-type.json +++ b/tests/qapi-schema/features-deprecated-type.json @@ -1,3 +1,3 @@ # Feature 'deprecated' is not supported for types -{ 'struct': 'S', 'data': {}, +{ 'struct': 'Foo', 'data': {}, 'features': [ 'deprecated' ] } diff --git a/tests/qapi-schema/meson.build b/tests/qapi-schema/meson.build index d5fa035507..ba11cb76ac 100644 --- a/tests/qapi-schema/meson.build +++ b/tests/qapi-schema/meson.build @@ -180,6 +180,7 @@ schemas = [ 'trailing-comma-list.json', 'trailing-comma-object.json', 'type-bypass-bad-gen.json', + 'type-case.json', 'unclosed-list.json', 'unclosed-object.json', 'unclosed-string.json', diff --git a/tests/qapi-schema/redefined-builtin.err b/tests/qapi-schema/redefined-builtin.err index 58c7e42ffc..92bc62dc76 100644 --- a/tests/qapi-schema/redefined-builtin.err +++ b/tests/qapi-schema/redefined-builtin.err @@ -1,2 +1,2 @@ -redefined-builtin.json: In struct 'size': -redefined-builtin.json:2: built-in type 'size' is already defined +redefined-builtin.json: In struct 'QType': +redefined-builtin.json:2: enum type 'QType' is already defined diff --git a/tests/qapi-schema/redefined-builtin.json b/tests/qapi-schema/redefined-builtin.json index 45b8a550ad..cad555cc73 100644 --- a/tests/qapi-schema/redefined-builtin.json +++ b/tests/qapi-schema/redefined-builtin.json @@ -1,2 +1,2 @@ -# we reject types that duplicate builtin names -{ 'struct': 'size', 'data': { 'myint': 'size' } } +# we reject types that clash with predefined types +{ 'struct': 'QType', 'data': { 'myint': 'size' } } diff --git a/tests/qapi-schema/redefined-type.err b/tests/qapi-schema/redefined-type.err index b7103fc15f..5e5406f811 100644 --- a/tests/qapi-schema/redefined-type.err +++ b/tests/qapi-schema/redefined-type.err @@ -1,4 +1,4 @@ -redefined-type.json: In enum 'foo': -redefined-type.json:3: 'foo' is already defined -redefined-type.json: In struct 'foo': +redefined-type.json: In enum 'Foo': +redefined-type.json:3: 'Foo' is already defined +redefined-type.json: In struct 'Foo': redefined-type.json:2: previous definition diff --git a/tests/qapi-schema/redefined-type.json b/tests/qapi-schema/redefined-type.json index a09e768bae..291453e70d 100644 --- a/tests/qapi-schema/redefined-type.json +++ b/tests/qapi-schema/redefined-type.json @@ -1,3 +1,3 @@ # we reject types defined more than once -{ 'struct': 'foo', 'data': { 'one': 'str' } } -{ 'enum': 'foo', 'data': [ 'two' ] } +{ 'struct': 'Foo', 'data': { 'one': 'str' } } +{ 'enum': 'Foo', 'data': [ 'two' ] } diff --git a/tests/qapi-schema/struct-data-invalid.err b/tests/qapi-schema/struct-data-invalid.err index 5ed4bec573..23cbfc60ea 100644 --- a/tests/qapi-schema/struct-data-invalid.err +++ b/tests/qapi-schema/struct-data-invalid.err @@ -1,2 +1,2 @@ -struct-data-invalid.json: In struct 'foo': +struct-data-invalid.json: In struct 'Foo': struct-data-invalid.json:1: 'data' should be an object or type name diff --git a/tests/qapi-schema/struct-data-invalid.json b/tests/qapi-schema/struct-data-invalid.json index 9adbc3bb6b..00ad11ef94 100644 --- a/tests/qapi-schema/struct-data-invalid.json +++ b/tests/qapi-schema/struct-data-invalid.json @@ -1,2 +1,2 @@ -{ 'struct': 'foo', +{ 'struct': 'Foo', 'data': false } diff --git a/tests/qapi-schema/struct-member-invalid-dict.err b/tests/qapi-schema/struct-member-invalid-dict.err index f9b3f33551..517793cc9b 100644 --- a/tests/qapi-schema/struct-member-invalid-dict.err +++ b/tests/qapi-schema/struct-member-invalid-dict.err @@ -1,2 +1,2 @@ -struct-member-invalid-dict.json: In struct 'foo': +struct-member-invalid-dict.json: In struct 'Foo': struct-member-invalid-dict.json:3: 'data' member '*a' misses key 'type' diff --git a/tests/qapi-schema/struct-member-invalid-dict.json b/tests/qapi-schema/struct-member-invalid-dict.json index bc3d62ae63..df5d018f65 100644 --- a/tests/qapi-schema/struct-member-invalid-dict.json +++ b/tests/qapi-schema/struct-member-invalid-dict.json @@ -1,4 +1,4 @@ # struct 'data' member with dict value is (longhand) member # definition, not inline complex type -{ 'struct': 'foo', +{ 'struct': 'Foo', 'data': { '*a': { 'case': 'foo' } } } diff --git a/tests/qapi-schema/struct-member-invalid.err b/tests/qapi-schema/struct-member-invalid.err index 9a2c934538..7e01a41d7c 100644 --- a/tests/qapi-schema/struct-member-invalid.err +++ b/tests/qapi-schema/struct-member-invalid.err @@ -1,2 +1,2 @@ -struct-member-invalid.json: In struct 'foo': +struct-member-invalid.json: In struct 'Foo': struct-member-invalid.json:1: 'data' member 'a' should be a type name diff --git a/tests/qapi-schema/struct-member-invalid.json b/tests/qapi-schema/struct-member-invalid.json index 8f172f7a87..a4cd860c67 100644 --- a/tests/qapi-schema/struct-member-invalid.json +++ b/tests/qapi-schema/struct-member-invalid.json @@ -1,2 +1,2 @@ -{ 'struct': 'foo', +{ 'struct': 'Foo', 'data': { 'a': false } } diff --git a/tests/qapi-schema/type-case.err b/tests/qapi-schema/type-case.err new file mode 100644 index 0000000000..36d2de2d00 --- /dev/null +++ b/tests/qapi-schema/type-case.err @@ -0,0 +1,2 @@ +type-case.json: In struct 'not-a-camel': +type-case.json:2: name of struct must use CamelCase diff --git a/tests/qapi-schema/type-case.json b/tests/qapi-schema/type-case.json new file mode 100644 index 0000000000..a43c68e7eb --- /dev/null +++ b/tests/qapi-schema/type-case.json @@ -0,0 +1,2 @@ +# Type names should use CamelCase +{ 'struct': 'not-a-camel', 'data': {} } diff --git a/tests/qapi-schema/type-case.out b/tests/qapi-schema/type-case.out new file mode 100644 index 0000000000..e69de29bb2 diff --git a/tests/qapi-schema/unknown-expr-key.err b/tests/qapi-schema/unknown-expr-key.err index c5f395bf79..f2538e3ce7 100644 --- a/tests/qapi-schema/unknown-expr-key.err +++ b/tests/qapi-schema/unknown-expr-key.err @@ -1,3 +1,3 @@ -unknown-expr-key.json: In struct 'bar': +unknown-expr-key.json: In struct 'Bar': unknown-expr-key.json:2: struct has unknown keys 'bogus', 'phony' Valid keys are 'base', 'data', 'features', 'if', 'struct'. diff --git a/tests/qapi-schema/unknown-expr-key.json b/tests/qapi-schema/unknown-expr-key.json index 13292d75ed..8003a0c36e 100644 --- a/tests/qapi-schema/unknown-expr-key.json +++ b/tests/qapi-schema/unknown-expr-key.json @@ -1,2 +1,2 @@ # we reject an expression with unknown top-level keys -{ 'struct': 'bar', 'data': { 'string': 'str'}, 'bogus': { }, 'phony': { } } +{ 'struct': 'Bar', 'data': { 'string': 'str'}, 'bogus': { }, 'phony': { } } -- 2.26.3
The previous commit changed this test to clash with a predefined enum type, not a built-in type. Adjust its name. Signed-off-by: Markus Armbruster <armbru@redhat.com> Message-Id: <20210323094025.3569441-16-armbru@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com> --- tests/qapi-schema/meson.build | 2 +- tests/qapi-schema/redefined-builtin.err | 2 -- tests/qapi-schema/redefined-predefined.err | 2 ++ .../{redefined-builtin.json => redefined-predefined.json} | 0 .../{redefined-builtin.out => redefined-predefined.out} | 0 5 files changed, 3 insertions(+), 3 deletions(-) delete mode 100644 tests/qapi-schema/redefined-builtin.err create mode 100644 tests/qapi-schema/redefined-predefined.err rename tests/qapi-schema/{redefined-builtin.json => redefined-predefined.json} (100%) rename tests/qapi-schema/{redefined-builtin.out => redefined-predefined.out} (100%) diff --git a/tests/qapi-schema/meson.build b/tests/qapi-schema/meson.build index ba11cb76ac..664f9ee22d 100644 --- a/tests/qapi-schema/meson.build +++ b/tests/qapi-schema/meson.build @@ -152,9 +152,9 @@ schemas = [ 'pragma-returns-whitelist-crap.json', 'qapi-schema-test.json', 'quoted-structural-chars.json', - 'redefined-builtin.json', 'redefined-command.json', 'redefined-event.json', + 'redefined-predefined.json', 'redefined-type.json', 'reserved-command-q.json', 'reserved-enum-q.json', diff --git a/tests/qapi-schema/redefined-builtin.err b/tests/qapi-schema/redefined-builtin.err deleted file mode 100644 index 92bc62dc76..0000000000 --- a/tests/qapi-schema/redefined-builtin.err +++ /dev/null @@ -1,2 +0,0 @@ -redefined-builtin.json: In struct 'QType': -redefined-builtin.json:2: enum type 'QType' is already defined diff --git a/tests/qapi-schema/redefined-predefined.err b/tests/qapi-schema/redefined-predefined.err new file mode 100644 index 0000000000..2924dde60b --- /dev/null +++ b/tests/qapi-schema/redefined-predefined.err @@ -0,0 +1,2 @@ +redefined-predefined.json: In struct 'QType': +redefined-predefined.json:2: enum type 'QType' is already defined diff --git a/tests/qapi-schema/redefined-builtin.json b/tests/qapi-schema/redefined-predefined.json similarity index 100% rename from tests/qapi-schema/redefined-builtin.json rename to tests/qapi-schema/redefined-predefined.json diff --git a/tests/qapi-schema/redefined-builtin.out b/tests/qapi-schema/redefined-predefined.out similarity index 100% rename from tests/qapi-schema/redefined-builtin.out rename to tests/qapi-schema/redefined-predefined.out -- 2.26.3
Signed-off-by: Markus Armbruster <armbru@redhat.com> Message-Id: <20210323094025.3569441-17-armbru@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com> --- scripts/qapi/parser.py | 19 +++++++++---------- 1 file changed, 9 insertions(+), 10 deletions(-) diff --git a/scripts/qapi/parser.py b/scripts/qapi/parser.py index 116afe549a..8eed69333f 100644 --- a/scripts/qapi/parser.py +++ b/scripts/qapi/parser.py @@ -119,6 +119,13 @@ def _include(self, include, info, incl_fname, previously_included): return QAPISchemaParser(incl_fname, previously_included, info) + def _check_pragma_list_of_str(self, name, value, info): + if (not isinstance(value, list) + or any([not isinstance(elt, str) for elt in value])): + raise QAPISemError( + info, + "pragma %s must be a list of strings" % name) + def _pragma(self, name, value, info): if name == 'doc-required': if not isinstance(value, bool): @@ -126,18 +133,10 @@ def _pragma(self, name, value, info): "pragma 'doc-required' must be boolean") info.pragma.doc_required = value elif name == 'returns-whitelist': - if (not isinstance(value, list) - or any([not isinstance(elt, str) for elt in value])): - raise QAPISemError( - info, - "pragma returns-whitelist must be a list of strings") + self._check_pragma_list_of_str(name, value, info) info.pragma.returns_whitelist = value elif name == 'name-case-whitelist': - if (not isinstance(value, list) - or any([not isinstance(elt, str) for elt in value])): - raise QAPISemError( - info, - "pragma name-case-whitelist must be a list of strings") + self._check_pragma_list_of_str(name, value, info) info.pragma.name_case_whitelist = value else: raise QAPISemError(info, "unknown pragma '%s'" % name) -- 2.26.3
Rename pragma-doc-required-crap to pragma-not-bool, pragma-returns-whitelist-crap to pragma-value-not-list, and pragma-name-case-whitelist-crap to pragma-value-not-list-of-str. Signed-off-by: Markus Armbruster <armbru@redhat.com> Message-Id: <20210323094025.3569441-18-armbru@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com> --- tests/qapi-schema/meson.build | 6 +++--- tests/qapi-schema/pragma-doc-required-crap.err | 1 - tests/qapi-schema/pragma-name-case-whitelist-crap.err | 1 - tests/qapi-schema/pragma-returns-whitelist-crap.err | 1 - tests/qapi-schema/pragma-value-not-bool.err | 1 + ...ma-doc-required-crap.json => pragma-value-not-bool.json} | 2 +- ...agma-doc-required-crap.out => pragma-value-not-bool.out} | 0 tests/qapi-schema/pragma-value-not-list-of-str.err | 1 + ...hitelist-crap.json => pragma-value-not-list-of-str.json} | 2 +- ...-whitelist-crap.out => pragma-value-not-list-of-str.out} | 0 tests/qapi-schema/pragma-value-not-list.err | 1 + ...-case-whitelist-crap.json => pragma-value-not-list.json} | 2 +- ...returns-whitelist-crap.out => pragma-value-not-list.out} | 0 13 files changed, 9 insertions(+), 9 deletions(-) delete mode 100644 tests/qapi-schema/pragma-doc-required-crap.err delete mode 100644 tests/qapi-schema/pragma-name-case-whitelist-crap.err delete mode 100644 tests/qapi-schema/pragma-returns-whitelist-crap.err create mode 100644 tests/qapi-schema/pragma-value-not-bool.err rename tests/qapi-schema/{pragma-doc-required-crap.json => pragma-value-not-bool.json} (55%) rename tests/qapi-schema/{pragma-doc-required-crap.out => pragma-value-not-bool.out} (100%) create mode 100644 tests/qapi-schema/pragma-value-not-list-of-str.err rename tests/qapi-schema/{pragma-returns-whitelist-crap.json => pragma-value-not-list-of-str.json} (57%) rename tests/qapi-schema/{pragma-name-case-whitelist-crap.out => pragma-value-not-list-of-str.out} (100%) create mode 100644 tests/qapi-schema/pragma-value-not-list.err rename tests/qapi-schema/{pragma-name-case-whitelist-crap.json => pragma-value-not-list.json} (50%) rename tests/qapi-schema/{pragma-returns-whitelist-crap.out => pragma-value-not-list.out} (100%) diff --git a/tests/qapi-schema/meson.build b/tests/qapi-schema/meson.build index 664f9ee22d..ffc276b765 100644 --- a/tests/qapi-schema/meson.build +++ b/tests/qapi-schema/meson.build @@ -144,12 +144,12 @@ schemas = [ 'oob-coroutine.json', 'oob-test.json', 'allow-preconfig-test.json', - 'pragma-doc-required-crap.json', 'pragma-extra-junk.json', - 'pragma-name-case-whitelist-crap.json', 'pragma-non-dict.json', 'pragma-unknown.json', - 'pragma-returns-whitelist-crap.json', + 'pragma-value-not-bool.json', + 'pragma-value-not-list-of-str.json', + 'pragma-value-not-list.json', 'qapi-schema-test.json', 'quoted-structural-chars.json', 'redefined-command.json', diff --git a/tests/qapi-schema/pragma-doc-required-crap.err b/tests/qapi-schema/pragma-doc-required-crap.err deleted file mode 100644 index 717062cb14..0000000000 --- a/tests/qapi-schema/pragma-doc-required-crap.err +++ /dev/null @@ -1 +0,0 @@ -pragma-doc-required-crap.json:3: pragma 'doc-required' must be boolean diff --git a/tests/qapi-schema/pragma-name-case-whitelist-crap.err b/tests/qapi-schema/pragma-name-case-whitelist-crap.err deleted file mode 100644 index fbea90d6c5..0000000000 --- a/tests/qapi-schema/pragma-name-case-whitelist-crap.err +++ /dev/null @@ -1 +0,0 @@ -pragma-name-case-whitelist-crap.json:3: pragma name-case-whitelist must be a list of strings diff --git a/tests/qapi-schema/pragma-returns-whitelist-crap.err b/tests/qapi-schema/pragma-returns-whitelist-crap.err deleted file mode 100644 index 69784259df..0000000000 --- a/tests/qapi-schema/pragma-returns-whitelist-crap.err +++ /dev/null @@ -1 +0,0 @@ -pragma-returns-whitelist-crap.json:3: pragma returns-whitelist must be a list of strings diff --git a/tests/qapi-schema/pragma-value-not-bool.err b/tests/qapi-schema/pragma-value-not-bool.err new file mode 100644 index 0000000000..6247539616 --- /dev/null +++ b/tests/qapi-schema/pragma-value-not-bool.err @@ -0,0 +1 @@ +pragma-value-not-bool.json:3: pragma 'doc-required' must be boolean diff --git a/tests/qapi-schema/pragma-doc-required-crap.json b/tests/qapi-schema/pragma-value-not-bool.json similarity index 55% rename from tests/qapi-schema/pragma-doc-required-crap.json rename to tests/qapi-schema/pragma-value-not-bool.json index ed763c5ffc..feb489f15b 100644 --- a/tests/qapi-schema/pragma-doc-required-crap.json +++ b/tests/qapi-schema/pragma-value-not-bool.json @@ -1,3 +1,3 @@ -# 'doc-required' must be bool +# pragma value must be bool { 'pragma': { 'doc-required': {} } } diff --git a/tests/qapi-schema/pragma-doc-required-crap.out b/tests/qapi-schema/pragma-value-not-bool.out similarity index 100% rename from tests/qapi-schema/pragma-doc-required-crap.out rename to tests/qapi-schema/pragma-value-not-bool.out diff --git a/tests/qapi-schema/pragma-value-not-list-of-str.err b/tests/qapi-schema/pragma-value-not-list-of-str.err new file mode 100644 index 0000000000..44870fe5d9 --- /dev/null +++ b/tests/qapi-schema/pragma-value-not-list-of-str.err @@ -0,0 +1 @@ +pragma-value-not-list-of-str.json:3: pragma returns-whitelist must be a list of strings diff --git a/tests/qapi-schema/pragma-returns-whitelist-crap.json b/tests/qapi-schema/pragma-value-not-list-of-str.json similarity index 57% rename from tests/qapi-schema/pragma-returns-whitelist-crap.json rename to tests/qapi-schema/pragma-value-not-list-of-str.json index f6b81b093f..12bbbed2e8 100644 --- a/tests/qapi-schema/pragma-returns-whitelist-crap.json +++ b/tests/qapi-schema/pragma-value-not-list-of-str.json @@ -1,3 +1,3 @@ -# 'returns-whitelist' must be list of strings +# pragma value must be list of strings { 'pragma': { 'returns-whitelist': [ 'good', [ 'bad' ] ] } } diff --git a/tests/qapi-schema/pragma-name-case-whitelist-crap.out b/tests/qapi-schema/pragma-value-not-list-of-str.out similarity index 100% rename from tests/qapi-schema/pragma-name-case-whitelist-crap.out rename to tests/qapi-schema/pragma-value-not-list-of-str.out diff --git a/tests/qapi-schema/pragma-value-not-list.err b/tests/qapi-schema/pragma-value-not-list.err new file mode 100644 index 0000000000..21736c5723 --- /dev/null +++ b/tests/qapi-schema/pragma-value-not-list.err @@ -0,0 +1 @@ +pragma-value-not-list.json:3: pragma name-case-whitelist must be a list of strings diff --git a/tests/qapi-schema/pragma-name-case-whitelist-crap.json b/tests/qapi-schema/pragma-value-not-list.json similarity index 50% rename from tests/qapi-schema/pragma-name-case-whitelist-crap.json rename to tests/qapi-schema/pragma-value-not-list.json index 734e1c617b..2c61a97dd3 100644 --- a/tests/qapi-schema/pragma-name-case-whitelist-crap.json +++ b/tests/qapi-schema/pragma-value-not-list.json @@ -1,3 +1,3 @@ -# 'name-case-whitelist' must be list of strings +# pragma value must be list { 'pragma': { 'name-case-whitelist': false } } diff --git a/tests/qapi-schema/pragma-returns-whitelist-crap.out b/tests/qapi-schema/pragma-value-not-list.out similarity index 100% rename from tests/qapi-schema/pragma-returns-whitelist-crap.out rename to tests/qapi-schema/pragma-value-not-list.out -- 2.26.3
This test covers returning "bad" types. Pragma returns-whitelist is just one aspect. Naming it returns-whitelist is suboptimal. Rename to returns-bad-type. Signed-off-by: Markus Armbruster <armbru@redhat.com> Message-Id: <20210323094025.3569441-19-armbru@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com> --- tests/qapi-schema/meson.build | 2 +- tests/qapi-schema/returns-bad-type.err | 2 ++ .../{returns-whitelist.json => returns-bad-type.json} | 0 .../qapi-schema/{returns-whitelist.out => returns-bad-type.out} | 0 tests/qapi-schema/returns-whitelist.err | 2 -- 5 files changed, 3 insertions(+), 3 deletions(-) create mode 100644 tests/qapi-schema/returns-bad-type.err rename tests/qapi-schema/{returns-whitelist.json => returns-bad-type.json} (100%) rename tests/qapi-schema/{returns-whitelist.out => returns-bad-type.out} (100%) delete mode 100644 tests/qapi-schema/returns-whitelist.err diff --git a/tests/qapi-schema/meson.build b/tests/qapi-schema/meson.build index ffc276b765..4e7635f0a8 100644 --- a/tests/qapi-schema/meson.build +++ b/tests/qapi-schema/meson.build @@ -166,9 +166,9 @@ schemas = [ 'reserved-type-list.json', 'returns-alternate.json', 'returns-array-bad.json', + 'returns-bad-type.json', 'returns-dict.json', 'returns-unknown.json', - 'returns-whitelist.json', 'string-code-point-31.json', 'string-code-point-127.json', 'struct-base-clash-deep.json', diff --git a/tests/qapi-schema/returns-bad-type.err b/tests/qapi-schema/returns-bad-type.err new file mode 100644 index 0000000000..2c270de9ad --- /dev/null +++ b/tests/qapi-schema/returns-bad-type.err @@ -0,0 +1,2 @@ +returns-bad-type.json: In command 'no-way-this-will-get-whitelisted': +returns-bad-type.json:14: command's 'returns' cannot take array type ['int'] diff --git a/tests/qapi-schema/returns-whitelist.json b/tests/qapi-schema/returns-bad-type.json similarity index 100% rename from tests/qapi-schema/returns-whitelist.json rename to tests/qapi-schema/returns-bad-type.json diff --git a/tests/qapi-schema/returns-whitelist.out b/tests/qapi-schema/returns-bad-type.out similarity index 100% rename from tests/qapi-schema/returns-whitelist.out rename to tests/qapi-schema/returns-bad-type.out diff --git a/tests/qapi-schema/returns-whitelist.err b/tests/qapi-schema/returns-whitelist.err deleted file mode 100644 index c6e46b9b86..0000000000 --- a/tests/qapi-schema/returns-whitelist.err +++ /dev/null @@ -1,2 +0,0 @@ -returns-whitelist.json: In command 'no-way-this-will-get-whitelisted': -returns-whitelist.json:14: command's 'returns' cannot take array type ['int'] -- 2.26.3
Rename pragma returns-whitelist to command-returns-exceptions, and name-case-whitelist to member-name-case-exceptions. Signed-off-by: Markus Armbruster <armbru@redhat.com> Message-Id: <20210323094025.3569441-20-armbru@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com> --- docs/devel/qapi-code-gen.txt | 21 ++++++++++--------- qapi/pragma.json | 4 ++-- qga/qapi-schema.json | 2 +- scripts/qapi/expr.py | 4 ++-- scripts/qapi/parser.py | 8 +++---- scripts/qapi/schema.py | 2 +- scripts/qapi/source.py | 8 +++---- tests/qapi-schema/enum-member-case.json | 2 +- .../pragma-value-not-list-of-str.err | 2 +- .../pragma-value-not-list-of-str.json | 2 +- tests/qapi-schema/pragma-value-not-list.err | 2 +- tests/qapi-schema/pragma-value-not-list.json | 3 +-- tests/qapi-schema/qapi-schema-test.json | 2 +- tests/qapi-schema/returns-bad-type.json | 2 +- 14 files changed, 32 insertions(+), 32 deletions(-) diff --git a/docs/devel/qapi-code-gen.txt b/docs/devel/qapi-code-gen.txt index 6906a06ad2..23e9823019 100644 --- a/docs/devel/qapi-code-gen.txt +++ b/docs/devel/qapi-code-gen.txt @@ -147,9 +147,10 @@ prevent incomplete include files. === Pragma directives === Syntax: - PRAGMA = { 'pragma': { '*doc-required': BOOL, - '*returns-whitelist': [ STRING, ... ], - '*name-case-whitelist': [ STRING, ... ] } } + PRAGMA = { 'pragma': { + '*doc-required': BOOL, + '*command-returns-exceptions': [ STRING, ... ], + '*member-name-exceptions': [ STRING, ... ] } } The pragma directive lets you control optional generator behavior. @@ -159,11 +160,11 @@ pragma to different values in parts of the schema doesn't work. Pragma 'doc-required' takes a boolean value. If true, documentation is required. Default is false. -Pragma 'returns-whitelist' takes a list of command names that may +Pragma 'command-returns-exceptions' takes a list of commands that may violate the rules on permitted return types. Default is none. -Pragma 'name-case-whitelist' takes a list of names that may violate -rules on use of upper- vs. lower-case letters. Default is none. +Pragma 'member-name-exceptions' takes a list of types whose member +names may contain uppercase letters. Default is none. === Enumeration types === @@ -490,9 +491,9 @@ are the arguments. A union type requires 'boxed': true. Member 'returns' defines the command's return type. It defaults to an empty struct type. It must normally be a complex type or an array of a complex type. To return anything else, the command must be listed -in pragma 'returns-whitelist'. If you do this, extending the command -to return additional information will be harder. Use of -'returns-whitelist' for new commands is strongly discouraged. +in pragma 'commands-returns-exceptions'. If you do this, extending +the command to return additional information will be harder. Use of +the pragma for new commands is strongly discouraged. A command's error responses are not specified in the QAPI schema. Error conditions should be documented in comments. @@ -755,7 +756,7 @@ Any name (command, event, type, member, or enum value) beginning with "x-" is marked experimental, and may be withdrawn or changed incompatibly in a future release. -Pragma 'name-case-whitelist' lets you violate the rules on use of +Pragma 'member-name-exceptions' lets you violate the rules on use of upper and lower case. Use for new code is strongly discouraged. diff --git a/qapi/pragma.json b/qapi/pragma.json index 7f158e183d..4895848c5e 100644 --- a/qapi/pragma.json +++ b/qapi/pragma.json @@ -4,13 +4,13 @@ # add to them! { 'pragma': { # Commands allowed to return a non-dictionary: - 'returns-whitelist': [ + 'command-returns-exceptions': [ 'human-monitor-command', 'qom-get', 'query-tpm-models', 'query-tpm-types', 'ringbuf-read' ], - 'name-case-whitelist': [ + 'member-name-exceptions': [ 'ACPISlotType', # DIMM, visible through query-acpi-ospm-status 'BlockdevVmdkSubformat', # all members, to match VMDK spec spellings 'BlockdevVmdkAdapterType', # legacyESX, to match VMDK spec spellings diff --git a/qga/qapi-schema.json b/qga/qapi-schema.json index 9a82b7e952..2b08b761c2 100644 --- a/qga/qapi-schema.json +++ b/qga/qapi-schema.json @@ -20,7 +20,7 @@ # add to them! { 'pragma': { # Commands allowed to return a non-dictionary: - 'returns-whitelist': [ + 'command-returns-exceptions': [ 'guest-file-open', 'guest-fsfreeze-freeze', 'guest-fsfreeze-freeze-list', diff --git a/scripts/qapi/expr.py b/scripts/qapi/expr.py index 7bd15559de..85493efbac 100644 --- a/scripts/qapi/expr.py +++ b/scripts/qapi/expr.py @@ -181,7 +181,7 @@ def check_type(value, info, source, raise QAPISemError(info, "%s should be an object or type name" % source) - permit_upper = allow_dict in info.pragma.name_case_whitelist + permit_upper = allow_dict in info.pragma.member_name_exceptions # value is a dictionary, check that each member is okay for (key, arg) in value.items(): @@ -224,7 +224,7 @@ def check_enum(expr, info): if prefix is not None and not isinstance(prefix, str): raise QAPISemError(info, "'prefix' must be a string") - permit_upper = name in info.pragma.name_case_whitelist + permit_upper = name in info.pragma.member_name_exceptions members[:] = [m if isinstance(m, dict) else {'name': m} for m in members] diff --git a/scripts/qapi/parser.py b/scripts/qapi/parser.py index 8eed69333f..c16b8b6995 100644 --- a/scripts/qapi/parser.py +++ b/scripts/qapi/parser.py @@ -132,12 +132,12 @@ def _pragma(self, name, value, info): raise QAPISemError(info, "pragma 'doc-required' must be boolean") info.pragma.doc_required = value - elif name == 'returns-whitelist': + elif name == 'command-returns-exceptions': self._check_pragma_list_of_str(name, value, info) - info.pragma.returns_whitelist = value - elif name == 'name-case-whitelist': + info.pragma.command_returns_exceptions = value + elif name == 'member-name-exceptions': self._check_pragma_list_of_str(name, value, info) - info.pragma.name_case_whitelist = value + info.pragma.member_name_exceptions = value else: raise QAPISemError(info, "unknown pragma '%s'" % name) diff --git a/scripts/qapi/schema.py b/scripts/qapi/schema.py index ff16578f6d..703b446fd2 100644 --- a/scripts/qapi/schema.py +++ b/scripts/qapi/schema.py @@ -779,7 +779,7 @@ def check(self, schema): if self._ret_type_name: self.ret_type = schema.resolve_type( self._ret_type_name, self.info, "command's 'returns'") - if self.name not in self.info.pragma.returns_whitelist: + if self.name not in self.info.pragma.command_returns_exceptions: typ = self.ret_type if isinstance(typ, QAPISchemaArrayType): typ = self.ret_type.element_type diff --git a/scripts/qapi/source.py b/scripts/qapi/source.py index d7a79a9b8a..d8f9ec377f 100644 --- a/scripts/qapi/source.py +++ b/scripts/qapi/source.py @@ -21,10 +21,10 @@ class QAPISchemaPragma: def __init__(self) -> None: # Are documentation comments required? self.doc_required = False - # Whitelist of commands allowed to return a non-dictionary - self.returns_whitelist: List[str] = [] - # Whitelist of entities allowed to violate case conventions - self.name_case_whitelist: List[str] = [] + # Commands allowed to return a non-dictionary + self.command_returns_exceptions: List[str] = [] + # Types whose member names may violate case conventions + self.member_name_exceptions: List[str] = [] class QAPISourceInfo: diff --git a/tests/qapi-schema/enum-member-case.json b/tests/qapi-schema/enum-member-case.json index f8af3e4622..b8969d224d 100644 --- a/tests/qapi-schema/enum-member-case.json +++ b/tests/qapi-schema/enum-member-case.json @@ -1,4 +1,4 @@ # Member names should be 'lower-case' unless the enum is whitelisted -{ 'pragma': { 'name-case-whitelist': [ 'UuidInfo' ] } } +{ 'pragma': { 'member-name-exceptions': [ 'UuidInfo' ] } } { 'enum': 'UuidInfo', 'data': [ 'Value' ] } # UuidInfo is whitelisted { 'enum': 'NoWayThisWillGetWhitelisted', 'data': [ 'Value' ] } diff --git a/tests/qapi-schema/pragma-value-not-list-of-str.err b/tests/qapi-schema/pragma-value-not-list-of-str.err index 44870fe5d9..b81983dd85 100644 --- a/tests/qapi-schema/pragma-value-not-list-of-str.err +++ b/tests/qapi-schema/pragma-value-not-list-of-str.err @@ -1 +1 @@ -pragma-value-not-list-of-str.json:3: pragma returns-whitelist must be a list of strings +pragma-value-not-list-of-str.json:3: pragma command-returns-exceptions must be a list of strings diff --git a/tests/qapi-schema/pragma-value-not-list-of-str.json b/tests/qapi-schema/pragma-value-not-list-of-str.json index 12bbbed2e8..27811cff73 100644 --- a/tests/qapi-schema/pragma-value-not-list-of-str.json +++ b/tests/qapi-schema/pragma-value-not-list-of-str.json @@ -1,3 +1,3 @@ # pragma value must be list of strings -{ 'pragma': { 'returns-whitelist': [ 'good', [ 'bad' ] ] } } +{ 'pragma': { 'command-returns-exceptions': [ 'good', [ 'bad' ] ] } } diff --git a/tests/qapi-schema/pragma-value-not-list.err b/tests/qapi-schema/pragma-value-not-list.err index 21736c5723..45dde1bf05 100644 --- a/tests/qapi-schema/pragma-value-not-list.err +++ b/tests/qapi-schema/pragma-value-not-list.err @@ -1 +1 @@ -pragma-value-not-list.json:3: pragma name-case-whitelist must be a list of strings +pragma-value-not-list.json:2: pragma member-name-exceptions must be a list of strings diff --git a/tests/qapi-schema/pragma-value-not-list.json b/tests/qapi-schema/pragma-value-not-list.json index 2c61a97dd3..3faa340c3b 100644 --- a/tests/qapi-schema/pragma-value-not-list.json +++ b/tests/qapi-schema/pragma-value-not-list.json @@ -1,3 +1,2 @@ # pragma value must be list - -{ 'pragma': { 'name-case-whitelist': false } } +{ 'pragma': { 'member-name-exceptions': false } } diff --git a/tests/qapi-schema/qapi-schema-test.json b/tests/qapi-schema/qapi-schema-test.json index a355321258..16b03bf308 100644 --- a/tests/qapi-schema/qapi-schema-test.json +++ b/tests/qapi-schema/qapi-schema-test.json @@ -7,7 +7,7 @@ # Whitelists to permit QAPI rule violations { 'pragma': { # Commands allowed to return a non-dictionary: - 'returns-whitelist': [ + 'command-returns-exceptions': [ 'guest-get-time', 'guest-sync' ] } } diff --git a/tests/qapi-schema/returns-bad-type.json b/tests/qapi-schema/returns-bad-type.json index da209329b1..0dd96e739e 100644 --- a/tests/qapi-schema/returns-bad-type.json +++ b/tests/qapi-schema/returns-bad-type.json @@ -1,6 +1,6 @@ # we enforce that 'returns' be a dict or array of dict unless whitelisted -{ 'pragma': { 'returns-whitelist': [ +{ 'pragma': { 'command-returns-exceptions': [ 'human-monitor-command', 'query-tpm-models', 'guest-get-time' ] } } { 'command': 'human-monitor-command', -- 2.26.3
Signed-off-by: Markus Armbruster <armbru@redhat.com> Message-Id: <20210323094025.3569441-21-armbru@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com> --- qapi/pragma.json | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/qapi/pragma.json b/qapi/pragma.json index 4895848c5e..4c47c802d1 100644 --- a/qapi/pragma.json +++ b/qapi/pragma.json @@ -10,11 +10,13 @@ 'query-tpm-models', 'query-tpm-types', 'ringbuf-read' ], - 'member-name-exceptions': [ - 'ACPISlotType', # DIMM, visible through query-acpi-ospm-status - 'BlockdevVmdkSubformat', # all members, to match VMDK spec spellings - 'BlockdevVmdkAdapterType', # legacyESX, to match VMDK spec spellings - 'QapiErrorClass', # all members, visible through errors - 'UuidInfo', # UUID, visible through query-uuid - 'X86CPURegister32' # all members, visible indirectly through qom-get + # Externally visible types whose member names may use uppercase + 'member-name-exceptions': [ # visible in: + 'ACPISlotType', # query-acpi-ospm-status + 'BlockdevVmdkAdapterType', # blockdev-create (to match VMDK spec) + 'BlockdevVmdkSubformat', # blockdev-create (to match VMDK spec) + 'QapiErrorClass', # QMP error replies + 'UuidInfo', # query-uuid + 'X86CPURegister32' # qom-get of x86 CPU properties + # feature-words, filtered-features ] } } -- 2.26.3
Commit 967c885108 "qapi: add 'if' to top-level expressions" added command TestIfCmd with an 'if' condition. It also added the qmp_TestIfCmd() to go with it, guarded by the corresponding #if. Commit ccadd6bcba "qapi: Add 'if' to implicit struct members" changed the command, but not the function. Compiles only because we don't satisfy the #if. Instead of fixing the function, simply drop it. Signed-off-by: Markus Armbruster <armbru@redhat.com> Message-Id: <20210323094025.3569441-22-armbru@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com> --- tests/unit/test-qmp-cmds.c | 7 ------- 1 file changed, 7 deletions(-) diff --git a/tests/unit/test-qmp-cmds.c b/tests/unit/test-qmp-cmds.c index 266db074b4..99973dde7c 100644 --- a/tests/unit/test-qmp-cmds.c +++ b/tests/unit/test-qmp-cmds.c @@ -13,13 +13,6 @@ static QmpCommandList qmp_commands; -#if defined(TEST_IF_STRUCT) && defined(TEST_IF_CMD) -UserDefThree *qmp_TestIfCmd(TestIfStruct *foo, Error **errp) -{ - return NULL; -} -#endif - UserDefThree *qmp_TestCmdReturnDefThree(Error **errp) { return NULL; -- 2.26.3
Command names and member names within a type should be all lower case with words separated by a hyphen. We also accept underscore. Rework check_name_lower() to optionally reject underscores, but don't use that option, yet. Update expected test output for the changed error message. Signed-off-by: Markus Armbruster <armbru@redhat.com> Message-Id: <20210323094025.3569441-23-armbru@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com> --- scripts/qapi/expr.py | 24 ++++++++++++++---------- tests/qapi-schema/args-member-case.err | 2 +- tests/qapi-schema/enum-member-case.err | 2 +- tests/qapi-schema/union-branch-case.err | 2 +- 4 files changed, 17 insertions(+), 13 deletions(-) diff --git a/scripts/qapi/expr.py b/scripts/qapi/expr.py index 85493efbac..e416cc5162 100644 --- a/scripts/qapi/expr.py +++ b/scripts/qapi/expr.py @@ -51,12 +51,13 @@ def check_name_upper(name, info, source): def check_name_lower(name, info, source, - permit_upper=False): + permit_upper=False, + permit_underscore=False): stem = check_name_str(name, info, source) - if not permit_upper and re.search(r'[A-Z]', stem): + if ((not permit_upper and re.search(r'[A-Z]', stem)) + or (not permit_underscore and '_' in stem)): raise QAPISemError( - info, "%s uses uppercase in name" % source) - # TODO reject '_' in stem + info, "name of %s must not use uppercase or '_'" % source) def check_name_camel(name, info, source): @@ -69,7 +70,8 @@ def check_defn_name_str(name, info, meta): if meta == 'event': check_name_upper(name, info, meta) elif meta == 'command': - check_name_lower(name, info, meta, permit_upper=True) + check_name_lower(name, info, meta, + permit_upper=True, permit_underscore=True) else: check_name_camel(name, info, meta) if name.endswith('Kind') or name.endswith('List'): @@ -188,7 +190,8 @@ def check_type(value, info, source, key_source = "%s member '%s'" % (source, key) if key.startswith('*'): key = key[1:] - check_name_lower(key, info, key_source, permit_upper) + check_name_lower(key, info, key_source, + permit_upper, permit_underscore=True) if c_name(key, False) == 'u' or c_name(key, False).startswith('has_'): raise QAPISemError(info, "%s uses reserved name" % key_source) check_keys(arg, info, key_source, ['type'], ['if', 'features']) @@ -210,7 +213,7 @@ def check_features(features, info): check_keys(f, info, source, ['name'], ['if']) check_name_is_str(f['name'], info, source) source = "%s '%s'" % (source, f['name']) - check_name_lower(f['name'], info, source) + check_name_lower(f['name'], info, source, permit_underscore=True) check_if(f, info, source) @@ -237,7 +240,8 @@ def check_enum(expr, info): # Enum members may start with a digit if member_name[0].isdigit(): member_name = 'd' + member_name # Hack: hide the digit - check_name_lower(member_name, info, source, permit_upper) + check_name_lower(member_name, info, source, + permit_upper, permit_underscore=True) check_if(member, info, source) @@ -267,7 +271,7 @@ def check_union(expr, info): for (key, value) in members.items(): source = "'data' member '%s'" % key if discriminator is None: - check_name_lower(key, info, source) + check_name_lower(key, info, source, permit_underscore=True) # else: name is in discriminator enum, which gets checked check_keys(value, info, source, ['type'], ['if']) check_if(value, info, source) @@ -281,7 +285,7 @@ def check_alternate(expr, info): raise QAPISemError(info, "'data' must not be empty") for (key, value) in members.items(): source = "'data' member '%s'" % key - check_name_lower(key, info, source) + check_name_lower(key, info, source, permit_underscore=True) check_keys(value, info, source, ['type'], ['if']) check_if(value, info, source) check_type(value['type'], info, source) diff --git a/tests/qapi-schema/args-member-case.err b/tests/qapi-schema/args-member-case.err index 4f33dbbc38..25c3910a17 100644 --- a/tests/qapi-schema/args-member-case.err +++ b/tests/qapi-schema/args-member-case.err @@ -1,2 +1,2 @@ args-member-case.json: In command 'no-way-this-will-get-whitelisted': -args-member-case.json:2: 'data' member 'Arg' uses uppercase in name +args-member-case.json:2: name of 'data' member 'Arg' must not use uppercase or '_' diff --git a/tests/qapi-schema/enum-member-case.err b/tests/qapi-schema/enum-member-case.err index 8b3aefe37a..c813522c50 100644 --- a/tests/qapi-schema/enum-member-case.err +++ b/tests/qapi-schema/enum-member-case.err @@ -1,2 +1,2 @@ enum-member-case.json: In enum 'NoWayThisWillGetWhitelisted': -enum-member-case.json:4: 'data' member 'Value' uses uppercase in name +enum-member-case.json:4: name of 'data' member 'Value' must not use uppercase or '_' diff --git a/tests/qapi-schema/union-branch-case.err b/tests/qapi-schema/union-branch-case.err index b1e9417303..d2d5cb8993 100644 --- a/tests/qapi-schema/union-branch-case.err +++ b/tests/qapi-schema/union-branch-case.err @@ -1,2 +1,2 @@ union-branch-case.json: In union 'Uni': -union-branch-case.json:2: 'data' member 'Branch' uses uppercase in name +union-branch-case.json:2: name of 'data' member 'Branch' must not use uppercase or '_' -- 2.26.3
Feature names should use '-', not '_'. Enforce this. Signed-off-by: Markus Armbruster <armbru@redhat.com> Message-Id: <20210323094025.3569441-24-armbru@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com> --- scripts/qapi/expr.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/scripts/qapi/expr.py b/scripts/qapi/expr.py index e416cc5162..d778107c18 100644 --- a/scripts/qapi/expr.py +++ b/scripts/qapi/expr.py @@ -213,7 +213,7 @@ def check_features(features, info): check_keys(f, info, source, ['name'], ['if']) check_name_is_str(f['name'], info, source) source = "%s '%s'" % (source, f['name']) - check_name_lower(f['name'], info, source, permit_underscore=True) + check_name_lower(f['name'], info, source) check_if(f, info, source) -- 2.26.3
Command names should be lower-case. Enforce this. Fix the fixable offenders (all in tests/), and add the remainder to pragma command-name-exceptions. Signed-off-by: Markus Armbruster <armbru@redhat.com> Message-Id: <20210323094025.3569441-25-armbru@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com> --- docs/devel/qapi-code-gen.txt | 8 ++++++-- qapi/pragma.json | 18 +++++++++++++++++ tests/unit/test-qmp-cmds.c | 10 +++++----- scripts/qapi/expr.py | 5 +++-- scripts/qapi/parser.py | 3 +++ scripts/qapi/source.py | 2 ++ tests/qapi-schema/qapi-schema-test.json | 21 +++++++++++--------- tests/qapi-schema/qapi-schema-test.out | 26 ++++++++++++------------- 8 files changed, 62 insertions(+), 31 deletions(-) diff --git a/docs/devel/qapi-code-gen.txt b/docs/devel/qapi-code-gen.txt index 23e9823019..10e9a0f829 100644 --- a/docs/devel/qapi-code-gen.txt +++ b/docs/devel/qapi-code-gen.txt @@ -149,6 +149,7 @@ prevent incomplete include files. Syntax: PRAGMA = { 'pragma': { '*doc-required': BOOL, + '*command-name-exceptions': [ STRING, ... ], '*command-returns-exceptions': [ STRING, ... ], '*member-name-exceptions': [ STRING, ... ] } } @@ -160,6 +161,9 @@ pragma to different values in parts of the schema doesn't work. Pragma 'doc-required' takes a boolean value. If true, documentation is required. Default is false. +Pragma 'command-name-exceptions' takes a list of commands whose names +may contain '_' instead of '-'. Default is none. + Pragma 'command-returns-exceptions' takes a list of commands that may violate the rules on permitted return types. Default is none. @@ -756,8 +760,8 @@ Any name (command, event, type, member, or enum value) beginning with "x-" is marked experimental, and may be withdrawn or changed incompatibly in a future release. -Pragma 'member-name-exceptions' lets you violate the rules on use of -upper and lower case. Use for new code is strongly discouraged. +Pragmas 'command-name-exceptions' and 'member-name-exceptions' let you +violate naming rules. Use for new code is strongly discouraged. === Downstream extensions === diff --git a/qapi/pragma.json b/qapi/pragma.json index 4c47c802d1..339f067943 100644 --- a/qapi/pragma.json +++ b/qapi/pragma.json @@ -4,6 +4,24 @@ # add to them! { 'pragma': { # Commands allowed to return a non-dictionary: + 'command-name-exceptions': [ + 'add_client', + 'block_passwd', + 'block_resize', + 'block_set_io_throttle', + 'client_migrate_info', + 'device_add', + 'device_del', + 'expire_password', + 'migrate_cancel', + 'netdev_add', + 'netdev_del', + 'qmp_capabilities', + 'set_link', + 'set_password', + 'system_powerdown', + 'system_reset', + 'system_wakeup' ], 'command-returns-exceptions': [ 'human-monitor-command', 'qom-get', diff --git a/tests/unit/test-qmp-cmds.c b/tests/unit/test-qmp-cmds.c index 99973dde7c..1b0b7d99df 100644 --- a/tests/unit/test-qmp-cmds.c +++ b/tests/unit/test-qmp-cmds.c @@ -13,7 +13,7 @@ static QmpCommandList qmp_commands; -UserDefThree *qmp_TestCmdReturnDefThree(Error **errp) +UserDefThree *qmp_test_cmd_return_def_three(Error **errp) { return NULL; } @@ -199,7 +199,7 @@ static void test_dispatch_cmd(void) ret = qobject_to(QDict, do_qmp_dispatch(false, - "{ 'execute': 'user_def_cmd' }")); + "{ 'execute': 'user-def-cmd' }")); assert(ret && qdict_size(ret) == 0); qobject_unref(ret); } @@ -220,11 +220,11 @@ static void test_dispatch_cmd_failure(void) { /* missing arguments */ do_qmp_dispatch_error(false, ERROR_CLASS_GENERIC_ERROR, - "{ 'execute': 'user_def_cmd2' }"); + "{ 'execute': 'user-def-cmd2' }"); /* extra arguments */ do_qmp_dispatch_error(false, ERROR_CLASS_GENERIC_ERROR, - "{ 'execute': 'user_def_cmd'," + "{ 'execute': 'user-def-cmd'," " 'arguments': { 'a': 66 } }"); } @@ -248,7 +248,7 @@ static void test_dispatch_cmd_io(void) int64_t val; ret = qobject_to(QDict, do_qmp_dispatch(false, - "{ 'execute': 'user_def_cmd2', 'arguments': {" + "{ 'execute': 'user-def-cmd2', 'arguments': {" " 'ud1a': { 'integer': 42, 'string': 'hello' }," " 'ud1b': { 'integer': 422, 'string': 'hello2' } } }")); diff --git a/scripts/qapi/expr.py b/scripts/qapi/expr.py index d778107c18..9193e68763 100644 --- a/scripts/qapi/expr.py +++ b/scripts/qapi/expr.py @@ -70,8 +70,9 @@ def check_defn_name_str(name, info, meta): if meta == 'event': check_name_upper(name, info, meta) elif meta == 'command': - check_name_lower(name, info, meta, - permit_upper=True, permit_underscore=True) + check_name_lower( + name, info, meta, + permit_underscore=name in info.pragma.command_name_exceptions) else: check_name_camel(name, info, meta) if name.endswith('Kind') or name.endswith('List'): diff --git a/scripts/qapi/parser.py b/scripts/qapi/parser.py index c16b8b6995..58267c3db9 100644 --- a/scripts/qapi/parser.py +++ b/scripts/qapi/parser.py @@ -132,6 +132,9 @@ def _pragma(self, name, value, info): raise QAPISemError(info, "pragma 'doc-required' must be boolean") info.pragma.doc_required = value + elif name == 'command-name-exceptions': + self._check_pragma_list_of_str(name, value, info) + info.pragma.command_name_exceptions = value elif name == 'command-returns-exceptions': self._check_pragma_list_of_str(name, value, info) info.pragma.command_returns_exceptions = value diff --git a/scripts/qapi/source.py b/scripts/qapi/source.py index d8f9ec377f..03b6ede082 100644 --- a/scripts/qapi/source.py +++ b/scripts/qapi/source.py @@ -21,6 +21,8 @@ class QAPISchemaPragma: def __init__(self) -> None: # Are documentation comments required? self.doc_required = False + # Commands whose names may use '_' + self.command_name_exceptions: List[str] = [] # Commands allowed to return a non-dictionary self.command_returns_exceptions: List[str] = [] # Types whose member names may violate case conventions diff --git a/tests/qapi-schema/qapi-schema-test.json b/tests/qapi-schema/qapi-schema-test.json index 16b03bf308..e635db4a35 100644 --- a/tests/qapi-schema/qapi-schema-test.json +++ b/tests/qapi-schema/qapi-schema-test.json @@ -31,7 +31,7 @@ 'base': { 'type': 'EnumOne' }, 'discriminator': 'type', 'data': { } } -{ 'command': 'user_def_cmd0', 'data': 'Empty2', 'returns': 'Empty2' } +{ 'command': 'user-def-cmd0', 'data': 'Empty2', 'returns': 'Empty2' } # for testing override of default naming heuristic { 'enum': 'QEnumTwo', @@ -141,9 +141,9 @@ { 'include': 'include/sub-module.json' } # testing commands -{ 'command': 'user_def_cmd', 'data': {} } -{ 'command': 'user_def_cmd1', 'data': {'ud1a': 'UserDefOne'} } -{ 'command': 'user_def_cmd2', +{ 'command': 'user-def-cmd', 'data': {} } +{ 'command': 'user-def-cmd1', 'data': {'ud1a': 'UserDefOne'} } +{ 'command': 'user-def-cmd2', 'data': {'ud1a': {'type': 'UserDefOne'}, '*ud1b': 'UserDefOne'}, 'returns': 'UserDefTwo' } @@ -230,7 +230,8 @@ 'union_bar': { 'type': 'str', 'if': 'defined(TEST_IF_UNION_BAR)'} }, 'if': 'defined(TEST_IF_UNION) && defined(TEST_IF_STRUCT)' } -{ 'command': 'TestIfUnionCmd', 'data': { 'union_cmd_arg': 'TestIfUnion' }, +{ 'command': 'test-if-union-cmd', + 'data': { 'union_cmd_arg': 'TestIfUnion' }, 'if': 'defined(TEST_IF_UNION)' } { 'alternate': 'TestIfAlternate', 'data': @@ -238,16 +239,18 @@ 'bar': { 'type': 'TestStruct', 'if': 'defined(TEST_IF_ALT_BAR)'} }, 'if': 'defined(TEST_IF_ALT) && defined(TEST_IF_STRUCT)' } -{ 'command': 'TestIfAlternateCmd', 'data': { 'alt_cmd_arg': 'TestIfAlternate' }, +{ 'command': 'test-if-alternate-cmd', + 'data': { 'alt_cmd_arg': 'TestIfAlternate' }, 'if': 'defined(TEST_IF_ALT)' } -{ 'command': 'TestIfCmd', 'data': - { 'foo': 'TestIfStruct', +{ 'command': 'test-if-cmd', + 'data': { + 'foo': 'TestIfStruct', 'bar': { 'type': 'TestIfEnum', 'if': 'defined(TEST_IF_CMD_BAR)' } }, 'returns': 'UserDefThree', 'if': ['defined(TEST_IF_CMD)', 'defined(TEST_IF_STRUCT)'] } -{ 'command': 'TestCmdReturnDefThree', 'returns': 'UserDefThree' } +{ 'command': 'test-cmd-return-def-three', 'returns': 'UserDefThree' } { 'event': 'TEST_IF_EVENT', 'data': { 'foo': 'TestIfStruct', diff --git a/tests/qapi-schema/qapi-schema-test.out b/tests/qapi-schema/qapi-schema-test.out index 882d0e7c56..3f1ea345fd 100644 --- a/tests/qapi-schema/qapi-schema-test.out +++ b/tests/qapi-schema/qapi-schema-test.out @@ -32,7 +32,7 @@ object Union case value2: q_empty case value3: q_empty case value4: q_empty -command user_def_cmd0 Empty2 -> Empty2 +command user-def-cmd0 Empty2 -> Empty2 gen=True success_response=True boxed=False oob=False preconfig=False enum QEnumTwo prefix QENUM_TWO @@ -190,16 +190,16 @@ object UserDefListUnion case any: q_obj_anyList-wrapper case user: q_obj_StatusList-wrapper include include/sub-module.json -command user_def_cmd None -> None +command user-def-cmd None -> None gen=True success_response=True boxed=False oob=False preconfig=False -object q_obj_user_def_cmd1-arg +object q_obj_user-def-cmd1-arg member ud1a: UserDefOne optional=False -command user_def_cmd1 q_obj_user_def_cmd1-arg -> None +command user-def-cmd1 q_obj_user-def-cmd1-arg -> None gen=True success_response=True boxed=False oob=False preconfig=False -object q_obj_user_def_cmd2-arg +object q_obj_user-def-cmd2-arg member ud1a: UserDefOne optional=False member ud1b: UserDefOne optional=True -command user_def_cmd2 q_obj_user_def_cmd2-arg -> UserDefTwo +command user-def-cmd2 q_obj_user-def-cmd2-arg -> UserDefTwo gen=True success_response=True boxed=False oob=False preconfig=False command cmd-success-response None -> None gen=True success_response=False boxed=False oob=False preconfig=False @@ -319,10 +319,10 @@ object TestIfUnion case union_bar: q_obj_str-wrapper if ['defined(TEST_IF_UNION_BAR)'] if ['defined(TEST_IF_UNION) && defined(TEST_IF_STRUCT)'] -object q_obj_TestIfUnionCmd-arg +object q_obj_test-if-union-cmd-arg member union_cmd_arg: TestIfUnion optional=False if ['defined(TEST_IF_UNION)'] -command TestIfUnionCmd q_obj_TestIfUnionCmd-arg -> None +command test-if-union-cmd q_obj_test-if-union-cmd-arg -> None gen=True success_response=True boxed=False oob=False preconfig=False if ['defined(TEST_IF_UNION)'] alternate TestIfAlternate @@ -331,21 +331,21 @@ alternate TestIfAlternate case bar: TestStruct if ['defined(TEST_IF_ALT_BAR)'] if ['defined(TEST_IF_ALT) && defined(TEST_IF_STRUCT)'] -object q_obj_TestIfAlternateCmd-arg +object q_obj_test-if-alternate-cmd-arg member alt_cmd_arg: TestIfAlternate optional=False if ['defined(TEST_IF_ALT)'] -command TestIfAlternateCmd q_obj_TestIfAlternateCmd-arg -> None +command test-if-alternate-cmd q_obj_test-if-alternate-cmd-arg -> None gen=True success_response=True boxed=False oob=False preconfig=False if ['defined(TEST_IF_ALT)'] -object q_obj_TestIfCmd-arg +object q_obj_test-if-cmd-arg member foo: TestIfStruct optional=False member bar: TestIfEnum optional=False if ['defined(TEST_IF_CMD_BAR)'] if ['defined(TEST_IF_CMD)', 'defined(TEST_IF_STRUCT)'] -command TestIfCmd q_obj_TestIfCmd-arg -> UserDefThree +command test-if-cmd q_obj_test-if-cmd-arg -> UserDefThree gen=True success_response=True boxed=False oob=False preconfig=False if ['defined(TEST_IF_CMD)', 'defined(TEST_IF_STRUCT)'] -command TestCmdReturnDefThree None -> UserDefThree +command test-cmd-return-def-three None -> UserDefThree gen=True success_response=True boxed=False oob=False preconfig=False array TestIfEnumList TestIfEnum if ['defined(TEST_IF_ENUM)'] -- 2.26.3
Test args-name-clash covers command parameter name clash. This effectively covers struct member name clash as well. The next commit will make parameter name clash impossible. Convert args-name-clash from testing command to testing a struct, and rename it to struct-member-name-clash. Signed-off-by: Markus Armbruster <armbru@redhat.com> Message-Id: <20210323094025.3569441-26-armbru@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com> [Commit message typo fixed] --- tests/qapi-schema/args-name-clash.err | 2 -- tests/qapi-schema/meson.build | 2 +- tests/qapi-schema/struct-member-name-clash.err | 2 ++ .../{args-name-clash.json => struct-member-name-clash.json} | 2 +- .../{args-name-clash.out => struct-member-name-clash.out} | 0 5 files changed, 4 insertions(+), 4 deletions(-) delete mode 100644 tests/qapi-schema/args-name-clash.err create mode 100644 tests/qapi-schema/struct-member-name-clash.err rename tests/qapi-schema/{args-name-clash.json => struct-member-name-clash.json} (64%) rename tests/qapi-schema/{args-name-clash.out => struct-member-name-clash.out} (100%) diff --git a/tests/qapi-schema/args-name-clash.err b/tests/qapi-schema/args-name-clash.err deleted file mode 100644 index 3e04817bc0..0000000000 --- a/tests/qapi-schema/args-name-clash.err +++ /dev/null @@ -1,2 +0,0 @@ -args-name-clash.json: In command 'oops': -args-name-clash.json:4: parameter 'a_b' collides with parameter 'a-b' diff --git a/tests/qapi-schema/meson.build b/tests/qapi-schema/meson.build index 4e7635f0a8..8ba6917132 100644 --- a/tests/qapi-schema/meson.build +++ b/tests/qapi-schema/meson.build @@ -30,7 +30,6 @@ schemas = [ 'args-member-array-bad.json', 'args-member-case.json', 'args-member-unknown.json', - 'args-name-clash.json', 'args-union.json', 'args-unknown.json', 'bad-base.json', @@ -177,6 +176,7 @@ schemas = [ 'struct-member-if-invalid.json', 'struct-member-invalid-dict.json', 'struct-member-invalid.json', + 'struct-member-name-clash.json', 'trailing-comma-list.json', 'trailing-comma-object.json', 'type-bypass-bad-gen.json', diff --git a/tests/qapi-schema/struct-member-name-clash.err b/tests/qapi-schema/struct-member-name-clash.err new file mode 100644 index 0000000000..6ac042d59d --- /dev/null +++ b/tests/qapi-schema/struct-member-name-clash.err @@ -0,0 +1,2 @@ +struct-member-name-clash.json: In struct 'Oops': +struct-member-name-clash.json:4: member 'a_b' collides with member 'a-b' diff --git a/tests/qapi-schema/args-name-clash.json b/tests/qapi-schema/struct-member-name-clash.json similarity index 64% rename from tests/qapi-schema/args-name-clash.json rename to tests/qapi-schema/struct-member-name-clash.json index 61423cb893..3fb69cc2ce 100644 --- a/tests/qapi-schema/args-name-clash.json +++ b/tests/qapi-schema/struct-member-name-clash.json @@ -1,4 +1,4 @@ # C member name collision # Reject members that clash when mapped to C names (we would have two 'a_b' # members). -{ 'command': 'oops', 'data': { 'a-b': 'str', 'a_b': 'str' } } +{ 'struct': 'Oops', 'data': { 'a-b': 'str', 'a_b': 'str' } } diff --git a/tests/qapi-schema/args-name-clash.out b/tests/qapi-schema/struct-member-name-clash.out similarity index 100% rename from tests/qapi-schema/args-name-clash.out rename to tests/qapi-schema/struct-member-name-clash.out -- 2.26.3
Struct members, including command arguments, event data, and union inline base members, should use '-', not '_'. Enforce this. Fix the fixable offenders (all in tests/), and add the remainder to pragma member-name-exceptions. Signed-off-by: Markus Armbruster <armbru@redhat.com> Message-Id: <20210323094025.3569441-27-armbru@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com> --- docs/devel/qapi-code-gen.txt | 3 ++- qapi/pragma.json | 17 +++++++++++++++++ qga/qapi-schema.json | 4 ++++ scripts/qapi/expr.py | 5 +++-- tests/qapi-schema/qapi-schema-test.json | 8 ++++++-- tests/qapi-schema/qapi-schema-test.out | 4 ++-- tests/qapi-schema/struct-member-name-clash.err | 2 +- tests/qapi-schema/struct-member-name-clash.json | 1 + 8 files changed, 36 insertions(+), 8 deletions(-) diff --git a/docs/devel/qapi-code-gen.txt b/docs/devel/qapi-code-gen.txt index 10e9a0f829..c1cb6f987d 100644 --- a/docs/devel/qapi-code-gen.txt +++ b/docs/devel/qapi-code-gen.txt @@ -168,7 +168,8 @@ Pragma 'command-returns-exceptions' takes a list of commands that may violate the rules on permitted return types. Default is none. Pragma 'member-name-exceptions' takes a list of types whose member -names may contain uppercase letters. Default is none. +names may contain uppercase letters, and '_' instead of '-'. Default +is none. === Enumeration types === diff --git a/qapi/pragma.json b/qapi/pragma.json index 339f067943..f422a1a2ac 100644 --- a/qapi/pragma.json +++ b/qapi/pragma.json @@ -31,10 +31,27 @@ # Externally visible types whose member names may use uppercase 'member-name-exceptions': [ # visible in: 'ACPISlotType', # query-acpi-ospm-status + 'AcpiTableOptions', # -acpitable + 'BlkdebugSetStateOptions', # blockdev-add, -blockdev + 'BlockDeviceInfo', # query-block + 'BlockDeviceStats', # query-blockstats + 'BlockDeviceTimedStats', # query-blockstats + 'BlockIOThrottle', # block_set_io_throttle + 'BlockInfo', # query-block 'BlockdevVmdkAdapterType', # blockdev-create (to match VMDK spec) 'BlockdevVmdkSubformat', # blockdev-create (to match VMDK spec) + 'ColoCompareProperties', # object_add, -object + 'FilterMirrorProperties', # object_add, -object + 'FilterRedirectorProperties', # object_add, -object + 'FilterRewriterProperties', # object_add, -object + 'InputLinuxProperties', # object_add, -object + 'NetdevTapOptions', # netdev_add, query-netdev, -netdev + 'PciBusInfo', # query-pci + 'PciDeviceInfo', # query-pci + 'PciMemoryRegion', # query-pci 'QapiErrorClass', # QMP error replies 'UuidInfo', # query-uuid + 'VncClientInfo', # query-vnc, query-vnc-servers, ... 'X86CPURegister32' # qom-get of x86 CPU properties # feature-words, filtered-features ] } } diff --git a/qga/qapi-schema.json b/qga/qapi-schema.json index 2b08b761c2..fb17eebde3 100644 --- a/qga/qapi-schema.json +++ b/qga/qapi-schema.json @@ -19,6 +19,10 @@ # Whitelists to permit QAPI rule violations; think twice before you # add to them! { 'pragma': { + # Types whose member names may use '_' + 'member-name-exceptions': [ + 'GuestAgentInfo' + ], # Commands allowed to return a non-dictionary: 'command-returns-exceptions': [ 'guest-file-open', diff --git a/scripts/qapi/expr.py b/scripts/qapi/expr.py index 9193e68763..ba9f7ad350 100644 --- a/scripts/qapi/expr.py +++ b/scripts/qapi/expr.py @@ -184,7 +184,7 @@ def check_type(value, info, source, raise QAPISemError(info, "%s should be an object or type name" % source) - permit_upper = allow_dict in info.pragma.member_name_exceptions + permissive = allow_dict in info.pragma.member_name_exceptions # value is a dictionary, check that each member is okay for (key, arg) in value.items(): @@ -192,7 +192,8 @@ def check_type(value, info, source, if key.startswith('*'): key = key[1:] check_name_lower(key, info, key_source, - permit_upper, permit_underscore=True) + permit_upper=permissive, + permit_underscore=permissive) if c_name(key, False) == 'u' or c_name(key, False).startswith('has_'): raise QAPISemError(info, "%s uses reserved name" % key_source) check_keys(arg, info, key_source, ['type'], ['if', 'features']) diff --git a/tests/qapi-schema/qapi-schema-test.json b/tests/qapi-schema/qapi-schema-test.json index e635db4a35..387678acbb 100644 --- a/tests/qapi-schema/qapi-schema-test.json +++ b/tests/qapi-schema/qapi-schema-test.json @@ -6,6 +6,10 @@ # Whitelists to permit QAPI rule violations { 'pragma': { + # Types whose member names may use '_' + 'member-name-exceptions': [ + 'UserDefA' + ], # Commands allowed to return a non-dictionary: 'command-returns-exceptions': [ 'guest-get-time', @@ -231,7 +235,7 @@ 'if': 'defined(TEST_IF_UNION) && defined(TEST_IF_STRUCT)' } { 'command': 'test-if-union-cmd', - 'data': { 'union_cmd_arg': 'TestIfUnion' }, + 'data': { 'union-cmd-arg': 'TestIfUnion' }, 'if': 'defined(TEST_IF_UNION)' } { 'alternate': 'TestIfAlternate', 'data': @@ -240,7 +244,7 @@ 'if': 'defined(TEST_IF_ALT) && defined(TEST_IF_STRUCT)' } { 'command': 'test-if-alternate-cmd', - 'data': { 'alt_cmd_arg': 'TestIfAlternate' }, + 'data': { 'alt-cmd-arg': 'TestIfAlternate' }, 'if': 'defined(TEST_IF_ALT)' } { 'command': 'test-if-cmd', diff --git a/tests/qapi-schema/qapi-schema-test.out b/tests/qapi-schema/qapi-schema-test.out index 3f1ea345fd..51efe5d7cd 100644 --- a/tests/qapi-schema/qapi-schema-test.out +++ b/tests/qapi-schema/qapi-schema-test.out @@ -320,7 +320,7 @@ object TestIfUnion if ['defined(TEST_IF_UNION_BAR)'] if ['defined(TEST_IF_UNION) && defined(TEST_IF_STRUCT)'] object q_obj_test-if-union-cmd-arg - member union_cmd_arg: TestIfUnion optional=False + member union-cmd-arg: TestIfUnion optional=False if ['defined(TEST_IF_UNION)'] command test-if-union-cmd q_obj_test-if-union-cmd-arg -> None gen=True success_response=True boxed=False oob=False preconfig=False @@ -332,7 +332,7 @@ alternate TestIfAlternate if ['defined(TEST_IF_ALT_BAR)'] if ['defined(TEST_IF_ALT) && defined(TEST_IF_STRUCT)'] object q_obj_test-if-alternate-cmd-arg - member alt_cmd_arg: TestIfAlternate optional=False + member alt-cmd-arg: TestIfAlternate optional=False if ['defined(TEST_IF_ALT)'] command test-if-alternate-cmd q_obj_test-if-alternate-cmd-arg -> None gen=True success_response=True boxed=False oob=False preconfig=False diff --git a/tests/qapi-schema/struct-member-name-clash.err b/tests/qapi-schema/struct-member-name-clash.err index 6ac042d59d..7e53a605d2 100644 --- a/tests/qapi-schema/struct-member-name-clash.err +++ b/tests/qapi-schema/struct-member-name-clash.err @@ -1,2 +1,2 @@ struct-member-name-clash.json: In struct 'Oops': -struct-member-name-clash.json:4: member 'a_b' collides with member 'a-b' +struct-member-name-clash.json:5: member 'a_b' collides with member 'a-b' diff --git a/tests/qapi-schema/struct-member-name-clash.json b/tests/qapi-schema/struct-member-name-clash.json index 3fb69cc2ce..571acf05ce 100644 --- a/tests/qapi-schema/struct-member-name-clash.json +++ b/tests/qapi-schema/struct-member-name-clash.json @@ -1,4 +1,5 @@ # C member name collision # Reject members that clash when mapped to C names (we would have two 'a_b' # members). +{ 'pragma': { 'member-name-exceptions': [ 'Oops' ] } } { 'struct': 'Oops', 'data': { 'a-b': 'str', 'a_b': 'str' } } -- 2.26.3
Enum members should use '-', not '_'. Enforce this. Fix the fixable offenders (all in tests/), and add the remainder to pragma member-name-exceptions. Signed-off-by: Markus Armbruster <armbru@redhat.com> Message-Id: <20210323094025.3569441-28-armbru@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com> --- qapi/pragma.json | 8 ++++++++ scripts/qapi/expr.py | 5 +++-- tests/qapi-schema/enum-clash-member.err | 2 +- tests/qapi-schema/enum-clash-member.json | 1 + 4 files changed, 13 insertions(+), 3 deletions(-) diff --git a/qapi/pragma.json b/qapi/pragma.json index f422a1a2ac..b4e17167e1 100644 --- a/qapi/pragma.json +++ b/qapi/pragma.json @@ -32,12 +32,15 @@ 'member-name-exceptions': [ # visible in: 'ACPISlotType', # query-acpi-ospm-status 'AcpiTableOptions', # -acpitable + 'BlkdebugEvent', # blockdev-add, -blockdev 'BlkdebugSetStateOptions', # blockdev-add, -blockdev 'BlockDeviceInfo', # query-block 'BlockDeviceStats', # query-blockstats 'BlockDeviceTimedStats', # query-blockstats 'BlockIOThrottle', # block_set_io_throttle 'BlockInfo', # query-block + 'BlockdevAioOptions', # blockdev-add, -blockdev + 'BlockdevDriver', # blockdev-add, query-blockstats, ... 'BlockdevVmdkAdapterType', # blockdev-create (to match VMDK spec) 'BlockdevVmdkSubformat', # blockdev-create (to match VMDK spec) 'ColoCompareProperties', # object_add, -object @@ -46,10 +49,15 @@ 'FilterRewriterProperties', # object_add, -object 'InputLinuxProperties', # object_add, -object 'NetdevTapOptions', # netdev_add, query-netdev, -netdev + 'ObjectType', # object-add, -object + 'PCIELinkSpeed', # internal only 'PciBusInfo', # query-pci 'PciDeviceInfo', # query-pci 'PciMemoryRegion', # query-pci + 'QKeyCode', # send-key, input-sent-event 'QapiErrorClass', # QMP error replies + 'SshHostKeyCheckMode', # blockdev-add, -blockdev + 'SysEmuTarget', # query-cpu-fast, query-target 'UuidInfo', # query-uuid 'VncClientInfo', # query-vnc, query-vnc-servers, ... 'X86CPURegister32' # qom-get of x86 CPU properties diff --git a/scripts/qapi/expr.py b/scripts/qapi/expr.py index ba9f7ad350..d968609c48 100644 --- a/scripts/qapi/expr.py +++ b/scripts/qapi/expr.py @@ -229,7 +229,7 @@ def check_enum(expr, info): if prefix is not None and not isinstance(prefix, str): raise QAPISemError(info, "'prefix' must be a string") - permit_upper = name in info.pragma.member_name_exceptions + permissive = name in info.pragma.member_name_exceptions members[:] = [m if isinstance(m, dict) else {'name': m} for m in members] @@ -243,7 +243,8 @@ def check_enum(expr, info): if member_name[0].isdigit(): member_name = 'd' + member_name # Hack: hide the digit check_name_lower(member_name, info, source, - permit_upper, permit_underscore=True) + permit_upper=permissive, + permit_underscore=permissive) check_if(member, info, source) diff --git a/tests/qapi-schema/enum-clash-member.err b/tests/qapi-schema/enum-clash-member.err index 5986571427..e4eb102ae2 100644 --- a/tests/qapi-schema/enum-clash-member.err +++ b/tests/qapi-schema/enum-clash-member.err @@ -1,2 +1,2 @@ enum-clash-member.json: In enum 'MyEnum': -enum-clash-member.json:2: value 'one_two' collides with value 'one-two' +enum-clash-member.json:3: value 'one_two' collides with value 'one-two' diff --git a/tests/qapi-schema/enum-clash-member.json b/tests/qapi-schema/enum-clash-member.json index b6928b8bfd..82bcbf724b 100644 --- a/tests/qapi-schema/enum-clash-member.json +++ b/tests/qapi-schema/enum-clash-member.json @@ -1,2 +1,3 @@ # we reject enums where members will clash when mapped to C enum +{ 'pragma': { 'member-name-exceptions': [ 'MyEnum' ] } } { 'enum': 'MyEnum', 'data': [ 'one-two', 'one_two' ] } -- 2.26.3
Union branch names should use '-', not '_'. Enforce this. The only offenders are in tests/. Fix them. Signed-off-by: Markus Armbruster <armbru@redhat.com> Message-Id: <20210323094025.3569441-29-armbru@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com> [Commit message typo fixed] --- scripts/qapi/expr.py | 4 ++-- tests/qapi-schema/alternate-clash.err | 2 +- tests/qapi-schema/alternate-clash.json | 6 ++++-- tests/qapi-schema/qapi-schema-test.json | 2 +- tests/qapi-schema/qapi-schema-test.out | 4 ++-- tests/qapi-schema/union-clash-branches.err | 2 +- tests/qapi-schema/union-clash-branches.json | 6 ++++-- 7 files changed, 15 insertions(+), 11 deletions(-) diff --git a/scripts/qapi/expr.py b/scripts/qapi/expr.py index d968609c48..540b3982b1 100644 --- a/scripts/qapi/expr.py +++ b/scripts/qapi/expr.py @@ -274,7 +274,7 @@ def check_union(expr, info): for (key, value) in members.items(): source = "'data' member '%s'" % key if discriminator is None: - check_name_lower(key, info, source, permit_underscore=True) + check_name_lower(key, info, source) # else: name is in discriminator enum, which gets checked check_keys(value, info, source, ['type'], ['if']) check_if(value, info, source) @@ -288,7 +288,7 @@ def check_alternate(expr, info): raise QAPISemError(info, "'data' must not be empty") for (key, value) in members.items(): source = "'data' member '%s'" % key - check_name_lower(key, info, source, permit_underscore=True) + check_name_lower(key, info, source) check_keys(value, info, source, ['type'], ['if']) check_if(value, info, source) check_type(value['type'], info, source) diff --git a/tests/qapi-schema/alternate-clash.err b/tests/qapi-schema/alternate-clash.err index 0fe02f2c99..caa2d42e3f 100644 --- a/tests/qapi-schema/alternate-clash.err +++ b/tests/qapi-schema/alternate-clash.err @@ -1,2 +1,2 @@ alternate-clash.json: In alternate 'Alt1': -alternate-clash.json:4: branch 'a_b' collides with branch 'a-b' +alternate-clash.json:6: name of 'data' member 'a_b' must not use uppercase or '_' diff --git a/tests/qapi-schema/alternate-clash.json b/tests/qapi-schema/alternate-clash.json index 039c4be658..87f061a74a 100644 --- a/tests/qapi-schema/alternate-clash.json +++ b/tests/qapi-schema/alternate-clash.json @@ -1,5 +1,7 @@ # Alternate branch name collision -# Reject an alternate that would result in a collision in generated C -# names (this would try to generate two union members named 'a_b'). +# Naming rules make collision impossible (even with the pragma). If +# that wasn't the case, then we'd get a collision in generated C: two +# union members a_b. +{ 'pragma': { 'member-name-exceptions': [ 'Alt1' ] } } { 'alternate': 'Alt1', 'data': { 'a-b': 'bool', 'a_b': 'int' } } diff --git a/tests/qapi-schema/qapi-schema-test.json b/tests/qapi-schema/qapi-schema-test.json index 387678acbb..84b9d41f15 100644 --- a/tests/qapi-schema/qapi-schema-test.json +++ b/tests/qapi-schema/qapi-schema-test.json @@ -231,7 +231,7 @@ { 'union': 'TestIfUnion', 'data': { 'foo': 'TestStruct', - 'union_bar': { 'type': 'str', 'if': 'defined(TEST_IF_UNION_BAR)'} }, + 'bar': { 'type': 'str', 'if': 'defined(TEST_IF_UNION_BAR)'} }, 'if': 'defined(TEST_IF_UNION) && defined(TEST_IF_STRUCT)' } { 'command': 'test-if-union-cmd', diff --git a/tests/qapi-schema/qapi-schema-test.out b/tests/qapi-schema/qapi-schema-test.out index 51efe5d7cd..e0b8a5f0b6 100644 --- a/tests/qapi-schema/qapi-schema-test.out +++ b/tests/qapi-schema/qapi-schema-test.out @@ -309,14 +309,14 @@ object q_obj_TestStruct-wrapper member data: TestStruct optional=False enum TestIfUnionKind member foo - member union_bar + member bar if ['defined(TEST_IF_UNION_BAR)'] if ['defined(TEST_IF_UNION) && defined(TEST_IF_STRUCT)'] object TestIfUnion member type: TestIfUnionKind optional=False tag type case foo: q_obj_TestStruct-wrapper - case union_bar: q_obj_str-wrapper + case bar: q_obj_str-wrapper if ['defined(TEST_IF_UNION_BAR)'] if ['defined(TEST_IF_UNION) && defined(TEST_IF_STRUCT)'] object q_obj_test-if-union-cmd-arg diff --git a/tests/qapi-schema/union-clash-branches.err b/tests/qapi-schema/union-clash-branches.err index 73bbc2cabd..ef53645728 100644 --- a/tests/qapi-schema/union-clash-branches.err +++ b/tests/qapi-schema/union-clash-branches.err @@ -1,2 +1,2 @@ union-clash-branches.json: In union 'TestUnion': -union-clash-branches.json:4: branch 'a_b' collides with branch 'a-b' +union-clash-branches.json:6: name of 'data' member 'a_b' must not use uppercase or '_' diff --git a/tests/qapi-schema/union-clash-branches.json b/tests/qapi-schema/union-clash-branches.json index 3bece8c948..7bdda0b0da 100644 --- a/tests/qapi-schema/union-clash-branches.json +++ b/tests/qapi-schema/union-clash-branches.json @@ -1,5 +1,7 @@ # Union branch name collision -# Reject a union that would result in a collision in generated C names (this -# would try to generate two members 'a_b'). +# Naming rules make collision impossible (even with the pragma). If +# that wasn't the case, then we'd get collisions in generated C: two +# union members a_b, and two enum members TEST_UNION_A_B. +{ 'pragma': { 'member-name-exceptions': [ 'TestUnion' ] } } { 'union': 'TestUnion', 'data': { 'a-b': 'int', 'a_b': 'str' } } -- 2.26.3
Command block_passwd always fails since Commit c01c214b69 "block: remove all encryption handling APIs" (v2.10.0) turned block_passwd into a stub that always fails, and hardcoded encryption_key_missing to false in query-named-block-nodes and query-block. Commit ad1324e044 "block: remove 'encryption_key_missing' flag from QAPI" just landed. Complete the cleanup job: remove block_passwd. Cc: Daniel P. Berrangé <berrange@redhat.com> Signed-off-by: Markus Armbruster <armbru@redhat.com> Message-Id: <20210323101951.3686029-1-armbru@redhat.com> Reviewed-by: Daniel P. Berrangé <berrange@redhat.com> --- qapi/block-core.json | 14 -------------- qapi/pragma.json | 1 - block/monitor/block-hmp-cmds.c | 10 ---------- blockdev.c | 8 -------- hmp-commands.hx | 15 --------------- 5 files changed, 48 deletions(-) diff --git a/qapi/block-core.json b/qapi/block-core.json index 1c3f1deb03..6d227924d0 100644 --- a/qapi/block-core.json +++ b/qapi/block-core.json @@ -1207,20 +1207,6 @@ ## { 'command': 'query-block-jobs', 'returns': ['BlockJobInfo'] } -## -# @block_passwd: -# -# This command sets the password of a block device that has not been open -# with a password and requires one. -# -# This command is now obsolete and will always return an error since 2.10 -# -## -{ 'command': 'block_passwd', - 'data': { '*device': 'str', - '*node-name': 'str', - 'password': 'str' } } - ## # @block_resize: # diff --git a/qapi/pragma.json b/qapi/pragma.json index b4e17167e1..3bc0335d1f 100644 --- a/qapi/pragma.json +++ b/qapi/pragma.json @@ -6,7 +6,6 @@ # Commands allowed to return a non-dictionary: 'command-name-exceptions': [ 'add_client', - 'block_passwd', 'block_resize', 'block_set_io_throttle', 'client_migrate_info', diff --git a/block/monitor/block-hmp-cmds.c b/block/monitor/block-hmp-cmds.c index 75d7fa9510..ebf1033f31 100644 --- a/block/monitor/block-hmp-cmds.c +++ b/block/monitor/block-hmp-cmds.c @@ -515,16 +515,6 @@ void hmp_block_stream(Monitor *mon, const QDict *qdict) hmp_handle_error(mon, error); } -void hmp_block_passwd(Monitor *mon, const QDict *qdict) -{ - const char *device = qdict_get_str(qdict, "device"); - const char *password = qdict_get_str(qdict, "password"); - Error *err = NULL; - - qmp_block_passwd(true, device, false, NULL, password, &err); - hmp_handle_error(mon, err); -} - void hmp_block_set_io_throttle(Monitor *mon, const QDict *qdict) { Error *err = NULL; diff --git a/blockdev.c b/blockdev.c index cf70bb4e43..621cc3b7c4 100644 --- a/blockdev.c +++ b/blockdev.c @@ -2407,14 +2407,6 @@ exit: job_txn_unref(block_job_txn); } -void qmp_block_passwd(bool has_device, const char *device, - bool has_node_name, const char *node_name, - const char *password, Error **errp) -{ - error_setg(errp, - "Setting block passwords directly is no longer supported"); -} - BlockDirtyBitmapSha256 *qmp_x_debug_block_dirty_bitmap_sha256(const char *node, const char *name, Error **errp) diff --git a/hmp-commands.hx b/hmp-commands.hx index 9b88c45174..435c591a1c 100644 --- a/hmp-commands.hx +++ b/hmp-commands.hx @@ -1493,21 +1493,6 @@ SRST used by another monitor command. ERST - { - .name = "block_passwd", - .args_type = "device:B,password:s", - .params = "block_passwd device password", - .help = "set the password of encrypted block devices", - .cmd = hmp_block_passwd, - }, - -SRST -``block_passwd`` *device* *password* - Set the encrypted device *device* password to *password* - - This command is now obsolete and will always return an error since 2.10 -ERST - { .name = "block_set_io_throttle", .args_type = "device:B,bps:l,bps_rd:l,bps_wr:l,iops:l,iops_rd:l,iops_wr:l", -- 2.26.3
On Tue, 23 Mar 2021 at 21:57, Markus Armbruster <armbru@redhat.com> wrote: > > The following changes since commit 9950da284fa5e2ea9ab57d87e05b693fb60c79ce: > > Merge remote-tracking branch 'remotes/alistair/tags/pull-riscv-to-apply-20210322-2' into staging (2021-03-23 15:30:46 +0000) > > are available in the Git repository at: > > git://repo.or.cz/qemu/armbru.git tags/pull-qapi-2021-03-23 > > for you to fetch changes up to bdabafc6836edc0f34732cac473899cb4e77a296: > > block: Remove monitor command block_passwd (2021-03-23 22:31:56 +0100) > > ---------------------------------------------------------------- > QAPI patches patches for 2021-03-23 > > ---------------------------------------------------------------- Applied, thanks. Please update the changelog at https://wiki.qemu.org/ChangeLog/6.0 for any user-visible changes. -- PMM