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. > * > * Author: Paolo Bonizni <pbonz...@redhat.com> > * > @@ -56,12 +56,6 @@ struct Visitor > /* May be NULL; most useful for input visitors. */ > void (*optional)(Visitor *v, bool *present, const char *name); > > - void (*type_uint8)(Visitor *v, uint8_t *obj, const char *name, Error > **errp); > - void (*type_uint16)(Visitor *v, uint16_t *obj, const char *name, Error > **errp); > - void (*type_uint32)(Visitor *v, uint32_t *obj, const char *name, Error > **errp); > - void (*type_int8)(Visitor *v, int8_t *obj, const char *name, Error > **errp); > - void (*type_int16)(Visitor *v, int16_t *obj, const char *name, Error > **errp); > - void (*type_int32)(Visitor *v, int32_t *obj, const char *name, Error > **errp); > bool (*start_union)(Visitor *v, bool data_present, Error **errp); > void (*end_union)(Visitor *v, bool data_present, Error **errp); > }; > diff --git a/qapi/qapi-visit-core.c b/qapi/qapi-visit-core.c > index 4a8ad43..a48fd4e 100644 > --- a/qapi/qapi-visit-core.c > +++ b/qapi/qapi-visit-core.c > @@ -100,129 +100,89 @@ void visit_type_int(Visitor *v, int64_t *obj, const > char *name, Error **errp) > v->type_int64(v, obj, name, errp); > } > > +static void visit_type_uintN(Visitor *v, uint64_t *obj, const char *name, > + uint64_t max, const char *type, Error **errp) > +{ > + Error *err = NULL; > + uint64_t value = *obj; > + > + v->type_uint64(v, &value, name, &err); > + if (err) { > + error_propagate(errp, err); > + } 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. > + } else { > + *obj = value; > + } > +} [...]