From nobody Fri Apr 19 21:45:02 2024 Delivered-To: importer@patchew.org Received-SPF: pass (zoho.com: domain of gnu.org designates 208.118.235.17 as permitted sender) client-ip=208.118.235.17; envelope-from=qemu-devel-bounces+importer=patchew.org@nongnu.org; helo=lists.gnu.org; Authentication-Results: mx.zoho.com; spf=pass (zoho.com: domain of gnu.org designates 208.118.235.17 as permitted sender) smtp.mailfrom=qemu-devel-bounces+importer=patchew.org@nongnu.org; Return-Path: Received: from lists.gnu.org (lists.gnu.org [208.118.235.17]) by mx.zohomail.com with SMTPS id 1487674986867580.1345554839703; Tue, 21 Feb 2017 03:03:06 -0800 (PST) Received: from localhost ([::1]:43663 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1cg8Dn-0003ZS-Jf for importer@patchew.org; Tue, 21 Feb 2017 06:03:03 -0500 Received: from eggs.gnu.org ([2001:4830:134:3::10]:52738) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1cg8Bv-0002Qd-S7 for qemu-devel@nongnu.org; Tue, 21 Feb 2017 06:01:09 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1cg8Bq-00073A-UH for qemu-devel@nongnu.org; Tue, 21 Feb 2017 06:01:07 -0500 Received: from mx1.redhat.com ([209.132.183.28]:45816) by eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1cg8Bq-00072s-O2 for qemu-devel@nongnu.org; Tue, 21 Feb 2017 06:01:02 -0500 Received: from int-mx13.intmail.prod.int.phx2.redhat.com (int-mx13.intmail.prod.int.phx2.redhat.com [10.5.11.26]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 226CB7E9D5; Tue, 21 Feb 2017 11:01:02 +0000 (UTC) Received: from blackfin.pond.sub.org (ovpn-116-55.ams2.redhat.com [10.36.116.55]) by int-mx13.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id v1LB10fO022698 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=NO); Tue, 21 Feb 2017 06:01:01 -0500 Received: by blackfin.pond.sub.org (Postfix, from userid 1000) id 22541113864A; Tue, 21 Feb 2017 12:00:59 +0100 (CET) From: Markus Armbruster To: qemu-devel@nongnu.org Date: Tue, 21 Feb 2017 12:00:58 +0100 Message-Id: <1487674859-26249-2-git-send-email-armbru@redhat.com> In-Reply-To: <1487674859-26249-1-git-send-email-armbru@redhat.com> References: <1487674859-26249-1-git-send-email-armbru@redhat.com> X-Scanned-By: MIMEDefang 2.68 on 10.5.11.26 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.26]); Tue, 21 Feb 2017 11:01:02 +0000 (UTC) X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic] [fuzzy] X-Received-From: 209.132.183.28 Subject: [Qemu-devel] [PATCH 1/2] qapi: Clean up after commit 3d344c2 X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.21 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: mdroth@linux.vnet.ibm.com Errors-To: qemu-devel-bounces+importer=patchew.org@nongnu.org Sender: "Qemu-devel" X-ZohoMail: RSF_0 Z_629925259 SPT_0 Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 Content-Type: text/plain; charset="utf-8" Drop unused QIV_STACK_SIZE and unused qobject_input_start_struct() parameter errp. Signed-off-by: Markus Armbruster Reviewed-by: Eric Blake --- qapi/qobject-input-visitor.c | 14 +++----------- 1 file changed, 3 insertions(+), 11 deletions(-) diff --git a/qapi/qobject-input-visitor.c b/qapi/qobject-input-visitor.c index 0063327..1e81317 100644 --- a/qapi/qobject-input-visitor.c +++ b/qapi/qobject-input-visitor.c @@ -21,8 +21,6 @@ #include "qapi/qmp/types.h" #include "qapi/qmp/qerror.h" =20 -#define QIV_STACK_SIZE 1024 - typedef struct StackObject { QObject *obj; /* Object being visited */ @@ -103,8 +101,7 @@ static void qdict_add_key(const char *key, QObject *obj= , void *opaque) } =20 static const QListEntry *qobject_input_push(QObjectInputVisitor *qiv, - QObject *obj, void *qapi, - Error **errp) + QObject *obj, void *qapi) { GHashTable *h; StackObject *tos =3D g_new0(StackObject, 1); @@ -170,7 +167,6 @@ static void qobject_input_start_struct(Visitor *v, cons= t char *name, void **obj, { QObjectInputVisitor *qiv =3D to_qiv(v); QObject *qobj =3D qobject_input_get_object(qiv, name, true, errp); - Error *err =3D NULL; =20 if (obj) { *obj =3D NULL; @@ -184,11 +180,7 @@ static void qobject_input_start_struct(Visitor *v, con= st char *name, void **obj, return; } =20 - qobject_input_push(qiv, qobj, obj, &err); - if (err) { - error_propagate(errp, err); - return; - } + qobject_input_push(qiv, qobj, obj); =20 if (obj) { *obj =3D g_malloc0(size); @@ -216,7 +208,7 @@ static void qobject_input_start_list(Visitor *v, const = char *name, return; } =20 - entry =3D qobject_input_push(qiv, qobj, list, errp); + entry =3D qobject_input_push(qiv, qobj, list); if (list) { if (entry) { *list =3D g_malloc0(size); --=20 2.7.4 From nobody Fri Apr 19 21:45:02 2024 Delivered-To: importer@patchew.org Received-SPF: pass (zoho.com: domain of gnu.org designates 208.118.235.17 as permitted sender) client-ip=208.118.235.17; envelope-from=qemu-devel-bounces+importer=patchew.org@nongnu.org; helo=lists.gnu.org; Authentication-Results: mx.zoho.com; spf=pass (zoho.com: domain of gnu.org designates 208.118.235.17 as permitted sender) smtp.mailfrom=qemu-devel-bounces+importer=patchew.org@nongnu.org; Return-Path: Received: from lists.gnu.org (lists.gnu.org [208.118.235.17]) by mx.zohomail.com with SMTPS id 1487675217028960.681098895639; Tue, 21 Feb 2017 03:06:57 -0800 (PST) Received: from localhost ([::1]:43690 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1cg8HX-0006WE-34 for importer@patchew.org; Tue, 21 Feb 2017 06:06:55 -0500 Received: from eggs.gnu.org ([2001:4830:134:3::10]:52737) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1cg8Bv-0002Qc-Ru for qemu-devel@nongnu.org; Tue, 21 Feb 2017 06:01:10 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1cg8Br-00073M-33 for qemu-devel@nongnu.org; Tue, 21 Feb 2017 06:01:07 -0500 Received: from mx1.redhat.com ([209.132.183.28]:60520) by eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1cg8Bq-00072y-O6 for qemu-devel@nongnu.org; Tue, 21 Feb 2017 06:01:03 -0500 Received: from int-mx14.intmail.prod.int.phx2.redhat.com (int-mx14.intmail.prod.int.phx2.redhat.com [10.5.11.27]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 743AA8124B; Tue, 21 Feb 2017 11:01:02 +0000 (UTC) Received: from blackfin.pond.sub.org (ovpn-116-55.ams2.redhat.com [10.36.116.55]) by int-mx14.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id v1LB10YM005683 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=NO); Tue, 21 Feb 2017 06:01:01 -0500 Received: by blackfin.pond.sub.org (Postfix, from userid 1000) id 25430113864D; Tue, 21 Feb 2017 12:00:59 +0100 (CET) From: Markus Armbruster To: qemu-devel@nongnu.org Date: Tue, 21 Feb 2017 12:00:59 +0100 Message-Id: <1487674859-26249-3-git-send-email-armbru@redhat.com> In-Reply-To: <1487674859-26249-1-git-send-email-armbru@redhat.com> References: <1487674859-26249-1-git-send-email-armbru@redhat.com> X-Scanned-By: MIMEDefang 2.68 on 10.5.11.27 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.25]); Tue, 21 Feb 2017 11:01:02 +0000 (UTC) X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic] [fuzzy] X-Received-From: 209.132.183.28 Subject: [Qemu-devel] [PATCH 2/2] qapi: Improve qobject input visitor error reporting X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.21 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: mdroth@linux.vnet.ibm.com Errors-To: qemu-devel-bounces+importer=patchew.org@nongnu.org Sender: "Qemu-devel" X-ZohoMail: RSF_0 Z_629925259 SPT_0 Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 Content-Type: text/plain; charset="utf-8" Error messages refer to nodes of the QObject being visited by name. Trouble is the names are sometimes less than helpful: * The name of the root QObject is whatever @name argument got passed to the visitor, except NULL gets mapped to "null". We commonly pass NULL. Not good. Avoiding errors "at the root" mitigates. For instance, visit_start_struct() can only fail when the visited object is not a dictionary, and we commonly ensure it is beforehand. * The name of a QDict's member is the member key. Good enough only when this happens to be unique. * The name of a QList's member is "null". Not good. Improve error messages by referring to nodes by path instead, as follows: * The path of the root QObject is whatever @name argument got passed to the visitor, except NULL gets mapped to "". * The path of a root QDict's member is the member key. * The path of a root QList's member is "[%zu]", where %zu is the list index, starting at zero. * The path of a non-root QDict's member is the path of the QDict concatenated with "." and the member key. * The path of a non-root QList's member is the path of the QList concatenated with "[%zu]", where %zu is the list index. For example, the incorrect QMP command { "execute": "blockdev-add", "arguments": { "node-name": "foo", "driver= ": "raw", "file": {"driver": "file" } } } now fails with {"error": {"class": "GenericError", "desc": "Parameter 'file.filename' = is missing"}} instead of {"error": {"class": "GenericError", "desc": "Parameter 'filename' is mi= ssing"}} and { "execute": "input-send-event", "arguments": { "device": "bar", "event= s": [ [] ] } } now fails with {"error": {"class": "GenericError", "desc": "Invalid parameter type for= 'events[0]', expected: object"}} instead of {"error": {"class": "GenericError", "desc": "Invalid parameter type for= 'null', expected: QDict"}} The qobject output visitor doesn't have this problem because it should not fail. Same for dealloc and clone visitors. The string visitors don't have this problem because they visit just one value, whose name needs to be passed to the visitor as @name. The string output visitor shouldn't fail anyway. The options visitor uses QemuOpts names. Their name space is flat, so the use of QDict member keys as names is fine. NULL names used with roots and lists could conceivably result in bad error messages. Left for another day. Signed-off-by: Markus Armbruster Reviewed-by: Eric Blake --- include/qapi/visitor.h | 6 --- qapi/qobject-input-visitor.c | 123 +++++++++++++++++++++++++++++++--------= ---- 2 files changed, 89 insertions(+), 40 deletions(-) diff --git a/include/qapi/visitor.h b/include/qapi/visitor.h index 9bb6cba..7c91a50 100644 --- a/include/qapi/visitor.h +++ b/include/qapi/visitor.h @@ -66,12 +66,6 @@ * object, @name is the key associated with the value; and when * visiting a member of a list, @name is NULL. * - * FIXME: Clients must pass NULL for @name when visiting a member of a - * list, but this leads to poor error messages; it might be nicer to - * require a non-NULL name such as "key.0" for '{ "key": [ "value" ] - * }' if an error is encountered on "value" (or to have the visitor - * core auto-generate the nicer name). - * * The visit_type_FOO() functions expect a non-null @obj argument; * they allocate *@obj during input visits, leave it unchanged on * output visits, and recursively free any resources during a dealloc diff --git a/qapi/qobject-input-visitor.c b/qapi/qobject-input-visitor.c index 1e81317..2439f1a 100644 --- a/qapi/qobject-input-visitor.c +++ b/qapi/qobject-input-visitor.c @@ -21,19 +21,19 @@ #include "qapi/qmp/types.h" #include "qapi/qmp/qerror.h" =20 -typedef struct StackObject -{ - QObject *obj; /* Object being visited */ +typedef struct StackObject { + const char *name; /* Name if parent is QDict */ + QObject *obj; /* Object being visited, either dict or list = */ void *qapi; /* sanity check that caller uses same pointer */ =20 - GHashTable *h; /* If obj is dict: unvisited keys */ - const QListEntry *entry; /* If obj is list: unvisited tail */ + GHashTable *h; /* If @obj is QDict: unvisited keys */ + const QListEntry *entry; /* If @obj is QList: unvisited tail */ + size_t index; /* If @obj is QList: list index of @entry */ =20 - QSLIST_ENTRY(StackObject) node; + QSLIST_ENTRY(StackObject) node; /* parent */ } StackObject; =20 -struct QObjectInputVisitor -{ +struct QObjectInputVisitor { Visitor visitor; =20 /* Root of visit at visitor creation. */ @@ -45,6 +45,8 @@ struct QObjectInputVisitor =20 /* True to reject parse in visit_end_struct() if unvisited keys remain= . */ bool strict; + + GString *errname; /* Accumulator for qobject_input_get_name() */ }; =20 static QObjectInputVisitor *to_qiv(Visitor *v) @@ -52,9 +54,43 @@ static QObjectInputVisitor *to_qiv(Visitor *v) return container_of(v, QObjectInputVisitor, visitor); } =20 -static QObject *qobject_input_get_object(QObjectInputVisitor *qiv, - const char *name, - bool consume, Error **errp) +static const char *qobject_input_get_name(QObjectInputVisitor *qiv, + const char *name) +{ + StackObject *so; + char buf[32]; + + if (qiv->errname) { + g_string_truncate(qiv->errname, 0); + } else { + qiv->errname =3D g_string_new(""); + } + + QSLIST_FOREACH(so , &qiv->stack, node) { + if (qobject_type(so->obj) =3D=3D QTYPE_QDICT) { + g_string_prepend(qiv->errname, name); + g_string_prepend_c(qiv->errname, '.'); + } else { + snprintf(buf, sizeof(buf), "[%zu]", so->index); + g_string_prepend(qiv->errname, buf); + } + name =3D so->name; + } + + if (name) { + g_string_prepend(qiv->errname, name); + } else if (qiv->errname->str[0] =3D=3D '.') { + g_string_erase(qiv->errname, 0, 1); + } else { + return ""; + } + + return qiv->errname->str; +} + +static QObject *qobject_input_try_get_object(QObjectInputVisitor *qiv, + const char *name, + bool consume) { StackObject *tos; QObject *qobj; @@ -78,9 +114,6 @@ static QObject *qobject_input_get_object(QObjectInputVis= itor *qiv, bool removed =3D g_hash_table_remove(tos->h, name); assert(removed); } - if (!ret) { - error_setg(errp, QERR_MISSING_PARAMETER, name); - } } else { assert(qobject_type(qobj) =3D=3D QTYPE_QLIST); assert(!name); @@ -88,12 +121,26 @@ static QObject *qobject_input_get_object(QObjectInputV= isitor *qiv, assert(ret); if (consume) { tos->entry =3D qlist_next(tos->entry); + tos->index++; } } =20 return ret; } =20 +static QObject *qobject_input_get_object(QObjectInputVisitor *qiv, + const char *name, + bool consume, Error **errp) +{ + QObject *obj =3D qobject_input_try_get_object(qiv, name, consume); + + if (!obj) { + error_setg(errp, QERR_MISSING_PARAMETER, + qobject_input_get_name(qiv, name)); + } + return obj; +} + static void qdict_add_key(const char *key, QObject *obj, void *opaque) { GHashTable *h =3D opaque; @@ -101,12 +148,14 @@ static void qdict_add_key(const char *key, QObject *o= bj, void *opaque) } =20 static const QListEntry *qobject_input_push(QObjectInputVisitor *qiv, + const char *name, QObject *obj, void *qapi) { GHashTable *h; StackObject *tos =3D g_new0(StackObject, 1); =20 assert(obj); + tos->name =3D name; tos->obj =3D obj; tos->qapi =3D qapi; =20 @@ -116,6 +165,7 @@ static const QListEntry *qobject_input_push(QObjectInpu= tVisitor *qiv, tos->h =3D h; } else if (qobject_type(obj) =3D=3D QTYPE_QLIST) { tos->entry =3D qlist_first(qobject_to_qlist(obj)); + tos->index =3D -1; } =20 QSLIST_INSERT_HEAD(&qiv->stack, tos, node); @@ -137,7 +187,8 @@ static void qobject_input_check_struct(Visitor *v, Erro= r **errp) =20 g_hash_table_iter_init(&iter, top_ht); if (g_hash_table_iter_next(&iter, (void **)&key, NULL)) { - error_setg(errp, QERR_QMP_EXTRA_MEMBER, key); + error_setg(errp, QERR_QMP_EXTRA_MEMBER, + qobject_input_get_name(qiv, key)); } } } @@ -175,12 +226,12 @@ static void qobject_input_start_struct(Visitor *v, co= nst char *name, void **obj, return; } if (qobject_type(qobj) !=3D QTYPE_QDICT) { - error_setg(errp, QERR_INVALID_PARAMETER_TYPE, name ? name : "null", - "QDict"); + error_setg(errp, QERR_INVALID_PARAMETER_TYPE, + qobject_input_get_name(qiv, name), "object"); return; } =20 - qobject_input_push(qiv, qobj, obj); + qobject_input_push(qiv, name, qobj, obj); =20 if (obj) { *obj =3D g_malloc0(size); @@ -203,12 +254,12 @@ static void qobject_input_start_list(Visitor *v, cons= t char *name, if (list) { *list =3D NULL; } - error_setg(errp, QERR_INVALID_PARAMETER_TYPE, name ? name : "null", - "list"); + error_setg(errp, QERR_INVALID_PARAMETER_TYPE, + qobject_input_get_name(qiv, name), "array"); return; } =20 - entry =3D qobject_input_push(qiv, qobj, list); + entry =3D qobject_input_push(qiv, name, qobj, list); if (list) { if (entry) { *list =3D g_malloc0(size); @@ -262,8 +313,8 @@ static void qobject_input_type_int64(Visitor *v, const = char *name, int64_t *obj, } qint =3D qobject_to_qint(qobj); if (!qint) { - error_setg(errp, QERR_INVALID_PARAMETER_TYPE, name ? name : "null", - "integer"); + error_setg(errp, QERR_INVALID_PARAMETER_TYPE, + qobject_input_get_name(qiv, name), "integer"); return; } =20 @@ -283,8 +334,8 @@ static void qobject_input_type_uint64(Visitor *v, const= char *name, } qint =3D qobject_to_qint(qobj); if (!qint) { - error_setg(errp, QERR_INVALID_PARAMETER_TYPE, name ? name : "null", - "integer"); + error_setg(errp, QERR_INVALID_PARAMETER_TYPE, + qobject_input_get_name(qiv, name), "integer"); return; } =20 @@ -303,8 +354,8 @@ static void qobject_input_type_bool(Visitor *v, const c= har *name, bool *obj, } qbool =3D qobject_to_qbool(qobj); if (!qbool) { - error_setg(errp, QERR_INVALID_PARAMETER_TYPE, name ? name : "null", - "boolean"); + error_setg(errp, QERR_INVALID_PARAMETER_TYPE, + qobject_input_get_name(qiv, name), "boolean"); return; } =20 @@ -324,8 +375,8 @@ static void qobject_input_type_str(Visitor *v, const ch= ar *name, char **obj, } qstr =3D qobject_to_qstring(qobj); if (!qstr) { - error_setg(errp, QERR_INVALID_PARAMETER_TYPE, name ? name : "null", - "string"); + error_setg(errp, QERR_INVALID_PARAMETER_TYPE, + qobject_input_get_name(qiv, name), "string"); return; } =20 @@ -355,8 +406,8 @@ static void qobject_input_type_number(Visitor *v, const= char *name, double *obj, return; } =20 - error_setg(errp, QERR_INVALID_PARAMETER_TYPE, name ? name : "null", - "number"); + error_setg(errp, QERR_INVALID_PARAMETER_TYPE, + qobject_input_get_name(qiv, name), "number"); } =20 static void qobject_input_type_any(Visitor *v, const char *name, QObject *= *obj, @@ -384,15 +435,15 @@ static void qobject_input_type_null(Visitor *v, const= char *name, Error **errp) } =20 if (qobject_type(qobj) !=3D QTYPE_QNULL) { - error_setg(errp, QERR_INVALID_PARAMETER_TYPE, name ? name : "null", - "null"); + error_setg(errp, QERR_INVALID_PARAMETER_TYPE, + qobject_input_get_name(qiv, name), "null"); } } =20 static void qobject_input_optional(Visitor *v, const char *name, bool *pre= sent) { QObjectInputVisitor *qiv =3D to_qiv(v); - QObject *qobj =3D qobject_input_get_object(qiv, name, false, NULL); + QObject *qobj =3D qobject_input_try_get_object(qiv, name, false); =20 if (!qobj) { *present =3D false; @@ -405,6 +456,7 @@ static void qobject_input_optional(Visitor *v, const ch= ar *name, bool *present) static void qobject_input_free(Visitor *v) { QObjectInputVisitor *qiv =3D to_qiv(v); + while (!QSLIST_EMPTY(&qiv->stack)) { StackObject *tos =3D QSLIST_FIRST(&qiv->stack); =20 @@ -413,6 +465,9 @@ static void qobject_input_free(Visitor *v) } =20 qobject_decref(qiv->root); + if (qiv->errname) { + g_string_free(qiv->errname, TRUE); + } g_free(qiv); } =20 --=20 2.7.4