[Qemu-devel] [PATCH v8 33/35] qapi: Simplify semantics of visit_next_list()

[Eric Blake <eblake@redhat.com>]

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()
prev = head
while (cur = next(prev)) {
    visit(cur)
    prev = &cur
}

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)
element = *head
while (element) {
    visit(element)
    element = next(element)
}

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 parameter after 'name'.

The spapr_drc case requires that visit_start_list() has to pay
attention to whether visit_next_list() will even be used to
visit a FooList qapi struct; this is 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 (it required hoisting the
advance of the current qlist entry out of qmp_input_next_list()
into qmp_input_get_object()).  But it turned out that the
string and opts visitors munge enough state during
visit_next_list() to make those conversions simpler if they
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 <eblake@redhat.com>

---
v8: consistent parameter order, fix qmp_input_get_next_type() to not
skip list entries
v7: new patch
---
 hw/ppc/spapr_drc.c           |  2 +-
 include/qapi/visitor-impl.h  |  5 ++--
 include/qapi/visitor.h       | 45 +++++++++++++++++--------------
 qapi/opts-visitor.c          | 32 +++++++++-------------
 qapi/qapi-dealloc-visitor.c  | 29 +++++---------------
 qapi/qapi-visit-core.c       |  7 ++---
 qapi/qmp-input-visitor.c     | 64 +++++++++++++++++++++-----------------------
 qapi/qmp-output-visitor.c    | 21 +++------------
 qapi/string-input-visitor.c  | 34 +++++++++++------------
 qapi/string-output-visitor.c | 36 ++++++++-----------------
 scripts/qapi-visit.py        | 21 ++++++++-------
 11 files changed, 126 insertions(+), 170 deletions(-)

