qapi: Reserve 'q_*' and 'has_*' member names
authorEric Blake <eblake@redhat.com>
Mon, 26 Oct 2015 22:34:44 +0000 (16:34 -0600)
committerMarkus Armbruster <armbru@redhat.com>
Mon, 2 Nov 2015 07:30:26 +0000 (08:30 +0100)
c_name() produces names starting with 'q_' when protecting a
dictionary member name that would fail to directly compile, but
in doing so can cause clashes with any member name already
beginning with 'q-' or 'q_'.  Likewise, we create a C name 'has_'
for any optional member that can clash with any member name
beginning with 'has-' or 'has_'.

Technically, rather than blindly reserving the namespace,
we could try to complain about user names only when an actual
collision occurs, or even teach c_name() how to munge names
to avoid collisions.  But it is not trivial, especially when
collisions can occur across multiple types (such as via
inheritance or flat unions).  Besides, no existing .json
files are trying to use these names.  So it's easier to just
outright forbid the potential for collision.  We can always
relax things in the future if a real need arises for QMP to
express member names that have been forbidden here.

'has_' only has to be reserved for struct/union member names,
while 'q_' is reserved everywhere (matching the fact that
only members can be optional, while we use c_name() for munging
both members and entities).  Note that we could relax 'q_'
restrictions on entities independently from member names; for
example, c_name('qmp_' + 'unix') would result in a different
function name than our current 'qmp_' + c_name('unix').

Update and add tests to cover the new error messages.

Signed-off-by: Eric Blake <eblake@redhat.com>
Message-Id: <1445898903-12082-6-git-send-email-eblake@redhat.com>
[Consistently pass protect=False to c_name(); commit message tweaked
slightly]
Signed-off-by: Markus Armbruster <armbru@redhat.com>
14 files changed:
docs/qapi-code-gen.txt
scripts/qapi.py
tests/qapi-schema/reserved-command-q.err
tests/qapi-schema/reserved-command-q.exit
tests/qapi-schema/reserved-command-q.json
tests/qapi-schema/reserved-command-q.out
tests/qapi-schema/reserved-member-has.err
tests/qapi-schema/reserved-member-has.exit
tests/qapi-schema/reserved-member-has.json
tests/qapi-schema/reserved-member-has.out
tests/qapi-schema/reserved-member-q.err
tests/qapi-schema/reserved-member-q.exit
tests/qapi-schema/reserved-member-q.json
tests/qapi-schema/reserved-member-q.out

index c4264a819b5620233c2d18b10dfe4c088b5814d0..4d8c2fcf02ff9b3f2ae4b9fbb8b04b64536b5628 100644 (file)
@@ -112,7 +112,9 @@ and field names within a type, should be all lower case with words
 separated by a hyphen.  However, some existing older commands and
 complex types use underscore; when extending such expressions,
 consistency is preferred over blindly avoiding underscore.  Event
-names should be ALL_CAPS with words separated by underscore.
+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.
 
 Any name (command, event, type, field, or enum value) beginning with
 "x-" is marked experimental, and may be withdrawn or changed
@@ -123,9 +125,10 @@ vendor), even if the rest of the name uses dash (example:
 __com.redhat_drive-mirror).  Other than downstream extensions (with
 leading underscore and the use of dots), all names should begin with a
 letter, and contain only ASCII letters, digits, dash, and underscore.
-It is okay to reuse names that match C keywords; the generator will
-rename a field named "default" in the QAPI to "q_default" in the
-generated C code.
+Names beginning with 'q_' are reserved for the generator: QMP names
+that resemble C keywords or other problematic strings will be munged
+in C to use this prefix.  For example, a field named "default" in
+qapi becomes "q_default" in the generated C code.
 
 In the rest of this document, usage lines are given for each
 expression type, with literal strings written in lower case and
