qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
* [PULL 00/29] QAPI patches patches for 2021-03-23
@ 2021-03-23 21:56 Markus Armbruster
  2021-03-23 21:56 ` [PULL 01/29] qapi/pragma: Tidy up after removal of deprecated commands Markus Armbruster
                   ` (29 more replies)
  0 siblings, 30 replies; 31+ messages in thread
From: Markus Armbruster @ 2021-03-23 21:56 UTC (permalink / raw)
  To: qemu-devel; +Cc: peter.maydell

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



^ permalink raw reply	[flat|nested] 31+ messages in thread

* [PULL 01/29] qapi/pragma: Tidy up after removal of deprecated commands
  2021-03-23 21:56 [PULL 00/29] QAPI patches patches for 2021-03-23 Markus Armbruster
@ 2021-03-23 21:56 ` Markus Armbruster
  2021-03-23 21:56 ` [PULL 02/29] tests/qapi-schema: Drop redundant flat-union-inline test Markus Armbruster
                   ` (28 subsequent siblings)
  29 siblings, 0 replies; 31+ messages in thread
From: Markus Armbruster @ 2021-03-23 21:56 UTC (permalink / raw)
  To: qemu-devel; +Cc: peter.maydell, John Snow

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



^ permalink raw reply related	[flat|nested] 31+ messages in thread

* [PULL 02/29] tests/qapi-schema: Drop redundant flat-union-inline test
  2021-03-23 21:56 [PULL 00/29] QAPI patches patches for 2021-03-23 Markus Armbruster
  2021-03-23 21:56 ` [PULL 01/29] qapi/pragma: Tidy up after removal of deprecated commands Markus Armbruster
@ 2021-03-23 21:56 ` Markus Armbruster
  2021-03-23 21:56 ` [PULL 03/29] tests/qapi-schema: Rework comments on longhand member definitions Markus Armbruster
                   ` (27 subsequent siblings)
  29 siblings, 0 replies; 31+ messages in thread
From: Markus Armbruster @ 2021-03-23 21:56 UTC (permalink / raw)
  To: qemu-devel; +Cc: peter.maydell, John Snow

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



^ permalink raw reply related	[flat|nested] 31+ messages in thread

* [PULL 03/29] tests/qapi-schema: Rework comments on longhand member definitions
  2021-03-23 21:56 [PULL 00/29] QAPI patches patches for 2021-03-23 Markus Armbruster
  2021-03-23 21:56 ` [PULL 01/29] qapi/pragma: Tidy up after removal of deprecated commands Markus Armbruster
  2021-03-23 21:56 ` [PULL 02/29] tests/qapi-schema: Drop redundant flat-union-inline test Markus Armbruster
@ 2021-03-23 21:56 ` Markus Armbruster
  2021-03-23 21:56 ` [PULL 04/29] tests/qapi-schema: Belatedly update comment on alternate clash Markus Armbruster
                   ` (26 subsequent siblings)
  29 siblings, 0 replies; 31+ messages in thread
From: Markus Armbruster @ 2021-03-23 21:56 UTC (permalink / raw)
  To: qemu-devel; +Cc: John Snow, peter.maydell

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



^ permalink raw reply related	[flat|nested] 31+ messages in thread

* [PULL 04/29] tests/qapi-schema: Belatedly update comment on alternate clash
  2021-03-23 21:56 [PULL 00/29] QAPI patches patches for 2021-03-23 Markus Armbruster
                   ` (2 preceding siblings ...)
  2021-03-23 21:56 ` [PULL 03/29] tests/qapi-schema: Rework comments on longhand member definitions Markus Armbruster
