[Qemu-block] [PATCH v1 4/6] qapi: add a text output visitor for pretty printing types

[Daniel P. Berrange]

The current approach for pretty-printing QAPI types is to
convert them to JSON using the QMP output visitor and then
pretty-print the JSON document. This has an unfixable problem
that structs get their keys printed out in random order, since
JSON dicts do not contain any key ordering information.

To address this, introduce a text output visitor that can
directly pretty print a QAPI type into a string.

Signed-off-by: Daniel P. Berrange <address@hidden>
---
 include/qapi/text-output-visitor.h |  73 ++++++++++++
 include/qapi/visitor-impl.h        |   5 +-
 include/qapi/visitor.h             |   5 +-
 qapi/Makefile.objs                 |   1 +
 qapi/opts-visitor.c                |   5 +-
 qapi/qapi-dealloc-visitor.c        |   4 +-
 qapi/qapi-visit-core.c             |   9 +-
 qapi/qmp-input-visitor.c           |   5 +-
 qapi/qmp-output-visitor.c          |   4 +-
 qapi/string-input-visitor.c        |   5 +-
 qapi/string-output-visitor.c       |   5 +-
 qapi/text-output-visitor.c         | 235 +++++++++++++++++++++++++++++++++++++
 scripts/qapi-visit.py              |   5 +-
 13 files changed, 339 insertions(+), 22 deletions(-)
 create mode 100644 include/qapi/text-output-visitor.h
 create mode 100644 qapi/text-output-visitor.c

diff --git a/include/qapi/text-output-visitor.h 
b/include/qapi/text-output-visitor.h
new file mode 100644
index 0000000..3b742b7
--- /dev/null
+++ b/include/qapi/text-output-visitor.h
@@ -0,0 +1,73 @@
+/*
+ * Text pretty printing Visitor
+ *
+ * Copyright Red Hat, Inc. 2016
+ *
+ * Author: Daniel Berrange <address@hidden>
+ *
+ * This work is licensed under the terms of the GNU LGPL, version 2.1 or later.
+ * See the COPYING.LIB file in the top-level directory.
+ *
+ */
+
+#ifndef TEXT_OUTPUT_VISITOR_H
+#define TEXT_OUTPUT_VISITOR_H
+
+#include "qapi/visitor.h"
+
+typedef struct TextOutputVisitor TextOutputVisitor;
+
+/**
+ * text_output_visitor_new:
+ * @extraIndent: number of extra levels of indent to apply
+ * @skipLevel: skip output of nodes less than depth @skipLevel
+ *
+ * Create a new output visitor for displaying objects
+ * in a pretty-printed text format. The @extraIdent
+ * parameter can be used to add extra levels of whitespace
+ * indentation on the output text. If there are some nodes
+ * at the top level of the QAPI object that should not be
+ * displayed, the @skipLevel parameter can be set to a
+ * non-zero value to hide them
+ *
+ * Returns: a pointer to new output visitor
+ */
+TextOutputVisitor *text_output_visitor_new(int extraIndent,
+                                           int skipLevel);
+
+/**
+ * text_output_visitor_cleanup:
+ * @v: the output visitor object
+ *
+ * Release all resources associated with the output
+ * visitor
+ */
+void text_output_visitor_cleanup(TextOutputVisitor *v);
+
+/**
+ * text_output_get_string:
+ * @v: the output visitor object
+ *
+ * Get the string containing the pretty-printed
+ * text output of the object(s) that have been
+ * visited. The memory for the returned string
+ * should be released with g_free() when no longer
+ * required.
+ *
+ * Returns: a string containing output.
+ */
+char *text_output_get_string(TextOutputVisitor *v);
+
+/**
+ * text_output_get_visitor
+ * @v: the output visitor object
+ *
+ * Get the visitor instance to be used to visit
+ * QAPI objects. The returned instance should not
+ * be freed by the callers.
+ *
+ * Returns: a visitor instance
+ */
+Visitor *text_output_get_visitor(TextOutputVisitor *v);
+
+#endif
diff --git a/include/qapi/visitor-impl.h b/include/qapi/visitor-impl.h
index 145afd0..23cc6aa 100644
--- a/include/qapi/visitor-impl.h
+++ b/include/qapi/visitor-impl.h
@@ -52,10 +52,11 @@ struct Visitor
     /* Must be set; implementations may require @list to be non-null,
      * but must document it. */
     void (*start_list)(Visitor *v, const char *name, GenericList **list,
-                       size_t size, Error **errp);
+                       size_t size, bool hasElements, Error **errp);
 
     /* Must be set */
