[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
sufficient.
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature
- Re: [Qemu-devel] [PATCH v9 02/37] qapi: Avoid use of misnamed DO_UPCAST(), (continued)
[Qemu-devel] [PATCH v9 12/37] qapi: Don't cast Enum* to int*, Eric Blake, 2016/01/19
[Qemu-devel] [PATCH v9 10/37] qapi: Make all visitors supply uint64 callbacks, Eric Blake, 2016/01/19
[Qemu-devel] [PATCH v9 11/37] qapi: Consolidate visitor small integer callbacks, Eric Blake, 2016/01/19
[Qemu-devel] [PATCH v9 08/37] qapi: Track all failures between visit_start/stop, Eric Blake, 2016/01/19
[Qemu-devel] [PATCH v9 09/37] qapi: Prefer type_int64 over type_int in visitors, Eric Blake, 2016/01/19