Commit 9fb081e0 authored by Eric Blake's avatar Eric Blake Committed by Markus Armbruster
Browse files

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: default avatarEric 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: default avatarMarkus Armbruster <armbru@redhat.com>
parent 255960dd

docs/qapi-code-gen.txt

+7 −4
Original line number Diff line number Diff line
@@ -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

scripts/qapi.py

+7 −1
Original line number Diff line number Diff line
@@ -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,

tests/qapi-schema/reserved-command-q.err

+1 −0
Original line number Diff line number Diff line 
tests/qapi-schema/reserved-command-q.json:5: 'command' uses invalid name 'q-unix'

tests/qapi-schema/reserved-command-q.exit

+1 −1
Original line number Diff line number Diff line 
0
 
1

tests/qapi-schema/reserved-command-q.json

+2 −4
Original line number Diff line number Diff line 
# 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' }