From ac1a61af47c495b9de47c3ec205f5ced3b98e7a2 Mon Sep 17 00:00:00 2001 From: Markus Armbruster Date: Fri, 2 Mar 2018 12:17:52 -0500 Subject: [PATCH] qapi: Make input visitors detect unvisited list tails Fix the design flaw demonstrated in the previous commit: new method check_list() lets input visitors report that unvisited input remains for a list, exactly like check_struct() lets them report that unvisited input remains for a struct or union. Implement the method for the qobject input visitor (straightforward), and the string input visitor (less so, due to the magic list syntax there). The opts visitor's list magic is even more impenetrable, and all I can do there today is a stub with a FIXME comment. No worse than before. Backports commit a4a1c70dc759e5b81627e96564f344ab43ea86eb from qemu --- qemu/include/qapi/visitor-impl.h | 3 +++ qemu/include/qapi/visitor.h | 13 +++++++++ qemu/qapi/qapi-visit-core.c | 9 +++++++ qemu/qapi/qobject-input-visitor.c | 44 ++++++++++++++++++++++--------- qemu/qapi/string-input-visitor.c | 30 +++++++++++++++++++++ 5 files changed, 86 insertions(+), 13 deletions(-) diff --git a/qemu/include/qapi/visitor-impl.h b/qemu/include/qapi/visitor-impl.h index 3f5341cb..d2a177c6 100644 --- a/qemu/include/qapi/visitor-impl.h +++ b/qemu/include/qapi/visitor-impl.h @@ -62,6 +62,9 @@ struct Visitor /* Must be set */ GenericList *(*next_list)(Visitor *v, GenericList *tail, size_t size); + /* Optional; intended for input visitors */ + void (*check_list)(Visitor *v, Error **errp); + /* Must be set */ void (*end_list)(Visitor *v, void **list); diff --git a/qemu/include/qapi/visitor.h b/qemu/include/qapi/visitor.h index 5cda4750..1342477b 100644 --- a/qemu/include/qapi/visitor.h +++ b/qemu/include/qapi/visitor.h @@ -377,6 +377,19 @@ void visit_start_list(Visitor *v, const char *name, GenericList **list, */ GenericList *visit_next_list(Visitor *v, GenericList *tail, size_t size); +/* + * Prepare for completing a list visit. + * + * @errp obeys typical error usage, and reports failures such as + * unvisited list tail remaining in the input stream. + * + * Should be called prior to visit_end_list() if all other + * intermediate visit steps were successful, to allow the visitor one + * last chance to report errors. May be skipped on a cleanup path, + * where there is no need to check for further errors. + */ +void visit_check_list(Visitor *v, Error **errp); + /* * Complete a list visit started earlier. * diff --git a/qemu/qapi/qapi-visit-core.c b/qemu/qapi/qapi-visit-core.c index 08ceb223..4d0a9df2 100644 --- a/qemu/qapi/qapi-visit-core.c +++ b/qemu/qapi/qapi-visit-core.c @@ -83,6 +83,15 @@ GenericList *visit_next_list(Visitor *v, GenericList *tail, size_t size) return v->next_list(v, tail, size); } +void visit_check_list(Visitor *v, Error **errp) +{ + // Unicorn: commented out + //trace_visit_check_list(v); + if (v->check_list) { + v->check_list(v, errp); + } +} + void visit_end_list(Visitor *v, void **obj) { v->end_list(v, obj); diff --git a/qemu/qapi/qobject-input-visitor.c b/qemu/qapi/qobject-input-visitor.c index dcd58ca1..aeb2892e 100644 --- a/qemu/qapi/qobject-input-visitor.c +++ b/qemu/qapi/qobject-input-visitor.c @@ -50,7 +50,8 @@ static QObjectInputVisitor *to_qiv(Visitor *v) return container_of(v, QObjectInputVisitor, visitor); } -static const char *full_name(QObjectInputVisitor *qiv, const char *name) +static const char *full_name_nth(QObjectInputVisitor *qiv, const char *name, + int n) { StackObject *so; char buf[32]; @@ -62,8 +63,10 @@ static const char *full_name(QObjectInputVisitor *qiv, const char *name) } QSLIST_FOREACH(so , &qiv->stack, node) { - if (qobject_type(so->obj) == QTYPE_QDICT) { - g_string_prepend(qiv->errname, name); + if (n) { + n--; + } else if (qobject_type(so->obj) == QTYPE_QDICT) { + g_string_prepend(qiv->errname, name ?: ""); g_string_prepend_c(qiv->errname, '.'); } else { snprintf(buf, sizeof(buf), "[%u]", so->index); @@ -71,18 +74,24 @@ static const char *full_name(QObjectInputVisitor *qiv, const char *name) } name = so->name; } + assert(!n); if (name) { g_string_prepend(qiv->errname, name); } else if (qiv->errname->str[0] == '.') { g_string_erase(qiv->errname, 0, 1); - } else { + } else if (!qiv->errname->str[0]) { return ""; } return qiv->errname->str; } +static const char *full_name(QObjectInputVisitor *qiv, const char *name) +{ + return full_name_nth(qiv, name, 0); +} + static QObject *qobject_input_try_get_object(QObjectInputVisitor *qiv, const char *name, bool consume) @@ -167,13 +176,6 @@ static const QListEntry *qobject_input_push(QObjectInputVisitor *qiv, return tos->entry; } -/** Only for qobject_input_pop. */ -static gboolean always_true(gpointer key, gpointer val, gpointer user_pkey) -{ - *(const char **)user_pkey = (const char *)key; - return TRUE; -} - static void qobject_input_check_struct(Visitor *v, Error **errp) { QObjectInputVisitor *qiv = to_qiv(v); @@ -263,15 +265,30 @@ static GenericList *qobject_input_next_list(Visitor *v, GenericList *tail, size_t size) { QObjectInputVisitor *qiv = to_qiv(v); - StackObject *so = QSLIST_FIRST(&qiv->stack); + StackObject *tos = QSLIST_FIRST(&qiv->stack); - if (!so->entry) { + assert(tos && tos->obj && qobject_type(tos->obj) == QTYPE_QLIST); + + if (!tos->entry) { return NULL; } tail->next = g_malloc0(size); return tail->next; } +static void qobject_input_check_list(Visitor *v, Error **errp) +{ + QObjectInputVisitor *qiv = to_qiv(v); + StackObject *tos = QSLIST_FIRST(&qiv->stack); + + assert(tos && tos->obj && qobject_type(tos->obj) == QTYPE_QLIST); + + if (tos->entry) { + error_setg(errp, "Only %u list elements expected in %s", + tos->index + 1, full_name_nth(qiv, NULL, 1)); + } +} + static void qobject_input_start_alternate(Visitor *v, const char *name, GenericAlternate **obj, size_t size, bool promote_int, Error **errp) @@ -474,6 +491,7 @@ Visitor *qobject_input_visitor_new(QObject *obj) v->visitor.start_alternate = qobject_input_start_alternate; v->visitor.start_list = qobject_input_start_list; v->visitor.next_list = qobject_input_next_list; + v->visitor.check_list = qobject_input_check_list; v->visitor.end_list = qobject_input_pop; v->visitor.type_int64 = qobject_input_type_int64; v->visitor.type_uint64 = qobject_input_type_uint64; diff --git a/qemu/qapi/string-input-visitor.c b/qemu/qapi/string-input-visitor.c index 4dbff4bf..8180be3f 100644 --- a/qemu/qapi/string-input-visitor.c +++ b/qemu/qapi/string-input-visitor.c @@ -170,6 +170,35 @@ static GenericList *next_list(Visitor *v, GenericList *tail, size_t size) return tail->next; } +static void check_list(Visitor *v, Error **errp) +{ + const StringInputVisitor *siv = to_siv(v); + Range *r; + GList *cur_range; + + if (!siv->ranges || !siv->cur_range) { + return; + } + + r = siv->cur_range->data; + if (!r) { + return; + } + + if (!range_contains(r, siv->cur)) { + cur_range = g_list_next(siv->cur_range); + if (!cur_range) { + return; + } + r = cur_range->data; + if (!r) { + return; + } + } + + error_setg(errp, "Range contains too many values"); +} + static void end_list(Visitor *v, void **obj) { StringInputVisitor *siv = to_siv(v); @@ -302,6 +331,7 @@ Visitor *string_input_visitor_new(const char *str) v->visitor.type_number = parse_type_number; v->visitor.start_list = start_list; v->visitor.next_list = next_list; + v->visitor.check_list = check_list; v->visitor.end_list = end_list; v->visitor.free = string_input_free;