On Tue, Mar 04, 2014 at 10:03:49AM +0100, Paolo Bonzini wrote:
> Il 04/03/2014 08:28, Hu Tao ha scritto:
> >From: Hu Tao <hu.t...@gmail.com>
> >
> >Signed-off-by: Hu Tao <hu...@cn.fujitsu.com>
> >---
> > qapi/string-output-visitor.c | 156
> > +++++++++++++++++++++++++++++++++++--
> > tests/test-string-output-visitor.c | 26 +++++++
> > 2 files changed, 177 insertions(+), 5 deletions(-)
> >
> >diff --git a/qapi/string-output-visitor.c b/qapi/string-output-visitor.c
> >index fb1d2e8..bc9bb36 100644
> >--- a/qapi/string-output-visitor.c
> >+++ b/qapi/string-output-visitor.c
> >@@ -17,11 +17,57 @@
> > #include "qemu/host-utils.h"
> > #include <math.h>
> >
> >+enum ListMode {
> >+ LM_NONE, /* not traversing a list of repeated options */
> >+ LM_STARTED, /* start_list() succeeded */
> >+
> >+ LM_IN_PROGRESS, /* next_list() has been called.
> >+ *
> >+ * Generating the next list link will consume the
> >most
> >+ * recently parsed QemuOpt instance of the repeated
> >+ * option.
> >+ *
> >+ * Parsing a value into the list link will examine
> >the
> >+ * next QemuOpt instance of the repeated option,
> >and
> >+ * possibly enter LM_SIGNED_INTERVAL or
> >+ * LM_UNSIGNED_INTERVAL.
> >+ */
> >+
> >+ LM_SIGNED_INTERVAL, /* next_list() has been called.
> >+ *
> >+ * Generating the next list link will consume the
> >most
> >+ * recently stored element from the signed
> >interval,
> >+ * parsed from the most recent QemuOpt instance of
> >the
> >+ * repeated option. This may consume QemuOpt itself
> >+ * and return to LM_IN_PROGRESS.
> >+ *
> >+ * Parsing a value into the list link will store
> >the
> >+ * next element of the signed interval.
> >+ */
> >+
> >+ LM_UNSIGNED_INTERVAL,/* Same as above, only for an unsigned interval. */
> >+
> >+ LM_END
> >+};
> >+
> >+typedef enum ListMode ListMode;
> >+
> > struct StringOutputVisitor
> > {
> > Visitor visitor;
> > bool human;
> >+ bool head;
> > char *string;
> >+ ListMode list_mode;
> >+ /* When parsing a list of repeating options as integers, values of the
> >form
> >+ * "a-b", representing a closed interval, are allowed. Elements in the
> >+ * range are generated individually.
> >+ */
> >+ union {
> >+ int64_t s;
> >+ uint64_t u;
> >+ } range_start, range_end;
> >+
> > };
> >
> > static void string_output_set(StringOutputVisitor *sov, char *string)
> >@@ -34,13 +80,60 @@ static void print_type_int(Visitor *v, int64_t *obj,
> >const char *name,
> > Error **errp)
> > {
> > StringOutputVisitor *sov = DO_UPCAST(StringOutputVisitor, visitor, v);
> >- char *out;
> >+ char *out = NULL;
> >
> >- if (sov->human) {
> >- out = g_strdup_printf("%lld (%#llx)", (long long) *obj, (long long)
> >*obj);
> >- } else {
> >- out = g_strdup_printf("%lld", (long long) *obj);
> >+ switch (sov->list_mode) {
> >+ case LM_NONE:
> >+ if (sov->human) {
> >+ out = g_strdup_printf("%lld (%#llx)", (long long) *obj,
> >+ (long long) *obj);
> >+ } else {
> >+ out = g_strdup_printf("%lld", (long long) *obj);
> >+ }
> >+ sov->list_mode = LM_END;
> >+ break;
> >+
> >+ case LM_STARTED:
> >+ sov->range_start.s = *obj;
> >+ sov->range_end.s = *obj;
> >+ sov->list_mode = LM_IN_PROGRESS;
> >+ break;
> >+
> >+ case LM_IN_PROGRESS:
> >+ assert(sov->range_end.s + 1 == *obj);
> >+ sov->range_end.s++;
> >+ break;
> >+
> >+ case LM_END:
> >+ assert(sov->range_end.s + 1 == *obj);
> >+ sov->range_end.s++;
> >+ if (sov->range_end.s == sov->range_start.s) {
> >+ if (sov->human) {
> >+ out = g_strdup_printf("%lld (%#llx)",
> >+ (long long)sov->range_start.s,
> >+ (long long)sov->range_start.s);
> >+ } else {
> >+ out = g_strdup_printf("%lld", (long
> >long)sov->range_start.s);
> >+ }
> >+ } else {
> >+ if (sov->human) {
> >+ out = g_strdup_printf("%lld(%#llx)-%lld(%#llx)",
> >+ (long long) sov->range_start.s,
> >+ (long long) sov->range_start.s,
> >+ (long long) sov->range_end.s,
> >+ (long long) sov->range_end.s);
>
> Perhaps "10-15 (0xa-0xf)"?
Looks better.
>
> >+ } else {
> >+ out = g_strdup_printf("%lld-%lld",
> >+ (long long) sov->range_start.s,
> >+ (long long) sov->range_end.s);
> >+ }
> >+ }
>
> This looks wrong. You do not insert any separator, and you do not
> handle things like "0-3,8-11". You probably should use a GString
> instead of string_output_set.
Right. We should also handle "0-3,8-11"-like lists in string input
visitor and opts visitor.
