qapi: Split visit_end_struct() into pieces

As mentioned in previous patches, we want to call visit_end_struct()
functions unconditionally, so that visitors can release resources
tied up since the matching visit_start_struct() without also having
to worry about error priority if more than one error occurs.

Even though error_propagate() can be safely used to ignore a second
error during cleanup caused by a first error, it is simpler if the
cleanup cannot set an error.  So, split out the error checking
portion (basically, input visitors checking for unvisited keys) into
a new function visit_check_struct(), which can be safely skipped if
any earlier errors are encountered, and leave the cleanup portion
(which never fails, but must be called unconditionally if
visit_start_struct() succeeded) in visit_end_struct().

Generated code in qapi-visit.c has diffs resembling:

|@@ -59,10 +59,12 @@ void visit_type_ACPIOSTInfo(Visitor *v,
|         goto out_obj;
|     }
|     visit_type_ACPIOSTInfo_members(v, obj, &err);
|-    error_propagate(errp, err);
|-    err = NULL;
|+    if (err) {
|+        goto out_obj;
|+    }
|+    visit_check_struct(v, &err);
| out_obj:
|-    visit_end_struct(v, &err);
|+    visit_end_struct(v);
| out:

and in qapi-event.c:

@@ -47,7 +47,10 @@ void qapi_event_send_acpi_device_ost(ACP
|         goto out;
|     }
|     visit_type_q_obj_ACPI_DEVICE_OST_arg_members(v, &param, &err);
|-    visit_end_struct(v, err ? NULL : &err);
|+    if (!err) {
|+        visit_check_struct(v, &err);
|+    }
|+    visit_end_struct(v);
|     if (err) {
|         goto out;

Signed-off-by: Eric Blake <eblake@redhat.com>
Message-Id: <1461879932-9020-20-git-send-email-eblake@redhat.com>
[Conflict with a doc fixup resolved]
Signed-off-by: Markus Armbruster <armbru@redhat.com>

5 files changed, 46 insertions, 21 deletions

diff --git a/qapi/opts-visitor.c b/qapi/opts-visitor.c
index 4cb6436a1f..ece0598e34 100644
--- a/qapi/opts-visitor.c
+++ b/qapi/opts-visitor.c
@@ -159,13 +159,13 @@ opts_start_struct(Visitor *v, const char *name, void **obj,
 
 
 static void
-opts_end_struct(Visitor *v, Error **errp)
+opts_check_struct(Visitor *v, Error **errp)
 {
     OptsVisitor *ov = to_ov(v);
     GHashTableIter iter;
     GQueue *any;
 
-    if (--ov->depth > 0) {
+    if (ov->depth > 0) {
         return;
     }
 
@@ -177,6 +177,18 @@ opts_end_struct(Visitor *v, Error **errp)
         first = g_queue_peek_head(any);
         error_setg(errp, QERR_INVALID_PARAMETER, first->name);
     }
+}
+
+
+static void
+opts_end_struct(Visitor *v)
+{
+    OptsVisitor *ov = to_ov(v);
+
+    if (--ov->depth > 0) {
+        return;
+    }
+
     g_hash_table_destroy(ov->unprocessed_opts);
     ov->unprocessed_opts = NULL;
     if (ov->fake_id_opt) {
@@ -516,6 +528,7 @@ opts_visitor_new(const QemuOpts *opts)
     ov->visitor.type = VISITOR_INPUT;
 
     ov->visitor.start_struct = &opts_start_struct;
+    ov->visitor.check_struct = &opts_check_struct;
     ov->visitor.end_struct   = &opts_end_struct;
 
     ov->visitor.start_list = &opts_start_list;
diff --git a/qapi/qapi-dealloc-visitor.c b/qapi/qapi-dealloc-visitor.c
index 413d525ad8..9005badb25 100644
--- a/qapi/qapi-dealloc-visitor.c
+++ b/qapi/qapi-dealloc-visitor.c
@@ -67,7 +67,7 @@ static void qapi_dealloc_start_struct(Visitor *v, const char *name, void **obj,
     qapi_dealloc_push(qov, obj);
 }
 
-static void qapi_dealloc_end_struct(Visitor *v, Error **errp)
+static void qapi_dealloc_end_struct(Visitor *v)
 {
     QapiDeallocVisitor *qov = to_qov(v);
     void **obj = qapi_dealloc_pop(qov);
diff --git a/qapi/qapi-visit-core.c b/qapi/qapi-visit-core.c
index fe6afe8ca8..58144f67b7 100644
--- a/qapi/qapi-visit-core.c
+++ b/qapi/qapi-visit-core.c
@@ -36,9 +36,16 @@ void visit_start_struct(Visitor *v, const char *name, void **obj,
     error_propagate(errp, err);
 }
 
-void visit_end_struct(Visitor *v, Error **errp)
+void visit_check_struct(Visitor *v, Error **errp)
 {
-    v->end_struct(v, errp);
+    if (v->check_struct) {
+        v->check_struct(v, errp);
+    }
+}
+
+void visit_end_struct(Visitor *v)
+{
+    v->end_struct(v);
 }
 
 void visit_start_list(Visitor *v, const char *name, Error **errp)
diff --git a/qapi/qmp-input-visitor.c b/qapi/qmp-input-visitor.c
index 30e7cb38f2..90b9df11c4 100644
--- a/qapi/qmp-input-visitor.c
+++ b/qapi/qmp-input-visitor.c
@@ -125,9 +125,11 @@ static void qmp_input_push(QmpInputVisitor *qiv, QObject *obj, Error **errp)
 }
 
 
-static void qmp_input_pop(QmpInputVisitor *qiv, Error **errp)
+static void qmp_input_check_struct(Visitor *v, Error **errp)
 {
+    QmpInputVisitor *qiv = to_qiv(v);
     StackObject *tos = &qiv->stack[qiv->nb_stack - 1];
+
     assert(qiv->nb_stack > 0);
 
     if (qiv->strict) {
@@ -140,6 +142,20 @@ static void qmp_input_pop(QmpInputVisitor *qiv, Error **errp)
             if (g_hash_table_iter_next(&iter, (void **)&key, NULL)) {
                 error_setg(errp, QERR_QMP_EXTRA_MEMBER, key);
             }
+        }
+    }
+}
+
+static void qmp_input_pop(Visitor *v)
+{
+    QmpInputVisitor *qiv = to_qiv(v);
+    StackObject *tos = &qiv->stack[qiv->nb_stack - 1];
+
+    assert(qiv->nb_stack > 0);
+
+    if (qiv->strict) {
+        GHashTable * const top_ht = qiv->stack[qiv->nb_stack - 1].h;
+        if (top_ht) {
             g_hash_table_unref(top_ht);
         }
         tos->h = NULL;
@@ -175,12 +191,6 @@ static void qmp_input_start_struct(Visitor *v, const char *name, void **obj,
     }
 }
 
-static void qmp_input_end_struct(Visitor *v, Error **errp)
-{
-    QmpInputVisitor *qiv = to_qiv(v);
-
-    qmp_input_pop(qiv, errp);
-}
 
 static void qmp_input_start_list(Visitor *v, const char *name, Error **errp)
 {
@@ -218,12 +228,6 @@ static GenericList *qmp_input_next_list(Visitor *v, GenericList **list,
     return entry;
 }
 
-static void qmp_input_end_list(Visitor *v)
-{
-    QmpInputVisitor *qiv = to_qiv(v);
-
-    qmp_input_pop(qiv, &error_abort);
-}
 
 static void qmp_input_start_alternate(Visitor *v, const char *name,
                                       GenericAlternate **obj, size_t size,
@@ -383,10 +387,11 @@ QmpInputVisitor *qmp_input_visitor_new(QObject *obj, bool strict)
 
     v->visitor.type = VISITOR_INPUT;
     v->visitor.start_struct = qmp_input_start_struct;
-    v->visitor.end_struct = qmp_input_end_struct;
+    v->visitor.check_struct = qmp_input_check_struct;
+    v->visitor.end_struct = qmp_input_pop;
     v->visitor.start_list = qmp_input_start_list;
     v->visitor.next_list = qmp_input_next_list;
-    v->visitor.end_list = qmp_input_end_list;
+    v->visitor.end_list = qmp_input_pop;
     v->visitor.start_alternate = qmp_input_start_alternate;
     v->visitor.type_int64 = qmp_input_type_int64;
     v->visitor.type_uint64 = qmp_input_type_uint64;
diff --git a/qapi/qmp-output-visitor.c b/qapi/qmp-output-visitor.c
index 4e21f1ddf0..232ad58221 100644
--- a/qapi/qmp-output-visitor.c
+++ b/qapi/qmp-output-visitor.c
@@ -111,7 +111,7 @@ static void qmp_output_start_struct(Visitor *v, const char *name, void **obj,
     qmp_output_push(qov, dict);
 }
 
-static void qmp_output_end_struct(Visitor *v, Error **errp)
+static void qmp_output_end_struct(Visitor *v)
 {
     QmpOutputVisitor *qov = to_qov(v);
     QObject *value = qmp_output_pop(qov);