@ 2021-03-23 21:56 ` Markus Armbruster
  2021-03-23 21:56 ` [PULL 05/29] tests/qapi-schema: Drop TODO comment on simple unions Markus Armbruster
                   ` (25 subsequent siblings)
  29 siblings, 0 replies; 31+ messages in thread
From: Markus Armbruster @ 2021-03-23 21:56 UTC (permalink / raw)
  To: qemu-devel; +Cc: peter.maydell, John Snow

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



^ permalink raw reply related	[flat|nested] 31+ messages in thread

* [PULL 05/29] tests/qapi-schema: Drop TODO comment on simple unions
  2021-03-23 21:56 [PULL 00/29] QAPI patches patches for 2021-03-23 Markus Armbruster
                   ` (3 preceding siblings ...)
  2021-03-23 21:56 ` [PULL 04/29] tests/qapi-schema: Belatedly update comment on alternate clash Markus Armbruster
@ 2021-03-23 21:56 ` Markus Armbruster
  2021-03-23 21:56 ` [PULL 06/29] tests/qapi-schema: Tweak to demonstrate buggy member name check Markus Armbruster
                   ` (24 subsequent siblings)
  29 siblings, 0 replies; 31+ messages in thread
From: Markus Armbruster @ 2021-03-23 21:56 UTC (permalink / raw)
  To: qemu-devel; +Cc: peter.maydell, John Snow

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



^ permalink raw reply related	[flat|nested] 31+ messages in thread

* [PULL 06/29] tests/qapi-schema: Tweak to demonstrate buggy member name check
  2021-03-23 21:56 [PULL 00/29] QAPI patches patches for 2021-03-23 Markus Armbruster
                   ` (4 preceding siblings ...)
  2021-03-23 21:56 ` [PULL 05/29] tests/qapi-schema: Drop TODO comment on simple unions Markus Armbruster
@ 2021-03-23 21:56 ` Markus Armbruster
  2021-03-23 21:56 ` [PULL 07/29] qapi: Fix to reject optional members with reserved names Markus Armbruster
                   ` (23 subsequent siblings)
  29 siblings, 0 replies; 31+ messages in thread
From: Markus Armbruster @ 2021-03-23 21:56 UTC (permalink / raw)
  To: qemu-devel; +Cc: peter.maydell, John Snow

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



^ permalink raw reply related	[flat|nested] 31+ messages in thread

* [PULL 07/29] qapi: Fix to reject optional members with reserved names
  2021-03-23 21:56 [PULL 00/29] QAPI patches patches for 2021-03-23 Markus Armbruster
                   ` (5 preceding siblings ...)
  2021-03-23 21:56 ` [PULL 06/29] tests/qapi-schema: Tweak to demonstrate buggy member name check Markus Armbruster
@ 2021-03-23 21:56 ` Markus Armbruster
  2021-03-23 21:56 ` [PULL 08/29] qapi: Permit flat union members for any tag value Markus Armbruster
                   ` (22 subsequent siblings)
  29 siblings, 0 replies; 31+ messages in thread
From: Markus Armbruster @ 2021-03-23 21:56 UTC (permalink / raw)
  To: qemu-devel; +Cc: peter.maydell, John Snow

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



^ permalink raw reply related	[flat|nested] 31+ messages in thread

* [PULL 08/29] qapi: Permit flat union members for any tag value
  2021-03-23 21:56 [PULL 00/29] QAPI patches patches for 2021-03-23 Markus Armbruster
                   ` (6 preceding siblings ...)
  2021-03-23 21:56 ` [PULL 07/29] qapi: Fix to reject optional members with reserved names Markus Armbruster
