[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v6 01/18] build-sys: define QEMU_VERSION_{MAJOR,
From: |
Markus Armbruster |
Subject: |
Re: [Qemu-devel] [PATCH v6 01/18] build-sys: define QEMU_VERSION_{MAJOR, MINOR, MICRO} |
Date: |
Tue, 13 Sep 2016 09:33:48 +0200 |
User-agent: |
Gnus/5.13 (Gnus v5.13) Emacs/24.5 (gnu/linux) |
Eric Blake <address@hidden> writes:
> On 09/12/2016 04:18 AM, Marc-André Lureau wrote:
>> There are better chances to find what went wrong at build time than a
>> later assert in qmp_query_version
>>
>> Signed-off-by: Marc-André Lureau <address@hidden>
>> ---
>> qmp.c | 16 +++-------------
>> scripts/create_config | 6 ++++++
>> 2 files changed, 9 insertions(+), 13 deletions(-)
>>
>> diff --git a/qmp.c b/qmp.c
>> index dea8f81..6733463 100644
>> --- a/qmp.c
>> +++ b/qmp.c
>> @@ -51,21 +51,11 @@ NameInfo *qmp_query_name(Error **errp)
>> VersionInfo *qmp_query_version(Error **errp)
>> {
>> VersionInfo *info = g_new0(VersionInfo, 1);
>> - const char *version = QEMU_VERSION;
>> - const char *tmp;
>> - int err;
>>
>> info->qemu = g_new0(VersionTriple, 1);
>> - err = qemu_strtoll(version, &tmp, 10, &info->qemu->major);
>> - assert(err == 0);
>> - tmp++;
>> -
>> - err = qemu_strtoll(tmp, &tmp, 10, &info->qemu->micro);
>> - assert(err == 0);
>> + info->qemu->major = QEMU_VERSION_MAJOR;
>> + info->qemu->minor = QEMU_VERSION_MINOR;
>> + info->qemu->micro = QEMU_VERSION_MICRO;
>> info->package = g_strdup(QEMU_PKGVERSION);
>>
>> return info;
>
> The old code silently ignores any garbage after the third integer (it
> populates &tmp, but never checks the value of tmp).
It also accepts any separator character between the integers, not just
'.'.
>> diff --git a/scripts/create_config b/scripts/create_config
>> index 1dd6a35..e6929dd 100755
>> --- a/scripts/create_config
>> +++ b/scripts/create_config
>> @@ -7,7 +7,13 @@ while read line; do
>> case $line in
>> VERSION=*) # configuration
>> version=${line#*=}
>> + major=$(echo "$version" | cut -d. -f1)
>> + minor=$(echo "$version" | cut -d. -f2)
>> + micro=$(echo "$version" | cut -d. -f3)
>> echo "#define QEMU_VERSION \"$version\""
>> + echo "#define QEMU_VERSION_MAJOR $major"
>> + echo "#define QEMU_VERSION_MINOR $minor"
>> + echo "#define QEMU_VERSION_MICRO $micro"
>
> The new code likewise ignores any fourth field.
Nope:
$ echo "2.7.50garbage" | cut -d. -f3
50garbage
The compiler will choke on
info->qemu->micro = QEMU_VERSION_MICRO;
because it's
info->qemu->micro = 50garbage;
after preprocessing.
The commit message could mention that VERSION is now parsed more
strictly (configure checks '.', the compiler checks integers), but I
guess it's not worth the bother now.
> Do we care either way? Unless someone else has a reason for why we
> should care, I'm fine with:
> Reviewed-by: Eric Blake <address@hidden>
Does your R-by stand?
- [Qemu-devel] [PATCH v6 00/18] qapi: remove the 'middle' mode, Marc-André Lureau, 2016/09/12
- [Qemu-devel] [PATCH v6 02/18] qapi-schema: use generated marshaller for 'qmp_capabilities', Marc-André Lureau, 2016/09/12
- [Qemu-devel] [PATCH v6 03/18] qapi-schema: add 'device_add', Marc-André Lureau, 2016/09/12
- [Qemu-devel] [PATCH v6 04/18] monitor: simplify invalid_qmp_mode(), Marc-André Lureau, 2016/09/12
- [Qemu-devel] [PATCH v6 05/18] monitor: register gen:false commands manually, Marc-André Lureau, 2016/09/12
- [Qemu-devel] [PATCH v6 06/18] qapi: Support unregistering QMP commands, Marc-André Lureau, 2016/09/12
- [Qemu-devel] [PATCH v6 07/18] qmp: Hack to keep commands configuration-specific, Marc-André Lureau, 2016/09/12
- [Qemu-devel] [PATCH v6 08/18] qapi: export the marshallers, Marc-André Lureau, 2016/09/12
- [Qemu-devel] [PATCH v6 10/18] monitor: implement 'qmp_query_commands' without qmp_cmds, Marc-André Lureau, 2016/09/12
- [Qemu-devel] [PATCH v6 09/18] monitor: use qmp_find_command() (using generated qapi code), Marc-André Lureau, 2016/09/12