[Qemu-devel] [PATCH v11 20/28] qapi: Forbid case-insensitive clashes

[Eric Blake]

In general, designing user interfaces that rely on case
distinction is poor practice.  Another benefit of enforcing
a restriction against case-insensitive clashes is that we
no longer have to worry about the situation 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_upper().  Thus, 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.

There is also the possibility that we may want to add 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.  Of course, the idea of a future extension is
not as strong of a reason to make the change.

At any rate, it is easier to be strict now, and relax things
later if we find a reason to need case-sensitive QMP members,
than it would be to wish we had the restriction in place.

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

---
v11: rebase to latest, don't focus so hard on future case-insensitive
extensions, adjust commit message
v10: new patch
---
 docs/qapi-code-gen.txt                 | 3 +++
 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 | 4 ++++
 tests/qapi-schema/args-case-clash.out  | 0
 7 files changed, 12 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..54a6a7b 100644
--- a/docs/qapi-code-gen.txt
+++ b/docs/qapi-code-gen.txt
@@ -102,6 +102,9 @@ single-dimension array of that type; multi-dimension arrays 
are not
 directly supported (although an array of a complex struct that
 contains an array member is possible).

+Client JSON Protocol is case-sensitive.  However, the generator
+rejects attempts to create entities that differ only in case.
+
 Types, commands, and events share a common namespace.  Therefore,
 generally speaking, type definitions should always use CamelCase for
 user-defined type names, while built-in types are lowercase. Type
diff --git a/scripts/qapi.py b/scripts/qapi.py
index d0f2308..ed7a32b 100644
--- a/scripts/qapi.py
+++ b/scripts/qapi.py
@@ -1040,7 +1040,7 @@ class QAPISchemaObjectTypeMember(object):
         assert self.type

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

     def check(self, schema, seen):
         if not self.tag_member:    # 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
         assert isinstance(self.tag_member.type, QAPISchemaEnumType)
         for v in self.variants:
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..0fafe75
--- /dev/null
+++ b/tests/qapi-schema/args-case-clash.err
@@ -0,0 +1 @@
+tests/qapi-schema/args-case-clash.json:4: 'A' (parameter of oops) collides 
with 'a' (parameter 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..e6f0625
--- /dev/null
+++ b/tests/qapi-schema/args-case-clash.json
@@ -0,0 +1,4 @@
+# C member name collision
+# Reject members that clash case-insensitively, even though our mapping to
+# C names preserves case.
+{ '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