index d53b5c4b45e1cfa4494add3003b13dc1558f1f52..3ff7b11e619c36ca5ed8a6576fce695c69c9e42e 100644 (file)
@@ -376,7 +376,9 @@ def check_name(expr_info, source, name, allow_optional=False,
     # code always prefixes it with the enum name
     if enum_member:
         membername = '_' + membername
-    if not valid_name.match(membername):
+    # Reserve the entire 'q_' namespace for c_name()
+    if not valid_name.match(membername) or \
+       c_name(membername, False).startswith('q_'):
         raise QAPIExprError(expr_info,
                             "%s uses invalid name '%s'" % (source, name))
 
@@ -488,6 +490,10 @@ def check_type(expr_info, source, value, allow_array=False,
     for (key, arg) in value.items():
         check_name(expr_info, "Member of %s" % source, key,
                    allow_optional=allow_optional)
+        if c_name(key, False).startswith('has_'):
+            raise QAPIExprError(expr_info,
+                                "Member of %s uses reserved name '%s'"
+                                % (source, key))
         # Todo: allow dictionaries to represent default values of
         # an optional argument.
         check_type(expr_info, "Member '%s' of %s" % (key, source), arg,
index e69de29bb2d1d6434b8b29ae775ad8c2e48c5391..f939e044eba01713c57ca727882753af8adaf1dd 100644 (file)
@@ -0,0 +1 @@
+tests/qapi-schema/reserved-command-q.json:5: 'command' uses invalid name 'q-unix'
index 573541ac9702dd3969c9bc859d2b91ec1f7e6e56..d00491fd7e5bb6fa28c517a0bb32b8b506539d4d 100644 (file)
@@ -1 +1 @@
-0
+1
index be9944c68af7097a42b67fa9ffebae6d6eaa788c..99f8aae314bc1295a354d89546c3fdbebbc02358 100644 (file)
@@ -1,7 +1,5 @@
 # C entity name collision
-# FIXME - This parses, but fails to compile, because it attempts to declare
-# two 'qmp_q_unix' functions (one for 'q-unix', the other because c_name()
-# munges 'unix' to 'q_unix' to avoid reserved word collisions). We should
-# reject attempts to explicitly use 'q_' names, to reserve it for qapi.
+# We reject names like 'q-unix', because they can collide with the mangled
+# name for 'unix' in generated C.
 { 'command': 'unix' }
 { 'command': 'q-unix' }
index b31b38ff0d1c501d6624a41a7ed5df4789b1ff3d..e69de29bb2d1d6434b8b29ae775ad8c2e48c5391 100644 (file)
@@ -1,5 +0,0 @@
-object :empty
-command q-unix None -> None
-   gen=True success_response=True
-command unix None -> None
-   gen=True success_response=True
index e69de29bb2d1d6434b8b29ae775ad8c2e48c5391..e75577144613cccc031fd515b51e3cb584963d70 100644 (file)
@@ -0,0 +1 @@
+tests/qapi-schema/reserved-member-has.json:5: Member of 'data' for command 'oops' uses reserved name 'has-a'
index 573541ac9702dd3969c9bc859d2b91ec1f7e6e56..d00491fd7e5bb6fa28c517a0bb32b8b506539d4d 100644 (file)
@@ -1 +1 @@
-0
+1
index a2197de6b51fe789f0945d71c1e84375308dcdd1..45b9109bdc5e5edd67a8c6b385a11c0d22db4bf9 100644 (file)
@@ -1,6 +1,5 @@
 # C member name collision
-# FIXME - This parses, but fails to compile, because the C struct is given
-# two 'has_a' members, one from the flag for optional 'a', and the other
-# from member 'has-a'.  Either reject this at parse time, or munge the C
-# names to avoid the collision.
+# We reject names like 'has-a', because they can collide with the flag
+# for an optional 'a' in generated C.
+# TODO we could munge the optional flag name to avoid the collision.
 { 'command': 'oops', 'data': { '*a': 'str', 'has-a': 'str' } }
index 5a18b6be8c313148ca348abd507894c6e8dc6255..e69de29bb2d1d6434b8b29ae775ad8c2e48c5391 100644 (file)
@@ -1,6 +0,0 @@
-object :empty
-object :obj-oops-arg
-    member a: str optional=True
-    member has-a: str optional=False
-command oops :obj-oops-arg -> None
-   gen=True success_response=True
index e69de29bb2d1d6434b8b29ae775ad8c2e48c5391..f3d5dd78187759a3b515333e26688760e505ee09 100644 (file)
@@ -0,0 +1 @@
+tests/qapi-schema/reserved-member-q.json:4: Member of 'data' for struct 'Foo' uses invalid name 'q-unix'
index 573541ac9702dd3969c9bc859d2b91ec1f7e6e56..d00491fd7e5bb6fa28c517a0bb32b8b506539d4d 100644 (file)
@@ -1 +1 @@
-0
+1
index 1602ed3281f1a7b02e53a0bb9c34383dabb5efca..62fed8fddfe8a4d584fa7b99120cfcb943c7354b 100644 (file)
@@ -1,6 +1,4 @@
 # C member name collision
-# FIXME - This parses, but fails to compile, because it attempts to declare
-# two 'q_unix' members (one for 'q-unix', the other because c_name()
-# munges 'unix' to 'q_unix' to avoid reserved word collisions). We should
-# reject attempts to explicitly use 'q_' names, to reserve it for qapi.
+# We reject names like 'q-unix', because they can collide with the mangled
+# name for 'unix' in generated C.
 { 'struct': 'Foo', 'data': { 'unix':'int', 'q-unix':'bool' } }
index 0d8685aeb00ca2e6152de548590e168601a1e10e..e69de29bb2d1d6434b8b29ae775ad8c2e48c5391 100644 (file)
@@ -1,4 +0,0 @@
-object :empty
-object Foo
-    member unix: int optional=False
-    member q-unix: bool optional=False