Eric Blake <ebl...@redhat.com> writes:
> Commit 4e27e819 introduced optional visitor callbacks for all
> sorts of int types, but except for type_uint64() and type_size(),
> none of them have ever been supplied (the generic implementation
> based on using type_[u]int64() then 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.
>
> Signed-off-by: Eric Blake <ebl...@redhat.com>
> ---
> v6: split off from v5 23/46
> original version also appeared in v6-v9 of subset D
> ---
> include/qapi/visitor-impl.h | 22 ++++-----
> qapi/qapi-visit-core.c | 117
> ++++++++++++++++++--------------------------
> 2 files changed, 59 insertions(+), 80 deletions(-)
>
> diff --git a/include/qapi/visitor-impl.h b/include/qapi/visitor-impl.h
> index 70326e0..d071c06 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 Red Hat, Inc.
> *
> * Author: Paolo Bonizni <pbonz...@redhat.com>
> *
> @@ -36,6 +36,16 @@ struct Visitor
> void (*get_next_type)(Visitor *v, QType *type, bool promote_int,
> const char *name, Error **errp);
>
> + /* Must be set. */
> + void (*type_int64)(Visitor *v, int64_t *obj, const char *name,
> + Error **errp);
> + /* Must be set. */
> + void (*type_uint64)(Visitor *v, uint64_t *obj, const char *name,
> + Error **errp);
> + /* Only required to visit sizes differently than (*type_uint64)(). */
What about:
/* Optional; fallback is type_uint64() */
> + void (*type_size)(Visitor *v, uint64_t *obj, const char *name,
> + Error **errp);
> + /* Must be set. */
> 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,
> @@ -46,16 +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_uint64)(Visitor *v, uint64_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);
> - void (*type_int64)(Visitor *v, int64_t *obj, const char *name, Error
> **errp);
> - /* visit_type_size() falls back to (*type_uint64)() if type_size is
> unset */
> - void (*type_size)(Visitor *v, uint64_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 88bed9c..be1fcdd 100644
> --- a/qapi/qapi-visit-core.c
> +++ b/qapi/qapi-visit-core.c
> @@ -104,57 +104,48 @@ void visit_type_uint8(Visitor *v, uint8_t *obj, const
> char *name, Error **errp)
> {
> uint64_t value;
>
> - if (v->type_uint8) {
> - v->type_uint8(v, obj, name, errp);
> - } else {
> - value = *obj;
> - v->type_uint64(v, &value, name, errp);
> - if (value > UINT8_MAX) {
> - error_setg(errp, QERR_INVALID_PARAMETER_VALUE,
> - name ? name : "null", "uint8_t");
> - return;
> - }
> - *obj = value;
> + value = *obj;
> + v->type_uint64(v, &value, name, errp);
> + if (value > UINT8_MAX) {
> + error_setg(errp, QERR_INVALID_PARAMETER_VALUE,
> + name ? name : "null", "uint8_t");
> + return;
> }
> + *obj = value;
> }
Best reviewed with whitespace ignored.
Have you considered doing this before PATCH 02? Hmm, swapping them now
is probably not worth the bother.
[...]
