qapi: Reserve 'q_*' and 'has_*' member names

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>
This commit is contained in:
Eric Blake 2015-10-26 16:34:44 -06:00 committed by Markus Armbruster
parent 255960dd37
commit 9fb081e0b9
14 changed files with 27 additions and 35 deletions

View 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

View 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,

View File

@ -0,0 +1 @@
tests/qapi-schema/reserved-command-q.json:5: 'command' uses invalid name 'q-unix'

View File

@ -1 +1 @@
0
1

View 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' }

View 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

View File

@ -0,0 +1 @@
tests/qapi-schema/reserved-member-has.json:5: Member of 'data' for command 'oops' uses reserved name 'has-a'

View File

@ -1 +1 @@
0
1

View 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' } }

View 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

View File

@ -0,0 +1 @@
tests/qapi-schema/reserved-member-q.json:4: Member of 'data' for struct 'Foo' uses invalid name 'q-unix'

View File

@ -1 +1 @@
0
1

View 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' } }

View File

@ -1,4 +0,0 @@
object :empty
object Foo
member unix: int optional=False
member q-unix: bool optional=False