@ 2021-03-23 21:56 ` Markus Armbruster
  2021-03-23 21:56 ` [PULL 09/29] qapi: Lift enum-specific code out of check_name_str() Markus Armbruster
                   ` (21 subsequent siblings)
  29 siblings, 0 replies; 31+ messages in thread
From: Markus Armbruster @ 2021-03-23 21:56 UTC (permalink / raw)
  To: qemu-devel; +Cc: peter.maydell

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



^ permalink raw reply related	[flat|nested] 31+ messages in thread

* [PULL 09/29] qapi: Lift enum-specific code out of check_name_str()
  2021-03-23 21:56 [PULL 00/29] QAPI patches patches for 2021-03-23 Markus Armbruster
                   ` (7 preceding siblings ...)
  2021-03-23 21:56 ` [PULL 08/29] qapi: Permit flat union members for any tag value Markus Armbruster
@ 2021-03-23 21:56 ` Markus Armbruster
  2021-03-23 21:56 ` [PULL 10/29] qapi: Rework name checking in preparation of stricter checking Markus Armbruster
                   ` (20 subsequent siblings)
  29 siblings, 0 replies; 31+ messages in thread
From: Markus Armbruster @ 2021-03-23 21:56 UTC (permalink / raw)
  To: qemu-devel; +Cc: peter.maydell

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



^ permalink raw reply related	[flat|nested] 31+ messages in thread

* [PULL 10/29] qapi: Rework name checking in preparation of stricter checking
  2021-03-23 21:56 [PULL 00/29] QAPI patches patches for 2021-03-23 Markus Armbruster
                   ` (8 preceding siblings ...)
  2021-03-23 21:56 ` [PULL 09/29] qapi: Lift enum-specific code out of check_name_str() Markus Armbruster
@ 2021-03-23 21:56 ` Markus Armbruster
  2021-03-23 21:56 ` [PULL 11/29] qapi: Move uppercase rejection to check_name_lower() Markus Armbruster
                   ` (19 subsequent siblings)
  29 siblings, 0 replies; 31+ messages in thread
From: Markus Armbruster @ 2021-03-23 21:56 UTC (permalink / raw)
  To: qemu-devel; +Cc: peter.maydell

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



^ permalink raw reply related	[flat|nested] 31+ messages in thread

* [PULL 11/29] qapi: Move uppercase rejection to check_name_lower()
  2021-03-23 21:56 [PULL 00/29] QAPI patches patches for 2021-03-23 Markus Armbruster
                   ` (9 preceding siblings ...)
  2021-03-23 21:56 ` [PULL 10/29] qapi: Rework name checking in preparation of stricter checking Markus Armbruster
@ 2021-03-23 21:56 ` Markus Armbruster
  2021-03-23 21:56 ` [PULL 12/29] qapi: Consistently permit any case in downstream prefixes Markus Armbruster
                   ` (18 subsequent siblings)
  29 siblings, 0 replies; 31+ messages in thread
From: Markus Armbruster @ 2021-03-23 21:56 UTC (permalink / raw)
  To: qemu-devel; +Cc: peter.maydell

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



^ permalink raw reply related	[flat|nested] 31+ messages in thread

* [PULL 12/29] qapi: Consistently permit any case in downstream prefixes
  2021-03-23 21:56 [PULL 00/29] QAPI patches patches for 2021-03-23 Markus Armbruster
                   ` (10 preceding siblings ...)
  2021-03-23 21:56 ` [PULL 11/29] qapi: Move uppercase rejection to check_name_lower() Markus Armbruster
@ 2021-03-23 21:56 ` Markus Armbruster
  2021-03-23 21:56 ` [PULL 13/29] qapi: Enforce event naming rules Markus Armbruster
                   ` (17 subsequent siblings)
  29 siblings, 0 replies; 31+ messages in thread
From: Markus Armbruster @ 2021-03-23 21:56 UTC (permalink / raw)
  To: qemu-devel; +Cc: peter.maydell

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



^ permalink raw reply related	[flat|nested] 31+ messages in thread

* [PULL 13/29] qapi: Enforce event naming rules
  2021-03-23 21:56 [PULL 00/29] QAPI patches patches for 2021-03-23 Markus Armbruster
                   ` (11 preceding siblings ...)
  2021-03-23 21:56 ` [PULL 12/29] qapi: Consistently permit any case in downstream prefixes Markus Armbruster
@ 2021-03-23 21:56 ` Markus Armbruster
  2021-03-23 21:56 ` [PULL 14/29] qapi: Enforce type " Markus Armbruster
                   ` (16 subsequent siblings)
  29 siblings, 0 replies; 31+ messages in thread