diff --git a/hw/ppc/spapr_drc.c b/hw/ppc/spapr_drc.c
index 91becc2..e3e2127 100644
--- a/hw/ppc/spapr_drc.c
+++ b/hw/ppc/spapr_drc.c
@@ -299,7 +299,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, &err);
             if (err) {
                 error_propagate(errp, err);
                 return;
diff --git a/include/qapi/visitor-impl.h b/include/qapi/visitor-impl.h
index 08b1b94..94d65fa 100644
--- a/include/qapi/visitor-impl.h
+++ b/include/qapi/visitor-impl.h
@@ -40,9 +40,10 @@ struct Visitor
     void (*end_implicit_struct)(Visitor *v);

     /* Must be set */
-    void (*start_list)(Visitor *v, const char *name, Error **errp);
+    void (*start_list)(Visitor *v, const char *name, GenericList **list,
+                       Error **errp);
     /* Must be set */
-    GenericList *(*next_list)(Visitor *v, GenericList **list, Error **errp);
+    GenericList *(*next_list)(Visitor *v, GenericList *element, Error **errp);
     /* Must be set */
     void (*end_list)(Visitor *v);

diff --git a/include/qapi/visitor.h b/include/qapi/visitor.h
index e5dcde4..4638863 100644
--- a/include/qapi/visitor.h
+++ b/include/qapi/visitor.h
@@ -1,6 +1,7 @@
 /*
  * Core Definitions for QAPI Visitor Classes
  *
+ * Copyright (C) 2015 Red Hat, Inc.
  * Copyright IBM, Corp. 2011
  *
  * Authors:
@@ -111,32 +112,36 @@ void visit_end_implicit_struct(Visitor *v);
 /**
  * Prepare to visit a list tied to an object key @name.
  * @name will be NULL if this is visited as part of another list.
- * After calling this, the elements must be collected until
- * visit_next_list() returns NULL, then visit_end_list() must be
- * used to complete the visit.
- */
-void visit_start_list(Visitor *v, const char *name, Error **errp);
-/**
- * Iterate over a GenericList during a list visit.
- * @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
- * 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.
+ * Input visitors malloc a qapi List struct into *@list, or set it to
+ * NULL if there are no elements in the list; and output visitors
+ * expect *@list to point to the start of the list, if any.  On
+ * return, if *@list is non-NULL, the caller should enter a loop
+ * visiting the current element, then using visit_next_list() to
+ * advance to the next element, until that returns NULL; then
+ * visit_end_list() must be used to complete the visit.
  *
- * Note that for some visitors (qapi-dealloc and qmp-output), when a
- * qapi GenericList linked list is not being used (comparable to when
- * a NULL obj is used for visit_start_struct()), it is acceptable to
- * bypass the use of visit_next_list() and just directly call the
- * appropriate visit_type_*() for each element in between the
- * visit_start_list() and visit_end_list() calls.
+ * If supported by a visitor, @list can be NULL to indicate that there
+ * is no qapi List struct, and that the upcoming visit calls are
+ * parsing input to or creating output from some other representation;
+ * in this case, visit_next_list() will not be needed, but
+ * visit_end_list() is still mandatory.
  *
  * FIXME: For input visitors, *@list can be assigned here even if
  * later visits will fail; this can lead to memory leaks if clients
  * aren't careful.
  */
-GenericList *visit_next_list(Visitor *v, GenericList **list, Error **errp);
+void visit_start_list(Visitor *v, const char *name, GenericList **list,
+                      Error **errp);
+/**
+ * Iterate over a GenericList during a list visit.
+ * Before calling this function, the caller should use the appropriate
+ * visit_type_FOO() for the current list element at @element->value, and
+ * check for errors. @element must not be NULL; on the first iteration,
+ * it should be the value in *list after visit_start_list(); on other
+ * calls it should be the previous return value.  This function
+ * returns NULL once there are no further list elements.
+ */
+GenericList *visit_next_list(Visitor *v, GenericList *element, Error **errp);
 /**
  * Complete the list started earlier.
  * Must be called after any successful use of visit_start_list(),
diff --git a/qapi/opts-visitor.c b/qapi/opts-visitor.c
index b469573..c5a7396 100644
--- a/qapi/opts-visitor.c
+++ b/qapi/opts-visitor.c
@@ -21,9 +21,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
@@ -212,35 +211,32 @@ 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, 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_new0(GenericList, 1);
+    } else {
+        *list = NULL;
     }
 }


 static GenericList *
-opts_next_list(Visitor *v, GenericList **list, Error **errp)
+opts_next_list(Visitor *v, GenericList *list, Error **errp)
 {
     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;
@@ -261,7 +257,6 @@ opts_next_list(Visitor *v, GenericList **list, Error **errp)
             g_hash_table_remove(ov->unprocessed_opts, opt->name);
             return NULL;
         }
-        link = &(*list)->next;
         break;
     }

@@ -269,8 +264,8 @@ opts_next_list(Visitor *v, GenericList **list, Error **errp)
         abort();
     }

-    *link = g_malloc0(sizeof **link);
-    return *link;
+    list->next = g_new0(GenericList, 1);
+    return list->next;
 }


@@ -279,8 +274,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 a89e6d1..839049e 100644
--- a/qapi/qapi-dealloc-visitor.c
+++ b/qapi/qapi-dealloc-visitor.c
@@ -20,7 +20,6 @@
 typedef struct StackEntry
 {
     void *value;
-    bool is_list_head;
     QTAILQ_ENTRY(StackEntry) node;
 } StackEntry;

@@ -41,10 +40,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);
 }

@@ -92,31 +87,19 @@ static void qapi_dealloc_end_implicit_struct(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, 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 *list,
                                            Error **errp)
 {
-    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 = list->next;
+    g_free(list);
+    return next;
 }

 static void qapi_dealloc_end_list(Visitor *v)
diff --git a/qapi/qapi-visit-core.c b/qapi/qapi-visit-core.c
index 9506a02..f391a70 100644
--- a/qapi/qapi-visit-core.c
+++ b/qapi/qapi-visit-core.c
@@ -67,12 +67,13 @@ void visit_end_implicit_struct(Visitor *v)
     }
 }

-void visit_start_list(Visitor *v, const char *name, Error **errp)
+void visit_start_list(Visitor *v, const char *name, GenericList **list,
+                      Error **errp)
 {
-    v->start_list(v, name, errp);
+    v->start_list(v, name, list, errp);
 }

-GenericList *visit_next_list(Visitor *v, GenericList **list, Error **errp)
+GenericList *visit_next_list(Visitor *v, GenericList *list, Error **errp)
 {
     assert(list);
     return v->next_list(v, list, errp);
diff --git a/qapi/qmp-input-visitor.c b/qapi/qmp-input-visitor.c
index 52d8c9a..550ee93 100644
--- a/qapi/qmp-input-visitor.c
+++ b/qapi/qmp-input-visitor.c
@@ -44,16 +44,20 @@ static QObject *qmp_input_get_object(QmpInputVisitor *qiv,
                                      const char *name,
                                      bool consume)
 {
-    QObject *qobj = qiv->stack[qiv->nb_stack - 1].obj;
+    StackObject *so = &qiv->stack[qiv->nb_stack - 1];
+    QObject *qobj = so->obj;

     if (qobj) {
         if (name && qobject_type(qobj) == QTYPE_QDICT) {
-            if (qiv->stack[qiv->nb_stack - 1].h && consume) {
-                g_hash_table_remove(qiv->stack[qiv->nb_stack - 1].h, name);
+            if (so->h && consume) {
+                g_hash_table_remove(so->h, name);
+            }
+            qobj = qdict_get(qobject_to_qdict(qobj), name);
+        } else if (so->entry) {
+            qobj = qlist_entry_obj(so->entry);
+            if (consume) {
+                so->entry = qlist_next(so->entry);
             }
-            return qdict_get(qobject_to_qdict(qobj), name);
-        } else if (qiv->stack[qiv->nb_stack - 1].entry) {
-            return qlist_entry_obj(qiv->stack[qiv->nb_stack - 1].entry);
         }
     }

@@ -66,7 +70,8 @@ static void qdict_add_key(const char *key, QObject *obj, void *opaque)
     g_hash_table_insert(h, (gpointer) key, NULL);
 }

-static void qmp_input_push(QmpInputVisitor *qiv, QObject *obj, Error **errp)
+static void qmp_input_push(QmpInputVisitor *qiv, QObject *obj,
+                           const QListEntry *entry, Error **errp)
 {
     GHashTable *h;

@@ -76,7 +81,7 @@ static void qmp_input_push(QmpInputVisitor *qiv, QObject *obj, Error **errp)
     }

     qiv->stack[qiv->nb_stack].obj = obj;
-    qiv->stack[qiv->nb_stack].entry = NULL;
+    qiv->stack[qiv->nb_stack].entry = entry;
     qiv->stack[qiv->nb_stack].h = NULL;

     if (qiv->strict && qobject_type(obj) == QTYPE_QDICT) {
@@ -138,7 +143,7 @@ static void qmp_input_start_struct(Visitor *v, const char *name, void **obj,
         return;
     }

-    qmp_input_push(qiv, qobj, &err);
+    qmp_input_push(qiv, qobj, NULL, &err);
     if (err) {
         error_propagate(errp, err);
         return;
@@ -158,10 +163,12 @@ static void qmp_input_start_implicit_struct(Visitor *v, 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, 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) {
         error_setg(errp, QERR_INVALID_PARAMETER_TYPE, name ? name : "null",
@@ -169,37 +176,28 @@ static void qmp_input_start_list(Visitor *v, const char *name, Error **errp)
         return;
     }

-    qmp_input_push(qiv, qobj, errp);
+    entry = qlist_first(qobject_to_qlist(qobj));
+    qmp_input_push(qiv, qobj, entry, errp);
+    if (list) {
+        if (entry) {
+            *list = g_new0(GenericList, 1);
+        } else {
+            *list = NULL;
+        }
+    }
 }

-static GenericList *qmp_input_next_list(Visitor *v, GenericList **list,
+static GenericList *qmp_input_next_list(Visitor *v, GenericList *list,
                                         Error **errp)
 {
     QmpInputVisitor *qiv = to_qiv(v);
-    GenericList *entry;
     StackObject *so = &qiv->stack[qiv->nb_stack - 1];
-    bool first;

-    if (so->entry == NULL) {
-        so->entry = qlist_first(qobject_to_qlist(so->obj));
-        first = true;
-    } else {
-        so->entry = qlist_next(so->entry);
-        first = false;
-    }
-
-    if (so->entry == NULL) {
+    if (!so->entry) {
         return NULL;
     }
-
-    entry = g_malloc0(sizeof(*entry));
-    if (first) {
-        *list = entry;
-    } else {
-        (*list)->next = entry;
-    }
-
-    return entry;
+    list->next = g_new0(GenericList, 1);
+    return list->next;
 }


@@ -361,7 +359,7 @@ QmpInputVisitor *qmp_input_visitor_new(QObject *obj)
     v->visitor.optional = qmp_input_optional;
     v->visitor.get_next_type = qmp_input_get_next_type;

-    qmp_input_push(v, obj, NULL);
+    qmp_input_push(v, obj, NULL, NULL);
     qobject_incref(obj);

     return v;
diff --git a/qapi/qmp-output-visitor.c b/qapi/qmp-output-visitor.c
index 5a1767b..c9615fe 100644
--- a/qapi/qmp-output-visitor.c
+++ b/qapi/qmp-output-visitor.c
@@ -21,7 +21,6 @@
 typedef struct QStackEntry
 {
     QObject *value;
-    bool is_list_head;
     QTAILQ_ENTRY(QStackEntry) node;
 } QStackEntry;

@@ -51,9 +50,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);
 }

@@ -123,7 +119,8 @@ static void qmp_output_end_struct(Visitor *v)
     qmp_output_pop(qov);
 }

-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, Error **errp)
 {
     QmpOutputVisitor *qov = to_qov(v);
     QList *list = qlist_new();
@@ -132,20 +129,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 *list,
                                          Error **errp)
 {
-    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 list->next;
 }

 static void qmp_output_end_list(Visitor *v)
diff --git a/qapi/string-input-visitor.c b/qapi/string-input-visitor.c
index 610c233..582a62a 100644
--- a/qapi/string-input-visitor.c
+++ b/qapi/string-input-visitor.c
@@ -23,8 +23,6 @@ struct StringInputVisitor
 {
     Visitor visitor;

-    bool head;
-
     GList *ranges;
     GList *cur_range;
     int64_t cur;
@@ -123,11 +121,19 @@ error:
 }

 static void
-start_list(Visitor *v, const char *name, Error **errp)
+start_list(Visitor *v, const char *name, GenericList **list, 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) {
+        error_propagate(errp, err);
+        return;
+    }

     siv->cur_range = g_list_first(siv->ranges);
     if (siv->cur_range) {
@@ -135,14 +141,16 @@ start_list(Visitor *v, const char *name, Error **errp)
         if (r) {
             siv->cur = r->begin;
         }
+        *list = g_new0(GenericList, 1);
+    } else {
+        *list = NULL;
     }
 }

 static GenericList *
-next_list(Visitor *v, GenericList **list, Error **errp)
+next_list(Visitor *v, GenericList *list, Error **errp)
 {
     StringInputVisitor *siv = to_siv(v);
-    GenericList **link;
     Range *r;

     if (!siv->ranges || !siv->cur_range) {
@@ -166,22 +174,13 @@ next_list(Visitor *v, GenericList **list, Error **errp)
         siv->cur = r->begin;
     }

-    if (siv->head) {
-        link = list;
-        siv->head = false;
-    } else {
-        link = &(*list)->next;
-    }
-
-    *link = g_malloc0(sizeof **link);
-    return *link;
+    list->next = g_new0(GenericList, 1);
+    return list->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 +360,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 fd917a4..7209d80 100644
--- a/qapi/string-output-visitor.c
+++ b/qapi/string-output-visitor.c
@@ -57,7 +57,6 @@ struct StringOutputVisitor
     Visitor visitor;
     bool human;
     GString *string;
-    bool head;
     ListMode list_mode;
     union {
         int64_t s;
@@ -265,40 +264,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, 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, Error **errp)
+next_list(Visitor *v, GenericList *list, Error **errp)
 {
     StringOutputVisitor *sov = to_sov(v);
-    GenericList *ret = NULL;
-    if (*list) {
-        if (sov->head) {
-            ret = *list;
-        } else {
-            ret = (*list)->next;
-        }
+    GenericList *ret = list->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;
 }

@@ -312,8 +300,6 @@ 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/scripts/qapi-visit.py b/scripts/qapi-visit.py
index 8039b97..6016734 100644
--- a/scripts/qapi-visit.py
+++ b/scripts/qapi-visit.py
@@ -125,20 +125,23 @@ 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 *elt;

-    visit_start_list(v, name, &err);
+    visit_start_list(v, name, (GenericList **)obj, &err);
     if (err) {
         goto out;
     }
-
-    for (prev = (GenericList **)obj;
-         !err && (i = visit_next_list(v, prev, &err)) != NULL;
-         prev = &i) {
-        %(c_name)s *native_i = (%(c_name)s *)i;
-        visit_type_%(c_elt_type)s(v, NULL, &native_i->value, &err);
+    elt = *obj;
+    while (elt) {
+        visit_type_%(c_elt_type)s(v, NULL, &elt->value, &err);
+        if (err) {
+            break;
+        }
+        elt = (%(c_name)s *)visit_next_list(v, (GenericList *)elt, &err);
+        if (err) {
+            break;
+        }
     }
-
     visit_end_list(v);
 out:
     error_propagate(errp, err);
-- 
2.4.3