Eric Blake <ebl...@redhat.com> writes: > Now that all visitors supply both type_int64() and type_uint64() > callbacks, we can drop the redundant type_int() callback (the > public interface visit_type_int() remains, but calls into > type_int64() under the hood). > > Signed-off-by: Eric Blake <ebl...@redhat.com> > > --- > v6: new patch, but stems from v5 23/46 > --- > include/qapi/visitor-impl.h | 1 - > qapi/opts-visitor.c | 1 - > qapi/qapi-dealloc-visitor.c | 1 - > qapi/qapi-visit-core.c | 50 > ++++++++++++++------------------------------ > qapi/qmp-input-visitor.c | 1 - > qapi/qmp-output-visitor.c | 1 - > qapi/string-input-visitor.c | 1 - > qapi/string-output-visitor.c | 1 - > 8 files changed, 16 insertions(+), 41 deletions(-) > > diff --git a/include/qapi/visitor-impl.h b/include/qapi/visitor-impl.h > index 44a21b7..70326e0 100644 > --- a/include/qapi/visitor-impl.h > +++ b/include/qapi/visitor-impl.h > @@ -36,7 +36,6 @@ struct Visitor > void (*get_next_type)(Visitor *v, QType *type, bool promote_int, > const char *name, Error **errp); > > - void (*type_int)(Visitor *v, int64_t *obj, const char *name, Error > **errp); > void (*type_bool)(Visitor *v, bool *obj, const char *name, Error **errp); > void (*type_str)(Visitor *v, char **obj, const char *name, Error **errp); > void (*type_number)(Visitor *v, double *obj, const char *name, > diff --git a/qapi/opts-visitor.c b/qapi/opts-visitor.c > index bc2b976..8104e42 100644 > --- a/qapi/opts-visitor.c > +++ b/qapi/opts-visitor.c > @@ -522,7 +522,6 @@ opts_visitor_new(const QemuOpts *opts) > */ > ov->visitor.type_enum = &input_type_enum; > > - ov->visitor.type_int = &opts_type_int64; > ov->visitor.type_int64 = &opts_type_int64; > ov->visitor.type_uint64 = &opts_type_uint64; > ov->visitor.type_size = &opts_type_size; > diff --git a/qapi/qapi-dealloc-visitor.c b/qapi/qapi-dealloc-visitor.c > index 5d1b2e6..5133d37 100644 > --- a/qapi/qapi-dealloc-visitor.c > +++ b/qapi/qapi-dealloc-visitor.c > @@ -220,7 +220,6 @@ QapiDeallocVisitor *qapi_dealloc_visitor_new(void) > v->visitor.next_list = qapi_dealloc_next_list; > v->visitor.end_list = qapi_dealloc_end_list; > v->visitor.type_enum = qapi_dealloc_type_enum; > - v->visitor.type_int = qapi_dealloc_type_int64; > v->visitor.type_int64 = qapi_dealloc_type_int64; > v->visitor.type_uint64 = qapi_dealloc_type_uint64; > v->visitor.type_bool = qapi_dealloc_type_bool; > diff --git a/qapi/qapi-visit-core.c b/qapi/qapi-visit-core.c > index 6d63e40..88bed9c 100644 > --- a/qapi/qapi-visit-core.c > +++ b/qapi/qapi-visit-core.c > @@ -97,19 +97,19 @@ void visit_type_enum(Visitor *v, int *obj, const char * > const strings[], > > void visit_type_int(Visitor *v, int64_t *obj, const char *name, Error **errp) > { > - v->type_int(v, obj, name, errp); > + v->type_int64(v, obj, name, errp); > } > > void visit_type_uint8(Visitor *v, uint8_t *obj, const char *name, Error > **errp) > { > - int64_t value; > + uint64_t value; > > if (v->type_uint8) { > v->type_uint8(v, obj, name, errp); > } else { > value = *obj; > - v->type_int(v, &value, name, errp); > - if (value < 0 || value > UINT8_MAX) { > + v->type_uint64(v, &value, name, errp); > + if (value > UINT8_MAX) { > error_setg(errp, QERR_INVALID_PARAMETER_VALUE, > name ? name : "null", "uint8_t"); > return;
Note that this relies on value being in range after type_uint64() fails. If it isn't, we call error_setg() with non-null *errp. Two solutions: 1. Stipulate that type_uint64() & friends leave value alone on error. Works, because its initial value *obj is in range. 2. Avoid using value on error. A clean way to do this: Error *err = NULL; value = *obj; v->type_uint64(v, &value, name, &err); if (err) { error_propagate(errp, err); return; } if (value < 0 || value > UINT8_MAX) { error_setg(errp, QERR_INVALID_PARAMETER_VALUE, name ? name : "null", "uint8_t"); return; } *obj = value; More boilerplate. If we pick this solution, we'll want a separate PATCH 1.5 cleaning up the preexisting instances. > @@ -120,14 +120,14 @@ void visit_type_uint8(Visitor *v, uint8_t *obj, const > char *name, Error **errp) > > void visit_type_uint16(Visitor *v, uint16_t *obj, const char *name, Error > **errp) > { > - int64_t value; > + uint64_t value; > > if (v->type_uint16) { > v->type_uint16(v, obj, name, errp); > } else { > value = *obj; > - v->type_int(v, &value, name, errp); > - if (value < 0 || value > UINT16_MAX) { > + v->type_uint64(v, &value, name, errp); > + if (value > UINT16_MAX) { > error_setg(errp, QERR_INVALID_PARAMETER_VALUE, > name ? name : "null", "uint16_t"); > return; Good. The fewer signed-unsigned conversions, the better. [...]