From patchwork Tue Mar 1 05:14:31 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Eric Blake X-Patchwork-Id: 8462061 Return-Path: X-Original-To: patchwork-qemu-devel@patchwork.kernel.org Delivered-To: patchwork-parsemail@patchwork1.web.kernel.org Received: from mail.kernel.org (mail.kernel.org [198.145.29.136]) by patchwork1.web.kernel.org (Postfix) with ESMTP id A94129F372 for ; Tue, 1 Mar 2016 05:24:39 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id DCC6F20320 for ; Tue, 1 Mar 2016 05:24:37 +0000 (UTC) Received: from lists.gnu.org (lists.gnu.org [208.118.235.17]) (using TLSv1 with cipher AES256-SHA (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id C019720304 for ; Tue, 1 Mar 2016 05:24:35 +0000 (UTC) Received: from localhost ([::1]:40982 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1aacnT-0000Uc-5C for patchwork-qemu-devel@patchwork.kernel.org; Tue, 01 Mar 2016 00:24:35 -0500 Received: from eggs.gnu.org ([2001:4830:134:3::10]:59850) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1aace3-0000W2-MD for qemu-devel@nongnu.org; Tue, 01 Mar 2016 00:14:54 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1aacdz-0006Sz-B6 for qemu-devel@nongnu.org; Tue, 01 Mar 2016 00:14:51 -0500 Received: from mx1.redhat.com ([209.132.183.28]:58716) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1aacdy-0006Sf-OZ; Tue, 01 Mar 2016 00:14:47 -0500 Received: from int-mx14.intmail.prod.int.phx2.redhat.com (int-mx14.intmail.prod.int.phx2.redhat.com [10.5.11.27]) by mx1.redhat.com (Postfix) with ESMTPS id 76003C00FBB7; Tue, 1 Mar 2016 05:14:46 +0000 (UTC) Received: from red.redhat.com (ovpn-113-165.phx2.redhat.com [10.3.113.165]) by int-mx14.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id u215EZ9b018132; Tue, 1 Mar 2016 00:14:45 -0500 From: Eric Blake To: qemu-devel@nongnu.org Date: Mon, 29 Feb 2016 22:14:31 -0700 Message-Id: <1456809272-14184-18-git-send-email-eblake@redhat.com> In-Reply-To: <1456809272-14184-1-git-send-email-eblake@redhat.com> References: <1456809272-14184-1-git-send-email-eblake@redhat.com> X-Scanned-By: MIMEDefang 2.68 on 10.5.11.27 X-detected-operating-system: by eggs.gnu.org: GNU/Linux 3.x X-Received-From: 209.132.183.28 Cc: Alexander Graf , David Gibson , "open list:sPAPR" , armbru@redhat.com, Michael Roth Subject: [Qemu-devel] [PATCH v12 17/18] qapi: Simplify semantics of visit_next_list() X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.14 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: qemu-devel-bounces+patchwork-qemu-devel=patchwork.kernel.org@nongnu.org Sender: qemu-devel-bounces+patchwork-qemu-devel=patchwork.kernel.org@nongnu.org X-Spam-Status: No, score=-6.9 required=5.0 tests=BAYES_00, RCVD_IN_DNSWL_HI, UNPARSEABLE_RELAY autolearn=unavailable version=3.3.1 X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on mail.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP We have two uses of list visits in the entire code base: one in spapr_drc (which completely avoids visit_next_list(), feeding in integers from a different source than uint8List), and one in qapi-visit.py (that is, all other list visitors are generated in qapi-visit.c, and share the same paradigm based on a qapi FooList type). What's more, the semantics of the list visit are somewhat baroque, with the following pseudocode when FooList is used: start() for (prev = head; cur = next(prev); prev = &cur) { visit(&cur->value) } Note that these semantics (advance before visit) requires that the first call to next() return the list head, while all other calls return the next element of the list; that is, every visitor implementation is required to track extra state to decide whether to return the input as-is, or to advance. It also requires an argument of 'GenericList **' to next(), solely because the first iteration might need to modify the caller's GenericList head, so that all other calls have to do a layer of dereferencing. We can greatly simplify things by hoisting the special case into the start() routine, and flipping the order in the loop to visit before advance: start(head) for (tail = *head; tail; tail = next(tail)) { visit(&tail->value) } With the simpler semantics, visitors have less state to track, the argument to next() is reduced to 'GenericList *', and it also becomes obvious whether an input visitor is allocating a FooList during visit_start_list() (rather than the old way of not knowing if an allocation happened until the first visit_next_list()). The signature of visit_start_list() is chosen to match visit_start_struct(), with the new parameters after 'name'. The spapr_drc case is a virtual visit, done by passing NULL for list, similarly to how NULL is passed to visit_start_struct() when a qapi type is not used in those visits. It was easy to provide these semantics for qmp-output and dealloc visitors, and a bit harder for qmp-input (several prerequisite patches refactored things to make this patch straightforward). But it turned out that the string and opts visitors munge enough other state during visit_next_list() to make it easier to just document and require a GenericList visit for now; an assertion will remind us to adjust things if we need the semantics in the future. Signed-off-by: Eric Blake --- v12: rebase to earlier changes, drop R-b, improve documentation, split out qmp-input cleanups into prereq patches [no v10, v11] v9: no change v8: consistent parameter order, fix qmp_input_get_next_type() to not skip list entries v7: new patch --- include/qapi/visitor.h | 46 ++++++++++++++++++++---------------- include/qapi/visitor-impl.h | 7 +++--- scripts/qapi-visit.py | 18 +++++++------- include/qapi/opts-visitor.h | 2 +- include/qapi/string-input-visitor.h | 3 ++- include/qapi/string-output-visitor.h | 3 ++- qapi/qapi-visit-core.c | 12 ++++++---- hw/ppc/spapr_drc.c | 2 +- qapi/opts-visitor.c | 33 +++++++++++--------------- qapi/qapi-dealloc-visitor.c | 35 ++++++--------------------- qapi/qmp-input-visitor.c | 32 ++++++++++++------------- qapi/qmp-output-visitor.c | 22 ++++------------- qapi/string-input-visitor.c | 36 ++++++++++++++-------------- qapi/string-output-visitor.c | 41 +++++++++++--------------------- docs/qapi-code-gen.txt | 17 ++++++------- 15 files changed, 133 insertions(+), 176 deletions(-) diff --git a/include/qapi/visitor.h b/include/qapi/visitor.h index fb4582a..a81878d 100644 --- a/include/qapi/visitor.h +++ b/include/qapi/visitor.h @@ -166,7 +166,7 @@ * if (err) { * goto outobj; * } - * visit_start_list(v, "list", &err); + * visit_start_list(v, "list", NULL, 0, &err); * if (err) { * goto outobj; * } @@ -268,39 +268,43 @@ void visit_end_struct(Visitor *v); * @name expresses the relationship of this list to its parent * container; see the general description of @name above. * + * @list must be non-NULL for a real walk, in which case @size + * determines how much memory an input visitor will allocate into + * *@list (at least sizeof(GenericList)). Some visitors also allow + * @list to be NULL for a virtual walk, in which case @size is + * ignored. + * * @errp must be NULL-initialized, and is set if an error is detected * (such as if a member @name is not present, or is present but not a - * list). + * list). On error, input visitors set *@obj to NULL. * * After visit_start_list() succeeds, the caller may visit its members - * one after the other. A real visit uses visit_next_list() for - * traversing the linked list, while a virtual visit 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 list element. - * Finally, visit_end_list() needs to be called to clean up. See the - * examples above. + * one after the other. A real visit (where @obj was non-NULL) uses + * visit_next_list() for traversing the linked list, while a virtual + * visit (where @obj was 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 list element. Finally, + * visit_end_list() needs to be called to clean up. See the examples + * above. */ -void visit_start_list(Visitor *v, const char *name, Error **errp); +void visit_start_list(Visitor *v, const char *name, GenericList **list, + size_t size, Error **errp); /* - * Iterate over a GenericList during a list visit. + * Iterate over a GenericList during a non-virtual list visit. * - * @size represents the size of a linked list node. + * @size represents the size of a linked list node (at least + * sizeof(GenericList)). * - * @list must not be NULL; on the first call, @list contains the - * address of the list head, and on subsequent calls *@list must be - * the previously returned value. Must be called in a loop until a + * @tail must not be NULL; on the first call, @tail is the value of + * *list after visit_start_list(), and on subsequent calls @tail must + * be the previously returned value. Must be called in a loop until a * NULL return or error occurs; for each non-NULL return, the caller * must then call the appropriate visit_type_*() for the element type * of the list, with that function's name parameter set to NULL and - * obj set to the address of (*@list)->value. - * - * FIXME: This interface is awkward; it requires all callbacks to - * track whether it is the first or a subsequent call. A better - * interface would pass the head of the list through - * visit_start_list(). + * obj set to the address of @tail->value. */ -GenericList *visit_next_list(Visitor *v, GenericList **list, size_t size); +GenericList *visit_next_list(Visitor *v, GenericList *tail, size_t size); /* * Complete a list visit started earlier. diff --git a/include/qapi/visitor-impl.h b/include/qapi/visitor-impl.h index d44fcd1..0471465 100644 --- a/include/qapi/visitor-impl.h +++ b/include/qapi/visitor-impl.h @@ -45,11 +45,12 @@ struct Visitor /* Must be set to visit structs. */ void (*end_struct)(Visitor *v); - /* Must be set. */ - void (*start_list)(Visitor *v, const char *name, Error **errp); + /* Must be set; document if @list may not be NULL. */ + void (*start_list)(Visitor *v, const char *name, GenericList **list, + size_t size, Error **errp); /* Must be set. */ - GenericList *(*next_list)(Visitor *v, GenericList **list, size_t size); + GenericList *(*next_list)(Visitor *v, GenericList *tail, size_t size); /* Must be set. */ void (*end_list)(Visitor *v); diff --git a/scripts/qapi-visit.py b/scripts/qapi-visit.py index db756a0..5921bad 100644 --- a/scripts/qapi-visit.py +++ b/scripts/qapi-visit.py @@ -119,20 +119,20 @@ def gen_visit_list(name, element_type): void visit_type_%(c_name)s(Visitor *v, const char *name, %(c_name)s **obj, Error **errp) { Error *err = NULL; - GenericList *i, **prev; + %(c_name)s *tail; + size_t size = sizeof(**obj); - visit_start_list(v, name, &err); + visit_start_list(v, name, (GenericList **)obj, size, &err); if (err) { goto out; } - - for (prev = (GenericList **)obj; - !err && (i = visit_next_list(v, prev, sizeof(**obj))) != NULL; - prev = &i) { - %(c_name)s *native_i = (%(c_name)s *)i; - visit_type_%(c_elt_type)s(v, NULL, &native_i->value, &err); + for (tail = *obj; tail; + tail = (%(c_name)s *)visit_next_list(v, (GenericList *)tail, size)) { + visit_type_%(c_elt_type)s(v, NULL, &tail->value, &err); + if (err) { + break; + } } - visit_end_list(v); out: error_propagate(errp, err); diff --git a/include/qapi/opts-visitor.h b/include/qapi/opts-visitor.h index 3fcd327..7c9a7e5 100644 --- a/include/qapi/opts-visitor.h +++ b/include/qapi/opts-visitor.h @@ -32,7 +32,7 @@ typedef struct OptsVisitor OptsVisitor; * * The Opts input visitor does not yet implement support for visiting * QAPI alternates, numbers (other than integers), null, or arbitrary - * QTypes. + * QTypes. It also requires non-NULL list to visit_start_list(). */ OptsVisitor *opts_visitor_new(const QemuOpts *opts); void opts_visitor_cleanup(OptsVisitor *nv); diff --git a/include/qapi/string-input-visitor.h b/include/qapi/string-input-visitor.h index 1a34c52..d26a845 100644 --- a/include/qapi/string-input-visitor.h +++ b/include/qapi/string-input-visitor.h @@ -19,7 +19,8 @@ typedef struct StringInputVisitor StringInputVisitor; /* * The string input visitor does not yet implement support for - * visiting QAPI structs, alternates, null, or arbitrary QTypes. + * visiting QAPI structs, alternates, null, or arbitrary QTypes. It + * also requires non-NULL list to visit_start_list(). */ StringInputVisitor *string_input_visitor_new(const char *str); void string_input_visitor_cleanup(StringInputVisitor *v); diff --git a/include/qapi/string-output-visitor.h b/include/qapi/string-output-visitor.h index 2564833..0a9354c 100644 --- a/include/qapi/string-output-visitor.h +++ b/include/qapi/string-output-visitor.h @@ -19,7 +19,8 @@ typedef struct StringOutputVisitor StringOutputVisitor; /* * The string output visitor does not yet implement support for - * visiting QAPI structs, alternates, null, or arbitrary QTypes. + * visiting QAPI structs, alternates, null, or arbitrary QTypes. It + * also requires non-NULL list to visit_start_list(). */ StringOutputVisitor *string_output_visitor_new(bool human); void string_output_visitor_cleanup(StringOutputVisitor *v); diff --git a/qapi/qapi-visit-core.c b/qapi/qapi-visit-core.c index c00a9e1..bbcedb1 100644 --- a/qapi/qapi-visit-core.c +++ b/qapi/qapi-visit-core.c @@ -41,15 +41,17 @@ void visit_end_struct(Visitor *v) v->end_struct(v); } -void visit_start_list(Visitor *v, const char *name, Error **errp) +void visit_start_list(Visitor *v, const char *name, GenericList **list, + size_t size, Error **errp) { - v->start_list(v, name, errp); + assert(!list || size >= sizeof(GenericList)); + v->start_list(v, name, list, size, errp); } -GenericList *visit_next_list(Visitor *v, GenericList **list, size_t size) +GenericList *visit_next_list(Visitor *v, GenericList *tail, size_t size) { - assert(list && size >= sizeof(GenericList)); - return v->next_list(v, list, size); + assert(tail && size >= sizeof(GenericList)); + return v->next_list(v, tail, size); } void visit_end_list(Visitor *v) diff --git a/hw/ppc/spapr_drc.c b/hw/ppc/spapr_drc.c index 77fe01a..4f42449 100644 --- a/hw/ppc/spapr_drc.c +++ b/hw/ppc/spapr_drc.c @@ -300,7 +300,7 @@ static void prop_get_fdt(Object *obj, Visitor *v, const char *name, int i; prop = fdt_get_property_by_offset(fdt, fdt_offset, &prop_len); name = fdt_string(fdt, fdt32_to_cpu(prop->nameoff)); - visit_start_list(v, name, &err); + visit_start_list(v, name, NULL, 0, &err); if (err) { error_propagate(errp, err); return; diff --git a/qapi/opts-visitor.c b/qapi/opts-visitor.c index f2fcfc3..525805e 100644 --- a/qapi/opts-visitor.c +++ b/qapi/opts-visitor.c @@ -22,9 +22,8 @@ enum ListMode { LM_NONE, /* not traversing a list of repeated options */ - LM_STARTED, /* opts_start_list() succeeded */ - LM_IN_PROGRESS, /* opts_next_list() has been called. + LM_IN_PROGRESS, /* opts_next_list() ready to be called. * * Generating the next list link will consume the most * recently parsed QemuOpt instance of the repeated @@ -213,35 +212,33 @@ lookup_distinct(const OptsVisitor *ov, const char *name, Error **errp) static void -opts_start_list(Visitor *v, const char *name, Error **errp) +opts_start_list(Visitor *v, const char *name, GenericList **list, size_t size, + Error **errp) { OptsVisitor *ov = to_ov(v); /* we can't traverse a list in a list */ assert(ov->list_mode == LM_NONE); + /* we don't support visits without GenericList, yet */ + assert(list); ov->repeated_opts = lookup_distinct(ov, name, errp); - if (ov->repeated_opts != NULL) { - ov->list_mode = LM_STARTED; + if (ov->repeated_opts) { + ov->list_mode = LM_IN_PROGRESS; + *list = g_malloc0(size); + } else { + *list = NULL; } } static GenericList * -opts_next_list(Visitor *v, GenericList **list, size_t size) +opts_next_list(Visitor *v, GenericList *tail, size_t size) { OptsVisitor *ov = to_ov(v); - GenericList **link; switch (ov->list_mode) { - case LM_STARTED: - ov->list_mode = LM_IN_PROGRESS; - link = list; - break; - case LM_SIGNED_INTERVAL: case LM_UNSIGNED_INTERVAL: - link = &(*list)->next; - if (ov->list_mode == LM_SIGNED_INTERVAL) { if (ov->range_next.s < ov->range_limit.s) { ++ov->range_next.s; @@ -262,7 +259,6 @@ opts_next_list(Visitor *v, GenericList **list, size_t size) g_hash_table_remove(ov->unprocessed_opts, opt->name); return NULL; } - link = &(*list)->next; break; } @@ -270,8 +266,8 @@ opts_next_list(Visitor *v, GenericList **list, size_t size) abort(); } - *link = g_malloc0(size); - return *link; + tail->next = g_malloc0(size); + return tail->next; } @@ -280,8 +276,7 @@ opts_end_list(Visitor *v) { OptsVisitor *ov = to_ov(v); - assert(ov->list_mode == LM_STARTED || - ov->list_mode == LM_IN_PROGRESS || + assert(ov->list_mode == LM_IN_PROGRESS || ov->list_mode == LM_SIGNED_INTERVAL || ov->list_mode == LM_UNSIGNED_INTERVAL); ov->repeated_opts = NULL; diff --git a/qapi/qapi-dealloc-visitor.c b/qapi/qapi-dealloc-visitor.c index 9005bad..cd68b55 100644 --- a/qapi/qapi-dealloc-visitor.c +++ b/qapi/qapi-dealloc-visitor.c @@ -22,7 +22,6 @@ typedef struct StackEntry { void *value; - bool is_list_head; QTAILQ_ENTRY(StackEntry) node; } StackEntry; @@ -43,10 +42,6 @@ static void qapi_dealloc_push(QapiDeallocVisitor *qov, void *value) e->value = value; - /* see if we're just pushing a list head tracker */ - if (value == NULL) { - e->is_list_head = true; - } QTAILQ_INSERT_HEAD(&qov->stack, e, node); } @@ -93,38 +88,22 @@ static void qapi_dealloc_end_alternate(Visitor *v) } } -static void qapi_dealloc_start_list(Visitor *v, const char *name, Error **errp) +static void qapi_dealloc_start_list(Visitor *v, const char *name, + GenericList **list, size_t size, + Error **errp) { - QapiDeallocVisitor *qov = to_qov(v); - qapi_dealloc_push(qov, NULL); } -static GenericList *qapi_dealloc_next_list(Visitor *v, GenericList **listp, +static GenericList *qapi_dealloc_next_list(Visitor *v, GenericList *tail, size_t size) { - GenericList *list = *listp; - QapiDeallocVisitor *qov = to_qov(v); - StackEntry *e = QTAILQ_FIRST(&qov->stack); - - if (e && e->is_list_head) { - e->is_list_head = false; - return list; - } - - if (list) { - list = list->next; - g_free(*listp); - return list; - } - - return NULL; + GenericList *next = tail->next; + g_free(tail); + return next; } static void qapi_dealloc_end_list(Visitor *v) { - QapiDeallocVisitor *qov = to_qov(v); - void *obj = qapi_dealloc_pop(qov); - assert(obj == NULL); /* should've been list head tracker with no payload */ } static void qapi_dealloc_type_str(Visitor *v, const char *name, char **obj, diff --git a/qapi/qmp-input-visitor.c b/qapi/qmp-input-visitor.c index 2daf83f..e1f4085 100644 --- a/qapi/qmp-input-visitor.c +++ b/qapi/qmp-input-visitor.c @@ -28,8 +28,6 @@ typedef struct StackObject /* If obj is list: tail of list still needing visits */ const QListEntry *entry; - /* If obj is list: true if head is not visited yet */ - bool first; GHashTable *h; /* If obj is dict: remaining keys needing visits */ } StackObject; @@ -86,7 +84,6 @@ static QObject *qmp_input_get_object(QmpInputVisitor *qiv, assert(!name); if (tos->entry) { - assert(!tos->first); qobj = qlist_entry_obj(tos->entry); if (consume) { tos->entry = qlist_next(tos->entry); @@ -116,7 +113,6 @@ static void qmp_input_push(QmpInputVisitor *qiv, QObject *obj, tos->obj = obj; tos->entry = entry; - tos->first = true; tos->h = NULL; if (qiv->strict && qobject_type(obj) == QTYPE_QDICT) { @@ -193,13 +189,17 @@ static void qmp_input_start_struct(Visitor *v, const char *name, void **obj, } -static void qmp_input_start_list(Visitor *v, const char *name, Error **errp) +static void qmp_input_start_list(Visitor *v, const char *name, + GenericList **list, size_t size, Error **errp) { QmpInputVisitor *qiv = to_qiv(v); QObject *qobj = qmp_input_get_object(qiv, name, true); const QListEntry *entry; if (!qobj || qobject_type(qobj) != QTYPE_QLIST) { + if (list) { + *list = NULL; + } error_setg(errp, QERR_INVALID_PARAMETER_TYPE, name ? name : "null", "list"); return; @@ -207,28 +207,26 @@ static void qmp_input_start_list(Visitor *v, const char *name, Error **errp) entry = qlist_first(qobject_to_qlist(qobj)); qmp_input_push(qiv, qobj, entry, errp); + if (list) { + if (entry) { + *list = g_malloc0(size); + } else { + *list = NULL; + } + } } -static GenericList *qmp_input_next_list(Visitor *v, GenericList **list, +static GenericList *qmp_input_next_list(Visitor *v, GenericList *tail, size_t size) { QmpInputVisitor *qiv = to_qiv(v); - GenericList *entry; StackObject *so = &qiv->stack[qiv->nb_stack - 1]; if (!so->entry) { return NULL; } - - entry = g_malloc0(size); - if (so->first) { - *list = entry; - so->first = false; - } else { - (*list)->next = entry; - } - - return entry; + tail->next = g_malloc0(size); + return tail->next; } diff --git a/qapi/qmp-output-visitor.c b/qapi/qmp-output-visitor.c index ecb2005..40657be 100644 --- a/qapi/qmp-output-visitor.c +++ b/qapi/qmp-output-visitor.c @@ -22,7 +22,6 @@ typedef struct QStackEntry { QObject *value; - bool is_list_head; QTAILQ_ENTRY(QStackEntry) node; } QStackEntry; @@ -52,9 +51,6 @@ static void qmp_output_push_obj(QmpOutputVisitor *qov, QObject *value) assert(qov->root); assert(value); e->value = value; - if (qobject_type(e->value) == QTYPE_QLIST) { - e->is_list_head = true; - } QTAILQ_INSERT_HEAD(&qov->stack, e, node); } @@ -120,7 +116,9 @@ static void qmp_output_end_struct(Visitor *v) assert(qobject_type(value) == QTYPE_QDICT); } -static void qmp_output_start_list(Visitor *v, const char *name, Error **errp) +static void qmp_output_start_list(Visitor *v, const char *name, + GenericList **listp, size_t size, + Error **errp) { QmpOutputVisitor *qov = to_qov(v); QList *list = qlist_new(); @@ -129,20 +127,10 @@ static void qmp_output_start_list(Visitor *v, const char *name, Error **errp) qmp_output_push(qov, list); } -static GenericList *qmp_output_next_list(Visitor *v, GenericList **listp, +static GenericList *qmp_output_next_list(Visitor *v, GenericList *tail, size_t size) { - GenericList *list = *listp; - QmpOutputVisitor *qov = to_qov(v); - QStackEntry *e = QTAILQ_FIRST(&qov->stack); - - assert(e); - if (e->is_list_head) { - e->is_list_head = false; - return list; - } - - return list ? list->next : NULL; + return tail->next; } static void qmp_output_end_list(Visitor *v) diff --git a/qapi/string-input-visitor.c b/qapi/string-input-visitor.c index 4087bf9..a514526 100644 --- a/qapi/string-input-visitor.c +++ b/qapi/string-input-visitor.c @@ -24,8 +24,6 @@ struct StringInputVisitor { Visitor visitor; - bool head; - GList *ranges; GList *cur_range; int64_t cur; @@ -124,11 +122,21 @@ error: } static void -start_list(Visitor *v, const char *name, Error **errp) +start_list(Visitor *v, const char *name, GenericList **list, size_t size, + Error **errp) { StringInputVisitor *siv = to_siv(v); + Error *err = NULL; - parse_str(siv, errp); + /* We don't support visits without a GenericList, yet */ + assert(list); + + parse_str(siv, &err); + if (err) { + *list = NULL; + error_propagate(errp, err); + return; + } siv->cur_range = g_list_first(siv->ranges); if (siv->cur_range) { @@ -136,13 +144,15 @@ start_list(Visitor *v, const char *name, Error **errp) if (r) { siv->cur = r->begin; } + *list = g_malloc0(size); + } else { + *list = NULL; } } -static GenericList *next_list(Visitor *v, GenericList **list, size_t size) +static GenericList *next_list(Visitor *v, GenericList *tail, size_t size) { StringInputVisitor *siv = to_siv(v); - GenericList **link; Range *r; if (!siv->ranges || !siv->cur_range) { @@ -166,21 +176,12 @@ static GenericList *next_list(Visitor *v, GenericList **list, size_t size) siv->cur = r->begin; } - if (siv->head) { - link = list; - siv->head = false; - } else { - link = &(*list)->next; - } - - *link = g_malloc0(size); - return *link; + tail->next = g_malloc0(size); + return tail->next; } static void end_list(Visitor *v) { - StringInputVisitor *siv = to_siv(v); - siv->head = true; } static void parse_type_int64(Visitor *v, const char *name, int64_t *obj, @@ -361,6 +362,5 @@ StringInputVisitor *string_input_visitor_new(const char *str) v->visitor.optional = parse_optional; v->string = str; - v->head = true; return v; } diff --git a/qapi/string-output-visitor.c b/qapi/string-output-visitor.c index 0d44d7e..e27e2df 100644 --- a/qapi/string-output-visitor.c +++ b/qapi/string-output-visitor.c @@ -20,7 +20,7 @@ enum ListMode { LM_NONE, /* not traversing a list of repeated options */ - LM_STARTED, /* start_list() succeeded */ + LM_STARTED, /* start_list() succeeded with multiple elements */ LM_IN_PROGRESS, /* next_list() has been called. * @@ -48,7 +48,7 @@ enum ListMode { LM_UNSIGNED_INTERVAL,/* Same as above, only for an unsigned interval. */ - LM_END + LM_END, /* next_list() called, about to see last element. */ }; typedef enum ListMode ListMode; @@ -58,7 +58,6 @@ struct StringOutputVisitor Visitor visitor; bool human; GString *string; - bool head; ListMode list_mode; union { int64_t s; @@ -266,39 +265,29 @@ static void print_type_number(Visitor *v, const char *name, double *obj, } static void -start_list(Visitor *v, const char *name, Error **errp) +start_list(Visitor *v, const char *name, GenericList **list, size_t size, + Error **errp) { StringOutputVisitor *sov = to_sov(v); /* we can't traverse a list in a list */ assert(sov->list_mode == LM_NONE); - sov->list_mode = LM_STARTED; - sov->head = true; + /* We don't support visits without a GenericList, yet */ + assert(list); + /* List handling is only needed if there are at least two elements */ + if (*list && (*list)->next) { + sov->list_mode = LM_STARTED; + } } -static GenericList *next_list(Visitor *v, GenericList **list, size_t size) +static GenericList *next_list(Visitor *v, GenericList *tail, size_t size) { StringOutputVisitor *sov = to_sov(v); - GenericList *ret = NULL; - if (*list) { - if (sov->head) { - ret = *list; - } else { - ret = (*list)->next; - } + GenericList *ret = tail->next; - if (sov->head) { - if (ret && ret->next == NULL) { - sov->list_mode = LM_NONE; - } - sov->head = false; - } else { - if (ret && ret->next == NULL) { - sov->list_mode = LM_END; - } - } + if (ret && !ret->next) { + sov->list_mode = LM_END; } - return ret; } @@ -311,8 +300,6 @@ static void end_list(Visitor *v) sov->list_mode == LM_NONE || sov->list_mode == LM_IN_PROGRESS); sov->list_mode = LM_NONE; - sov->head = true; - } char *string_output_get_string(StringOutputVisitor *sov) diff --git a/docs/qapi-code-gen.txt b/docs/qapi-code-gen.txt index 41d856c..06f22b7 100644 --- a/docs/qapi-code-gen.txt +++ b/docs/qapi-code-gen.txt @@ -911,18 +911,19 @@ Example: void visit_type_UserDefOneList(Visitor *v, const char *name, UserDefOneList **obj, Error **errp) { Error *err = NULL; - GenericList *i, **prev; + UserDefOneList *tail; + size_t size = sizeof(**obj); - visit_start_list(v, name, &err); + visit_start_list(v, name, (GenericList **)obj, size, &err); if (err) { goto out; } - - for (prev = (GenericList **)obj; - !err && (i = visit_next_list(v, prev, sizeof(**obj))) != NULL; - prev = &i) { - UserDefOneList *native_i = (UserDefOneList *)i; - visit_type_UserDefOne(v, NULL, &native_i->value, &err); + for (tail = *obj; tail; + tail = (UserDefOneList *)visit_next_list(v, (GenericList *)tail, size)) { + visit_type_UserDefOne(v, NULL, &tail->value, &err); + if (err) { + break; + } } visit_end_list(v);