From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:48011) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1a69NE-0004K4-Tu for qemu-devel@nongnu.org; Mon, 07 Dec 2015 22:55:34 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1a69NC-0003Y0-Nq for qemu-devel@nongnu.org; Mon, 07 Dec 2015 22:55:32 -0500 Received: from mx1.redhat.com ([209.132.183.28]:46687) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1a69NC-0003Xf-Dq for qemu-devel@nongnu.org; Mon, 07 Dec 2015 22:55:30 -0500 From: Eric Blake Date: Mon, 7 Dec 2015 20:55:00 -0700 Message-Id: <1449546921-6378-11-git-send-email-eblake@redhat.com> In-Reply-To: <1449546921-6378-1-git-send-email-eblake@redhat.com> References: <1449546921-6378-1-git-send-email-eblake@redhat.com> Subject: [Qemu-devel] [PATCH v7 10/31] qapi: Make all visitors supply uint64 callbacks List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: qemu-devel@nongnu.org Cc: armbru@redhat.com, Michael Roth Our qapi visitor contract supports multiple integer visitors, but left the type_uint64 visitor as optional (falling back on type_int64); it also marks the type_size visitor as optional (falling back on type_uint64 or even type_int64). Note that the default of falling back on type_int for unsigned visitors can cause confusing results for values larger than INT64_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 handling large values as negatives, but does move towards a cleaner situation where EVERY visitor provides both type_int64 and type_uint64 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. The dealloc visitor no longer needs a type_size callback, since that now safely falls back to the type_uint64 callback. Then, in qapi-visit-core.c, we can now use the guaranteed type_uint64 callback as the fallback for all smaller unsigned int visits. Signed-off-by: Eric Blake --- v7: split off int64 callbacks and retitle, add more FIXMEs in the code, hoist use of type_uint64 here from 3/23, improved commit message v6: new patch, but stems from v5 23/46 --- qapi/qapi-dealloc-visitor.c | 12 ++++++------ qapi/qapi-visit-core.c | 42 ++++++++++++++---------------------------- qapi/qmp-input-visitor.c | 17 +++++++++++++++++ qapi/qmp-output-visitor.c | 9 +++++++++ qapi/string-input-visitor.c | 15 +++++++++++++++ qapi/string-output-visitor.c | 9 +++++++++ 6 files changed, 70 insertions(+), 34 deletions(-) diff --git a/qapi/qapi-dealloc-visitor.c b/qapi/qapi-dealloc-visitor.c index e9b9f3f..11eb828 100644 --- a/qapi/qapi-dealloc-visitor.c +++ b/qapi/qapi-dealloc-visitor.c @@ -140,6 +140,11 @@ static void qapi_dealloc_type_int64(Visitor *v, int64_t *obj, const char *name, { } +static void qapi_dealloc_type_uint64(Visitor *v, uint64_t *obj, + const char *name, Error **errp) +{ +} + static void qapi_dealloc_type_bool(Visitor *v, bool *obj, const char *name, Error **errp) { @@ -158,11 +163,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,11 +220,11 @@ QapiDeallocVisitor *qapi_dealloc_visitor_new(void) v->visitor.end_list = qapi_dealloc_end_list; v->visitor.type_enum = qapi_dealloc_type_enum; 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/qapi-visit-core.c b/qapi/qapi-visit-core.c index 6295fa8..4a8ad43 100644 --- a/qapi/qapi-visit-core.c +++ b/qapi/qapi-visit-core.c @@ -102,15 +102,15 @@ void visit_type_int(Visitor *v, int64_t *obj, const char *name, Error **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_int64(v, &value, name, errp); - if (value < 0 || value > UINT8_MAX) { - /* FIXME questionable reuse of errp if type_int64() changes + v->type_uint64(v, &value, name, errp); + if (value > UINT8_MAX) { + /* FIXME questionable reuse of errp if type_uint64() changes value on error */ error_setg(errp, QERR_INVALID_PARAMETER_VALUE, name ? name : "null", "uint8_t"); @@ -122,15 +122,15 @@ 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_int64(v, &value, name, errp); - if (value < 0 || value > UINT16_MAX) { - /* FIXME questionable reuse of errp if type_int64() changes + v->type_uint64(v, &value, name, errp); + if (value > UINT16_MAX) { + /* FIXME questionable reuse of errp if type_uint64() changes value on error */ error_setg(errp, QERR_INVALID_PARAMETER_VALUE, name ? name : "null", "uint16_t"); @@ -142,15 +142,15 @@ void visit_type_uint16(Visitor *v, uint16_t *obj, const char *name, Error **errp void visit_type_uint32(Visitor *v, uint32_t *obj, const char *name, Error **errp) { - int64_t value; + uint64_t value; if (v->type_uint32) { v->type_uint32(v, obj, name, errp); } else { value = *obj; - v->type_int64(v, &value, name, errp); - if (value < 0 || value > UINT32_MAX) { - /* FIXME questionable reuse of errp if type_int64() changes + v->type_uint64(v, &value, name, errp); + if (value > UINT32_MAX) { + /* FIXME questionable reuse of errp if type_uint64() changes value on error */ error_setg(errp, QERR_INVALID_PARAMETER_VALUE, name ? name : "null", "uint32_t"); @@ -162,15 +162,7 @@ void visit_type_uint32(Visitor *v, uint32_t *obj, const char *name, Error **errp void visit_type_uint64(Visitor *v, uint64_t *obj, const char *name, Error **errp) { - int64_t value; - - if (v->type_uint64) { - v->type_uint64(v, obj, name, errp); - } else { - value = *obj; - v->type_int64(v, &value, name, errp); - *obj = value; - } + v->type_uint64(v, obj, name, errp); } void visit_type_int8(Visitor *v, int8_t *obj, const char *name, Error **errp) @@ -240,16 +232,10 @@ void visit_type_int64(Visitor *v, int64_t *obj, const char *name, Error **errp) void visit_type_size(Visitor *v, uint64_t *obj, const char *name, Error **errp) { - int64_t value; - if (v->type_size) { v->type_size(v, obj, name, errp); - } else if (v->type_uint64) { - v->type_uint64(v, obj, name, errp); } else { - value = *obj; - v->type_int64(v, &value, name, errp); - *obj = value; + v->type_uint64(v, obj, name, errp); } } diff --git a/qapi/qmp-input-visitor.c b/qapi/qmp-input-visitor.c index 0d8a3c3..32b60bb 100644 --- a/qapi/qmp-input-visitor.c +++ b/qapi/qmp-input-visitor.c @@ -239,6 +239,22 @@ static void qmp_input_type_int64(Visitor *v, int64_t *obj, const char *name, *obj = qint_get_int(qint); } +static void qmp_input_type_uint64(Visitor *v, uint64_t *obj, const char *name, + Error **errp) +{ + /* FIXME: qobject_to_qint mishandles values over INT64_MAX */ + 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_bool(Visitor *v, bool *obj, const char *name, Error **errp) { @@ -342,6 +358,7 @@ QmpInputVisitor *qmp_input_visitor_new(QObject *obj) v->visitor.end_list = qmp_input_end_list; v->visitor.type_enum = input_type_enum; 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 3984011..f8eebaa 100644 --- a/qapi/qmp-output-visitor.c +++ b/qapi/qmp-output-visitor.c @@ -165,6 +165,14 @@ static void qmp_output_type_int64(Visitor *v, int64_t *obj, const char *name, 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) +{ + /* FIXME: QMP outputs values larger than INT64_MAX as negative */ + QmpOutputVisitor *qov = to_qov(v); + qmp_output_add(qov, name, qint_from_int(*obj)); +} + static void qmp_output_type_bool(Visitor *v, bool *obj, const char *name, Error **errp) { @@ -242,6 +250,7 @@ QmpOutputVisitor *qmp_output_visitor_new(void) v->visitor.end_list = qmp_output_end_list; v->visitor.type_enum = output_type_enum; 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 2f422f0..d7546b5 100644 --- a/qapi/string-input-visitor.c +++ b/qapi/string-input-visitor.c @@ -226,6 +226,20 @@ error: "an int64 value or range"); } +static void parse_type_uint64(Visitor *v, uint64_t *obj, const char *name, + Error **errp) +{ + /* FIXME: parse_type_int64 mishandles values over INT64_MAX */ + 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) { @@ -336,6 +350,7 @@ StringInputVisitor *string_input_visitor_new(const char *str) v->visitor.type_enum = input_type_enum; 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 c0a9331..3ed2b2c 100644 --- a/qapi/string-output-visitor.c +++ b/qapi/string-output-visitor.c @@ -197,6 +197,14 @@ static void print_type_int64(Visitor *v, int64_t *obj, const char *name, } } +static void print_type_uint64(Visitor *v, uint64_t *obj, const char *name, + Error **errp) +{ + /* FIXME: print_type_int64 mishandles values over INT64_MAX */ + 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) { @@ -346,6 +354,7 @@ StringOutputVisitor *string_output_visitor_new(bool human) v->human = human; v->visitor.type_enum = output_type_enum; 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