From: Markus Armbruster @ 2021-03-23 21:56 UTC (permalink / raw)
  To: qemu-devel; +Cc: peter.maydell

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



^ permalink raw reply related	[flat|nested] 31+ messages in thread

* [PULL 14/29] qapi: Enforce type naming rules
  2021-03-23 21:56 [PULL 00/29] QAPI patches patches for 2021-03-23 Markus Armbruster
                   ` (12 preceding siblings ...)
  2021-03-23 21:56 ` [PULL 13/29] qapi: Enforce event naming rules Markus Armbruster
@ 2021-03-23 21:56 ` Markus Armbruster
  2021-03-23 21:56 ` [PULL 15/29] tests/qapi-schema: Rename redefined-builtin to redefined-predefined Markus Armbruster
                   ` (15 subsequent siblings)
  29 siblings, 0 replies; 31+ messages in thread
From: Markus Armbruster @ 2021-03-23 21:56 UTC (permalink / raw)
  To: qemu-devel; +Cc: peter.maydell

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



^ permalink raw reply related	[flat|nested] 31+ messages in thread

* [PULL 15/29] tests/qapi-schema: Rename redefined-builtin to redefined-predefined
  2021-03-23 21:56 [PULL 00/29] QAPI patches patches for 2021-03-23 Markus Armbruster
                   ` (13 preceding siblings ...)
  2021-03-23 21:56 ` [PULL 14/29] qapi: Enforce type " Markus Armbruster
@ 2021-03-23 21:56 ` Markus Armbruster
  2021-03-23 21:56 ` [PULL 16/29] qapi: Factor out QAPISchemaParser._check_pragma_list_of_str() Markus Armbruster
                   ` (14 subsequent siblings)
  29 siblings, 0 replies; 31+ messages in thread
From: Markus Armbruster @ 2021-03-23 21:56 UTC (permalink / raw)
  To: qemu-devel; +Cc: peter.maydell

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



^ permalink raw reply related	[flat|nested] 31+ messages in thread

* [PULL 16/29] qapi: Factor out QAPISchemaParser._check_pragma_list_of_str()
  2021-03-23 21:56 [PULL 00/29] QAPI patches patches for 2021-03-23 Markus Armbruster
                   ` (14 preceding siblings ...)
  2021-03-23 21:56 ` [PULL 15/29] tests/qapi-schema: Rename redefined-builtin to redefined-predefined Markus Armbruster
@ 2021-03-23 21:56 ` Markus Armbruster
  2021-03-23 21:56 ` [PULL 17/29] tests/qapi-schema: Rename pragma-*-crap to pragma-value-not-* Markus Armbruster
                   ` (13 subsequent siblings)
  29 siblings, 0 replies; 31+ messages in thread
From: Markus Armbruster @ 2021-03-23 21:56 UTC (permalink / raw)
  To: qemu-devel; +Cc: peter.maydell

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



^ permalink raw reply related	[flat|nested] 31+ messages in thread

* [PULL 17/29] tests/qapi-schema: Rename pragma-*-crap to pragma-value-not-*
  2021-03-23 21:56 [PULL 00/29] QAPI patches patches for 2021-03-23 Markus Armbruster
                   ` (15 preceding siblings ...)
  2021-03-23 21:56 ` [PULL 16/29] qapi: Factor out QAPISchemaParser._check_pragma_list_of_str() Markus Armbruster
@ 2021-03-23 21:56 ` Markus Armbruster
  2021-03-23 21:56 ` [PULL 18/29] tests/qapi-schema: Rename returns-whitelist to returns-bad-type Markus Armbruster
                   ` (12 subsequent siblings)
  29 siblings, 0 replies; 31+ messages in thread