-    GenericList *(*next_list)(Visitor *v, GenericList *tail, size_t size);
+    GenericList *(*next_list)(Visitor *v, GenericList *tail, size_t size,
+                              size_t element);
 
     /* Must be set */
     void (*end_list)(Visitor *v);
diff --git a/include/qapi/visitor.h b/include/qapi/visitor.h
index 4d12167..c8c39d7 100644
--- a/include/qapi/visitor.h
+++ b/include/qapi/visitor.h
@@ -303,7 +303,7 @@ void visit_end_struct(Visitor *v);
  * even if intermediate visits fail.  See the examples above.
  */
 void visit_start_list(Visitor *v, const char *name, GenericList **list,
-                      size_t size, Error **errp);
+                      size_t size, bool hasElements, Error **errp);
 
 /*
  * Iterate over a GenericList during a non-virtual list visit.
@@ -319,7 +319,8 @@ void visit_start_list(Visitor *v, const char *name, 
GenericList **list,
  * the list, with that function's name parameter set to NULL and obj
  * set to the address of @tail->value.
  */
-GenericList *visit_next_list(Visitor *v, GenericList *tail, size_t size);
+GenericList *visit_next_list(Visitor *v, GenericList *tail, size_t size,
+                             size_t element);
 
 /*
  * Complete a list visit started earlier.
diff --git a/qapi/Makefile.objs b/qapi/Makefile.objs
index 2278970..7caf64b 100644
--- a/qapi/Makefile.objs
+++ b/qapi/Makefile.objs
@@ -1,6 +1,7 @@
 util-obj-y = qapi-visit-core.o qapi-dealloc-visitor.o qmp-input-visitor.o
 util-obj-y += qmp-output-visitor.o qmp-registry.o qmp-dispatch.o
 util-obj-y += string-input-visitor.o string-output-visitor.o
+util-obj-y += text-output-visitor.o
 util-obj-y += opts-visitor.o
 util-obj-y += qmp-event.o
 util-obj-y += qapi-util.o
diff --git a/qapi/opts-visitor.c b/qapi/opts-visitor.c
index 4cf1cf8..8c59597 100644
--- a/qapi/opts-visitor.c
+++ b/qapi/opts-visitor.c
@@ -214,7 +214,7 @@ lookup_distinct(const OptsVisitor *ov, const char *name, 
Error **errp)
 
 static void
 opts_start_list(Visitor *v, const char *name, GenericList **list, size_t size,
-                Error **errp)
+                bool hasElements, Error **errp)
 {
     OptsVisitor *ov = to_ov(v);
 
@@ -233,7 +233,8 @@ opts_start_list(Visitor *v, const char *name, GenericList 
**list, size_t size,
 
 
 static GenericList *
-opts_next_list(Visitor *v, GenericList *tail, size_t size)
+opts_next_list(Visitor *v, GenericList *tail,
+               size_t size, size_t element)
 {
     OptsVisitor *ov = to_ov(v);
 
diff --git a/qapi/qapi-dealloc-visitor.c b/qapi/qapi-dealloc-visitor.c
index cd68b55..025b7d3 100644
--- a/qapi/qapi-dealloc-visitor.c
+++ b/qapi/qapi-dealloc-visitor.c
@@ -90,12 +90,12 @@ static void qapi_dealloc_end_alternate(Visitor *v)
 
 static void qapi_dealloc_start_list(Visitor *v, const char *name,
                                     GenericList **list, size_t size,
-                                    Error **errp)
+                                    bool hasElements, Error **errp)
 {
 }
 
 static GenericList *qapi_dealloc_next_list(Visitor *v, GenericList *tail,
-                                           size_t size)
+                                           size_t size, size_t element)
 {
     GenericList *next = tail->next;
     g_free(tail);
diff --git a/qapi/qapi-visit-core.c b/qapi/qapi-visit-core.c
index 3b5efbe..a2f31d5 100644
--- a/qapi/qapi-visit-core.c
+++ b/qapi/qapi-visit-core.c
@@ -51,24 +51,25 @@ void visit_end_struct(Visitor *v)
 }
 
 void visit_start_list(Visitor *v, const char *name, GenericList **list,
-                      size_t size, Error **errp)
+                      size_t size, bool hasElements, Error **errp)
 {
     Error *err = NULL;
 
     assert(!list || size >= sizeof(GenericList));
     assert(v->start_list != NULL);
-    v->start_list(v, name, list, size, &err);
+    v->start_list(v, name, list, size, hasElements, &err);
     if (list && v->type == VISITOR_INPUT) {
         assert(!(err && *list));
     }
     error_propagate(errp, err);
 }
 
-GenericList *visit_next_list(Visitor *v, GenericList *tail, size_t size)
+GenericList *visit_next_list(Visitor *v, GenericList *tail,
+                             size_t size, size_t element)
 {
     assert(tail && size >= sizeof(GenericList));
     assert(v->next_list != NULL);
-    return v->next_list(v, tail, size);
+    return v->next_list(v, tail, size, element);
 }
 
 void visit_end_list(Visitor *v)
diff --git a/qapi/qmp-input-visitor.c b/qapi/qmp-input-visitor.c
index aea90a1..2509af4 100644
--- a/qapi/qmp-input-visitor.c
+++ b/qapi/qmp-input-visitor.c
@@ -192,7 +192,8 @@ static void qmp_input_start_struct(Visitor *v, const char 
*name, void **obj,
 
 
 static void qmp_input_start_list(Visitor *v, const char *name,
-                                 GenericList **list, size_t size, Error **errp)
+                                 GenericList **list, size_t size,
+                                 bool hasElements, Error **errp)
 {
     QmpInputVisitor *qiv = to_qiv(v);
     QObject *qobj = qmp_input_get_object(qiv, name, true);
@@ -218,7 +219,7 @@ static void qmp_input_start_list(Visitor *v, const char 
*name,
 }
 
 static GenericList *qmp_input_next_list(Visitor *v, GenericList *tail,
-                                        size_t size)
+                                        size_t size, size_t element)
 {
     QmpInputVisitor *qiv = to_qiv(v);
     StackObject *so = &qiv->stack[qiv->nb_stack - 1];
diff --git a/qapi/qmp-output-visitor.c b/qapi/qmp-output-visitor.c
index 4d3cf78..395a8a3 100644
--- a/qapi/qmp-output-visitor.c
+++ b/qapi/qmp-output-visitor.c
@@ -116,7 +116,7 @@ static void qmp_output_end_struct(Visitor *v)
 
 static void qmp_output_start_list(Visitor *v, const char *name,
                                   GenericList **listp, size_t size,
-                                  Error **errp)
+                                  bool hasElements, Error **errp)
 {
     QmpOutputVisitor *qov = to_qov(v);
     QList *list = qlist_new();
@@ -126,7 +126,7 @@ static void qmp_output_start_list(Visitor *v, const char 
*name,
 }
 
 static GenericList *qmp_output_next_list(Visitor *v, GenericList *tail,
-                                         size_t size)
+                                         size_t size, size_t element)
 {
     return tail->next;
 }
diff --git a/qapi/string-input-visitor.c b/qapi/string-input-visitor.c
index 30b5879..ab804c8 100644
--- a/qapi/string-input-visitor.c
+++ b/qapi/string-input-visitor.c
@@ -127,7 +127,7 @@ error:
 
 static void
 start_list(Visitor *v, const char *name, GenericList **list, size_t size,
-           Error **errp)
+           bool hasElements, Error **errp)
 {
     StringInputVisitor *siv = to_siv(v);
 
@@ -151,7 +151,8 @@ start_list(Visitor *v, const char *name, GenericList 
**list, size_t size,
     }
 }
 
-static GenericList *next_list(Visitor *v, GenericList *tail, size_t size)
+static GenericList *next_list(Visitor *v, GenericList *tail,
+                              size_t size, size_t element)
 {
     StringInputVisitor *siv = to_siv(v);
     Range *r;
diff --git a/qapi/string-output-visitor.c b/qapi/string-output-visitor.c
index d013196..a645a47 100644
--- a/qapi/string-output-visitor.c
+++ b/qapi/string-output-visitor.c
@@ -266,7 +266,7 @@ static void print_type_number(Visitor *v, const char *name, 
double *obj,
 
 static void
 start_list(Visitor *v, const char *name, GenericList **list, size_t size,
-           Error **errp)
+           bool hasElements, Error **errp)
 {
     StringOutputVisitor *sov = to_sov(v);
 
@@ -280,7 +280,8 @@ start_list(Visitor *v, const char *name, GenericList 
**list, size_t size,
     }
 }
 
-static GenericList *next_list(Visitor *v, GenericList *tail, size_t size)
+static GenericList *next_list(Visitor *v, GenericList *tail,
+                              size_t size, size_t element)
 {
     StringOutputVisitor *sov = to_sov(v);
     GenericList *ret = tail->next;
diff --git a/qapi/text-output-visitor.c b/qapi/text-output-visitor.c
new file mode 100644
index 0000000..35afe58
--- /dev/null
+++ b/qapi/text-output-visitor.c
@@ -0,0 +1,235 @@
+/*
+ * Text pretty printing Visitor
+ *
+ * Copyright Red Hat, Inc. 2016
+ *
+ * Author: Daniel Berrange <address@hidden>
+ *
+ * This work is licensed under the terms of the GNU LGPL, version 2.1 or later.
+ * See the COPYING.LIB file in the top-level directory.
+ *
+ */
+
+#include "qemu/osdep.h"
+#include "qemu-common.h"
+#include "qapi/text-output-visitor.h"
+#include "qapi/visitor-impl.h"
+#include <math.h>
+
+struct TextOutputVisitor {
+    Visitor visitor;
+    GString *string;
+    int level;
+    int skipLevel;
+    int extraIndent;
+};
+
+static TextOutputVisitor *to_sov(Visitor *v)
+{
+    return container_of(v, TextOutputVisitor, visitor);
+}
+
+
+#define INDENT (sov->extraIndent + ((sov->level - sov->skipLevel) * 4))
+
+static void print_type_int64(Visitor *v, const char *name, int64_t *obj,
+                             Error **errp)
+{
+    TextOutputVisitor *sov = to_sov(v);
+
+    if (sov->level < sov->skipLevel) {
+        return;
+    }
+    g_string_append_printf(sov->string,
+                           "%*s%s: %" PRIu64 "\n",
+                           INDENT, "",
+                           name, *obj);
+}
+
+static void print_type_uint64(Visitor *v, const char *name, uint64_t *obj,
+                             Error **errp)
+{
+    TextOutputVisitor *sov = to_sov(v);
+
+    if (sov->level < sov->skipLevel) {
+        return;
+    }
+    g_string_append_printf(sov->string,
+                           "%*s%s: %" PRIi64 "\n",
+                           INDENT, "",
+                           name, *obj);
+}
+
+static void print_type_size(Visitor *v, const char *name, uint64_t *obj,
+                            Error **errp)
+{
+    TextOutputVisitor *sov = to_sov(v);
+    static const char suffixes[] = { 'B', 'K', 'M', 'G', 'T', 'P', 'E' };
+    uint64_t div, val;
+    int i;
+
+    if (sov->level < sov->skipLevel) {
+        return;
+    }
+
+    val = *obj;
+
+    /* The exponent (returned in i) minus one gives us
+     * floor(log2(val * 1024 / 1000).  The correction makes us
+     * switch to the higher power when the integer part is >= 1000.
+     */
+    frexp(val / (1000.0 / 1024.0), &i);
+    i = (i - 1) / 10;
+    assert(i < ARRAY_SIZE(suffixes));
+    div = 1ULL << (i * 10);
+
+    g_string_append_printf(sov->string,
+                           "%*s%s: %" PRIu64 " (%0.3g %c%s)\n",
+                           INDENT, "",
+                           name, val,
+                           (double)val / div, suffixes[i], i ? "iB" : "");
+}
+
+static void print_type_bool(Visitor *v, const char *name, bool *obj,
+                            Error **errp)
+{
+    TextOutputVisitor *sov = to_sov(v);
+
+    if (sov->level < sov->skipLevel) {
+        return;
+    }
+    g_string_append_printf(sov->string,
+                           "%*s%s: %s\n",
+                           INDENT, "",
+                           name, *obj ? "true" : "false");
+}
+
+static void print_type_str(Visitor *v, const char *name, char **obj,
+                           Error **errp)
+{
+    TextOutputVisitor *sov = to_sov(v);
+
+    if (sov->level < sov->skipLevel) {
+        return;
+    }
+    g_string_append_printf(sov->string,
+                           "%*s%s: %s\n",
+                           INDENT, "",
+                           name, *obj ? *obj : "<null>");
+}
+
+static void print_type_number(Visitor *v, const char *name, double *obj,
+                              Error **errp)
+{
+    TextOutputVisitor *sov = to_sov(v);
+
+    if (sov->level < sov->skipLevel) {
+        return;
+    }
+    g_string_append_printf(sov->string,
+                           "%*s%s: %f\n",
+                           INDENT, "",
+                           name, *obj);
+}
+
+static void
+start_list(Visitor *v, const char *name, GenericList **list, size_t size,
+           bool hasElements, Error **errp)
+{
+    TextOutputVisitor *sov = to_sov(v);
+
+    if (sov->level >= sov->skipLevel) {
+        g_string_append_printf(sov->string,
+                               "%*s%s:\n",
+                               INDENT, "",
+                               name);
+    }
+    sov->level++;
+    if (hasElements) {
+        if (sov->level >= sov->skipLevel) {
+            g_string_append_printf(sov->string,
+                                   "%*s[0]:\n",
+                                   INDENT, "");
+        }
+    }
+}
+
+static GenericList *next_list(Visitor *v, GenericList *tail,
+                              size_t size, size_t element)
+{
+    TextOutputVisitor *sov = to_sov(v);
+    GenericList *ret = tail->next;
+    if (ret && sov->level >= sov->skipLevel) {
+        g_string_append_printf(sov->string,
+                               "%*s[%zu]:\n",
+                               INDENT, "", element);
+    }
+    return ret;
+}
+
+static void end_list(Visitor *v)
+{
+    TextOutputVisitor *sov = to_sov(v);
+    sov->level--;
+}
+
+static void start_struct(Visitor *v, const char *name, void **obj,
+                         size_t size, Error **errp)
+{
+    TextOutputVisitor *sov = to_sov(v);
+    sov->level++;
+}
+
+
+static void end_struct(Visitor *v)
+{
+    TextOutputVisitor *sov = to_sov(v);
+    sov->level--;
+}
+
+
+char *text_output_get_string(TextOutputVisitor *sov)
+{
+    char *string = g_string_free(sov->string, false);
+    sov->string = NULL;
+    return string;
+}
+
+Visitor *text_output_get_visitor(TextOutputVisitor *sov)
+{
+    return &sov->visitor;
+}
+
+void text_output_visitor_cleanup(TextOutputVisitor *sov)
+{
+    if (sov->string) {
+        g_string_free(sov->string, true);
+    }
+    g_free(sov);
+}
+
+TextOutputVisitor *text_output_visitor_new(int extraIndent,
+                                           int skipLevel)
+{
+    TextOutputVisitor *v;
+
+    v = g_malloc0(sizeof(*v));
+
+    v->extraIndent = extraIndent;
+    v->skipLevel = skipLevel;
+    v->string = g_string_new(NULL);
+    v->visitor.type = VISITOR_OUTPUT;
+    v->visitor.type_int64 = print_type_int64;
+    v->visitor.type_uint64 = print_type_uint64;
+    v->visitor.type_size = print_type_size;
+    v->visitor.type_bool = print_type_bool;
+    v->visitor.type_str = print_type_str;
+    v->visitor.type_number = print_type_number;
+    v->visitor.start_list = start_list;
+    v->visitor.next_list = next_list;
+    v->visitor.end_list = end_list;
+    v->visitor.start_struct = start_struct;
+    v->visitor.end_struct = end_struct;
+
+    return v;
+}
diff --git a/scripts/qapi-visit.py b/scripts/qapi-visit.py
index 70ea8ca..72efab8 100644
--- a/scripts/qapi-visit.py
+++ b/scripts/qapi-visit.py
@@ -115,14 +115,15 @@ void visit_type_%(c_name)s(Visitor *v, const char *name, 
%(c_name)s **obj, Error
     Error *err = NULL;
     %(c_name)s *tail;
     size_t size = sizeof(**obj);
+    size_t element = 0;
 
-    visit_start_list(v, name, (GenericList **)obj, size, &err);
+    visit_start_list(v, name, (GenericList **)obj, size, *obj != NULL, &err);
     if (err) {
         goto out;
     }
 
     for (tail = *obj; tail;
-         tail = (%(c_name)s *)visit_next_list(v, (GenericList *)tail, size)) {
+         tail = (%(c_name)s *)visit_next_list(v, (GenericList *)tail, size, 
++element)) {
         visit_type_%(c_elt_type)s(v, NULL, &tail->value, &err);
         if (err) {
             break;
-- 
2.5.5