qapi: Add new visit_complete() function

Making each output visitor provide its own output collection
function was the only remaining reason for exposing visitor
sub-types to the rest of the code base.  Add a polymorphic
visit_complete() function which is a no-op for input visitors,
and which populates an opaque pointer for output visitors.  For
maximum type-safety, also add a parameter to the output visitor
constructors with a type-correct version of the output pointer,
and assert that the two uses match.

This approach was considered superior to either passing the
output parameter only during construction (action at a distance
during visit_free() feels awkward) or only during visit_complete()
(defeating type safety makes it easier to use incorrectly).

Most callers were function-local, and therefore a mechanical
conversion; the testsuite was a bit trickier, but the previous
cleanup patch minimized the churn here.

The visit_complete() function may be called at most once; doing
so lets us use transfer semantics rather than duplication or
ref-count semantics to get the just-built output back to the
caller, even though it means our behavior is not idempotent.

Generated code is simplified as follows for events:

|@@ -26,7 +26,7 @@ void qapi_event_send_acpi_device_ost(ACP
|     QDict *qmp;
|     Error *err = NULL;
|     QMPEventFuncEmit emit;
|-    QmpOutputVisitor *qov;
|+    QObject *obj;
|     Visitor *v;
|     q_obj_ACPI_DEVICE_OST_arg param = {
|         info
|@@ -39,8 +39,7 @@ void qapi_event_send_acpi_device_ost(ACP
|
|     qmp = qmp_event_build_dict("ACPI_DEVICE_OST");
|
|-    qov = qmp_output_visitor_new();
|-    v = qmp_output_get_visitor(qov);
|+    v = qmp_output_visitor_new(&obj);
|
|     visit_start_struct(v, "ACPI_DEVICE_OST", NULL, 0, &err);
|     if (err) {
|@@ -55,7 +54,8 @@ void qapi_event_send_acpi_device_ost(ACP
|         goto out;
|     }
|
|-    qdict_put_obj(qmp, "data", qmp_output_get_qobject(qov));
|+    visit_complete(v, &obj);
|+    qdict_put_obj(qmp, "data", obj);
|     emit(QAPI_EVENT_ACPI_DEVICE_OST, qmp, &err);

and for commands:

| {
|     Error *err = NULL;
|-    QmpOutputVisitor *qov = qmp_output_visitor_new();
|     Visitor *v;
|
|-    v = qmp_output_get_visitor(qov);
|+    v = qmp_output_visitor_new(ret_out);
|     visit_type_AddfdInfo(v, "unused", &ret_in, &err);
|-    if (err) {
|-        goto out;
|+    if (!err) {
|+        visit_complete(v, ret_out);
|     }
|-    *ret_out = qmp_output_get_qobject(qov);
|-
|-out:
|     error_propagate(errp, err);

Signed-off-by: Eric Blake <eblake@redhat.com>
Message-Id: <1465490926-28625-13-git-send-email-eblake@redhat.com>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>

4 files changed, 21 insertions, 29 deletions

diff --git a/tests/check-qnull.c b/tests/check-qnull.c
index 6310bf7d41..dc906b116e 100644
--- a/tests/check-qnull.c
+++ b/tests/check-qnull.c
@@ -37,7 +37,6 @@ static void qnull_ref_test(void)
 static void qnull_visit_test(void)
 {
     QObject *obj;
-    QmpOutputVisitor *qov;
     Visitor *v;
 
     /*
@@ -53,12 +52,12 @@ static void qnull_visit_test(void)
     visit_type_null(v, NULL, &error_abort);
     visit_free(v);
 
-    qov = qmp_output_visitor_new();
-    visit_type_null(qmp_output_get_visitor(qov), NULL, &error_abort);
-    obj = qmp_output_get_qobject(qov);
+    v = qmp_output_visitor_new(&obj);
+    visit_type_null(v, NULL, &error_abort);
+    visit_complete(v, &obj);
     g_assert(obj == &qnull_);
     qobject_decref(obj);
-    visit_free(qmp_output_get_visitor(qov));
+    visit_free(v);
 
     g_assert(qnull_.refcnt == 1);
 }
diff --git a/tests/test-qmp-output-visitor.c b/tests/test-qmp-output-visitor.c
index af24b1087f..513d71f0d1 100644
--- a/tests/test-qmp-output-visitor.c
+++ b/tests/test-qmp-output-visitor.c
@@ -21,7 +21,6 @@
 #include "qapi/qmp/qjson.h"
 
 typedef struct TestOutputVisitorData {
-    QmpOutputVisitor *qov;
     Visitor *ov;
     QObject *obj;
 } TestOutputVisitorData;
@@ -29,18 +28,14 @@ typedef struct TestOutputVisitorData {
 static void visitor_output_setup(TestOutputVisitorData *data,
                                  const void *unused)
 {
-    data->qov = qmp_output_visitor_new();
-    g_assert(data->qov != NULL);
-
-    data->ov = qmp_output_get_visitor(data->qov);
-    g_assert(data->ov != NULL);
+    data->ov = qmp_output_visitor_new(&data->obj);
+    g_assert(data->ov);
 }
 
 static void visitor_output_teardown(TestOutputVisitorData *data,
                                     const void *unused)
 {
     visit_free(data->ov);
-    data->qov = NULL;
     data->ov = NULL;
     qobject_decref(data->obj);
     data->obj = NULL;
@@ -48,7 +43,7 @@ static void visitor_output_teardown(TestOutputVisitorData *data,
 
 static QObject *visitor_get(TestOutputVisitorData *data)
 {
-    data->obj = qmp_output_get_qobject(data->qov);
+    visit_complete(data->ov, &data->obj);
     g_assert(data->obj);
     return data->obj;
 }
diff --git a/tests/test-string-output-visitor.c b/tests/test-string-output-visitor.c
index e17035be5e..444844a15a 100644
--- a/tests/test-string-output-visitor.c
+++ b/tests/test-string-output-visitor.c
@@ -20,7 +20,6 @@
 #include "qapi/qmp/types.h"
 
 typedef struct TestOutputVisitorData {
-    StringOutputVisitor *sov;
     Visitor *ov;
     char *str;
     bool human;
@@ -30,9 +29,7 @@ static void visitor_output_setup_internal(TestOutputVisitorData *data,
                                           bool human)
 {
     data->human = human;
-    data->sov = string_output_visitor_new(human);
-    g_assert(data->sov);
-    data->ov = string_output_get_visitor(data->sov);
+    data->ov = string_output_visitor_new(human, &data->str);
     g_assert(data->ov);
 }
 
@@ -52,7 +49,6 @@ static void visitor_output_teardown(TestOutputVisitorData *data,
                                     const void *unused)
 {
     visit_free(data->ov);
-    data->sov = NULL;
     data->ov = NULL;
     g_free(data->str);
     data->str = NULL;
@@ -60,7 +56,7 @@ static void visitor_output_teardown(TestOutputVisitorData *data,
 
 static char *visitor_get(TestOutputVisitorData *data)
 {
-    data->str = string_output_get_string(data->sov);
+    visit_complete(data->ov, &data->str);
     g_assert(data->str);
     return data->str;
 }
diff --git a/tests/test-visitor-serialization.c b/tests/test-visitor-serialization.c
index 377ee3e788..dba4670762 100644
--- a/tests/test-visitor-serialization.c
+++ b/tests/test-visitor-serialization.c
@@ -1012,7 +1012,8 @@ static PrimitiveType pt_values[] = {
 /* visitor-specific op implementations */
 
 typedef struct QmpSerializeData {
-    QmpOutputVisitor *qov;
+    Visitor *qov;
+    QObject *obj;
     Visitor *qiv;
 } QmpSerializeData;
 
@@ -1021,8 +1022,8 @@ static void qmp_serialize(void *native_in, void **datap,
 {
     QmpSerializeData *d = g_malloc0(sizeof(*d));
 
-    d->qov = qmp_output_visitor_new();
-    visit(qmp_output_get_visitor(d->qov), &native_in, errp);
+    d->qov = qmp_output_visitor_new(&d->obj);
+    visit(d->qov, &native_in, errp);
     *datap = d;
 }
 
@@ -1033,7 +1034,8 @@ static void qmp_deserialize(void **native_out, void *datap,
     QString *output_json;
     QObject *obj_orig, *obj;
 
-    obj_orig = qmp_output_get_qobject(d->qov);
+    visit_complete(d->qov, &d->obj);
+    obj_orig = d->obj;
     output_json = qobject_to_json(obj_orig);
     obj = qobject_from_json(qstring_get_str(output_json));
 
@@ -1047,7 +1049,7 @@ static void qmp_deserialize(void **native_out, void *datap,
 static void qmp_cleanup(void *datap)
 {
     QmpSerializeData *d = datap;
-    visit_free(qmp_output_get_visitor(d->qov));
+    visit_free(d->qov);
     visit_free(d->qiv);
 
     g_free(d);
@@ -1055,7 +1057,7 @@ static void qmp_cleanup(void *datap)
 
 typedef struct StringSerializeData {
     char *string;
-    StringOutputVisitor *sov;
+    Visitor *sov;
     Visitor *siv;
 } StringSerializeData;
 
@@ -1064,8 +1066,8 @@ static void string_serialize(void *native_in, void **datap,
 {
     StringSerializeData *d = g_malloc0(sizeof(*d));
 
-    d->sov = string_output_visitor_new(false);
-    visit(string_output_get_visitor(d->sov), &native_in, errp);
+    d->sov = string_output_visitor_new(false, &d->string);
+    visit(d->sov, &native_in, errp);
     *datap = d;
 }
 
@@ -1074,7 +1076,7 @@ static void string_deserialize(void **native_out, void *datap,
 {
     StringSerializeData *d = datap;
 
-    d->string = string_output_get_string(d->sov);
+    visit_complete(d->sov, &d->string);
     d->siv = string_input_visitor_new(d->string);
     visit(d->siv, native_out, errp);
 }
@@ -1083,7 +1085,7 @@ static void string_cleanup(void *datap)
 {
     StringSerializeData *d = datap;
 
-    visit_free(string_output_get_visitor(d->sov));
+    visit_free(d->sov);
     visit_free(d->siv);
     g_free(d->string);
     g_free(d);