diff mbox

[v13,17/18] qapi: Simplify semantics of visit_next_list()

Message ID 1457221861-18067-18-git-send-email-eblake@redhat.com (mailing list archive)
State New, archived
Headers show

Commit Message

Eric Blake March 5, 2016, 11:51 p.m. UTC
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 <eblake@redhat.com>

---
v13: documentation wording tweaks
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               | 47 +++++++++++++++++++-----------------
 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(+), 177 deletions(-)
diff mbox

Patch

diff --git a/include/qapi/visitor.h b/include/qapi/visitor.h
index 5f86262..8d4837d 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;
  *  }
@@ -269,40 +269,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 value member of
- * the list element.  Finally, visit_end_list() needs to be called to
- * clean up, even if intermediate visits fail.  See the examples
- * above.
+ * one after the other.  A real visit (where @obj 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
+ * 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 to clean up,
+ * even if intermediate visits fail.  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 d2eaea9..fdb10c0 100644
--- a/scripts/qapi-visit.py
+++ b/scripts/qapi-visit.py
@@ -100,20 +100,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 2de8d25..6107fa4 100644
--- a/docs/qapi-code-gen.txt
+++ b/docs/qapi-code-gen.txt
@@ -912,18 +912,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);