From: Markus Armbruster @ 2021-03-23 21:56 UTC (permalink / raw)
  To: qemu-devel; +Cc: peter.maydell

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



^ permalink raw reply related	[flat|nested] 31+ messages in thread

* [PULL 18/29] tests/qapi-schema: Rename returns-whitelist to returns-bad-type
  2021-03-23 21:56 [PULL 00/29] QAPI patches patches for 2021-03-23 Markus Armbruster
                   ` (16 preceding siblings ...)
  2021-03-23 21:56 ` [PULL 17/29] tests/qapi-schema: Rename pragma-*-crap to pragma-value-not-* Markus Armbruster
@ 2021-03-23 21:56 ` Markus Armbruster
  2021-03-23 21:56 ` [PULL 19/29] qapi: Rename pragma *-whitelist to *-exceptions Markus Armbruster
                   ` (11 subsequent siblings)
  29 siblings, 0 replies; 31+ messages in thread
From: Markus Armbruster @ 2021-03-23 21:56 UTC (permalink / raw)
  To: qemu-devel; +Cc: peter.maydell

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



^ permalink raw reply related	[flat|nested] 31+ messages in thread

* [PULL 19/29] qapi: Rename pragma *-whitelist to *-exceptions
  2021-03-23 21:56 [PULL 00/29] QAPI patches patches for 2021-03-23 Markus Armbruster
                   ` (17 preceding siblings ...)
  2021-03-23 21:56 ` [PULL 18/29] tests/qapi-schema: Rename returns-whitelist to returns-bad-type Markus Armbruster
@ 2021-03-23 21:56 ` Markus Armbruster
  2021-03-23 21:56 ` [PULL 20/29] qapi/pragma: Streamline comments on member-name-exceptions Markus Armbruster
                   ` (10 subsequent siblings)
  29 siblings, 0 replies; 31+ messages in thread
From: Markus Armbruster @ 2021-03-23 21:56 UTC (permalink / raw)
  To: qemu-devel; +Cc: peter.maydell

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



^ permalink raw reply related	[flat|nested] 31+ messages in thread

* [PULL 20/29] qapi/pragma: Streamline comments on member-name-exceptions
  2021-03-23 21:56 [PULL 00/29] QAPI patches patches for 2021-03-23 Markus Armbruster
                   ` (18 preceding siblings ...)
  2021-03-23 21:56 ` [PULL 19/29] qapi: Rename pragma *-whitelist to *-exceptions Markus Armbruster
@ 2021-03-23 21:56 ` Markus Armbruster
  2021-03-23 21:56 ` [PULL 21/29] tests-qmp-cmds: Drop unused and incorrect qmp_TestIfCmd() Markus Armbruster
                   ` (9 subsequent siblings)
  29 siblings, 0 replies; 31+ messages in thread
From: Markus Armbruster @ 2021-03-23 21:56 UTC (permalink / raw)
  To: qemu-devel; +Cc: peter.maydell

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



^ permalink raw reply related	[flat|nested] 31+ messages in thread

* [PULL 21/29] tests-qmp-cmds: Drop unused and incorrect qmp_TestIfCmd()
  2021-03-23 21:56 [PULL 00/29] QAPI patches patches for 2021-03-23 Markus Armbruster
                   ` (19 preceding siblings ...)
  2021-03-23 21:56 ` [PULL 20/29] qapi/pragma: Streamline comments on member-name-exceptions Markus Armbruster
@ 2021-03-23 21:56 ` Markus Armbruster
  2021-03-23 21:56 ` [PULL 22/29] qapi: Prepare for rejecting underscore in command and member names Markus Armbruster
                   ` (8 subsequent siblings)
  29 siblings, 0 replies; 31+ messages in thread
From: Markus Armbruster @ 2021-03-23 21:56 UTC (permalink / raw)
  To: qemu-devel; +Cc: peter.maydell

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



^ permalink raw reply related	[flat|nested] 31+ messages in thread

