From 64594e2bcc9a734bfabfaa8a75e08b4c9d920569 Mon Sep 17 00:00:00 2001 From: Markus Armbruster Date: Wed, 15 Apr 2020 10:30:44 +0200 Subject: qobject: Clean up QLIST_FOREACH_ENTRY() QLIST_FOREACH_ENTRY() traverses a tail queue manually. Use QTAILQ_FIRST() and QTAILQ_NEXT() instead. Signed-off-by: Markus Armbruster Message-Id: <20200415083048.14339-2-armbru@redhat.com> Reviewed-by: Eric Blake --- include/qapi/qmp/qlist.h | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/include/qapi/qmp/qlist.h b/include/qapi/qmp/qlist.h index 8d2c32ca28..07ecae81e4 100644 --- a/include/qapi/qmp/qlist.h +++ b/include/qapi/qmp/qlist.h @@ -34,10 +34,10 @@ void qlist_append_int(QList *qlist, int64_t value); void qlist_append_null(QList *qlist); void qlist_append_str(QList *qlist, const char *value); -#define QLIST_FOREACH_ENTRY(qlist, var) \ - for ((var) = ((qlist)->head.tqh_first); \ - (var); \ - (var) = ((var)->next.tqe_next)) +#define QLIST_FOREACH_ENTRY(qlist, var) \ + for ((var) = QTAILQ_FIRST(&(qlist)->head); \ + (var); \ + (var) = QTAILQ_NEXT((var), next)) static inline QObject *qlist_entry_obj(const QListEntry *entry) { -- cgit v1.2.3 From 1cd7741ef18e4333b17d64aaed3cd48e3e182a57 Mon Sep 17 00:00:00 2001 From: Markus Armbruster Date: Wed, 15 Apr 2020 10:30:45 +0200 Subject: qobject: Factor out helper json_pretty_newline() Signed-off-by: Markus Armbruster Message-Id: <20200415083048.14339-3-armbru@redhat.com> Reviewed-by: Eric Blake [Coding style in moved code tidied up] --- qobject/qjson.c | 40 ++++++++++++++++------------------------ 1 file changed, 16 insertions(+), 24 deletions(-) diff --git a/qobject/qjson.c b/qobject/qjson.c index db36101f3b..87422f600d 100644 --- a/qobject/qjson.c +++ b/qobject/qjson.c @@ -159,21 +159,28 @@ typedef struct ToJsonIterState static void to_json(const QObject *obj, QString *str, int pretty, int indent); +static void json_pretty_newline(QString *str, bool pretty, int indent) +{ + int i; + + if (pretty) { + qstring_append(str, "\n"); + for (i = 0; i < indent; i++) { + qstring_append(str, " "); + } + } +} + static void to_json_dict_iter(const char *key, QObject *obj, void *opaque) { ToJsonIterState *s = opaque; QString *qkey; - int j; if (s->count) { qstring_append(s->str, s->pretty ? "," : ", "); } - if (s->pretty) { - qstring_append(s->str, "\n"); - for (j = 0 ; j < s->indent ; j++) - qstring_append(s->str, " "); - } + json_pretty_newline(s->str, s->pretty, s->indent); qkey = qstring_from_str(key); to_json(QOBJECT(qkey), s->str, s->pretty, s->indent); @@ -187,17 +194,12 @@ static void to_json_dict_iter(const char *key, QObject *obj, void *opaque) static void to_json_list_iter(QObject *obj, void *opaque) { ToJsonIterState *s = opaque; - int j; if (s->count) { qstring_append(s->str, s->pretty ? "," : ", "); } - if (s->pretty) { - qstring_append(s->str, "\n"); - for (j = 0 ; j < s->indent ; j++) - qstring_append(s->str, " "); - } + json_pretty_newline(s->str, s->pretty, s->indent); to_json(obj, s->str, s->pretty, s->indent); s->count++; @@ -282,12 +284,7 @@ static void to_json(const QObject *obj, QString *str, int pretty, int indent) s.pretty = pretty; qstring_append(str, "{"); qdict_iter(val, to_json_dict_iter, &s); - if (pretty) { - int j; - qstring_append(str, "\n"); - for (j = 0 ; j < indent ; j++) - qstring_append(str, " "); - } + json_pretty_newline(str, pretty, indent); qstring_append(str, "}"); break; } @@ -301,12 +298,7 @@ static void to_json(const QObject *obj, QString *str, int pretty, int indent) s.pretty = pretty; qstring_append(str, "["); qlist_iter(val, (void *)to_json_list_iter, &s); - if (pretty) { - int j; - qstring_append(str, "\n"); - for (j = 0 ; j < indent ; j++) - qstring_append(str, " "); - } + json_pretty_newline(str, pretty, indent); qstring_append(str, "]"); break; } -- cgit v1.2.3 From 2f2ec111795119b2e020bc0cbf4b5f42878574b2 Mon Sep 17 00:00:00 2001 From: Markus Armbruster Date: Wed, 15 Apr 2020 10:30:46 +0200 Subject: qobject: Eliminate qlist_iter(), use QLIST_FOREACH_ENTRY() instead qlist_iter() has just three uses outside tests/. Replace by QLIST_FOREACH_ENTRY() for more concise code and less type punning. Signed-off-by: Markus Armbruster Message-Id: <20200415083048.14339-4-armbru@redhat.com> Reviewed-by: Eric Blake --- include/qapi/qmp/qlist.h | 2 -- qobject/qjson.c | 31 +++++++++++-------------------- qobject/qlist.c | 44 ++++++++++++-------------------------------- tests/check-qlist.c | 37 ++++++++++++++----------------------- 4 files changed, 37 insertions(+), 77 deletions(-) diff --git a/include/qapi/qmp/qlist.h b/include/qapi/qmp/qlist.h index 07ecae81e4..595b7943e1 100644 --- a/include/qapi/qmp/qlist.h +++ b/include/qapi/qmp/qlist.h @@ -47,8 +47,6 @@ static inline QObject *qlist_entry_obj(const QListEntry *entry) QList *qlist_new(void); QList *qlist_copy(QList *src); void qlist_append_obj(QList *qlist, QObject *obj); -void qlist_iter(const QList *qlist, - void (*iter)(QObject *obj, void *opaque), void *opaque); QObject *qlist_pop(QList *qlist); QObject *qlist_peek(QList *qlist); int qlist_empty(const QList *qlist); diff --git a/qobject/qjson.c b/qobject/qjson.c index 87422f600d..f0eebc5fda 100644 --- a/qobject/qjson.c +++ b/qobject/qjson.c @@ -191,20 +191,6 @@ static void to_json_dict_iter(const char *key, QObject *obj, void *opaque) s->count++; } -static void to_json_list_iter(QObject *obj, void *opaque) -{ - ToJsonIterState *s = opaque; - - if (s->count) { - qstring_append(s->str, s->pretty ? "," : ", "); - } - - json_pretty_newline(s->str, s->pretty, s->indent); - - to_json(obj, s->str, s->pretty, s->indent); - s->count++; -} - static void to_json(const QObject *obj, QString *str, int pretty, int indent) { switch (qobject_type(obj)) { @@ -289,15 +275,20 @@ static void to_json(const QObject *obj, QString *str, int pretty, int indent) break; } case QTYPE_QLIST: { - ToJsonIterState s; QList *val = qobject_to(QList, obj); + const char *comma = pretty ? "," : ", "; + const char *sep = ""; + QListEntry *entry; - s.count = 0; - s.str = str; - s.indent = indent + 1; - s.pretty = pretty; qstring_append(str, "["); - qlist_iter(val, (void *)to_json_list_iter, &s); + + QLIST_FOREACH_ENTRY(val, entry) { + qstring_append(str, sep); + json_pretty_newline(str, pretty, indent + 1); + to_json(qlist_entry_obj(entry), str, pretty, indent + 1); + sep = comma; + } + json_pretty_newline(str, pretty, indent); qstring_append(str, "]"); break; diff --git a/qobject/qlist.c b/qobject/qlist.c index b3274af88b..1be95367d1 100644 --- a/qobject/qlist.c +++ b/qobject/qlist.c @@ -34,20 +34,17 @@ QList *qlist_new(void) return qlist; } -static void qlist_copy_elem(QObject *obj, void *opaque) -{ - QList *dst = opaque; - - qobject_ref(obj); - qlist_append_obj(dst, obj); -} - QList *qlist_copy(QList *src) { QList *dst = qlist_new(); + QListEntry *entry; + QObject *elt; - qlist_iter(src, qlist_copy_elem, dst); - + QLIST_FOREACH_ENTRY(src, entry) { + elt = qlist_entry_obj(entry); + qobject_ref(elt); + qlist_append_obj(dst, elt); + } return dst; } @@ -86,21 +83,6 @@ void qlist_append_null(QList *qlist) qlist_append(qlist, qnull()); } -/** - * qlist_iter(): Iterate over all the list's stored values. - * - * This function allows the user to provide an iterator, which will be - * called for each stored value in the list. - */ -void qlist_iter(const QList *qlist, - void (*iter)(QObject *obj, void *opaque), void *opaque) -{ - QListEntry *entry; - - QTAILQ_FOREACH(entry, &qlist->head, next) - iter(entry->value, opaque); -} - QObject *qlist_pop(QList *qlist) { QListEntry *entry; @@ -137,16 +119,14 @@ int qlist_empty(const QList *qlist) return QTAILQ_EMPTY(&qlist->head); } -static void qlist_size_iter(QObject *obj, void *opaque) -{ - size_t *count = opaque; - (*count)++; -} - size_t qlist_size(const QList *qlist) { size_t count = 0; - qlist_iter(qlist, qlist_size_iter, &count); + QListEntry *entry; + + QLIST_FOREACH_ENTRY(qlist, entry) { + count++; + } return count; } diff --git a/tests/check-qlist.c b/tests/check-qlist.c index ece83e293d..3cd0ccbf19 100644 --- a/tests/check-qlist.c +++ b/tests/check-qlist.c @@ -61,40 +61,31 @@ static void qobject_to_qlist_test(void) qobject_unref(qlist); } -static int iter_called; -static const int iter_max = 42; - -static void iter_func(QObject *obj, void *opaque) -{ - QNum *qi; - int64_t val; - - g_assert(opaque == NULL); - - qi = qobject_to(QNum, obj); - g_assert(qi != NULL); - - g_assert(qnum_get_try_int(qi, &val)); - g_assert_cmpint(val, >=, 0); - g_assert_cmpint(val, <=, iter_max); - - iter_called++; -} - static void qlist_iter_test(void) { + const int iter_max = 42; int i; QList *qlist; + QListEntry *entry; + QNum *qi; + int64_t val; qlist = qlist_new(); for (i = 0; i < iter_max; i++) qlist_append_int(qlist, i); - iter_called = 0; - qlist_iter(qlist, iter_func, NULL); + i = 0; + QLIST_FOREACH_ENTRY(qlist, entry) { + qi = qobject_to(QNum, qlist_entry_obj(entry)); + g_assert(qi != NULL); + + g_assert(qnum_get_try_int(qi, &val)); + g_assert_cmpint(val, ==, i); + i++; + } - g_assert(iter_called == iter_max); + g_assert(i == iter_max); qobject_unref(qlist); } -- cgit v1.2.3 From 7b1cd1c65abad70b26fbe9b11991bd88f0d956e1 Mon Sep 17 00:00:00 2001 From: Markus Armbruster Date: Wed, 15 Apr 2020 10:30:47 +0200 Subject: qobject: Eliminate qdict_iter(), use qdict_first(), qdict_next() qdict_iter() has just three uses and no test coverage. Replace by qdict_first(), qdict_next() for more concise code and less type punning. Signed-off-by: Markus Armbruster Message-Id: <20200415083048.14339-5-armbru@redhat.com> Reviewed-by: Eric Blake --- include/qapi/qmp/qdict.h | 3 --- qapi/qobject-input-visitor.c | 21 +++++++++-------- qobject/qdict.c | 19 ---------------- qobject/qjson.c | 54 ++++++++++++++++---------------------------- util/qemu-option.c | 10 +++++++- 5 files changed, 40 insertions(+), 67 deletions(-) diff --git a/include/qapi/qmp/qdict.h b/include/qapi/qmp/qdict.h index 7f3ec10a10..da942347a7 100644 --- a/include/qapi/qmp/qdict.h +++ b/include/qapi/qmp/qdict.h @@ -40,9 +40,6 @@ void qdict_del(QDict *qdict, const char *key); int qdict_haskey(const QDict *qdict, const char *key); QObject *qdict_get(const QDict *qdict, const char *key); bool qdict_is_equal(const QObject *x, const QObject *y); -void qdict_iter(const QDict *qdict, - void (*iter)(const char *key, QObject *obj, void *opaque), - void *opaque); const QDictEntry *qdict_first(const QDict *qdict); const QDictEntry *qdict_next(const QDict *qdict, const QDictEntry *entry); void qdict_destroy_obj(QObject *obj); diff --git a/qapi/qobject-input-visitor.c b/qapi/qobject-input-visitor.c index 32236cbcb1..5ce3ec2e5f 100644 --- a/qapi/qobject-input-visitor.c +++ b/qapi/qobject-input-visitor.c @@ -203,31 +203,32 @@ static const char *qobject_input_get_keyval(QObjectInputVisitor *qiv, return qstring_get_str(qstr); } -static void qdict_add_key(const char *key, QObject *obj, void *opaque) -{ - GHashTable *h = opaque; - g_hash_table_insert(h, (gpointer) key, NULL); -} - static const QListEntry *qobject_input_push(QObjectInputVisitor *qiv, const char *name, QObject *obj, void *qapi) { GHashTable *h; StackObject *tos = g_new0(StackObject, 1); + QDict *qdict = qobject_to(QDict, obj); + QList *qlist = qobject_to(QList, obj); + const QDictEntry *entry; assert(obj); tos->name = name; tos->obj = obj; tos->qapi = qapi; - if (qobject_type(obj) == QTYPE_QDICT) { + if (qdict) { h = g_hash_table_new(g_str_hash, g_str_equal); - qdict_iter(qobject_to(QDict, obj), qdict_add_key, h); + for (entry = qdict_first(qdict); + entry; + entry = qdict_next(qdict, entry)) { + g_hash_table_insert(h, (void *)qdict_entry_key(entry), NULL); + } tos->h = h; } else { - assert(qobject_type(obj) == QTYPE_QLIST); - tos->entry = qlist_first(qobject_to(QList, obj)); + assert(qlist); + tos->entry = qlist_first(qlist); tos->index = -1; } diff --git a/qobject/qdict.c b/qobject/qdict.c index 3d8c2f7bbc..526de54ceb 100644 --- a/qobject/qdict.c +++ b/qobject/qdict.c @@ -298,25 +298,6 @@ const char *qdict_get_try_str(const QDict *qdict, const char *key) return qstr ? qstring_get_str(qstr) : NULL; } -/** - * qdict_iter(): Iterate over all the dictionary's stored values. - * - * This function allows the user to provide an iterator, which will be - * called for each stored value in the dictionary. - */ -void qdict_iter(const QDict *qdict, - void (*iter)(const char *key, QObject *obj, void *opaque), - void *opaque) -{ - int i; - QDictEntry *entry; - - for (i = 0; i < QDICT_BUCKET_MAX; i++) { - QLIST_FOREACH(entry, &qdict->table[i], next) - iter(entry->key, entry->value, opaque); - } -} - static QDictEntry *qdict_next_entry(const QDict *qdict, int first_bucket) { int i; diff --git a/qobject/qjson.c b/qobject/qjson.c index f0eebc5fda..f1f2c69704 100644 --- a/qobject/qjson.c +++ b/qobject/qjson.c @@ -149,14 +149,6 @@ QDict *qdict_from_jsonf_nofail(const char *string, ...) return qdict; } -typedef struct ToJsonIterState -{ - int indent; - int pretty; - int count; - QString *str; -} ToJsonIterState; - static void to_json(const QObject *obj, QString *str, int pretty, int indent); static void json_pretty_newline(QString *str, bool pretty, int indent) @@ -171,26 +163,6 @@ static void json_pretty_newline(QString *str, bool pretty, int indent) } } -static void to_json_dict_iter(const char *key, QObject *obj, void *opaque) -{ - ToJsonIterState *s = opaque; - QString *qkey; - - if (s->count) { - qstring_append(s->str, s->pretty ? "," : ", "); - } - - json_pretty_newline(s->str, s->pretty, s->indent); - - qkey = qstring_from_str(key); - to_json(QOBJECT(qkey), s->str, s->pretty, s->indent); - qobject_unref(qkey); - - qstring_append(s->str, ": "); - to_json(obj, s->str, s->pretty, s->indent); - s->count++; -} - static void to_json(const QObject *obj, QString *str, int pretty, int indent) { switch (qobject_type(obj)) { @@ -261,15 +233,29 @@ static void to_json(const QObject *obj, QString *str, int pretty, int indent) break; } case QTYPE_QDICT: { - ToJsonIterState s; QDict *val = qobject_to(QDict, obj); + const char *comma = pretty ? "," : ", "; + const char *sep = ""; + const QDictEntry *entry; + QString *qkey; - s.count = 0; - s.str = str; - s.indent = indent + 1; - s.pretty = pretty; qstring_append(str, "{"); - qdict_iter(val, to_json_dict_iter, &s); + + for (entry = qdict_first(val); + entry; + entry = qdict_next(val, entry)) { + qstring_append(str, sep); + json_pretty_newline(str, pretty, indent + 1); + + qkey = qstring_from_str(qdict_entry_key(entry)); + to_json(QOBJECT(qkey), str, pretty, indent + 1); + qobject_unref(qkey); + + qstring_append(str, ": "); + to_json(qdict_entry_value(entry), str, pretty, indent + 1); + sep = comma; + } + json_pretty_newline(str, pretty, indent); qstring_append(str, "}"); break; diff --git a/util/qemu-option.c b/util/qemu-option.c index 9542988183..2784757ef5 100644 --- a/util/qemu-option.c +++ b/util/qemu-option.c @@ -1013,6 +1013,7 @@ QemuOpts *qemu_opts_from_qdict(QemuOptsList *list, const QDict *qdict, OptsFromQDictState state; Error *local_err = NULL; QemuOpts *opts; + const QDictEntry *entry; opts = qemu_opts_create(list, qdict_get_try_str(qdict, "id"), 1, &local_err); @@ -1025,7 +1026,14 @@ QemuOpts *qemu_opts_from_qdict(QemuOptsList *list, const QDict *qdict, state.errp = &local_err; state.opts = opts; - qdict_iter(qdict, qemu_opts_from_qdict_1, &state); + + for (entry = qdict_first(qdict); + entry; + entry = qdict_next(qdict, entry)) { + qemu_opts_from_qdict_1(qdict_entry_key(entry), + qdict_entry_value(entry), + &state); + } if (local_err) { error_propagate(errp, local_err); qemu_opts_del(opts); -- cgit v1.2.3 From 2500f6f30b547b9c301dfe7571a16b028f7b848d Mon Sep 17 00:00:00 2001 From: Markus Armbruster Date: Wed, 15 Apr 2020 10:30:48 +0200 Subject: qemu-option: Clean up after the previous commit Signed-off-by: Markus Armbruster Message-Id: <20200415083048.14339-6-armbru@redhat.com> Reviewed-by: Eric Blake --- util/qemu-option.c | 43 +++++++++++++++---------------------------- 1 file changed, 15 insertions(+), 28 deletions(-) diff --git a/util/qemu-option.c b/util/qemu-option.c index 2784757ef5..0ebfd97a98 100644 --- a/util/qemu-option.c +++ b/util/qemu-option.c @@ -965,18 +965,16 @@ void qemu_opts_set_defaults(QemuOptsList *list, const char *params, assert(opts); } -typedef struct OptsFromQDictState { - QemuOpts *opts; - Error **errp; -} OptsFromQDictState; - -static void qemu_opts_from_qdict_1(const char *key, QObject *obj, void *opaque) +static void qemu_opts_from_qdict_entry(QemuOpts *opts, + const QDictEntry *entry, + Error **errp) { - OptsFromQDictState *state = opaque; + const char *key = qdict_entry_key(entry); + QObject *obj = qdict_entry_value(entry); char buf[32], *tmp = NULL; const char *value; - if (!strcmp(key, "id") || *state->errp) { + if (!strcmp(key, "id")) { return; } @@ -997,7 +995,7 @@ static void qemu_opts_from_qdict_1(const char *key, QObject *obj, void *opaque) return; } - qemu_opt_set(state->opts, key, value, state->errp); + qemu_opt_set(opts, key, value, errp); g_free(tmp); } @@ -1010,7 +1008,6 @@ static void qemu_opts_from_qdict_1(const char *key, QObject *obj, void *opaque) QemuOpts *qemu_opts_from_qdict(QemuOptsList *list, const QDict *qdict, Error **errp) { - OptsFromQDictState state; Error *local_err = NULL; QemuOpts *opts; const QDictEntry *entry; @@ -1024,20 +1021,15 @@ QemuOpts *qemu_opts_from_qdict(QemuOptsList *list, const QDict *qdict, assert(opts != NULL); - state.errp = &local_err; - state.opts = opts; - for (entry = qdict_first(qdict); entry; entry = qdict_next(qdict, entry)) { - qemu_opts_from_qdict_1(qdict_entry_key(entry), - qdict_entry_value(entry), - &state); - } - if (local_err) { - error_propagate(errp, local_err); - qemu_opts_del(opts); - return NULL; + qemu_opts_from_qdict_entry(opts, entry, &local_err); + if (local_err) { + error_propagate(errp, local_err); + qemu_opts_del(opts); + return NULL; + } } return opts; @@ -1056,21 +1048,16 @@ void qemu_opts_absorb_qdict(QemuOpts *opts, QDict *qdict, Error **errp) while (entry != NULL) { Error *local_err = NULL; - OptsFromQDictState state = { - .errp = &local_err, - .opts = opts, - }; next = qdict_next(qdict, entry); if (find_desc_by_name(opts->list->desc, entry->key)) { - qemu_opts_from_qdict_1(entry->key, entry->value, &state); + qemu_opts_from_qdict_entry(opts, entry, &local_err); if (local_err) { error_propagate(errp, local_err); return; - } else { - qdict_del(qdict, entry->key); } + qdict_del(qdict, entry->key); } entry = next; -- cgit v1.2.3 From 3777d36e674ca3b75b63413b4fb506b108309144 Mon Sep 17 00:00:00 2001 From: Markus Armbruster Date: Fri, 24 Apr 2020 10:43:24 +0200 Subject: qapi: Belatedly update visitor.h's big comment for QAPI modules Signed-off-by: Markus Armbruster Reviewed-by: Eric Blake Message-Id: <20200424084338.26803-2-armbru@redhat.com> --- include/qapi/visitor.h | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/include/qapi/visitor.h b/include/qapi/visitor.h index c5b23851a1..f8a0fc1ea9 100644 --- a/include/qapi/visitor.h +++ b/include/qapi/visitor.h @@ -58,7 +58,7 @@ * * where T is FOO for scalar types, and FOO * otherwise. The scalar * visitors are declared here; the remaining visitors are generated in - * qapi-visit.h. + * qapi-visit-MODULE.h. * * The @name parameter of visit_type_FOO() describes the relation * between this QAPI value and its parent container. When visiting @@ -86,16 +86,16 @@ * by manual construction. * * For the QAPI object types (structs, unions, and alternates), there - * is an additional generated function in qapi-visit.h compatible - * with: + * is an additional generated function in qapi-visit-MODULE.h + * compatible with: * * void visit_type_FOO_members(Visitor *v, FOO *obj, Error **errp); * * for visiting the members of a type without also allocating the QAPI * struct. * - * Additionally, in qapi-types.h, all QAPI pointer types (structs, - * unions, alternates, and lists) have a generated function compatible + * Additionally, QAPI pointer types (structs, unions, alternates, and + * lists) have a generated function in qapi-types-MODULE.h compatible * with: * * void qapi_free_FOO(FOO *obj); -- cgit v1.2.3 From 294c90662ac9daae2c3bbf608296cf037925bd95 Mon Sep 17 00:00:00 2001 From: Markus Armbruster Date: Fri, 24 Apr 2020 10:43:25 +0200 Subject: qapi: Fix the virtual walk example in visitor.h's big comment Call visit_check_list(). Missed in commit a4a1c70dc7 "qapi: Make input visitors detect unvisited list tails". Drop an irrelevant error_propagate() while there. Signed-off-by: Markus Armbruster Reviewed-by: Eric Blake Message-Id: <20200424084338.26803-3-armbru@redhat.com> --- include/qapi/visitor.h | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/include/qapi/visitor.h b/include/qapi/visitor.h index f8a0fc1ea9..7f63e4c381 100644 --- a/include/qapi/visitor.h +++ b/include/qapi/visitor.h @@ -215,6 +215,9 @@ * goto outlist; * } * outlist: + * if (!err) { + * visit_check_list(v, &err); + * } * visit_end_list(v, NULL); * if (!err) { * visit_check_struct(v, &err); @@ -222,7 +225,6 @@ * outobj: * visit_end_struct(v, NULL); * out: - * error_propagate(errp, err); * visit_free(v); * */ -- cgit v1.2.3 From 782586c77103d297f38e78950dfbe2a31a3b2924 Mon Sep 17 00:00:00 2001 From: Markus Armbruster Date: Fri, 24 Apr 2020 10:43:26 +0200 Subject: qapi: Fix typo in visit_start_list()'s contract Signed-off-by: Markus Armbruster Reviewed-by: Eric Blake Message-Id: <20200424084338.26803-4-armbru@redhat.com> --- include/qapi/visitor.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/include/qapi/visitor.h b/include/qapi/visitor.h index 7f63e4c381..c5d0ce9184 100644 --- a/include/qapi/visitor.h +++ b/include/qapi/visitor.h @@ -345,9 +345,9 @@ void visit_end_struct(Visitor *v, void **obj); * input visitors set *@list to NULL. * * After visit_start_list() succeeds, the caller may visit its members - * one after the other. A real visit (where @obj is non-NULL) uses + * one after the other. A real visit (where @list is non-NULL) uses * visit_next_list() for traversing the linked list, while a virtual - * visit (where @obj is NULL) uses other means. For each list + * visit (where @list is NULL) uses other means. For each list * element, call the appropriate visit_type_FOO() with name set to * NULL and obj set to the address of the value member of the list * element. Finally, visit_end_list() needs to be called with the -- cgit v1.2.3 From c5460d5e19b4c531e86f8be0e00ab463fae7548b Mon Sep 17 00:00:00 2001 From: Markus Armbruster Date: Fri, 24 Apr 2020 10:43:27 +0200 Subject: qapi: Document @errp usage more thoroughly in visitor.h Signed-off-by: Markus Armbruster Reviewed-by: Eric Blake Message-Id: <20200424084338.26803-5-armbru@redhat.com> --- include/qapi/visitor.h | 37 +++++++++++++++++++++++-------------- 1 file changed, 23 insertions(+), 14 deletions(-) diff --git a/include/qapi/visitor.h b/include/qapi/visitor.h index c5d0ce9184..09df7099c6 100644 --- a/include/qapi/visitor.h +++ b/include/qapi/visitor.h @@ -284,9 +284,7 @@ void visit_free(Visitor *v); * into *@obj. @obj may also be NULL for a virtual walk, in which * case @size is ignored. * - * @errp obeys typical error usage, and reports failures such as a - * member @name is not present, or present but not an object. On - * error, input visitors set *@obj to NULL. + * On failure, set *@obj to NULL and store an error through @errp. * * After visit_start_struct() succeeds, the caller may visit its * members one after the other, passing the member's name and address @@ -303,8 +301,7 @@ void visit_start_struct(Visitor *v, const char *name, void **obj, /* * Prepare for completing an object visit. * - * @errp obeys typical error usage, and reports failures such as - * unparsed keys remaining in the input stream. + * On failure, store an error through @errp. * * Should be called prior to visit_end_struct() if all other * intermediate visit steps were successful, to allow the visitor one @@ -340,9 +337,7 @@ void visit_end_struct(Visitor *v, void **obj); * allow @list to be NULL for a virtual walk, in which case @size is * ignored. * - * @errp obeys typical error usage, and reports failures such as a - * member @name is not present, or present but not a list. On error, - * input visitors set *@list to NULL. + * On failure, set *@list to NULL and store an error through @errp. * * After visit_start_list() succeeds, the caller may visit its members * one after the other. A real visit (where @list is non-NULL) uses @@ -376,8 +371,7 @@ 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. + * On failure, store an error through @errp. * * Should be called prior to visit_end_list() if all other * intermediate visit steps were successful, to allow the visitor one @@ -409,8 +403,10 @@ void visit_end_list(Visitor *v, void **list); * * @obj must not be NULL. Input and clone visitors use @size to * determine how much memory to allocate into *@obj, then determine - * the qtype of the next thing to be visited, stored in (*@obj)->type. - * Other visitors will leave @obj unchanged. + * the qtype of the next thing to be visited, and store it in + * (*@obj)->type. Other visitors leave @obj unchanged. + * + * On failure, set *@obj to NULL and store an error through @errp. * * If successful, this must be paired with visit_end_alternate() with * the same @obj to clean up, even if visiting the contents of the @@ -463,8 +459,9 @@ bool visit_optional(Visitor *v, const char *name, bool *present); * * Currently, all input visitors parse text input, and all output * visitors produce text output. The mapping between enumeration - * values and strings is done by the visitor core, using @strings; it - * should be the ENUM_lookup array from visit-types.h. + * values and strings is done by the visitor core, using @lookup. + * + * On failure, store an error through @errp. * * May call visit_type_str() under the hood, and the enum visit may * fail even if the corresponding string visit succeeded; this implies @@ -488,6 +485,8 @@ bool visit_is_input(Visitor *v); * * @obj must be non-NULL. Input visitors set *@obj to the value; * other visitors will leave *@obj unchanged. + * + * On failure, store an error through @errp. */ void visit_type_int(Visitor *v, const char *name, int64_t *obj, Error **errp); @@ -564,6 +563,8 @@ void visit_type_size(Visitor *v, const char *name, uint64_t *obj, * * @obj must be non-NULL. Input visitors set *@obj to the value; * other visitors will leave *@obj unchanged. + * + * On failure, store an error through @errp. */ void visit_type_bool(Visitor *v, const char *name, bool *obj, Error **errp); @@ -581,6 +582,8 @@ void visit_type_bool(Visitor *v, const char *name, bool *obj, Error **errp); * It is safe to cast away const when preparing a (const char *) value * into @obj for use by an output visitor. * + * On failure, set *@obj to NULL and store an error through @errp. + * * FIXME: Callers that try to output NULL *obj should not be allowed. */ void visit_type_str(Visitor *v, const char *name, char **obj, Error **errp); @@ -594,6 +597,8 @@ void visit_type_str(Visitor *v, const char *name, char **obj, Error **errp); * @obj must be non-NULL. Input visitors set *@obj to the value; * other visitors will leave *@obj unchanged. Visitors should * document if infinity or NaN are not permitted. + * + * On failure, store an error through @errp. */ void visit_type_number(Visitor *v, const char *name, double *obj, Error **errp); @@ -608,6 +613,8 @@ void visit_type_number(Visitor *v, const char *name, double *obj, * other visitors will leave *@obj unchanged. *@obj must be non-NULL * for output visitors. * + * On failure, set *@obj to NULL and store an error through @errp. + * * Note that some kinds of input can't express arbitrary QObject. * E.g. the visitor returned by qobject_input_visitor_new_keyval() * can't create numbers or booleans, only strings. @@ -622,6 +629,8 @@ void visit_type_any(Visitor *v, const char *name, QObject **obj, Error **errp); * * @obj must be non-NULL. Input visitors set *@obj to the value; * other visitors ignore *@obj. + * + * On failure, set *@obj to NULL and store an error through @errp. */ void visit_type_null(Visitor *v, const char *name, QNull **obj, Error **errp); -- cgit v1.2.3 From 554d6586ae96216771fe701c9d3a321e2adb057b Mon Sep 17 00:00:00 2001 From: Markus Armbruster Date: Fri, 24 Apr 2020 10:43:28 +0200 Subject: qapi: Polish prose in visitor.h Signed-off-by: Markus Armbruster Reviewed-by: Eric Blake Message-Id: <20200424084338.26803-6-armbru@redhat.com> --- include/qapi/visitor.h | 104 +++++++++++++++++++++++++------------------------ 1 file changed, 54 insertions(+), 50 deletions(-) diff --git a/include/qapi/visitor.h b/include/qapi/visitor.h index 09df7099c6..a425ea514c 100644 --- a/include/qapi/visitor.h +++ b/include/qapi/visitor.h @@ -25,19 +25,21 @@ * for doing work at each node of a QAPI graph; it can also be used * for a virtual walk, where there is no actual QAPI C struct. * - * There are four kinds of visitor classes: input visitors (QObject, - * string, and QemuOpts) parse an external representation and build - * the corresponding QAPI graph, output visitors (QObject and string) take - * a completed QAPI graph and generate an external representation, the - * dealloc visitor can take a QAPI graph (possibly partially - * constructed) and recursively free its resources, and the clone - * visitor performs a deep clone of one QAPI object to another. While - * the dealloc and QObject input/output visitors are general, the string, - * QemuOpts, and clone visitors have some implementation limitations; - * see the documentation for each visitor for more details on what it - * supports. Also, see visitor-impl.h for the callback contracts - * implemented by each visitor, and docs/devel/qapi-code-gen.txt for more - * about the QAPI code generator. + * There are four kinds of visitors: input visitors (QObject, string, + * and QemuOpts) parse an external representation and build the + * corresponding QAPI object, output visitors (QObject and string) + * take a QAPI object and generate an external representation, the + * dealloc visitor takes a QAPI object (possibly partially + * constructed) and recursively frees it, and the clone visitor + * performs a deep clone of a QAPI object. + * + * While the dealloc and QObject input/output visitors are general, + * the string, QemuOpts, and clone visitors have some implementation + * limitations; see the documentation for each visitor for more + * details on what it supports. Also, see visitor-impl.h for the + * callback contracts implemented by each visitor, and + * docs/devel/qapi-code-gen.txt for more about the QAPI code + * generator. * * All of the visitors are created via: * @@ -45,11 +47,15 @@ * * A visitor should be used for exactly one top-level visit_type_FOO() * or virtual walk; if that is successful, the caller can optionally - * call visit_complete() (for now, useful only for output visits, but - * safe to call on all visits). Then, regardless of success or - * failure, the user should call visit_free() to clean up resources. - * It is okay to free the visitor without completing the visit, if - * some other error is detected in the meantime. + * call visit_complete() (useful only for output visits, but safe to + * call on all visits). Then, regardless of success or failure, the + * user should call visit_free() to clean up resources. It is okay to + * free the visitor without completing the visit, if some other error + * is detected in the meantime. + * + * The clone and dealloc visitor should not be used directly outside + * of QAPI code. Use the qapi_free_FOO() and QAPI_CLONE() instead, + * described below. * * All QAPI types have a corresponding function with a signature * roughly compatible with this: @@ -68,22 +74,26 @@ * alternate, @name should equal the name used for visiting the * alternate. * - * 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 - * visit. Each function also takes the customary @errp argument (see + * The visit_type_FOO() functions take a non-null @obj argument; they + * allocate *@obj during input visits, leave it unchanged during + * output and clone visits, and free it (recursively) during a dealloc + * visit. + * + * Each function also takes the customary @errp argument (see * qapi/error.h for details), for reporting any errors (such as if a * member @name is not present, or is present but not the specified * type). * * If an error is detected during visit_type_FOO() with an input - * visitor, then *@obj will be NULL for pointer types, and left - * unchanged for scalar types. Using an output or clone visitor with - * an incomplete object has undefined behavior (other than a special - * case for visit_type_str() treating NULL like ""), while the dealloc - * visitor safely handles incomplete objects. Since input visitors - * never produce an incomplete object, such an object is possible only - * by manual construction. + * visitor, then *@obj will be set to NULL for pointer types, and left + * unchanged for scalar types. + * + * Using an output or clone visitor with an incomplete object has + * undefined behavior (other than a special case for visit_type_str() + * treating NULL like ""), while the dealloc visitor safely handles + * incomplete objects. Since input visitors never produce an + * incomplete object, such an object is possible only by manual + * construction. * * For the QAPI object types (structs, unions, and alternates), there * is an additional generated function in qapi-visit-MODULE.h @@ -100,23 +110,20 @@ * * void qapi_free_FOO(FOO *obj); * - * where behaves like free() in that @obj may be NULL. Such objects - * may also be used with the following macro, provided alongside the - * clone visitor: + * Does nothing when @obj is NULL. + * + * Such objects may also be used with macro * * Type *QAPI_CLONE(Type, src); * - * in order to perform a deep clone of @src. Because of the generated - * qapi_free functions and the QAPI_CLONE() macro, the clone and - * dealloc visitor should not be used directly outside of QAPI code. + * in order to perform a deep clone of @src. * - * QAPI types can also inherit from a base class; when this happens, a - * function is generated for easily going from the derived type to the - * base type: + * For QAPI types can that inherit from a base type, a function is + * generated for going from the derived type to the base type: * * BASE *qapi_CHILD_base(CHILD *obj); * - * For a real QAPI struct, typical input usage involves: + * Typical input visitor usage involves: * * * Foo *f; @@ -153,7 +160,7 @@ * qapi_free_FooList(l); * * - * Similarly, typical output usage is: + * Typical output visitor usage: * * * Foo *f = ...obtain populated object... @@ -172,17 +179,8 @@ * visit_free(v); * * - * When visiting a real QAPI struct, this file provides several - * helpers that rely on in-tree information to control the walk: - * visit_optional() for the 'has_member' field associated with - * optional 'member' in the C struct; and visit_next_list() for - * advancing through a FooList linked list. Similarly, the - * visit_is_input() helper makes it possible to write code that is - * visitor-agnostic everywhere except for cleanup. Only the generated - * visit_type functions need to use these helpers. - * * It is also possible to use the visitors to do a virtual walk, where - * no actual QAPI struct is present. In this situation, decisions + * no actual QAPI object is present. In this situation, decisions * about what needs to be walked are made by the calling code, and * structured visits are split between pairs of start and end methods * (where the end method must be called if the start function @@ -227,6 +225,12 @@ * out: * visit_free(v); * + * + * This file provides helpers for use by the generated + * visit_type_FOO(): visit_optional() for the 'has_member' field + * associated with optional 'member' in the C struct, + * visit_next_list() for advancing through a FooList linked list, and + * visit_is_input() for cleaning up on failure. */ /*** Useful types ***/ -- cgit v1.2.3 From 8e08bf4ea24c3e6e07fab2c1b5bdcc7b104012c4 Mon Sep 17 00:00:00 2001 From: Markus Armbruster Date: Fri, 24 Apr 2020 10:43:29 +0200 Subject: qapi: Assert incomplete object occurs only in dealloc visitor Signed-off-by: Markus Armbruster Reviewed-by: Eric Blake Message-Id: <20200424084338.26803-7-armbru@redhat.com> --- docs/devel/qapi-code-gen.txt | 2 ++ include/qapi/visitor.h | 5 +++++ qapi/qapi-visit-core.c | 5 +++++ scripts/qapi/visit.py | 4 ++++ 4 files changed, 16 insertions(+) diff --git a/docs/devel/qapi-code-gen.txt b/docs/devel/qapi-code-gen.txt index 1967adfa92..c6dd1891c3 100644 --- a/docs/devel/qapi-code-gen.txt +++ b/docs/devel/qapi-code-gen.txt @@ -1446,6 +1446,8 @@ Example: goto out; } if (!*obj) { + /* incomplete */ + assert(visit_is_dealloc(v)); goto out_obj; } visit_type_UserDefOne_members(v, *obj, &err); diff --git a/include/qapi/visitor.h b/include/qapi/visitor.h index a425ea514c..2d40d2fe0f 100644 --- a/include/qapi/visitor.h +++ b/include/qapi/visitor.h @@ -479,6 +479,11 @@ void visit_type_enum(Visitor *v, const char *name, int *obj, */ bool visit_is_input(Visitor *v); +/* + * Check if visitor is a dealloc visitor. + */ +bool visit_is_dealloc(Visitor *v); + /*** Visiting built-in types ***/ /* diff --git a/qapi/qapi-visit-core.c b/qapi/qapi-visit-core.c index 5365561b07..d4aac206cf 100644 --- a/qapi/qapi-visit-core.c +++ b/qapi/qapi-visit-core.c @@ -142,6 +142,11 @@ bool visit_is_input(Visitor *v) return v->type == VISITOR_INPUT; } +bool visit_is_dealloc(Visitor *v) +{ + return v->type == VISITOR_DEALLOC; +} + void visit_type_int(Visitor *v, const char *name, int64_t *obj, Error **errp) { assert(obj); diff --git a/scripts/qapi/visit.py b/scripts/qapi/visit.py index 23d9194aa4..e3467b770b 100644 --- a/scripts/qapi/visit.py +++ b/scripts/qapi/visit.py @@ -189,6 +189,8 @@ void visit_type_%(c_name)s(Visitor *v, const char *name, %(c_name)s **obj, Error goto out; } if (!*obj) { + /* incomplete */ + assert(visit_is_dealloc(v)); goto out_obj; } switch ((*obj)->type) { @@ -260,6 +262,8 @@ void visit_type_%(c_name)s(Visitor *v, const char *name, %(c_name)s **obj, Error goto out; } if (!*obj) { + /* incomplete */ + assert(visit_is_dealloc(v)); goto out_obj; } visit_type_%(c_name)s_members(v, *obj, &err); -- cgit v1.2.3 From 8b7ce95b46815f274aed52857260cb180f6fade3 Mon Sep 17 00:00:00 2001 From: Markus Armbruster Date: Fri, 24 Apr 2020 10:43:30 +0200 Subject: qapi: Fix Visitor contract for start_alternate() The contract demands v->start_alternate() for input and dealloc visitors, but visit_start_alternate() actually requires it for input and clone visitors. Fix the contract, and delete superfluous qapi_dealloc_start_alternate(). Signed-off-by: Markus Armbruster Reviewed-by: Eric Blake Message-Id: <20200424084338.26803-8-armbru@redhat.com> --- include/qapi/visitor-impl.h | 5 ++--- qapi/qapi-dealloc-visitor.c | 7 ------- 2 files changed, 2 insertions(+), 10 deletions(-) diff --git a/include/qapi/visitor-impl.h b/include/qapi/visitor-impl.h index 8ccb3b6c20..252206dc0d 100644 --- a/include/qapi/visitor-impl.h +++ b/include/qapi/visitor-impl.h @@ -67,13 +67,12 @@ struct Visitor /* Must be set */ void (*end_list)(Visitor *v, void **list); - /* Must be set by input and dealloc visitors to visit alternates; - * optional for output visitors. */ + /* Must be set by input and clone visitors to visit alternates */ void (*start_alternate)(Visitor *v, const char *name, GenericAlternate **obj, size_t size, Error **errp); - /* Optional, needed for dealloc visitor */ + /* Optional */ void (*end_alternate)(Visitor *v, void **obj); /* Must be set */ diff --git a/qapi/qapi-dealloc-visitor.c b/qapi/qapi-dealloc-visitor.c index d192724b13..2239fc6417 100644 --- a/qapi/qapi-dealloc-visitor.c +++ b/qapi/qapi-dealloc-visitor.c @@ -34,12 +34,6 @@ static void qapi_dealloc_end_struct(Visitor *v, void **obj) } } -static void qapi_dealloc_start_alternate(Visitor *v, const char *name, - GenericAlternate **obj, size_t size, - Error **errp) -{ -} - static void qapi_dealloc_end_alternate(Visitor *v, void **obj) { if (obj) { @@ -123,7 +117,6 @@ Visitor *qapi_dealloc_visitor_new(void) v->visitor.type = VISITOR_DEALLOC; v->visitor.start_struct = qapi_dealloc_start_struct; v->visitor.end_struct = qapi_dealloc_end_struct; - v->visitor.start_alternate = qapi_dealloc_start_alternate; v->visitor.end_alternate = qapi_dealloc_end_alternate; v->visitor.start_list = qapi_dealloc_start_list; v->visitor.next_list = qapi_dealloc_next_list; -- cgit v1.2.3 From 777d20cfa5735de298f378e9b90f0cd1caafdc2d Mon Sep 17 00:00:00 2001 From: Markus Armbruster Date: Fri, 24 Apr 2020 10:43:31 +0200 Subject: qapi: Assert output visitors see only valid enum values output_type_enum() fails when *obj is not a valid value of the enum type. Should not happen. Drop the check, along with its unit tests. This unmasks qapi_enum_lookup()'s assertion. Signed-off-by: Markus Armbruster Reviewed-by: Eric Blake Message-Id: <20200424084338.26803-9-armbru@redhat.com> [Commit message tweaked] --- qapi/qapi-visit-core.c | 9 --------- tests/test-qobject-output-visitor.c | 39 ------------------------------------- tests/test-string-output-visitor.c | 19 ------------------ 3 files changed, 67 deletions(-) diff --git a/qapi/qapi-visit-core.c b/qapi/qapi-visit-core.c index d4aac206cf..80ca83bcb9 100644 --- a/qapi/qapi-visit-core.c +++ b/qapi/qapi-visit-core.c @@ -341,15 +341,6 @@ static void output_type_enum(Visitor *v, const char *name, int *obj, int value = *obj; char *enum_str; - /* - * TODO why is this an error, not an assertion? If assertion: - * delete, and rely on qapi_enum_lookup() - */ - if (value < 0 || value >= lookup->size) { - error_setg(errp, QERR_INVALID_PARAMETER, name ? name : "null"); - return; - } - enum_str = (char *)qapi_enum_lookup(lookup, value); visit_type_str(v, name, &enum_str, errp); } diff --git a/tests/test-qobject-output-visitor.c b/tests/test-qobject-output-visitor.c index d7761ebf84..1c856d9bd2 100644 --- a/tests/test-qobject-output-visitor.c +++ b/tests/test-qobject-output-visitor.c @@ -141,21 +141,6 @@ static void test_visitor_out_enum(TestOutputVisitorData *data, } } -static void test_visitor_out_enum_errors(TestOutputVisitorData *data, - const void *unused) -{ - EnumOne i, bad_values[] = { ENUM_ONE__MAX, -1 }; - - for (i = 0; i < ARRAY_SIZE(bad_values) ; i++) { - Error *err = NULL; - - visit_type_EnumOne(data->ov, "unused", &bad_values[i], &err); - error_free_or_abort(&err); - visitor_reset(data); - } -} - - static void test_visitor_out_struct(TestOutputVisitorData *data, const void *unused) { @@ -234,26 +219,6 @@ static void test_visitor_out_struct_nested(TestOutputVisitorData *data, qapi_free_UserDefTwo(ud2); } -static void test_visitor_out_struct_errors(TestOutputVisitorData *data, - const void *unused) -{ - EnumOne bad_values[] = { ENUM_ONE__MAX, -1 }; - UserDefOne u = {0}; - UserDefOne *pu = &u; - int i; - - for (i = 0; i < ARRAY_SIZE(bad_values) ; i++) { - Error *err = NULL; - - u.has_enum1 = true; - u.enum1 = bad_values[i]; - visit_type_UserDefOne(data->ov, "unused", &pu, &err); - error_free_or_abort(&err); - visitor_reset(data); - } -} - - static void test_visitor_out_list(TestOutputVisitorData *data, const void *unused) { @@ -821,14 +786,10 @@ int main(int argc, char **argv) &out_visitor_data, test_visitor_out_no_string); output_visitor_test_add("/visitor/output/enum", &out_visitor_data, test_visitor_out_enum); - output_visitor_test_add("/visitor/output/enum-errors", - &out_visitor_data, test_visitor_out_enum_errors); output_visitor_test_add("/visitor/output/struct", &out_visitor_data, test_visitor_out_struct); output_visitor_test_add("/visitor/output/struct-nested", &out_visitor_data, test_visitor_out_struct_nested); - output_visitor_test_add("/visitor/output/struct-errors", - &out_visitor_data, test_visitor_out_struct_errors); output_visitor_test_add("/visitor/output/list", &out_visitor_data, test_visitor_out_list); output_visitor_test_add("/visitor/output/any", diff --git a/tests/test-string-output-visitor.c b/tests/test-string-output-visitor.c index 1be1540767..3bd732222c 100644 --- a/tests/test-string-output-visitor.c +++ b/tests/test-string-output-visitor.c @@ -203,19 +203,6 @@ static void test_visitor_out_enum(TestOutputVisitorData *data, } } -static void test_visitor_out_enum_errors(TestOutputVisitorData *data, - const void *unused) -{ - EnumOne i, bad_values[] = { ENUM_ONE__MAX, -1 }; - - for (i = 0; i < ARRAY_SIZE(bad_values) ; i++) { - Error *err = NULL; - - visit_type_EnumOne(data->ov, "unused", &bad_values[i], &err); - error_free_or_abort(&err); - } -} - static void output_visitor_test_add(const char *testpath, TestOutputVisitorData *data, @@ -260,12 +247,6 @@ int main(int argc, char **argv) &out_visitor_data, test_visitor_out_enum, false); output_visitor_test_add("/string-visitor/output/enum-human", &out_visitor_data, test_visitor_out_enum, true); - output_visitor_test_add("/string-visitor/output/enum-errors", - &out_visitor_data, test_visitor_out_enum_errors, - false); - output_visitor_test_add("/string-visitor/output/enum-errors-human", - &out_visitor_data, test_visitor_out_enum_errors, - true); output_visitor_test_add("/string-visitor/output/intList", &out_visitor_data, test_visitor_out_intList, false); output_visitor_test_add("/string-visitor/output/intList-human", -- cgit v1.2.3 From faad584adb48737aa3b39984786442a1a9c42aa4 Mon Sep 17 00:00:00 2001 From: Markus Armbruster Date: Fri, 24 Apr 2020 10:43:32 +0200 Subject: qapi: Assert non-input visitors see only valid narrow integers visit_type_intN() and visit_type_uintN() fail when the value is out of bounds. This is appropriate with an input visitor: the value comes from input, and input may be bad. It should never happen with the other visitors: the value comes from the caller, and callers must keep it within bounds. Assert that. Signed-off-by: Markus Armbruster Reviewed-by: Eric Blake Message-Id: <20200424084338.26803-10-armbru@redhat.com> --- qapi/qapi-visit-core.c | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/qapi/qapi-visit-core.c b/qapi/qapi-visit-core.c index 80ca83bcb9..74aa9c04bd 100644 --- a/qapi/qapi-visit-core.c +++ b/qapi/qapi-visit-core.c @@ -160,10 +160,13 @@ static void visit_type_uintN(Visitor *v, uint64_t *obj, const char *name, Error *err = NULL; uint64_t value = *obj; + assert(v->type == VISITOR_INPUT || value <= max); + v->type_uint64(v, name, &value, &err); if (err) { error_propagate(errp, err); } else if (value > max) { + assert(v->type == VISITOR_INPUT); error_setg(errp, QERR_INVALID_PARAMETER_VALUE, name ? name : "null", type); } else { @@ -219,10 +222,13 @@ static void visit_type_intN(Visitor *v, int64_t *obj, const char *name, Error *err = NULL; int64_t value = *obj; + assert(v->type == VISITOR_INPUT || (value >= min && value <= max)); + v->type_int64(v, name, &value, &err); if (err) { error_propagate(errp, err); } else if (value < min || value > max) { + assert(v->type == VISITOR_INPUT); error_setg(errp, QERR_INVALID_PARAMETER_VALUE, name ? name : "null", type); } else { -- cgit v1.2.3 From c978bd5226f1ed73456ac468a7ed50af3d24dc84 Mon Sep 17 00:00:00 2001 From: Markus Armbruster Date: Fri, 24 Apr 2020 10:43:33 +0200 Subject: qapi: Clean up visitor's recovery from input with invalid type An alternate type's visit_type_FOO() fails when it runs into an invalid ->type. If it's an input visit, we then need to free the the object we got from visit_start_alternate(). We do that with qapi_free_FOO(), which uses the dealloc visitor. Trouble is that object is in a bad state: its ->type is invalid. So the dealloc visitor will run into the same error again, and the error recovery skips deallocating the alternate's (invalid) alternative. Works, because qapi_free_FOO() ignores the error. Avoid it instead: free the messed up object with by g_free(). Signed-off-by: Markus Armbruster Message-Id: <20200424084338.26803-11-armbru@redhat.com> Reviewed-by: Eric Blake --- scripts/qapi/visit.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/scripts/qapi/visit.py b/scripts/qapi/visit.py index e3467b770b..678109dfb5 100644 --- a/scripts/qapi/visit.py +++ b/scripts/qapi/visit.py @@ -234,6 +234,9 @@ void visit_type_%(c_name)s(Visitor *v, const char *name, %(c_name)s **obj, Error default: error_setg(&err, QERR_INVALID_PARAMETER_TYPE, name ? name : "null", "%(name)s"); + /* Avoid passing invalid *obj to qapi_free_%(c_name)s() */ + g_free(*obj); + *obj = NULL; } out_obj: visit_end_alternate(v, (void **)obj); -- cgit v1.2.3 From 7111a86e1ba23ff8d59886df03d8a1cd6c5aab43 Mon Sep 17 00:00:00 2001 From: Markus Armbruster Date: Fri, 24 Apr 2020 10:43:34 +0200 Subject: qapi: Assert non-input visitors see only valid alternate tags An alternate type's visit_type_FOO() fails when it runs into an invalid ->type. This is appropriate with an input visitor: visit_start_alternate() sets ->type according to the input, and bad input can lead to bad ->type. It should never happen with an output, clone or dealloc visitor: if it did, the alternate being output, cloned or deallocated would be messed up beyond repair. Assert that. Signed-off-by: Markus Armbruster Reviewed-by: Eric Blake Message-Id: <20200424084338.26803-12-armbru@redhat.com> --- scripts/qapi/visit.py | 1 + 1 file changed, 1 insertion(+) diff --git a/scripts/qapi/visit.py b/scripts/qapi/visit.py index 678109dfb5..d5d7a1031f 100644 --- a/scripts/qapi/visit.py +++ b/scripts/qapi/visit.py @@ -232,6 +232,7 @@ void visit_type_%(c_name)s(Visitor *v, const char *name, %(c_name)s **obj, Error case QTYPE_NONE: abort(); default: + assert(visit_is_input(v)); error_setg(&err, QERR_INVALID_PARAMETER_TYPE, name ? name : "null", "%(name)s"); /* Avoid passing invalid *obj to qapi_free_%(c_name)s() */ -- cgit v1.2.3 From 1f5842487ad5b3d59ea32742e30dc7441f413e6c Mon Sep 17 00:00:00 2001 From: Markus Armbruster Date: Fri, 24 Apr 2020 10:43:35 +0200 Subject: qapi: Only input visitors can actually fail The previous few commits have made this more obvious, and removed the one exception. Time to clarify the documentation, and drop dead error checking. Signed-off-by: Markus Armbruster Reviewed-by: Eric Blake Message-Id: <20200424084338.26803-13-armbru@redhat.com> --- block.c | 9 +-------- block/sheepdog.c | 9 +-------- blockdev.c | 16 ++-------------- hw/core/machine-hmp-cmds.c | 2 +- include/qapi/visitor-impl.h | 4 ++++ include/qapi/visitor.h | 40 ++++++++++++++++++++++++---------------- monitor/hmp-cmds.c | 3 ++- 7 files changed, 35 insertions(+), 48 deletions(-) diff --git a/block.c b/block.c index 2e3905c99e..c11385ae05 100644 --- a/block.c +++ b/block.c @@ -2982,7 +2982,6 @@ BdrvChild *bdrv_open_child(const char *filename, BlockDriverState *bdrv_open_blockdev_ref(BlockdevRef *ref, Error **errp) { BlockDriverState *bs = NULL; - Error *local_err = NULL; QObject *obj = NULL; QDict *qdict = NULL; const char *reference = NULL; @@ -2995,11 +2994,7 @@ BlockDriverState *bdrv_open_blockdev_ref(BlockdevRef *ref, Error **errp) assert(ref->type == QTYPE_QDICT); v = qobject_output_visitor_new(&obj); - visit_type_BlockdevOptions(v, NULL, &options, &local_err); - if (local_err) { - error_propagate(errp, local_err); - goto fail; - } + visit_type_BlockdevOptions(v, NULL, &options, &error_abort); visit_complete(v, &obj); qdict = qobject_to(QDict, obj); @@ -3017,8 +3012,6 @@ BlockDriverState *bdrv_open_blockdev_ref(BlockdevRef *ref, Error **errp) bs = bdrv_open_inherit(NULL, reference, qdict, 0, NULL, NULL, errp); obj = NULL; - -fail: qobject_unref(obj); visit_free(v); return bs; diff --git a/block/sheepdog.c b/block/sheepdog.c index 59f7ebb171..5f3aead038 100644 --- a/block/sheepdog.c +++ b/block/sheepdog.c @@ -1854,19 +1854,12 @@ static int sd_create_prealloc(BlockdevOptionsSheepdog *location, int64_t size, Visitor *v; QObject *obj = NULL; QDict *qdict; - Error *local_err = NULL; int ret; v = qobject_output_visitor_new(&obj); - visit_type_BlockdevOptionsSheepdog(v, NULL, &location, &local_err); + visit_type_BlockdevOptionsSheepdog(v, NULL, &location, &error_abort); visit_free(v); - if (local_err) { - error_propagate(errp, local_err); - qobject_unref(obj); - return -EINVAL; - } - qdict = qobject_to(QDict, obj); qdict_flatten(qdict); diff --git a/blockdev.c b/blockdev.c index 5faddaa705..9da960b1e7 100644 --- a/blockdev.c +++ b/blockdev.c @@ -3725,14 +3725,8 @@ void qmp_blockdev_add(BlockdevOptions *options, Error **errp) QObject *obj; Visitor *v = qobject_output_visitor_new(&obj); QDict *qdict; - Error *local_err = NULL; - - visit_type_BlockdevOptions(v, NULL, &options, &local_err); - if (local_err) { - error_propagate(errp, local_err); - goto fail; - } + visit_type_BlockdevOptions(v, NULL, &options, &error_abort); visit_complete(v, &obj); qdict = qobject_to(QDict, obj); @@ -3760,7 +3754,6 @@ void qmp_x_blockdev_reopen(BlockdevOptions *options, Error **errp) AioContext *ctx; QObject *obj; Visitor *v = qobject_output_visitor_new(&obj); - Error *local_err = NULL; BlockReopenQueue *queue; QDict *qdict; @@ -3777,12 +3770,7 @@ void qmp_x_blockdev_reopen(BlockdevOptions *options, Error **errp) } /* Put all options in a QDict and flatten it */ - visit_type_BlockdevOptions(v, NULL, &options, &local_err); - if (local_err) { - error_propagate(errp, local_err); - goto fail; - } - + visit_type_BlockdevOptions(v, NULL, &options, &error_abort); visit_complete(v, &obj); qdict = qobject_to(QDict, obj); diff --git a/hw/core/machine-hmp-cmds.c b/hw/core/machine-hmp-cmds.c index b76f7223af..39999c47c5 100644 --- a/hw/core/machine-hmp-cmds.c +++ b/hw/core/machine-hmp-cmds.c @@ -113,7 +113,7 @@ void hmp_info_memdev(Monitor *mon, const QDict *qdict) while (m) { v = string_output_visitor_new(false, &str); - visit_type_uint16List(v, NULL, &m->value->host_nodes, NULL); + visit_type_uint16List(v, NULL, &m->value->host_nodes, &error_abort); monitor_printf(mon, "memory backend: %s\n", m->value->id); monitor_printf(mon, " size: %" PRId64 "\n", m->value->size); monitor_printf(mon, " merge: %s\n", diff --git a/include/qapi/visitor-impl.h b/include/qapi/visitor-impl.h index 252206dc0d..98dc533d39 100644 --- a/include/qapi/visitor-impl.h +++ b/include/qapi/visitor-impl.h @@ -43,6 +43,10 @@ typedef enum VisitorType { struct Visitor { + /* + * Only input visitors may fail! + */ + /* Must be set to visit structs */ void (*start_struct)(Visitor *v, const char *name, void **obj, size_t size, Error **errp); diff --git a/include/qapi/visitor.h b/include/qapi/visitor.h index 2d40d2fe0f..5573906966 100644 --- a/include/qapi/visitor.h +++ b/include/qapi/visitor.h @@ -82,7 +82,7 @@ * Each function also takes the customary @errp argument (see * qapi/error.h for details), for reporting any errors (such as if a * member @name is not present, or is present but not the specified - * type). + * type). Only input visitors can fail. * * If an error is detected during visit_type_FOO() with an input * visitor, then *@obj will be set to NULL for pointer types, and left @@ -164,19 +164,14 @@ * * * Foo *f = ...obtain populated object... - * Error *err = NULL; * Visitor *v; * Type *result; * * v = FOO_visitor_new(..., &result); - * visit_type_Foo(v, NULL, &f, &err); - * if (err) { - * ...handle error... - * } else { - * visit_complete(v, &result); - * ...use result... - * } + * visit_type_Foo(v, NULL, &f, &error_abort); + * visit_complete(v, &result); * visit_free(v); + * ...use result... * * * It is also possible to use the visitors to do a virtual walk, where @@ -289,6 +284,7 @@ void visit_free(Visitor *v); * case @size is ignored. * * On failure, set *@obj to NULL and store an error through @errp. + * Can happen only when @v is an input visitor. * * After visit_start_struct() succeeds, the caller may visit its * members one after the other, passing the member's name and address @@ -305,7 +301,8 @@ void visit_start_struct(Visitor *v, const char *name, void **obj, /* * Prepare for completing an object visit. * - * On failure, store an error through @errp. + * On failure, store an error through @errp. Can happen only when @v + * is an input visitor. * * Should be called prior to visit_end_struct() if all other * intermediate visit steps were successful, to allow the visitor one @@ -342,6 +339,7 @@ void visit_end_struct(Visitor *v, void **obj); * ignored. * * On failure, set *@list to NULL and store an error through @errp. + * Can happen only when @v is an input visitor. * * After visit_start_list() succeeds, the caller may visit its members * one after the other. A real visit (where @list is non-NULL) uses @@ -375,7 +373,8 @@ GenericList *visit_next_list(Visitor *v, GenericList *tail, size_t size); /* * Prepare for completing a list visit. * - * On failure, store an error through @errp. + * On failure, store an error through @errp. Can happen only when @v + * is an input visitor. * * Should be called prior to visit_end_list() if all other * intermediate visit steps were successful, to allow the visitor one @@ -411,6 +410,7 @@ void visit_end_list(Visitor *v, void **list); * (*@obj)->type. Other visitors leave @obj unchanged. * * On failure, set *@obj to NULL and store an error through @errp. + * Can happen only when @v is an input visitor. * * If successful, this must be paired with visit_end_alternate() with * the same @obj to clean up, even if visiting the contents of the @@ -465,11 +465,13 @@ bool visit_optional(Visitor *v, const char *name, bool *present); * visitors produce text output. The mapping between enumeration * values and strings is done by the visitor core, using @lookup. * - * On failure, store an error through @errp. + * On failure, store an error through @errp. Can happen only when @v + * is an input visitor. * * May call visit_type_str() under the hood, and the enum visit may * fail even if the corresponding string visit succeeded; this implies - * that visit_type_str() must have no unwelcome side effects. + * that an input visitor's visit_type_str() must have no unwelcome + * side effects. */ void visit_type_enum(Visitor *v, const char *name, int *obj, const QEnumLookup *lookup, Error **errp); @@ -495,7 +497,8 @@ bool visit_is_dealloc(Visitor *v); * @obj must be non-NULL. Input visitors set *@obj to the value; * other visitors will leave *@obj unchanged. * - * On failure, store an error through @errp. + * On failure, store an error through @errp. Can happen only when @v + * is an input visitor. */ void visit_type_int(Visitor *v, const char *name, int64_t *obj, Error **errp); @@ -573,7 +576,8 @@ void visit_type_size(Visitor *v, const char *name, uint64_t *obj, * @obj must be non-NULL. Input visitors set *@obj to the value; * other visitors will leave *@obj unchanged. * - * On failure, store an error through @errp. + * On failure, store an error through @errp. Can happen only when @v + * is an input visitor. */ void visit_type_bool(Visitor *v, const char *name, bool *obj, Error **errp); @@ -592,6 +596,7 @@ void visit_type_bool(Visitor *v, const char *name, bool *obj, Error **errp); * into @obj for use by an output visitor. * * On failure, set *@obj to NULL and store an error through @errp. + * Can happen only when @v is an input visitor. * * FIXME: Callers that try to output NULL *obj should not be allowed. */ @@ -607,7 +612,8 @@ void visit_type_str(Visitor *v, const char *name, char **obj, Error **errp); * other visitors will leave *@obj unchanged. Visitors should * document if infinity or NaN are not permitted. * - * On failure, store an error through @errp. + * On failure, store an error through @errp. Can happen only when @v + * is an input visitor. */ void visit_type_number(Visitor *v, const char *name, double *obj, Error **errp); @@ -623,6 +629,7 @@ void visit_type_number(Visitor *v, const char *name, double *obj, * for output visitors. * * On failure, set *@obj to NULL and store an error through @errp. + * Can happen only when @v is an input visitor. * * Note that some kinds of input can't express arbitrary QObject. * E.g. the visitor returned by qobject_input_visitor_new_keyval() @@ -640,6 +647,7 @@ void visit_type_any(Visitor *v, const char *name, QObject **obj, Error **errp); * other visitors ignore *@obj. * * On failure, set *@obj to NULL and store an error through @errp. + * Can happen only when @v is an input visitor. */ void visit_type_null(Visitor *v, const char *name, QNull **obj, Error **errp); diff --git a/monitor/hmp-cmds.c b/monitor/hmp-cmds.c index 9b94e67879..7f6e982dc8 100644 --- a/monitor/hmp-cmds.c +++ b/monitor/hmp-cmds.c @@ -334,7 +334,8 @@ void hmp_info_migrate(Monitor *mon, const QDict *qdict) Visitor *v; char *str; v = string_output_visitor_new(false, &str); - visit_type_uint32List(v, NULL, &info->postcopy_vcpu_blocktime, NULL); + visit_type_uint32List(v, NULL, &info->postcopy_vcpu_blocktime, + &error_abort); visit_complete(v, &str); monitor_printf(mon, "postcopy vcpu blocktime: %s\n", str); g_free(str); -- cgit v1.2.3 From ea097dff0f011c07202ea7e52a421429e01ef351 Mon Sep 17 00:00:00 2001 From: Markus Armbruster Date: Fri, 24 Apr 2020 10:43:36 +0200 Subject: qom: Simplify object_property_get_enum() Signed-off-by: Markus Armbruster Reviewed-by: Eric Blake Message-Id: <20200424084338.26803-14-armbru@redhat.com> --- qom/object.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/qom/object.c b/qom/object.c index 1812f79224..be700e831f 100644 --- a/qom/object.c +++ b/qom/object.c @@ -1550,11 +1550,9 @@ int object_property_get_enum(Object *obj, const char *name, } visit_complete(v, &str); visit_free(v); - v = string_input_visitor_new(str); - visit_type_enum(v, name, &ret, enumprop->lookup, errp); + ret = qapi_enum_parse(enumprop->lookup, str, -1, errp); g_free(str); - visit_free(v); return ret; } -- cgit v1.2.3 From 2061487bdba7fb9077efc09210224b42fad7d18f Mon Sep 17 00:00:00 2001 From: Markus Armbruster Date: Fri, 24 Apr 2020 10:43:37 +0200 Subject: qapi: Disallow qmp_marshal_FOO(NULL, ...) For QMP commands without arguments, gen_marshal() laboriously generates a qmp_marshal_FOO() that copes with null @args. Turns there's just one caller that passes null instead of an empty QDict. Adjust that caller, and simplify gen_marshal(). Signed-off-by: Markus Armbruster Message-Id: <20200424084338.26803-15-armbru@redhat.com> Reviewed-by: Eric Blake --- docs/devel/qapi-code-gen.txt | 2 +- monitor/qmp.c | 5 ++++- scripts/qapi/commands.py | 26 ++------------------------ 3 files changed, 7 insertions(+), 26 deletions(-) diff --git a/docs/devel/qapi-code-gen.txt b/docs/devel/qapi-code-gen.txt index c6dd1891c3..a7794ef658 100644 --- a/docs/devel/qapi-code-gen.txt +++ b/docs/devel/qapi-code-gen.txt @@ -1579,8 +1579,8 @@ Example: void qmp_marshal_my_command(QDict *args, QObject **ret, Error **errp) { Error *err = NULL; - UserDefOne *retval; Visitor *v; + UserDefOne *retval; q_obj_my_command_arg arg = {0}; v = qobject_input_visitor_new(QOBJECT(args)); diff --git a/monitor/qmp.c b/monitor/qmp.c index f89e7daf27..d433ceae5b 100644 --- a/monitor/qmp.c +++ b/monitor/qmp.c @@ -322,9 +322,12 @@ static QDict *qmp_greeting(MonitorQMP *mon) { QList *cap_list = qlist_new(); QObject *ver = NULL; + QDict *args; QMPCapability cap; - qmp_marshal_query_version(NULL, &ver, NULL); + args = qdict_new(); + qmp_marshal_query_version(args, &ver, NULL); + qobject_unref(args); for (cap = 0; cap < QMP_CAPABILITY__MAX; cap++) { if (mon->capab_offered[cap]) { diff --git a/scripts/qapi/commands.py b/scripts/qapi/commands.py index bc30876c88..f545903567 100644 --- a/scripts/qapi/commands.py +++ b/scripts/qapi/commands.py @@ -104,6 +104,7 @@ def gen_marshal(name, arg_type, boxed, ret_type): %(proto)s { Error *err = NULL; + Visitor *v; ''', proto=build_marshal_proto(name)) @@ -117,21 +118,14 @@ def gen_marshal(name, arg_type, boxed, ret_type): visit_members = ('visit_type_%s_members(v, &arg, &err);' % arg_type.c_name()) ret += mcgen(''' - Visitor *v; %(c_name)s arg = {0}; - ''', c_name=arg_type.c_name()) else: visit_members = '' - ret += mcgen(''' - Visitor *v = NULL; - - if (args) { -''') - push_indent() ret += mcgen(''' + v = qobject_input_visitor_new(QOBJECT(args)); visit_start_struct(v, NULL, NULL, 0, &err); if (err) { @@ -148,12 +142,6 @@ def gen_marshal(name, arg_type, boxed, ret_type): ''', visit_members=visit_members) - if not have_args: - pop_indent() - ret += mcgen(''' - } -''') - ret += gen_call(name, arg_type, boxed, ret_type) ret += mcgen(''' @@ -168,10 +156,6 @@ out: % arg_type.c_name()) else: visit_members = '' - ret += mcgen(''' - if (args) { -''') - push_indent() ret += mcgen(''' v = qapi_dealloc_visitor_new(); @@ -182,12 +166,6 @@ out: ''', visit_members=visit_members) - if not have_args: - pop_indent() - ret += mcgen(''' - } -''') - ret += mcgen(''' } ''') -- cgit v1.2.3 From 89bf68f933393a1bc0de4d07b59ffa8920da130f Mon Sep 17 00:00:00 2001 From: Markus Armbruster Date: Fri, 24 Apr 2020 10:43:38 +0200 Subject: qapi: Generate simpler marshalling code when no arguments When command FOO has no arguments, its generated qmp_marshal_FOO() is a bit confusing. Make it simpler: visit_start_struct(v, NULL, NULL, 0, &err); if (err) { goto out; } - - if (!err) { - visit_check_struct(v, &err); - } + visit_check_struct(v, &err); visit_end_struct(v, NULL); if (err) { goto out; } Signed-off-by: Markus Armbruster Message-Id: <20200424084338.26803-16-armbru@redhat.com> Reviewed-by: Eric Blake --- scripts/qapi/commands.py | 40 ++++++++++++++++++++++++---------------- 1 file changed, 24 insertions(+), 16 deletions(-) diff --git a/scripts/qapi/commands.py b/scripts/qapi/commands.py index f545903567..6809b0fb6e 100644 --- a/scripts/qapi/commands.py +++ b/scripts/qapi/commands.py @@ -115,14 +115,10 @@ def gen_marshal(name, arg_type, boxed, ret_type): c_type=ret_type.c_type()) if have_args: - visit_members = ('visit_type_%s_members(v, &arg, &err);' - % arg_type.c_name()) ret += mcgen(''' %(c_name)s arg = {0}; ''', c_name=arg_type.c_name()) - else: - visit_members = '' ret += mcgen(''' @@ -131,16 +127,27 @@ def gen_marshal(name, arg_type, boxed, ret_type): if (err) { goto out; } - %(visit_members)s +''') + + if have_args: + ret += mcgen(''' + visit_type_%(c_arg_type)s_members(v, &arg, &err); if (!err) { visit_check_struct(v, &err); } +''', + c_arg_type=arg_type.c_name()) + else: + ret += mcgen(''' + visit_check_struct(v, &err); +''') + + ret += mcgen(''' visit_end_struct(v, NULL); if (err) { goto out; } -''', - visit_members=visit_members) +''') ret += gen_call(name, arg_type, boxed, ret_type) @@ -151,20 +158,21 @@ out: visit_free(v); ''') - if have_args: - visit_members = ('visit_type_%s_members(v, &arg, NULL);' - % arg_type.c_name()) - else: - visit_members = '' - ret += mcgen(''' v = qapi_dealloc_visitor_new(); visit_start_struct(v, NULL, NULL, 0, NULL); - %(visit_members)s +''') + + if have_args: + ret += mcgen(''' + visit_type_%(c_arg_type)s_members(v, &arg, NULL); +''', + c_arg_type=arg_type.c_name()) + + ret += mcgen(''' visit_end_struct(v, NULL); visit_free(v); -''', - visit_members=visit_members) +''') ret += mcgen(''' } -- cgit v1.2.3