qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-devel] [PATCH v11 23/24] qapi: Reserve 'u' member name


From: Markus Armbruster
Subject: Re: [Qemu-devel] [PATCH v11 23/24] qapi: Reserve 'u' member name
Date: Tue, 27 Oct 2015 09:21:09 +0100
User-agent: Gnus/5.13 (Gnus v5.13) Emacs/24.5 (gnu/linux)

Eric Blake <address@hidden> writes:

> Now that we have separated union tag values from colliding with
> non-variant C names, by naming the union 'u', we should reserve
> this name for our use.  Note that we want to forbid 'u' even in
> a struct with no variants, because it is possible for a future
> qemu release to extend QMP in a backwards-compatible manner while
> converting from a struct to a flat union.  Fortunately, no
> existing clients were using this member name.  If we ever find
> the need for QMP to have a member 'u', we could at that time
> relax things, perhaps by having c_name() munge the QMP member to
> 'q_u'.
>
> Note that we cannot forbid 'u' everywhere (by adding the
> rejection code to check_name()), because the existing QKeyCode
> enum already uses it; therefore we only reserve it as a struct
> type member name.
>
> Signed-off-by: Eric Blake <address@hidden>
>
> ---
> v11: commit message tweaks, use c_name(), tweak test names
> v10: new patch, split from 1/17 and 3/17; by deferring this until
> after the rename was complete, it reduces churn due to the new feature
> ---
>  scripts/qapi.py                          | 2 +-
>  tests/Makefile                           | 1 +
>  tests/qapi-schema/reserved-member-u.err  | 1 +
>  tests/qapi-schema/reserved-member-u.exit | 1 +
>  tests/qapi-schema/reserved-member-u.json | 7 +++++++
>  tests/qapi-schema/reserved-member-u.out  | 0
>  6 files changed, 11 insertions(+), 1 deletion(-)
>  create mode 100644 tests/qapi-schema/reserved-member-u.err
>  create mode 100644 tests/qapi-schema/reserved-member-u.exit
>  create mode 100644 tests/qapi-schema/reserved-member-u.json
>  create mode 100644 tests/qapi-schema/reserved-member-u.out
>
> diff --git a/scripts/qapi.py b/scripts/qapi.py
> index 3fead04..89f3207 100644
> --- a/scripts/qapi.py
> +++ b/scripts/qapi.py
> @@ -490,7 +490,7 @@ 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).startswith('has_'):
> +        if c_name(key, False) == 'u' or c_name(key).startswith('has_'):

Slightly odd: new c_name() has protect=False, the existing one doesn't.
While we don't really need protect=False, it feels a bit cleaner.  If
you like, I can add it to the existing one when it gets added in PATCH
05.

>              raise QAPIExprError(expr_info,
>                                  "Member of %s uses reserved name '%s'"
>                                  % (source, key))
> diff --git a/tests/Makefile b/tests/Makefile
> index 6d7c07e..f448b5a 100644
> --- a/tests/Makefile
> +++ b/tests/Makefile
> @@ -318,6 +318,7 @@ qapi-schema += redefined-type.json
>  qapi-schema += reserved-command-q.json
>  qapi-schema += reserved-member-has.json
>  qapi-schema += reserved-member-q.json
> +qapi-schema += reserved-member-u.json
>  qapi-schema += reserved-type-kind.json
>  qapi-schema += reserved-type-list.json
>  qapi-schema += returns-alternate.json
> diff --git a/tests/qapi-schema/reserved-member-u.err 
> b/tests/qapi-schema/reserved-member-u.err
> new file mode 100644
> index 0000000..87d4229
> --- /dev/null
> +++ b/tests/qapi-schema/reserved-member-u.err
> @@ -0,0 +1 @@
> +tests/qapi-schema/reserved-member-u.json:7: Member of 'data' for struct 
> 'Oops' uses reserved name 'u'
> diff --git a/tests/qapi-schema/reserved-member-u.exit 
> b/tests/qapi-schema/reserved-member-u.exit
> new file mode 100644
> index 0000000..d00491f
> --- /dev/null
> +++ b/tests/qapi-schema/reserved-member-u.exit
> @@ -0,0 +1 @@
> +1
> diff --git a/tests/qapi-schema/reserved-member-u.json 
> b/tests/qapi-schema/reserved-member-u.json
> new file mode 100644
> index 0000000..1eaf0f3
> --- /dev/null
> +++ b/tests/qapi-schema/reserved-member-u.json
> @@ -0,0 +1,7 @@
> +# Potential C member name collision
> +# We reject use of 'u' as a member name, to allow it for internal use in
> +# putting union branch members in a separate namespace from QMP members.
> +# This is true even for non-unions, because it is possible to convert a
> +# struct to flat union while remaining backwards compatible in QMP.
> +# TODO - we could munge the member name to 'q_u' to avoid the collision
> +{ 'struct': 'Oops', 'data': { 'u': 'str' } }
> diff --git a/tests/qapi-schema/reserved-member-u.out 
> b/tests/qapi-schema/reserved-member-u.out
> new file mode 100644
> index 0000000..e69de29



reply via email to

[Prev in Thread] Current Thread [Next in Thread]