From 4c40314a35816de635e7170eaacdc0c35be83a8a Mon Sep 17 00:00:00 2001 From: Eric Blake Date: Fri, 29 Jan 2016 06:48:49 -0700 Subject: qapi: Prefer type_int64 over type_int in visitors The qapi builtin type 'int' is basically shorthand for the type 'int64'. In fact, since no visitor was providing the optional type_int64() callback, visit_type_int64() was just always falling back to type_int(), cementing the equivalence between the types. However, some visitors are providing a type_uint64() callback. For purposes of code consistency, it is nicer if all visitors use the paired type_int64/type_uint64 names rather than the mismatched type_int/type_uint64. So this patch just renames the signed int callbacks in place, dropping the type_int() callback as redundant, and a later patch will focus on the unsigned int callbacks. Add some FIXMEs to questionable reuse of errp in code touched by the rename, while at it (the reuse works as long as the callbacks don't modify value when setting an error, but it's not a good example to set) - a later patch will then fix those. No change in functionality here, although further cleanups are in the pipeline. Signed-off-by: Eric Blake Message-Id: <1454075341-13658-14-git-send-email-eblake@redhat.com> Signed-off-by: Markus Armbruster --- qapi/qmp-input-visitor.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) (limited to 'qapi/qmp-input-visitor.c') diff --git a/qapi/qmp-input-visitor.c b/qapi/qmp-input-visitor.c index edb8bd2..d1668e3 100644 --- a/qapi/qmp-input-visitor.c +++ b/qapi/qmp-input-visitor.c @@ -225,8 +225,8 @@ 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)); @@ -342,7 +342,7 @@ 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_int64 = qmp_input_type_int64; v->visitor.type_bool = qmp_input_type_bool; v->visitor.type_str = qmp_input_type_str; v->visitor.type_number = qmp_input_type_number; -- cgit v1.1 From f755dea79dc81b0d6a8f6414e0672e165e28d8ba Mon Sep 17 00:00:00 2001 From: Eric Blake Date: Fri, 29 Jan 2016 06:48:50 -0700 Subject: qapi: Make all visitors supply uint64 callbacks Our qapi visitor contract supports multiple integer visitors, but left the type_uint64 visitor as optional (falling back on type_int64); which in turn can lead to awkward behavior with numbers larger than INT64_MAX (the user has to be aware of twos complement, and deal with negatives). This patch does not address the disparity in handling large values as negatives. It merely moves the fallback from uint64 to int64 from the visitor core to the visitors, where the issue can actually be fixed, by implementing the missing type_uint64() callbacks on top of the respective type_int64() callbacks, and with a FIXME comment explaining why that's wrong. With that done, we now have a type_uint64() callback in every driver, so we can make it mandatory from the core. And although the type_int64() callback can cover the entire valid range of type_uint{8,16,32} on valid user input, using type_uint64() to avoid mixed signedness makes more sense. Signed-off-by: Eric Blake Message-Id: <1454075341-13658-15-git-send-email-eblake@redhat.com> Signed-off-by: Markus Armbruster --- qapi/qmp-input-visitor.c | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) (limited to 'qapi/qmp-input-visitor.c') diff --git a/qapi/qmp-input-visitor.c b/qapi/qmp-input-visitor.c index d1668e3..5eb67df 100644 --- a/qapi/qmp-input-visitor.c +++ b/qapi/qmp-input-visitor.c @@ -240,6 +240,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) { @@ -343,6 +359,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; -- cgit v1.1 From 0b2a0d6bb2446060944061e53e87d0c7addede79 Mon Sep 17 00:00:00 2001 From: Eric Blake Date: Fri, 29 Jan 2016 06:48:56 -0700 Subject: qapi: Swap 'name' in visit_* callbacks to match public API MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit As explained in the previous patches, matching argument order of 'name, &value' to JSON's "name":value makes sense. However, while the last two patches were easy with Coccinelle, I ended up doing this one all by hand. Now all the visitor callbacks match the main interface. The compiler is able to enforce that all clients match the changed interface in visitor-impl.h, even where two pointers are being swapped, because only one of the two pointers is const (if that were not the case, then C's looseness on treating 'char *' like 'void *' would have made review a bit harder). Signed-off-by: Eric Blake Reviewed-by: Marc-André Lureau Message-Id: <1454075341-13658-21-git-send-email-eblake@redhat.com> Signed-off-by: Markus Armbruster --- qapi/qmp-input-visitor.c | 22 +++++++++++----------- 1 file changed, 11 insertions(+), 11 deletions(-) (limited to 'qapi/qmp-input-visitor.c') diff --git a/qapi/qmp-input-visitor.c b/qapi/qmp-input-visitor.c index 5eb67df..f3a0fd5 100644 --- a/qapi/qmp-input-visitor.c +++ b/qapi/qmp-input-visitor.c @@ -115,8 +115,8 @@ static void qmp_input_pop(QmpInputVisitor *qiv, Error **errp) qiv->nb_stack--; } -static void qmp_input_start_struct(Visitor *v, void **obj, const char *kind, - const char *name, size_t size, Error **errp) +static void qmp_input_start_struct(Visitor *v, const char *name, void **obj, + const char *kind, size_t size, Error **errp) { QmpInputVisitor *qiv = to_qiv(v); QObject *qobj = qmp_input_get_object(qiv, name, true); @@ -209,8 +209,8 @@ static void qmp_input_end_list(Visitor *v, Error **errp) qmp_input_pop(qiv, errp); } -static void qmp_input_get_next_type(Visitor *v, QType *type, bool promote_int, - const char *name, Error **errp) +static void qmp_input_get_next_type(Visitor *v, const char *name, QType *type, + bool promote_int, Error **errp) { QmpInputVisitor *qiv = to_qiv(v); QObject *qobj = qmp_input_get_object(qiv, name, false); @@ -225,7 +225,7 @@ static void qmp_input_get_next_type(Visitor *v, QType *type, bool promote_int, } } -static void qmp_input_type_int64(Visitor *v, int64_t *obj, const char *name, +static void qmp_input_type_int64(Visitor *v, const char *name, int64_t *obj, Error **errp) { QmpInputVisitor *qiv = to_qiv(v); @@ -240,7 +240,7 @@ 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, +static void qmp_input_type_uint64(Visitor *v, const char *name, uint64_t *obj, Error **errp) { /* FIXME: qobject_to_qint mishandles values over INT64_MAX */ @@ -256,7 +256,7 @@ static void qmp_input_type_uint64(Visitor *v, uint64_t *obj, const char *name, *obj = qint_get_int(qint); } -static void qmp_input_type_bool(Visitor *v, bool *obj, const char *name, +static void qmp_input_type_bool(Visitor *v, const char *name, bool *obj, Error **errp) { QmpInputVisitor *qiv = to_qiv(v); @@ -271,7 +271,7 @@ static void qmp_input_type_bool(Visitor *v, bool *obj, const char *name, *obj = qbool_get_bool(qbool); } -static void qmp_input_type_str(Visitor *v, char **obj, const char *name, +static void qmp_input_type_str(Visitor *v, const char *name, char **obj, Error **errp) { QmpInputVisitor *qiv = to_qiv(v); @@ -286,7 +286,7 @@ static void qmp_input_type_str(Visitor *v, char **obj, const char *name, *obj = g_strdup(qstring_get_str(qstr)); } -static void qmp_input_type_number(Visitor *v, double *obj, const char *name, +static void qmp_input_type_number(Visitor *v, const char *name, double *obj, Error **errp) { QmpInputVisitor *qiv = to_qiv(v); @@ -310,7 +310,7 @@ static void qmp_input_type_number(Visitor *v, double *obj, const char *name, "number"); } -static void qmp_input_type_any(Visitor *v, QObject **obj, const char *name, +static void qmp_input_type_any(Visitor *v, const char *name, QObject **obj, Error **errp) { QmpInputVisitor *qiv = to_qiv(v); @@ -320,7 +320,7 @@ static void qmp_input_type_any(Visitor *v, QObject **obj, const char *name, *obj = qobj; } -static void qmp_input_optional(Visitor *v, bool *present, const char *name) +static void qmp_input_optional(Visitor *v, const char *name, bool *present) { QmpInputVisitor *qiv = to_qiv(v); QObject *qobj = qmp_input_get_object(qiv, name, true); -- cgit v1.1 From 337283dffbb5ad5860ed00408a5fd0665c21be07 Mon Sep 17 00:00:00 2001 From: Eric Blake Date: Fri, 29 Jan 2016 06:48:57 -0700 Subject: qapi: Drop unused 'kind' for struct/enum visit MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit visit_start_struct() and visit_type_enum() had a 'kind' argument that was usually set to either the stringized version of the corresponding qapi type name, or to NULL (although some clients didn't even get that right). But nothing ever used the argument. It's even hard to argue that it would be useful in a debugger, as a stack backtrace also tells which type is being visited. Therefore, drop the 'kind' argument as dead. Signed-off-by: Eric Blake Reviewed-by: Marc-André Lureau Message-Id: <1454075341-13658-22-git-send-email-eblake@redhat.com> [Harmless rebase mistake cleaned up] Signed-off-by: Markus Armbruster --- qapi/qmp-input-visitor.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'qapi/qmp-input-visitor.c') diff --git a/qapi/qmp-input-visitor.c b/qapi/qmp-input-visitor.c index f3a0fd5..92eee26 100644 --- a/qapi/qmp-input-visitor.c +++ b/qapi/qmp-input-visitor.c @@ -116,7 +116,7 @@ static void qmp_input_pop(QmpInputVisitor *qiv, Error **errp) } static void qmp_input_start_struct(Visitor *v, const char *name, void **obj, - const char *kind, size_t size, Error **errp) + size_t size, Error **errp) { QmpInputVisitor *qiv = to_qiv(v); QObject *qobj = qmp_input_get_object(qiv, name, true); -- cgit v1.1 From bdd8e6b5d8a9def83d491a3f41c10424fc366258 Mon Sep 17 00:00:00 2001 From: Eric Blake Date: Fri, 29 Jan 2016 06:48:58 -0700 Subject: qapi: Tighten qmp_input_end_list() The only way that qmp_input_pop() will set errp is if a dictionary was the most recent thing pushed. Since we don't have any push(struct)/pop(list) or push(list)/pop(struct) mismatches (such a mismatch is a programming bug), we therefore cannot set errp inside qmp_input_end_list(). Make this obvious by using &error_abort. A later patch will then remove the errp parameter of qmp_input_pop(), but that will first require the larger task of splitting visit_end_struct(). Signed-off-by: Eric Blake Message-Id: <1454075341-13658-23-git-send-email-eblake@redhat.com> Signed-off-by: Markus Armbruster --- qapi/qmp-input-visitor.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'qapi/qmp-input-visitor.c') diff --git a/qapi/qmp-input-visitor.c b/qapi/qmp-input-visitor.c index 92eee26..4563c20 100644 --- a/qapi/qmp-input-visitor.c +++ b/qapi/qmp-input-visitor.c @@ -206,7 +206,7 @@ static void qmp_input_end_list(Visitor *v, Error **errp) { QmpInputVisitor *qiv = to_qiv(v); - qmp_input_pop(qiv, errp); + qmp_input_pop(qiv, &error_abort); } static void qmp_input_get_next_type(Visitor *v, const char *name, QType *type, -- cgit v1.1 From 08f9541dec51700abef0c37994213164ca4e4fc9 Mon Sep 17 00:00:00 2001 From: Eric Blake Date: Fri, 29 Jan 2016 06:48:59 -0700 Subject: qapi: Drop unused error argument for list and implicit struct No backend was setting an error when ending the visit of a list or implicit struct, or when moving to the next list node. Make the callers a bit easier to follow by making this a part of the contract, and removing the errp argument - callers can then unconditionally end an object as part of cleanup without having to think about whether a second error is dominated by a first, because there is no second error. A later patch will then tackle the larger task of splitting visit_end_struct(), which can indeed set an error. Signed-off-by: Eric Blake Message-Id: <1454075341-13658-24-git-send-email-eblake@redhat.com> Signed-off-by: Markus Armbruster --- qapi/qmp-input-visitor.c | 11 +++-------- 1 file changed, 3 insertions(+), 8 deletions(-) (limited to 'qapi/qmp-input-visitor.c') diff --git a/qapi/qmp-input-visitor.c b/qapi/qmp-input-visitor.c index 4563c20..362a1a3 100644 --- a/qapi/qmp-input-visitor.c +++ b/qapi/qmp-input-visitor.c @@ -1,6 +1,7 @@ /* * Input Visitor * + * Copyright (C) 2012-2016 Red Hat, Inc. * Copyright IBM, Corp. 2011 * * Authors: @@ -154,10 +155,6 @@ static void qmp_input_start_implicit_struct(Visitor *v, void **obj, } } -static void qmp_input_end_implicit_struct(Visitor *v, Error **errp) -{ -} - static void qmp_input_start_list(Visitor *v, const char *name, Error **errp) { QmpInputVisitor *qiv = to_qiv(v); @@ -172,8 +169,7 @@ static void qmp_input_start_list(Visitor *v, const char *name, Error **errp) qmp_input_push(qiv, qobj, errp); } -static GenericList *qmp_input_next_list(Visitor *v, GenericList **list, - Error **errp) +static GenericList *qmp_input_next_list(Visitor *v, GenericList **list) { QmpInputVisitor *qiv = to_qiv(v); GenericList *entry; @@ -202,7 +198,7 @@ static GenericList *qmp_input_next_list(Visitor *v, GenericList **list, return entry; } -static void qmp_input_end_list(Visitor *v, Error **errp) +static void qmp_input_end_list(Visitor *v) { QmpInputVisitor *qiv = to_qiv(v); @@ -353,7 +349,6 @@ QmpInputVisitor *qmp_input_visitor_new(QObject *obj) v->visitor.start_struct = qmp_input_start_struct; v->visitor.end_struct = qmp_input_end_struct; v->visitor.start_implicit_struct = qmp_input_start_implicit_struct; - v->visitor.end_implicit_struct = qmp_input_end_implicit_struct; v->visitor.start_list = qmp_input_start_list; v->visitor.next_list = qmp_input_next_list; v->visitor.end_list = qmp_input_end_list; -- cgit v1.1