[Qemu-devel] [PATCH for-2.9 06/47] qapi: Have each QAPI schema declare its name rule violations

Markus Armbruster posted 47 patches 8 years, 7 months ago
There is a newer version of this series
[Qemu-devel] [PATCH for-2.9 06/47] qapi: Have each QAPI schema declare its name rule violations
Posted by Markus Armbruster 8 years, 7 months ago
qapi.py has a hardcoded white-list of type names that may violate the
rule on use of upper and lower case.  Add a new pragma directive
'name-case-whitelist', and use it to replace the hard-coded
white-list.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 docs/qapi-code-gen.txt                  |  6 ++++++
 qapi-schema.json                        | 11 ++++++++++-
 scripts/qapi.py                         | 22 ++++++++++------------
 tests/qapi-schema/enum-member-case.err  |  2 +-
 tests/qapi-schema/enum-member-case.json |  1 +
 5 files changed, 28 insertions(+), 14 deletions(-)

diff --git a/docs/qapi-code-gen.txt b/docs/qapi-code-gen.txt
index e907e57..349dc02 100644
--- a/docs/qapi-code-gen.txt
+++ b/docs/qapi-code-gen.txt
@@ -252,6 +252,9 @@ Any name (command, event, type, member, or enum value) beginning with
 "x-" is marked experimental, and may be withdrawn or changed
 incompatibly in a future release.
 
+Pragma 'name-case-whitelist' lets you violate the rules on use of
+upper and lower case.  Use for new code is strongly discouraged.
+
 In the rest of this document, usage lines are given for each
 expression type, with literal strings written in lower case and
 placeholders written in capitals.  If a literal string includes a
@@ -318,6 +321,9 @@ is required.  Default is false.
 Pragma 'returns-whitelist' takes a list of command names 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.
+
 
 === Struct types ===
 
diff --git a/qapi-schema.json b/qapi-schema.json
index 93e9e98..17c766e 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -61,7 +61,16 @@
         'query-migrate-cache-size',
         'query-tpm-models',
         'query-tpm-types',
-        'ringbuf-read' ] } }
+        '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
+        'QapiErrorClass',       # all members, visible through errors
+        'UuidInfo',             # UUID, visible through query-uuid
+        'X86CPURegister32',     # all members, visible indirectly through qom-get
+        'q_obj_CpuInfo-base'    # CPU, visible through query-cpu
+    ] } }
 
 # QAPI common definitions
 { 'include': 'qapi/common.json' }
diff --git a/scripts/qapi.py b/scripts/qapi.py
index a90b682..e98fd0c 100644
--- a/scripts/qapi.py
+++ b/scripts/qapi.py
@@ -44,16 +44,7 @@ doc_required = False
 returns_whitelist = []
 
 # Whitelist of entities allowed to violate case conventions
-case_whitelist = [
-    # From QMP:
-    'ACPISlotType',         # DIMM, visible through query-acpi-ospm-status
-    'CpuInfoMIPS',          # PC, visible through query-cpu
-    'CpuInfoTricore',       # PC, visible through query-cpu
-    'QapiErrorClass',       # all members, visible through errors
-    'UuidInfo',             # UUID, visible through query-uuid
-    'X86CPURegister32',     # all members, visible indirectly through qom-get
-    'q_obj_CpuInfo-base',   # CPU, visible through query-cpu
-]
+name_case_whitelist = []
 
 enum_types = []
 struct_types = []
@@ -298,7 +289,7 @@ class QAPISchemaParser(object):
         self.docs.extend(exprs_include.docs)
 
     def _pragma(self, name, value, info):
-        global doc_required, returns_whitelist
+        global doc_required, returns_whitelist, name_case_whitelist
         if name == 'doc-required':
             if not isinstance(value, bool):
                 raise QAPISemError(info,
@@ -311,6 +302,13 @@ class QAPISchemaParser(object):
                                    "Pragma returns-whitelist must be"
                                    " a list of strings")
             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")
+            name_case_whitelist = value
         else:
             raise QAPISemError(info, "Unknown pragma '%s'" % name)
 
