Our qapi visitor contract supports multiple integer visitors: type_int (64-bit signed; mandatory), type_int64 (64-bit signed, optional), type_uint64 (64-bit unsigned, optional), and type_size (64-bit unsigned, optional, with the possibility of parsing differently than type_uint64 for the case of suffixed sizes). But the default code treats any implementation without type_uint64 as just falling back on type_int, which can cause confusing results for values larger than LLONG_MAX (such as having to pass in a negative 2s complement value on input, and getting a negative result on output).
This patch does not fully address the disparity in parsing, but does move towards a cleaner situation where EVERY visitor provides both signed and unsigned variants as entry points; then each client can either implement sane differences between the two, or document in place with a FIXME that there is munging going on for large values. The next patch will then clean up the code to no longer allow conditional type_uint64. Signed-off-by: Eric Blake <ebl...@redhat.com> --- v6: new patch, but stems from v5 23/46 --- qapi/opts-visitor.c | 5 +++-- qapi/qapi-dealloc-visitor.c | 19 ++++++++++--------- qapi/qmp-input-visitor.c | 23 ++++++++++++++++++++--- qapi/qmp-output-visitor.c | 15 ++++++++++++--- qapi/string-input-visitor.c | 22 +++++++++++++++++++--- qapi/string-output-visitor.c | 16 +++++++++++++--- 6 files changed, 77 insertions(+), 23 deletions(-) diff --git a/qapi/opts-visitor.c b/qapi/opts-visitor.c index ef5fb8b..bc2b976 100644 --- a/qapi/opts-visitor.c +++ b/qapi/opts-visitor.c @@ -354,7 +354,7 @@ opts_type_bool(Visitor *v, bool *obj, const char *name, Error **errp) static void -opts_type_int(Visitor *v, int64_t *obj, const char *name, Error **errp) +opts_type_int64(Visitor *v, int64_t *obj, const char *name, Error **errp) { OptsVisitor *ov = DO_UPCAST(OptsVisitor, visitor, v); const QemuOpt *opt; @@ -522,7 +522,8 @@ opts_visitor_new(const QemuOpts *opts) */ ov->visitor.type_enum = &input_type_enum; - ov->visitor.type_int = &opts_type_int; + 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; ov->visitor.type_bool = &opts_type_bool; diff --git a/qapi/qapi-dealloc-visitor.c b/qapi/qapi-dealloc-visitor.c index 737deab..5d1b2e6 100644 --- a/qapi/qapi-dealloc-visitor.c +++ b/qapi/qapi-dealloc-visitor.c @@ -136,8 +136,13 @@ static void qapi_dealloc_type_str(Visitor *v, char **obj, const char *name, } } -static void qapi_dealloc_type_int(Visitor *v, int64_t *obj, const char *name, - Error **errp) +static void qapi_dealloc_type_int64(Visitor *v, int64_t *obj, + const char *name, Error **errp) +{ +} + +static void qapi_dealloc_type_uint64(Visitor *v, uint64_t *obj, + const char *name, Error **errp) { } @@ -159,11 +164,6 @@ static void qapi_dealloc_type_anything(Visitor *v, QObject **obj, } } -static void qapi_dealloc_type_size(Visitor *v, uint64_t *obj, const char *name, - Error **errp) -{ -} - static void qapi_dealloc_type_enum(Visitor *v, int *obj, const char * const strings[], const char *kind, const char *name, @@ -220,12 +220,13 @@ 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_int; + 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; v->visitor.type_str = qapi_dealloc_type_str; v->visitor.type_number = qapi_dealloc_type_number; v->visitor.type_any = qapi_dealloc_type_anything; - v->visitor.type_size = qapi_dealloc_type_size; v->visitor.start_union = qapi_dealloc_start_union; QTAILQ_INIT(&v->stack); diff --git a/qapi/qmp-input-visitor.c b/qapi/qmp-input-visitor.c index 932b5f3..96dafcb 100644 --- a/qapi/qmp-input-visitor.c +++ b/qapi/qmp-input-visitor.c @@ -224,8 +224,23 @@ static void qmp_input_get_next_type(Visitor *v, QType *type, bool promote_int, } } -static void qmp_input_type_int(Visitor *v, int64_t *obj, const char *name, - Error **errp) +static void qmp_input_type_int64(Visitor *v, int64_t *obj, const char *name, + Error **errp) +{ + QmpInputVisitor *qiv = to_qiv(v); + QInt *qint = qobject_to_qint(qmp_input_get_object(qiv, name, true)); + + if (!qint) { + error_setg(errp, QERR_INVALID_PARAMETER_TYPE, name ? name : "null", + "integer"); + return; + } + + *obj = qint_get_int(qint); +} + +static void qmp_input_type_uint64(Visitor *v, uint64_t *obj, const char *name, + Error **errp) { QmpInputVisitor *qiv = to_qiv(v); QInt *qint = qobject_to_qint(qmp_input_get_object(qiv, name, true)); @@ -341,7 +356,9 @@ QmpInputVisitor *qmp_input_visitor_new(QObject *obj) v->visitor.next_list = qmp_input_next_list; v->visitor.end_list = qmp_input_end_list; v->visitor.type_enum = input_type_enum; - v->visitor.type_int = qmp_input_type_int; + v->visitor.type_int = qmp_input_type_int64; + v->visitor.type_int64 = qmp_input_type_int64; + v->visitor.type_uint64 = qmp_input_type_uint64; v->visitor.type_bool = qmp_input_type_bool; v->visitor.type_str = qmp_input_type_str; v->visitor.type_number = qmp_input_type_number; diff --git a/qapi/qmp-output-visitor.c b/qapi/qmp-output-visitor.c index 29899ac..a52f26f 100644 --- a/qapi/qmp-output-visitor.c +++ b/qapi/qmp-output-visitor.c @@ -158,8 +158,15 @@ static void qmp_output_end_list(Visitor *v, Error **errp) qmp_output_pop(qov); } -static void qmp_output_type_int(Visitor *v, int64_t *obj, const char *name, - Error **errp) +static void qmp_output_type_int64(Visitor *v, int64_t *obj, const char *name, + Error **errp) +{ + QmpOutputVisitor *qov = to_qov(v); + qmp_output_add(qov, name, qint_from_int(*obj)); +} + +static void qmp_output_type_uint64(Visitor *v, uint64_t *obj, const char *name, + Error **errp) { QmpOutputVisitor *qov = to_qov(v); qmp_output_add(qov, name, qint_from_int(*obj)); @@ -241,7 +248,9 @@ QmpOutputVisitor *qmp_output_visitor_new(void) v->visitor.next_list = qmp_output_next_list; v->visitor.end_list = qmp_output_end_list; v->visitor.type_enum = output_type_enum; - v->visitor.type_int = qmp_output_type_int; + v->visitor.type_int = qmp_output_type_int64; + v->visitor.type_int64 = qmp_output_type_int64; + v->visitor.type_uint64 = qmp_output_type_uint64; v->visitor.type_bool = qmp_output_type_bool; v->visitor.type_str = qmp_output_type_str; v->visitor.type_number = qmp_output_type_number; diff --git a/qapi/string-input-visitor.c b/qapi/string-input-visitor.c index dee780a..0931321 100644 --- a/qapi/string-input-visitor.c +++ b/qapi/string-input-visitor.c @@ -179,8 +179,8 @@ end_list(Visitor *v, Error **errp) siv->head = true; } -static void parse_type_int(Visitor *v, int64_t *obj, const char *name, - Error **errp) +static void parse_type_int64(Visitor *v, int64_t *obj, const char *name, + Error **errp) { StringInputVisitor *siv = DO_UPCAST(StringInputVisitor, visitor, v); @@ -221,6 +221,20 @@ error: "an int64 value or range"); } +static void parse_type_uint64(Visitor *v, uint64_t *obj, const char *name, + Error **errp) +{ + /* FIXME - don't handle numbers > INT64_MAX as negative */ + int64_t i; + Error *err = NULL; + parse_type_int64(v, &i, name, &err); + if (err) { + error_propagate(errp, err); + } else { + *obj = i; + } +} + static void parse_type_size(Visitor *v, uint64_t *obj, const char *name, Error **errp) { @@ -330,7 +344,9 @@ StringInputVisitor *string_input_visitor_new(const char *str) v = g_malloc0(sizeof(*v)); v->visitor.type_enum = input_type_enum; - v->visitor.type_int = parse_type_int; + v->visitor.type_int = parse_type_int64; + v->visitor.type_int64 = parse_type_int64; + v->visitor.type_uint64 = parse_type_uint64; v->visitor.type_size = parse_type_size; v->visitor.type_bool = parse_type_bool; v->visitor.type_str = parse_type_str; diff --git a/qapi/string-output-visitor.c b/qapi/string-output-visitor.c index b86ce2c..83ca6cc 100644 --- a/qapi/string-output-visitor.c +++ b/qapi/string-output-visitor.c @@ -116,8 +116,8 @@ static void format_string(StringOutputVisitor *sov, Range *r, bool next, } } -static void print_type_int(Visitor *v, int64_t *obj, const char *name, - Error **errp) +static void print_type_int64(Visitor *v, int64_t *obj, const char *name, + Error **errp) { StringOutputVisitor *sov = DO_UPCAST(StringOutputVisitor, visitor, v); GList *l; @@ -192,6 +192,14 @@ static void print_type_int(Visitor *v, int64_t *obj, const char *name, } } +static void print_type_uint64(Visitor *v, uint64_t *obj, const char *name, + Error **errp) +{ + /* FIXME - don't handle numbers > INT64_MAX as negative */ + int64_t i = *obj; + print_type_int64(v, &i, name, errp); +} + static void print_type_size(Visitor *v, uint64_t *obj, const char *name, Error **errp) { @@ -340,7 +348,9 @@ StringOutputVisitor *string_output_visitor_new(bool human) v->string = g_string_new(NULL); v->human = human; v->visitor.type_enum = output_type_enum; - v->visitor.type_int = print_type_int; + v->visitor.type_int = print_type_int64; + v->visitor.type_int64 = print_type_int64; + v->visitor.type_uint64 = print_type_uint64; v->visitor.type_size = print_type_size; v->visitor.type_bool = print_type_bool; v->visitor.type_str = print_type_str; -- 2.4.3