qapi: Add new visit_free() function

Making each visitor provide its own (awkwardly-named) FOO_cleanup()
is unusual, when we can instead have a polymorphic visit_free()
interface.  Over the next few patches, we can use the polymorphic
functions to eliminate the need for a FOO_get_visitor() function
for accessing specific visitor functionality, once everything can
be accessed directly through the Visitor* interfaces.

The dealloc visitor is the first one converted to completely use
the new entry point, since qapi_dealloc_visitor_cleanup() was the
only reason that qapi_dealloc_get_visitor() existed, and only
generated and testsuite code was even using it.  With the new
visit_free() entry point in place, we no longer need to expose
the QapiDeallocVisitor subtype through qapi_dealloc_visitor_new(),
and can get by with less generated code, with diffs that look like:

| void qapi_free_ACPIOSTInfo(ACPIOSTInfo *obj)
| {
|-    QapiDeallocVisitor *qdv;
|     Visitor *v;
|
|     if (!obj) {
|         return;
|     }
|
|-    qdv = qapi_dealloc_visitor_new();
|-    v = qapi_dealloc_get_visitor(qdv);
|+    v = qapi_dealloc_visitor_new();
|     visit_type_ACPIOSTInfo(v, NULL, &obj, NULL);
|-    qapi_dealloc_visitor_cleanup(qdv);
|+    visit_free(v);
|}

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

3 files changed, 38 insertions, 7 deletions

diff --git a/include/qapi/dealloc-visitor.h b/include/qapi/dealloc-visitor.h
index 45b06b248c..b3e5c85fd8 100644
--- a/include/qapi/dealloc-visitor.h
+++ b/include/qapi/dealloc-visitor.h
@@ -23,9 +23,6 @@ typedef struct QapiDeallocVisitor QapiDeallocVisitor;
  * qapi_free_FOO() functions, and is the only visitor designed to work
  * correctly in the face of a partially-constructed QAPI tree.
  */
-QapiDeallocVisitor *qapi_dealloc_visitor_new(void);
-void qapi_dealloc_visitor_cleanup(QapiDeallocVisitor *d);
-
-Visitor *qapi_dealloc_get_visitor(QapiDeallocVisitor *v);
+Visitor *qapi_dealloc_visitor_new(void);
 
 #endif
diff --git a/include/qapi/visitor-impl.h b/include/qapi/visitor-impl.h
index a495bf0937..525b06872b 100644
--- a/include/qapi/visitor-impl.h
+++ b/include/qapi/visitor-impl.h
@@ -104,6 +104,9 @@ struct Visitor
 
     /* Must be set */
     VisitorType type;
+
+    /* Must be set */
+    void (*free)(Visitor *v);
 };
 
 #endif
diff --git a/include/qapi/visitor.h b/include/qapi/visitor.h
index 25d0cc275a..2ded852307 100644
--- a/include/qapi/visitor.h
+++ b/include/qapi/visitor.h
@@ -37,6 +37,24 @@
  * implemented by each visitor, and docs/qapi-code-gen.txt for more
  * about the QAPI code generator.
  *
+ * All of the visitors are created via:
+ *
+ * Type *subtype_visitor_new(parameters...);
+ *
+ * where Type is either directly 'Visitor *', or is a subtype that can
+ * be trivially upcast to Visitor * via another function:
+ *
+ * Visitor *subtype_get_visitor(SubtypeVisitor *);
+ *
+ * A visitor should be used for exactly one top-level visit_type_FOO()
+ * or virtual walk, then passed to visit_free() to clean up resources.
+ * It is okay to free the visitor without completing the visit, if
+ * some other error is detected in the meantime.  Output visitors
+ * provide an additional function, for collecting the final results of
+ * a successful visit: string_output_get_string() and
+ * qmp_output_get_qobject(); this collection function should not be
+ * called if any errors were reported during the visit.
+ *
  * All QAPI types have a corresponding function with a signature
  * roughly compatible with this:
  *
@@ -222,6 +240,19 @@ typedef struct GenericAlternate {
     char padding[];
 } GenericAlternate;
 
+/*** Visitor cleanup ***/
+
+/*
+ * Free @v and any resources it has tied up.
+ *
+ * May be called whether or not the visit has been successfully
+ * completed, but should not be called until a top-level
+ * visit_type_FOO() or visit_start_ITEM() has been performed on the
+ * visitor.  Safe if @v is NULL.
+ */
+void visit_free(Visitor *v);
+
+
 /*** Visiting structures ***/
 
 /*
@@ -272,7 +303,7 @@ void visit_check_struct(Visitor *v, Error **errp);
  * Must be called after any successful use of visit_start_struct(),
  * even if intermediate processing was skipped due to errors, to allow
  * the backend to release any resources.  Destroying the visitor early
- * behaves as if this was implicitly called.
+ * with visit_free() behaves as if this was implicitly called.
  */
 void visit_end_struct(Visitor *v, void **obj);
 
@@ -332,7 +363,7 @@ GenericList *visit_next_list(Visitor *v, GenericList *tail, size_t size);
  * Must be called after any successful use of visit_start_list(), even
  * if intermediate processing was skipped due to errors, to allow the
  * backend to release any resources.  Destroying the visitor early
- * behaves as if this was implicitly called.
+ * with visit_free() behaves as if this was implicitly called.
  */
 void visit_end_list(Visitor *v, void **list);
 
@@ -368,7 +399,7 @@ void visit_start_alternate(Visitor *v, const char *name,
  * Must be called after any successful use of visit_start_alternate(),
  * even if intermediate processing was skipped due to errors, to allow
  * the backend to release any resources.  Destroying the visitor early
- * behaves as if this was implicitly called.
+ * with visit_free() behaves as if this was implicitly called.
  *
  */
 void visit_end_alternate(Visitor *v, void **obj);