@@ -1283,7 +1281,7 @@ class QAPISchemaMember(object):
 
     def check_clash(self, info, seen):
         cname = c_name(self.name)
-        if cname.lower() != cname and self.owner not in case_whitelist:
+        if cname.lower() != cname and self.owner not in name_case_whitelist:
             raise QAPISemError(info,
                                "%s should not use uppercase" % self.describe())
         if cname in seen:
diff --git a/tests/qapi-schema/enum-member-case.err b/tests/qapi-schema/enum-member-case.err
index b652e9a..3c67a3a 100644
--- a/tests/qapi-schema/enum-member-case.err
+++ b/tests/qapi-schema/enum-member-case.err
@@ -1 +1 @@
-tests/qapi-schema/enum-member-case.json:3: 'Value' (member of NoWayThisWillGetWhitelisted) should not use uppercase
+tests/qapi-schema/enum-member-case.json:4: 'Value' (member of NoWayThisWillGetWhitelisted) should not use uppercase
diff --git a/tests/qapi-schema/enum-member-case.json b/tests/qapi-schema/enum-member-case.json
index 2096b35..f8af3e4 100644
--- a/tests/qapi-schema/enum-member-case.json
+++ b/tests/qapi-schema/enum-member-case.json
@@ -1,3 +1,4 @@
 # Member names should be 'lower-case' unless the enum is whitelisted
+{ 'pragma': { 'name-case-whitelist': [ 'UuidInfo' ] } }
 { 'enum': 'UuidInfo', 'data': [ 'Value' ] } # UuidInfo is whitelisted
 { 'enum': 'NoWayThisWillGetWhitelisted', 'data': [ 'Value' ] }
-- 
2.7.4


Re: [Qemu-devel] [PATCH for-2.9 06/47] qapi: Have each QAPI schema declare its name rule violations
Posted by Eric Blake 8 years, 7 months ago
On 03/13/2017 01:18 AM, Markus Armbruster wrote:
> qapi.py has a hardcoded white-list of type names that may violate the
> rule on use of upper and lower case.  Add a new pragma directive
> 'name-case-whitelist', and use it to replace the hard-coded
> white-list.
> 
> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> ---
>  docs/qapi-code-gen.txt                  |  6 ++++++
>  qapi-schema.json                        | 11 ++++++++++-
>  scripts/qapi.py                         | 22 ++++++++++------------
>  tests/qapi-schema/enum-member-case.err  |  2 +-
>  tests/qapi-schema/enum-member-case.json |  1 +
>  5 files changed, 28 insertions(+), 14 deletions(-)
> 

> +++ b/qapi-schema.json
> @@ -61,7 +61,16 @@
>          'query-migrate-cache-size',
>          'query-tpm-models',
>          'query-tpm-types',
> -        'ringbuf-read' ] } }
> +        '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
> +        'QapiErrorClass',       # all members, visible through errors
> +        'UuidInfo',             # UUID, visible through query-uuid
> +        'X86CPURegister32',     # all members, visible indirectly through qom-get
> +        'q_obj_CpuInfo-base'    # CPU, visible through query-cpu
> +    ] } }

Interesting - here you bunch up 2 of the 3 pragmas into one dict, while
still leaving the third related to documentation in its own dict.

That 'q_obj_CpuInfo-base' is ugly, and I had a patch around previously
that used a saner name rather than making callers reverse-engineer the
implicit naming rules.  Related to my work on anonymous bases to flat
unions, so I'll get to rebase that work and post it on top of yours.
But not a show-stopper for this patch, where it is just moving the location.


> @@ -311,6 +302,13 @@ class QAPISchemaParser(object):
>                                     "Pragma returns-whitelist must be"
>                                     " a list of strings")
>              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")
> +            name_case_whitelist = value

Same comments as before - new error message with no testsuite coverage,
and no checking for duplicate assignment where last one silently wins;
but where I'm okay with deferring if you don't want to delay 2.9 for a
v2 respin.

So if you use it unchanged,
Reviewed-by: Eric Blake <eblake@redhat.com>

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org