On 01/20/2016 10:34 AM, Markus Armbruster wrote: > Eric Blake <ebl...@redhat.com> 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 <ebl...@redhat.com> >> Reviewed-by: Marc-André Lureau <marcandre.lur...@redhat.com> >> >> --- >> 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
