On 09/13/2016 02:33 AM, Markus Armbruster wrote: >>> info->qemu = g_new0(VersionTriple, 1); >>> - err = qemu_strtoll(version, &tmp, 10, &info->qemu->major); >>> - assert(err == 0); >>> - tmp++; >>> -
>>
>> 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
> '.'.
True, and didn't we have a release of qemu a few years ago (perhaps it
was a downstream Red Hat release; I can't quickly find it now...) where
we accidentally released the version string with a comma instead of a dot?
>>> 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
That's not a fourth field. I was thinking '2.7.50.0' ignoring '.0'.
>
> 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.
Indeed, the fact that we now separate on exactly '.' instead of the end
of an integer is different.
>
>> 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?
Yes - while the parsing may have changed, it doesn't affect the common
case of a correctly-formed VERSION file. I'm okay if you want to tweak
the commit message to call out a summary of this conversation, but since
the patch is already on qapi-next staging, I'm also okay if it doesn't
change.
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature
