[Top][All Lists]

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

Re: [Qemu-devel] [PATCH v9 10/37] qapi: Make all visitors supply uint64

From: Eric Blake
Subject: Re: [Qemu-devel] [PATCH v9 10/37] qapi: Make all visitors supply uint64 callbacks
Date: Thu, 21 Jan 2016 10:22:20 -0700
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.5.0

On 01/21/2016 01:56 AM, Markus Armbruster wrote:

>>> Before: nobody implements type_uint64(), and the core falls back to
>>> type_int64(), casting negative values to large positive ones.  With an
>>> implementation of type_int64() that parses large positive values as
>>> negative, the two casts cancel out.
>>> After: everybody implements type_uint64() incorrectly, by reusing
>>> type_int64() code.  The problem moves from visitor core to visitor
>>> implementations, where we can actually fix it if we care.
>>> Correct?
>> Close. opts-visitor.c already implemented type_uint64, but also has its
>> known bugs (and Paolo has been pinged about his claim for fixes in that
>> file...)
> Ah, yes.  opts_type_uint64() doesn't reuse opts_type_int64(), but
> largely duplicates it.  Potentially less wrong :)
>> But otherwise, yes, in this patch, we are not fixing insanity so much as
>> localizing and better documenting it.
> Let me try to clarify the commit message a bit:
>     This patch does not address the disparity in handling large values
>     as negatives.  It merely moves the fallback from uint64 to int64
>     from the visitor core to the visitors, where the issue can actually
>     be fixed, by implementing the missing type_uint64() callbacks on top
>     of the respective type_int64() callbacks, with a FIXME comment
>     explaining why that's wrong.

Looks good.  I think we're starting to rack up enough tweaks to make it
worth me posting a v10 respin to fold them all in.

>>>> The dealloc visitor no longer needs a type_size callback,
>>>> since that now safely falls back to the type_uint64 callback.
>>> Did it need the callback before this patch?
>> Not really.  Should I split out the deletion of that callback as a
>> separate patch?
> Probably cleanest, but merely adjusting the commit message would also
> work for me:
>     The dealloc visitor doesn't actually need a type_size() callback,
>     since the visitor core can safely fall back to the type_uint64()
>     callback.  Drop it.

Okay, I won't bother to split this one; the commit message tweak seems

Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org

Attachment: signature.asc
Description: OpenPGP digital signature

reply via email to

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