* [PULL 22/29] qapi: Prepare for rejecting underscore in command and member names
  2021-03-23 21:56 [PULL 00/29] QAPI patches patches for 2021-03-23 Markus Armbruster
                   ` (20 preceding siblings ...)
  2021-03-23 21:56 ` [PULL 21/29] tests-qmp-cmds: Drop unused and incorrect qmp_TestIfCmd() Markus Armbruster
@ 2021-03-23 21:56 ` Markus Armbruster
  2021-03-23 21:56 ` [PULL 23/29] qapi: Enforce feature naming rules Markus Armbruster
                   ` (7 subsequent siblings)
  29 siblings, 0 replies; 31+ messages in thread
From: Markus Armbruster @ 2021-03-23 21:56 UTC (permalink / raw)
  To: qemu-devel; +Cc: peter.maydell

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



^ permalink raw reply related	[flat|nested] 31+ messages in thread

* [PULL 23/29] qapi: Enforce feature naming rules
  2021-03-23 21:56 [PULL 00/29] QAPI patches patches for 2021-03-23 Markus Armbruster
                   ` (21 preceding siblings ...)
  2021-03-23 21:56 ` [PULL 22/29] qapi: Prepare for rejecting underscore in command and member names Markus Armbruster
@ 2021-03-23 21:56 ` Markus Armbruster
  2021-03-23 21:56 ` [PULL 24/29] qapi: Enforce command " Markus Armbruster
                   ` (6 subsequent siblings)
  29 siblings, 0 replies; 31+ messages in thread
From: Markus Armbruster @ 2021-03-23 21:56 UTC (permalink / raw)
  To: qemu-devel; +Cc: peter.maydell

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



^ permalink raw reply related	[flat|nested] 31+ messages in thread

* [PULL 24/29] qapi: Enforce command naming rules
  2021-03-23 21:56 [PULL 00/29] QAPI patches patches for 2021-03-23 Markus Armbruster
                   ` (22 preceding siblings ...)
  2021-03-23 21:56 ` [PULL 23/29] qapi: Enforce feature naming rules Markus Armbruster
@ 2021-03-23 21:56 ` Markus Armbruster
  2021-03-23 21:56 ` [PULL 25/29] tests/qapi-schema: Switch member name clash test to struct Markus Armbruster
                   ` (5 subsequent siblings)
  29 siblings, 0 replies; 31+ messages in thread
From: Markus Armbruster @ 2021-03-23 21:56 UTC (permalink / raw)
  To: qemu-devel; +Cc: peter.maydell

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



^ permalink raw reply related	[flat|nested] 31+ messages in thread

* [PULL 25/29] tests/qapi-schema: Switch member name clash test to struct
  2021-03-23 21:56 [PULL 00/29] QAPI patches patches for 2021-03-23 Markus Armbruster
                   ` (23 preceding siblings ...)
  2021-03-23 21:56 ` [PULL 24/29] qapi: Enforce command " Markus Armbruster
@ 2021-03-23 21:56 ` Markus Armbruster
  2021-03-23 21:56 ` [PULL 26/29] qapi: Enforce struct member naming rules Markus Armbruster
                   ` (4 subsequent siblings)
  29 siblings, 0 replies; 31+ messages in thread
From: Markus Armbruster @ 2021-03-23 21:56 UTC (permalink / raw)
  To: qemu-devel; +Cc: peter.maydell

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



^ permalink raw reply related	[flat|nested] 31+ messages in thread

* [PULL 26/29] qapi: Enforce struct member naming rules
  2021-03-23 21:56 [PULL 00/29] QAPI patches patches for 2021-03-23 Markus Armbruster
                   ` (24 preceding siblings ...)
  2021-03-23 21:56 ` [PULL 25/29] tests/qapi-schema: Switch member name clash test to struct Markus Armbruster
