[Qemu-devel] [PATCH v10 30/30] qapi: Forbid case-insensitive clashes

[Eric Blake]

We have toyed on list with the idea of a future extension to
QMP of teaching it to be case-insensitive (the user could
request command 'Quit' instead of 'quit', or could spell a
struct field as 'CPU' instead of 'cpu').  But for that to be
a practical extension, we cannot break backwards compatibility
with any existing struct that was already relying on case
sensitivity.  Fortunately, after the previous patch cleaned
up CpuInfo, there are no such existing qapi structs.

Another benefit of enforcing a restriction against
case-insensitive is that we no longer have to worry about the
case of enum values that could be distinguished by case if
mapped by c_name(), but which cannot be distinguished when
mapped to C as ALL_CAPS by camel_to_uppper().  Having the
generator look for case collisions up front will prevent
developers from worrying whether different munging rules
for member names compared to enum values as a discriminator
will cause any problems in qapi unions.

Of course, if we never implement a case-insensitive QMP
extension, or find a legitimate reason to need qapi members
that differ only by case, we could always relax this
restriction.  But it is easier to relax later than it is to
wish we had the restriction in place earlier.

Signed-off-by: Eric Blake <address@hidden>

---
v10: new patch
---
 docs/qapi-code-gen.txt                 | 5 +++++
 scripts/qapi.py                        | 4 ++--
 tests/Makefile                         | 1 +
 tests/qapi-schema/args-case-clash.err  | 1 +
 tests/qapi-schema/args-case-clash.exit | 1 +
 tests/qapi-schema/args-case-clash.json | 5 +++++
 tests/qapi-schema/args-case-clash.out  | 0
 7 files changed, 15 insertions(+), 2 deletions(-)
 create mode 100644 tests/qapi-schema/args-case-clash.err
 create mode 100644 tests/qapi-schema/args-case-clash.exit
 create mode 100644 tests/qapi-schema/args-case-clash.json
 create mode 100644 tests/qapi-schema/args-case-clash.out

diff --git a/docs/qapi-code-gen.txt b/docs/qapi-code-gen.txt
index f9fa6f3..13cec10 100644
--- a/docs/qapi-code-gen.txt
+++ b/docs/qapi-code-gen.txt
@@ -116,6 +116,11 @@ names should be ALL_CAPS with words separated by 
underscore.  Field
 names cannot start with 'has-' or 'has_', as this is reserved for
 tracking optional fields.

+For now, Client JSON Protocol is case-sensitive, but future extensions
+may allow for case-insensitive recognition of command and event names,
+or of member field names.  As such, the generator rejects attempts to
+create entities that only differ by case.
+
 Any name (command, event, type, field, or enum value) beginning with
 "x-" is marked experimental, and may be withdrawn or changed
 incompatibly in a future release.  Downstream vendors may add
diff --git a/scripts/qapi.py b/scripts/qapi.py
index 3a359cb..7e7ad6e 100644
--- a/scripts/qapi.py
+++ b/scripts/qapi.py
@@ -1039,7 +1039,7 @@ class QAPISchemaObjectTypeMember(object):
         assert self.type

     def check_clash(self, info, seen):
-        name = c_name(self.name)
+        name = c_name(self.name).upper()
         if name in seen:
             raise QAPIExprError(info,
                                 "%s collides with %s"
@@ -1087,7 +1087,7 @@ class QAPISchemaObjectTypeVariants(object):

     def check(self, schema, seen):
         if self.tag_name:    # flat union
-            self.tag_member = seen[c_name(self.tag_name)]
+            self.tag_member = seen[c_name(self.tag_name).upper()]
             assert self.tag_name == self.tag_member.name
         tag_type = self.tag_member.type
         assert isinstance(tag_type, QAPISchemaEnumType)
diff --git a/tests/Makefile b/tests/Makefile
index c84c6cb..d1c6817 100644
--- a/tests/Makefile
+++ b/tests/Makefile
@@ -239,6 +239,7 @@ qapi-schema += args-alternate.json
 qapi-schema += args-any.json
 qapi-schema += args-array-empty.json
 qapi-schema += args-array-unknown.json
+qapi-schema += args-case-clash.json
 qapi-schema += args-int.json
 qapi-schema += args-invalid.json
 qapi-schema += args-member-array-bad.json
diff --git a/tests/qapi-schema/args-case-clash.err 
b/tests/qapi-schema/args-case-clash.err
new file mode 100644
index 0000000..5495ab8
--- /dev/null
+++ b/tests/qapi-schema/args-case-clash.err
@@ -0,0 +1 @@
+tests/qapi-schema/args-case-clash.json:5: 'A' (argument of oops) collides with 
'a' (argument of oops)
diff --git a/tests/qapi-schema/args-case-clash.exit 
b/tests/qapi-schema/args-case-clash.exit
new file mode 100644
index 0000000..d00491f
--- /dev/null
+++ b/tests/qapi-schema/args-case-clash.exit
@@ -0,0 +1 @@
+1
diff --git a/tests/qapi-schema/args-case-clash.json 
b/tests/qapi-schema/args-case-clash.json
new file mode 100644
index 0000000..55ae488
--- /dev/null
+++ b/tests/qapi-schema/args-case-clash.json
@@ -0,0 +1,5 @@
+# C member name collision
+# Reject members that clash case-insensitively (our mapping to C names
+# preserves case, but allowing these members now would prevent a future
+# relaxing of QMP to be case-insensitive).
+{ 'command': 'oops', 'data': { 'a': 'str', 'A': 'int' } }
diff --git a/tests/qapi-schema/args-case-clash.out 
b/tests/qapi-schema/args-case-clash.out
new file mode 100644
index 0000000..e69de29
-- 
2.4.3