From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:45972) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZuadD-0000JE-Od for qemu-devel@nongnu.org; Fri, 06 Nov 2015 01:36:17 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Zuad8-0004Db-Jo for qemu-devel@nongnu.org; Fri, 06 Nov 2015 01:36:15 -0500 Received: from mx1.redhat.com ([209.132.183.28]:45251) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Zuad8-0004Bu-8n for qemu-devel@nongnu.org; Fri, 06 Nov 2015 01:36:10 -0500 From: Eric Blake Date: Thu, 5 Nov 2015 23:35:54 -0700 Message-Id: <1446791754-23823-31-git-send-email-eblake@redhat.com> In-Reply-To: <1446791754-23823-1-git-send-email-eblake@redhat.com> References: <1446791754-23823-1-git-send-email-eblake@redhat.com> Subject: [Qemu-devel] [PATCH v10 30/30] qapi: Forbid case-insensitive clashes List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: qemu-devel@nongnu.org Cc: armbru@redhat.com, Michael Roth 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 --- 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