@ 2021-03-23 21:56 ` Markus Armbruster
  2021-03-23 21:56 ` [PULL 27/29] qapi: Enforce enum " Markus Armbruster
                   ` (3 subsequent siblings)
  29 siblings, 0 replies; 31+ messages in thread
From: Markus Armbruster @ 2021-03-23 21:56 UTC (permalink / raw)
  To: qemu-devel; +Cc: peter.maydell

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



^ permalink raw reply related	[flat|nested] 31+ messages in thread

* [PULL 27/29] qapi: Enforce enum member naming rules
  2021-03-23 21:56 [PULL 00/29] QAPI patches patches for 2021-03-23 Markus Armbruster
                   ` (25 preceding siblings ...)
  2021-03-23 21:56 ` [PULL 26/29] qapi: Enforce struct member naming rules Markus Armbruster
@ 2021-03-23 21:56 ` Markus Armbruster
  2021-03-23 21:56 ` [PULL 28/29] qapi: Enforce union and alternate branch " Markus Armbruster
                   ` (2 subsequent siblings)
  29 siblings, 0 replies; 31+ messages in thread
From: Markus Armbruster @ 2021-03-23 21:56 UTC (permalink / raw)
  To: qemu-devel; +Cc: peter.maydell

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



^ permalink raw reply related	[flat|nested] 31+ messages in thread

* [PULL 28/29] qapi: Enforce union and alternate branch naming rules
  2021-03-23 21:56 [PULL 00/29] QAPI patches patches for 2021-03-23 Markus Armbruster
                   ` (26 preceding siblings ...)
  2021-03-23 21:56 ` [PULL 27/29] qapi: Enforce enum " Markus Armbruster
@ 2021-03-23 21:56 ` Markus Armbruster
  2021-03-23 21:56 ` [PULL 29/29] block: Remove monitor command block_passwd Markus Armbruster
  2021-03-23 23:46 ` [PULL 00/29] QAPI patches patches for 2021-03-23 Peter Maydell
  29 siblings, 0 replies; 31+ messages in thread
From: Markus Armbruster @ 2021-03-23 21:56 UTC (permalink / raw)
  To: qemu-devel; +Cc: peter.maydell

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



^ permalink raw reply related	[flat|nested] 31+ messages in thread

* [PULL 29/29] block: Remove monitor command block_passwd
  2021-03-23 21:56 [PULL 00/29] QAPI patches patches for 2021-03-23 Markus Armbruster
                   ` (27 preceding siblings ...)
  2021-03-23 21:56 ` [PULL 28/29] qapi: Enforce union and alternate branch " Markus Armbruster
@ 2021-03-23 21:56 ` Markus Armbruster
  2021-03-23 23:46 ` [PULL 00/29] QAPI patches patches for 2021-03-23 Peter Maydell
  29 siblings, 0 replies; 31+ messages in thread
From: Markus Armbruster @ 2021-03-23 21:56 UTC (permalink / raw)
  To: qemu-devel; +Cc: peter.maydell, Daniel P . Berrangé

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



^ permalink raw reply related	[flat|nested] 31+ messages in thread

* Re: [PULL 00/29] QAPI patches patches for 2021-03-23
  2021-03-23 21:56 [PULL 00/29] QAPI patches patches for 2021-03-23 Markus Armbruster
                   ` (28 preceding siblings ...)
  2021-03-23 21:56 ` [PULL 29/29] block: Remove monitor command block_passwd Markus Armbruster
@ 2021-03-23 23:46 ` Peter Maydell
  29 siblings, 0 replies; 31+ messages in thread
From: Peter Maydell @ 2021-03-23 23:46 UTC (permalink / raw)
  To: Markus Armbruster; +Cc: QEMU Developers

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


^ permalink raw reply	[flat|nested] 31+ messages in thread

end of thread, other threads:[~2021-03-23 23:48 UTC | newest]

