[Qemu-devel] [PATCH v19 07/11] qapi: make string output visitor parse int list

2014-03-04 Thread Hu Tao
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);
+} else {
+out = g_strdup_printf(%lld-%lld,
+  (long long) sov-range_start.s,
+  (long long) sov-range_end.s);
+}
+}
+break;
+
+default:
+abort();
 }
+
 string_output_set(sov, out);
 }
 
@@ -103,6 +196,56 @@ static void print_type_number(Visitor *v, double *obj, 
const char *name,
 string_output_set(sov, g_strdup_printf(%f, *obj));
 }
 
+static void

Re: [Qemu-devel] [PATCH v19 07/11] qapi: make string output visitor parse int list

2014-03-04 Thread Paolo Bonzini

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)?


+} 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.


Paolo


+break;
+
+default:
+

Re: [Qemu-devel] [PATCH v19 07/11] qapi: make string output visitor parse int list

2014-03-04 Thread Hu Tao
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);
 +}
 

Re: [Qemu-devel] [PATCH v19 07/11] qapi: make string output visitor parse int list

2014-03-04 Thread Paolo Bonzini

Il 04/03/2014 10:23, Hu Tao ha scritto:



 +} 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.


Ok, no big deal; let's do one thing at a time.

Regarding string input visitor, I believe we can avoid the code 
duplication by:


(1) using Qmp input/output visitors in query-memdev

(2) using OptsVisitor in -object


The string output visitor becomes more important than the string input 
visitor, but it is only needed for HMP so it is not urgent.


We can then work on making OptsVisitor use the string input visitor 
internally.


But I'm already applying patches 1-5, 8, 9, 10 (the latter with the 
above changes) to the numa tree.


Paolo