[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v3 12/14] qapi: drop inline subtype in query-ver
From: |
Markus Armbruster |
Subject: |
Re: [Qemu-devel] [PATCH v3 12/14] qapi: drop inline subtype in query-version |
Date: |
Thu, 14 Aug 2014 13:45:06 +0200 |
User-agent: |
Gnus/5.13 (Gnus v5.13) Emacs/24.3 (gnu/linux) |
Eric Blake <address@hidden> writes:
> A future patch will be using a 'name':{dictionary} entry in the
> QAPI schema to specify a default value for an optional argument;
> but existing use of inline substructs conflicts with that goal.
> This patch fixes one of only two commands relying on nested
> subtypes, by breaking the nesting into an explicit type.
>
> Prefer the safer g_new0() while making the conversion.
>
> * qapi/common.json (VersionInfo): Split inline data...
> (VersionTriple): ...into new type.
> * qmp.c (qmp_query_version): Update caller.
> * hmp.c (hmp_info_version): Likewise.
>
> Signed-off-by: Eric Blake <address@hidden>
> ---
> hmp.c | 2 +-
> qapi/common.json | 26 +++++++++++++++++++-------
> qmp.c | 9 +++++----
> 3 files changed, 25 insertions(+), 12 deletions(-)
>
> diff --git a/hmp.c b/hmp.c
> index 4d1838e..e637fbc 100644
> --- a/hmp.c
> +++ b/hmp.c
> @@ -55,7 +55,7 @@ void hmp_info_version(Monitor *mon, const QDict *qdict)
> info = qmp_query_version(NULL);
>
> monitor_printf(mon, "%" PRId64 ".%" PRId64 ".%" PRId64 "%s\n",
> - info->qemu.major, info->qemu.minor, info->qemu.micro,
> + info->qemu->major, info->qemu->minor, info->qemu->micro,
> info->package);
>
> qapi_free_VersionInfo(info);
This demonstrates the difference between anonymous and named complex
member types: anonymous means unboxed, named means boxed. Already
visible in the previous patch, but it's easier to see here (less noise).
I'm no friend of excessive boxing in C, and QAPI code generator is too
boxing-happy for my taste already. However, I like tying boxed
vs. unboxed to named vs. anonymous even less.
> diff --git a/qapi/common.json b/qapi/common.json
> index 4e9a21f..d007095 100644
> --- a/qapi/common.json
> +++ b/qapi/common.json
> @@ -29,15 +29,28 @@
> 'DeviceNotActive', 'DeviceNotFound', 'KVMMissingCap' ] }
>
> ##
> +# @VersionTriple
> +#
> +# A three-part version number.
> +#
> +# @qemu.major: The major version number.
> +#
> +# @qemu.minor: The minor version number.
> +#
> +# @qemu.micro: The micro version number.
> +#
> +# Since: 2.2
> +##
> +{ 'type': 'VersionTriple',
> + 'data': {'major': 'int', 'minor': 'int', 'micro': 'int'} }
> +
> +
> +##
> # @VersionInfo:
> #
> # A description of QEMU's version.
> #
> -# @qemu.major: The major version of QEMU
> -#
> -# @qemu.minor: The minor version of QEMU
> -#
> -# @qemu.micro: The micro version of QEMU. By current convention, a micro
> +# @qemu: The version of QEMU. By current convention, a micro
> # version of 50 signifies a development branch. A micro
> version
> # greater than or equal to 90 signifies a release candidate for
> # the next minor version. A micro version of less than 50
> @@ -51,8 +64,7 @@
> # Since: 0.14.0
> ##
> { 'type': 'VersionInfo',
> - 'data': {'qemu': {'major': 'int', 'minor': 'int', 'micro': 'int'},
> - 'package': 'str'} }
> + 'data': {'qemu': 'VersionTriple', 'package': 'str'} }
>
> ##
> # @query-version:
> diff --git a/qmp.c b/qmp.c
> index 0d2553a..35e6f7a 100644
> --- a/qmp.c
> +++ b/qmp.c
> @@ -45,15 +45,16 @@ NameInfo *qmp_query_name(Error **errp)
>
> VersionInfo *qmp_query_version(Error **errp)
> {
> - VersionInfo *info = g_malloc0(sizeof(*info));
> + VersionInfo *info = g_new0(VersionInfo, 1);
Unrelated improvement. I figure you want it for consistency with the
g_new0() you add. Okay.
> const char *version = QEMU_VERSION;
> char *tmp;
>
> - info->qemu.major = strtol(version, &tmp, 10);
> + info->qemu = g_new0(VersionTriple, 1);
> + info->qemu->major = strtol(version, &tmp, 10);
> tmp++;
> - info->qemu.minor = strtol(tmp, &tmp, 10);
> + info->qemu->minor = strtol(tmp, &tmp, 10);
> tmp++;
> - info->qemu.micro = strtol(tmp, &tmp, 10);
> + info->qemu->micro = strtol(tmp, &tmp, 10);
> info->package = g_strdup(QEMU_PKGVERSION);
>
> return info;
- Re: [Qemu-devel] [PATCH v3 09/14] qapi: add check_type helper function, (continued)
- [Qemu-devel] [PATCH v3 05/14] qapi: add some expr tests, Eric Blake, 2014/08/05
- [Qemu-devel] [PATCH v3 02/14] qapi: ignore files created during make check, Eric Blake, 2014/08/05
- [Qemu-devel] [PATCH v3 14/14] qapi: drop support for inline subtypes, Eric Blake, 2014/08/05
- [Qemu-devel] [PATCH v3 07/14] qapi: add some type check tests, Eric Blake, 2014/08/05
- [Qemu-devel] [PATCH v3 13/14] qapi: drop inline subtype in query-pci, Eric Blake, 2014/08/05
- [Qemu-devel] [PATCH v3 10/14] qapi: merge UserDefTwo and UserDefNested in tests, Eric Blake, 2014/08/05
- [Qemu-devel] [PATCH v3 12/14] qapi: drop inline subtype in query-version, Eric Blake, 2014/08/05
- Re: [Qemu-devel] [PATCH v3 12/14] qapi: drop inline subtype in query-version,
Markus Armbruster <=
- [Qemu-devel] [PATCH v3 11/14] qapi: drop tests for inline subtypes, Eric Blake, 2014/08/05
- Re: [Qemu-devel] [PATCH v3 00/14] drop qapi nested structs, Eric Blake, 2014/08/12
- Re: [Qemu-devel] [PATCH v3 00/14] drop qapi nested structs, Markus Armbruster, 2014/08/14
- Re: [Qemu-devel] [PATCH v3 00/14] drop qapi nested structs, Luiz Capitulino, 2014/08/15