[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v9 11/37] qapi: Consolidate visitor small intege
From: |
Eric Blake |
Subject: |
Re: [Qemu-devel] [PATCH v9 11/37] qapi: Consolidate visitor small integer callbacks |
Date: |
Wed, 20 Jan 2016 11:17:31 -0700 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.5.0 |
On 01/20/2016 10:34 AM, Markus Armbruster wrote:
> Eric Blake <address@hidden> writes:
>
>> Commit 4e27e819 introduced optional visitor callbacks for all
>> sorts of int types, but no visitor has supplied any of the
>> callbacks for sizes less than 64 bits. In other words, the
>> generic implementation based on using type_[u]int64() followed
>> by bounds-checking works just fine. In the interest of
>> simplicity, it's easier to make the visitor callback interface
>> not have to worry about the other sizes.
>>
>> Adding some helper functions minimizes the boilerplate required
>> to correct FIXMEs added earlier with regards to questionable
>> reuse of errp, particularly now that we can guarantee from a
>> single file audit that value is unchanged if an error is set.
>>
>> Signed-off-by: Eric Blake <address@hidden>
>> Reviewed-by: Marc-André Lureau <address@hidden>
>>
>> ---
>> v9: hoist some of visitor-impl.h changes into 9/35 and 10/35
>> v8: no change
>> v7: further factor out helper functions that eliminate the
>> questionable errp reuse
>> v6: split off from v5 23/46
>> original version also appeared in v6-v9 of subset D
>> ---
>> include/qapi/visitor-impl.h | 8 +--
>> qapi/qapi-visit-core.c | 158
>> +++++++++++++++++---------------------------
>> 2 files changed, 60 insertions(+), 106 deletions(-)
>
> Nice diffstat.
>
>>
>> diff --git a/include/qapi/visitor-impl.h b/include/qapi/visitor-impl.h
>> index 45c1d3e..e6399d1 100644
>> --- a/include/qapi/visitor-impl.h
>> +++ b/include/qapi/visitor-impl.h
>> @@ -1,7 +1,7 @@
>> /*
>> * Core Definitions for QAPI Visitor implementations
>> *
>> - * Copyright (C) 2012 Red Hat, Inc.
>> + * Copyright (C) 2012, 2015-2016 Red Hat, Inc.
>
> git-log has authors @redhat.com in 2013 and 2014 as well.
I didn't bother to check whether those edits were complex enough to
warrant claiming copyright additions; but I'm also amenable to
shortening to '2012-2016' on a respin regardless of the sizes of those
edits. As it is, I was not very careful about adding 2016 in the v9
spin, so I may be inconsistent on which years are claimed where, in
comparison to where I felt I was adding new copyrightable content rather
than just minor fixing of existing content.
[While it's nice that we DON'T have to assign copyright to a central
organization to contribute to qemu, sometimes it is much nicer working
on FSF code where a single copyright holder makes discussions like this
moot.]
>> + } else if (value > max) {
>> + error_setg(errp, QERR_INVALID_PARAMETER_VALUE,
>> + name ? name : "null", type);
>
> We should clean up this name ? name : "null" nonsense some day.
It all stems from whether the visitor is locally visiting { 'name':value
} vs. [ value ]; in an array visit, there is no direct name.
Maybe we could go a step higher; if we have:
{ 'name': [ value ] }
then we could require callers to parse value by passing in "[name]" or
"name[0]" rather than NULL. Then audit the code to always pass in a
sensible name at the top level of a parse. It would even extend to 2-D
arrays, via "[[name]]" or "name[0][0]". But not in this series.
--
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 12/37] qapi: Don't cast Enum* to int*, (continued)
[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
[Qemu-devel] [PATCH v9 20/37] qmp: Don't abuse stack to track qmp-output root, Eric Blake, 2016/01/19