Thread overview: 31+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-03-23 21:56 [PULL 00/29] QAPI patches patches for 2021-03-23 Markus Armbruster
2021-03-23 21:56 ` [PULL 01/29] qapi/pragma: Tidy up after removal of deprecated commands Markus Armbruster
2021-03-23 21:56 ` [PULL 02/29] tests/qapi-schema: Drop redundant flat-union-inline test Markus Armbruster
2021-03-23 21:56 ` [PULL 03/29] tests/qapi-schema: Rework comments on longhand member definitions Markus Armbruster
2021-03-23 21:56 ` [PULL 04/29] tests/qapi-schema: Belatedly update comment on alternate clash Markus Armbruster
2021-03-23 21:56 ` [PULL 05/29] tests/qapi-schema: Drop TODO comment on simple unions Markus Armbruster
2021-03-23 21:56 ` [PULL 06/29] tests/qapi-schema: Tweak to demonstrate buggy member name check Markus Armbruster
2021-03-23 21:56 ` [PULL 07/29] qapi: Fix to reject optional members with reserved names Markus Armbruster
2021-03-23 21:56 ` [PULL 08/29] qapi: Permit flat union members for any tag value Markus Armbruster
2021-03-23 21:56 ` [PULL 09/29] qapi: Lift enum-specific code out of check_name_str() Markus Armbruster
2021-03-23 21:56 ` [PULL 10/29] qapi: Rework name checking in preparation of stricter checking Markus Armbruster
2021-03-23 21:56 ` [PULL 11/29] qapi: Move uppercase rejection to check_name_lower() Markus Armbruster
2021-03-23 21:56 ` [PULL 12/29] qapi: Consistently permit any case in downstream prefixes Markus Armbruster
2021-03-23 21:56 ` [PULL 13/29] qapi: Enforce event naming rules Markus Armbruster
2021-03-23 21:56 ` [PULL 14/29] qapi: Enforce type " Markus Armbruster
2021-03-23 21:56 ` [PULL 15/29] tests/qapi-schema: Rename redefined-builtin to redefined-predefined Markus Armbruster
2021-03-23 21:56 ` [PULL 16/29] qapi: Factor out QAPISchemaParser._check_pragma_list_of_str() Markus Armbruster
2021-03-23 21:56 ` [PULL 17/29] tests/qapi-schema: Rename pragma-*-crap to pragma-value-not-* Markus Armbruster
2021-03-23 21:56 ` [PULL 18/29] tests/qapi-schema: Rename returns-whitelist to returns-bad-type Markus Armbruster
2021-03-23 21:56 ` [PULL 19/29] qapi: Rename pragma *-whitelist to *-exceptions Markus Armbruster
2021-03-23 21:56 ` [PULL 20/29] qapi/pragma: Streamline comments on member-name-exceptions Markus Armbruster
2021-03-23 21:56 ` [PULL 21/29] tests-qmp-cmds: Drop unused and incorrect qmp_TestIfCmd() Markus Armbruster
2021-03-23 21:56 ` [PULL 22/29] qapi: Prepare for rejecting underscore in command and member names Markus Armbruster
2021-03-23 21:56 ` [PULL 23/29] qapi: Enforce feature naming rules Markus Armbruster
2021-03-23 21:56 ` [PULL 24/29] qapi: Enforce command " Markus Armbruster
2021-03-23 21:56 ` [PULL 25/29] tests/qapi-schema: Switch member name clash test to struct Markus Armbruster
2021-03-23 21:56 ` [PULL 26/29] qapi: Enforce struct member naming rules Markus Armbruster
2021-03-23 21:56 ` [PULL 27/29] qapi: Enforce enum " Markus Armbruster
2021-03-23 21:56 ` [PULL 28/29] qapi: Enforce union and alternate branch " Markus Armbruster
2021-03-23 21:56 ` [PULL 29/29] block: Remove monitor command block_passwd Markus Armbruster
2021-03-23 23:46 ` [PULL 00/29] QAPI patches patches for 2021-03-23 Peter Maydell

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).