Eric Blake <ebl...@redhat.com> 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 <marcandre.lur...@redhat.com>
>> ---
>> 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 <ebl...@redhat.com>
Does your R-by stand?
