[Qemu-devel] [PATCH 0/8] OptsVisitor: support / flatten integer ranges for repeating options

[Qemu-devel] [PATCH 0/8] OptsVisitor: support / flatten integer ranges for repeating options

[Laszlo Ersek]

rfc->v1:
- addressed Paolo's comments for patches 1 and 2,
- patches 7 and 8 are new (unit tests),
- updated the cover letter to take native lists into account, plus
  cleaned it up.

Consider the following QAPI schema fragment, for the purpose of command
line parsing with OptsVisitor:

  { 'union': 'NumaOptions',
    'data': {
      'node': 'NumaNodeOptions',
      'mem' : 'NumaMemOptions' }}

  { 'type': 'NumaNodeOptions',
    'data': {
      '*nodeid': 'int',
      '*cpus'  : ['uint16'] }}

  { 'type': 'NumaMemOptions',
    'data': {
      '*nodeid': 'int',
      '*size'  : 'size' }}

(Commit eb7ee2cb ("qapi: introduce OptsVisitor") had originally
documented OptsVisitor's general schema requirements for parsing
repeated options such that the list element type had to be a struct with
one mandatory scalar field. Accordingly, the RFC version of this series
required for interval flattening that this underlying scalar type be an
integer type. However, since commit a678e26c ("qapi: pad GenericList
value fields to 64 bits") we've had reliable native lists; OptsVisitor
turns out to support them automatically.)

OptsVisitor already accepts the following command line with the above
schema:

  -numa node,nodeid=3,cpus=0,cpus=1,cpus=2,cpus=6,cpus=7,cpus=8

Paolo suggested in
<http://thread.gmane.org/gmane.comp.emulators.qemu/222589/focus=222732>
that OptsVisitor should allow the following shortcut:

  -numa node,nodeid=3,cpus=0-2,cpus=6-8

and that the code processing the "cpus" list should encounter all six
elements (0, 1, 2, 6, 7, 8) individually.

The series implements this feature. Both signed and unsigned values and
intervals are supported in general:

  * 0     (zero)
  * 1-5   (one to five)
  * 4-4   (four to four, range with one element)
  * -2    (minus two)
  * -5-8  (minus five to plus eight)
  * -9--6 (minus nine to minus six)

The restrictions imposed by the native list element's signedness and
size (in the above schema example, 'uint16') are enforced element-wise
as usual. That is, for 'uint16', the command line option

  -numa node,nodeid=3,cpus=65534-65537

is equivalent to

  -numa node,nodeid=3,cpus=65534,cpus=65535,cpus=65536,cpus=65537

and visit_type_uint16() [qapi/qapi-visit-core.c] will catch the first
element (= 65536) that has been parsed by opts_type_int() but cannot be
represented as 'uint16'.

Laszlo Ersek (8):
  OptsVisitor: introduce basic list modes
  OptsVisitor: introduce list modes for interval flattening
  OptsVisitor: opts_type_int(): recognize intervals when LM_IN_PROGRESS
  OptsVisitor: rebase opts_type_uint64() to parse_uint_full()
  OptsVisitor: opts_type_uint64(): recognize intervals when
    LM_IN_PROGRESS
  OptsVisitor: don't try to flatten overlong integer ranges
  add "test-int128" to .gitignore
  OptsVisitor: introduce unit tests, with test cases for range
    flattening

 tests/Makefile              |    6 +-
 qapi-schema-test.json       |   15 +++
 include/qapi/opts-visitor.h |    6 +
 qapi/opts-visitor.c         |  184 ++++++++++++++++++++++++-----
 tests/test-opts-visitor.c   |  275 +++++++++++++++++++++++++++++++++++++++++++
 .gitignore                  |    2 +
 6 files changed, 456 insertions(+), 32 deletions(-)
 create mode 100644 tests/test-opts-visitor.c

[Qemu-devel] [PATCH 1/8] OptsVisitor: introduce basic list modes

[Laszlo Ersek]

We're going to need more state while processing a list of repeated
options. This change eliminates "repeated_opts_first" and adds a new state
variable:

  list_mode       repeated_opts  repeated_opts_first
  --------------  -------------  -------------------
  LM_NONE         NULL           false
  LM_STARTED      non-NULL       true
  LM_IN_PROGRESS  non-NULL       false

Additionally, it is documented that lookup_scalar() and processed(), both
called by opts_type_XXX(), are invalid in LM_STARTED -- generated qapi
code calls opts_next_list() to allocate the very first link before trying
to parse a scalar into it. List mode restrictions are expressed in
positive / inclusive form.

Signed-off-by: Laszlo Ersek <lersek@redhat.com>
---
 rfc->v1:
 - replace assert(0) with abort() for when opts_next_list() encounters a
   nonexistent / invalid list mode [Paolo]

 qapi/opts-visitor.c |   45 +++++++++++++++++++++++++++++++++++----------
 1 files changed, 35 insertions(+), 10 deletions(-)

diff --git a/qapi/opts-visitor.c b/qapi/opts-visitor.c
index 174bd8b..29fd1ab 100644
--- a/qapi/opts-visitor.c
+++ b/qapi/opts-visitor.c
@@ -1,7 +1,7 @@
 /*
  * Options Visitor
  *
- * Copyright Red Hat, Inc. 2012
+ * Copyright Red Hat, Inc. 2012, 2013
  *
  * Author: Laszlo Ersek <lersek@redhat.com>
  *
@@ -18,6 +18,15 @@
 #include "qapi/visitor-impl.h"
 
 
+enum ListMode
+{
+    LM_NONE,             /* not traversing a list of repeated options */
+    LM_STARTED,          /* opts_start_list() succeeded */
+    LM_IN_PROGRESS       /* opts_next_list() has been called */
+};
+
+typedef enum ListMode ListMode;
+
 struct OptsVisitor
 {
     Visitor visitor;
@@ -35,8 +44,8 @@ struct OptsVisitor
     /* The list currently being traversed with opts_start_list() /
      * opts_next_list(). The list must have a struct element type in the
      * schema, with a single mandatory scalar member. */
+    ListMode list_mode;
     GQueue *repeated_opts;
-    bool repeated_opts_first;
 
     /* If "opts_root->id" is set, reinstantiate it as a fake QemuOpt for
      * uniformity. Only its "name" and "str" fields are set. "fake_id_opt" does
@@ -156,9 +165,11 @@ opts_start_list(Visitor *v, const char *name, Error **errp)
     OptsVisitor *ov = DO_UPCAST(OptsVisitor, visitor, v);
 
     /* we can't traverse a list in a list */
-    assert(ov->repeated_opts == NULL);
+    assert(ov->list_mode == LM_NONE);
     ov->repeated_opts = lookup_distinct(ov, name, errp);
-    ov->repeated_opts_first = (ov->repeated_opts != NULL);
+    if (ov->repeated_opts != NULL) {
+        ov->list_mode = LM_STARTED;
+    }
 }
 
 
@@ -168,10 +179,13 @@ opts_next_list(Visitor *v, GenericList **list, Error **errp)
     OptsVisitor *ov = DO_UPCAST(OptsVisitor, visitor, v);
     GenericList **link;
 
-    if (ov->repeated_opts_first) {
-        ov->repeated_opts_first = false;
+    switch (ov->list_mode) {
+    case LM_STARTED:
+        ov->list_mode = LM_IN_PROGRESS;
         link = list;
-    } else {
+        break;
+
+    case LM_IN_PROGRESS: {
         const QemuOpt *opt;
 
         opt = g_queue_pop_head(ov->repeated_opts);
@@ -180,6 +194,11 @@ opts_next_list(Visitor *v, GenericList **list, Error **errp)
             return NULL;
         }
         link = &(*list)->next;
+        break;
+    }
+
+    default:
+        abort();
     }
 
     *link = g_malloc0(sizeof **link);
@@ -192,14 +211,16 @@ opts_end_list(Visitor *v, Error **errp)
 {
     OptsVisitor *ov = DO_UPCAST(OptsVisitor, visitor, v);
 
+    assert(ov->list_mode == LM_STARTED || ov->list_mode == LM_IN_PROGRESS);
     ov->repeated_opts = NULL;
+    ov->list_mode = LM_NONE;
 }
 
 
 static const QemuOpt *
 lookup_scalar(const OptsVisitor *ov, const char *name, Error **errp)
 {
-    if (ov->repeated_opts == NULL) {
+    if (ov->list_mode == LM_NONE) {
         GQueue *list;
 
         /* the last occurrence of any QemuOpt takes effect when queried by name
@@ -207,6 +228,7 @@ lookup_scalar(const OptsVisitor *ov, const char *name, Error **errp)
         list = lookup_distinct(ov, name, errp);
         return list ? g_queue_peek_tail(list) : NULL;
     }
+    assert(ov->list_mode == LM_IN_PROGRESS);
     return g_queue_peek_head(ov->repeated_opts);
 }
 
@@ -214,9 +236,12 @@ lookup_scalar(const OptsVisitor *ov, const char *name, Error **errp)
 static void
 processed(OptsVisitor *ov, const char *name)
 {
-    if (ov->repeated_opts == NULL) {
+    if (ov->list_mode == LM_NONE) {
         g_hash_table_remove(ov->unprocessed_opts, name);
+        return;
     }
+    assert(ov->list_mode == LM_IN_PROGRESS);
+    /* do nothing */
 }
 
 
@@ -365,7 +390,7 @@ opts_start_optional(Visitor *v, bool *present, const char *name,
     OptsVisitor *ov = DO_UPCAST(OptsVisitor, visitor, v);
 
     /* we only support a single mandatory scalar field in a list node */
-    assert(ov->repeated_opts == NULL);
+    assert(ov->list_mode == LM_NONE);
     *present = (lookup_distinct(ov, name, NULL) != NULL);
 }
 
-- 
1.7.1

[Qemu-devel] [PATCH 2/8] OptsVisitor: introduce list modes for interval flattening

[Laszlo Ersek]

The new modes are equal-rank, exclusive alternatives of LM_IN_PROGRESS.
Teach opts_next_list(), opts_type_int() and opts_type_uint64() to handle
them.

Also enumerate explicitly what functions are valid to call in what modes:
- opts_next_list() is valid to call while flattening a range,
- opts_end_list(): ditto,
- lookup_scalar() is invalid to call during flattening; generated qapi
  traversal code must continue asking for the same kind of signed/unsigned
  list element until the interval is fully flattened,
- processed(): ditto.

List mode restrictions are always formulated in positive / inclusive
sense. The restrictions for lookup_scalar() and processed() are
automatically satisfied by current qapi traversals if the schema to build
is compatible with OptsVisitor.

The new list modes are not entered yet.

Signed-off-by: Laszlo Ersek <lersek@redhat.com>
---
 rfc->v1:
 - replace "sub-modes" with "alternatives" in the commit message,
 - document list mode enum constants extensively [Paolo].

 qapi/opts-visitor.c |   67 +++++++++++++++++++++++++++++++++++++++++++++++++-
 1 files changed, 65 insertions(+), 2 deletions(-)

diff --git a/qapi/opts-visitor.c b/qapi/opts-visitor.c
index 29fd1ab..c2a57bd 100644
--- a/qapi/opts-visitor.c
+++ b/qapi/opts-visitor.c
@@ -22,7 +22,32 @@ enum ListMode
 {
     LM_NONE,             /* not traversing a list of repeated options */
     LM_STARTED,          /* opts_start_list() succeeded */
-    LM_IN_PROGRESS       /* opts_next_list() has been called */
+
+    LM_IN_PROGRESS,      /* opts_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,  /* opts_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. */
 };
 
 typedef enum ListMode ListMode;
@@ -47,6 +72,15 @@ struct OptsVisitor
     ListMode list_mode;
     GQueue *repeated_opts;
 
+    /* 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_next, range_limit;
+
     /* If "opts_root->id" is set, reinstantiate it as a fake QemuOpt for
      * uniformity. Only its "name" and "str" fields are set. "fake_id_opt" does
      * not survive or escape the OptsVisitor object.
@@ -185,6 +219,22 @@ opts_next_list(Visitor *v, GenericList **list, Error **errp)
         link = list;
         break;
 
+    case LM_SIGNED_INTERVAL:
+    case LM_UNSIGNED_INTERVAL:
+        link = &(*list)->next;
+
+        if (ov->list_mode == LM_SIGNED_INTERVAL) {
+            if (ov->range_next.s < ov->range_limit.s) {
+                ++ov->range_next.s;
+                break;
+            }
+        } else if (ov->range_next.u < ov->range_limit.u) {
+            ++ov->range_next.u;
+            break;
+        }
+        ov->list_mode = LM_IN_PROGRESS;
+        /* range has been completed, fall through in order to pop option */
+
     case LM_IN_PROGRESS: {
         const QemuOpt *opt;
 
@@ -211,7 +261,10 @@ opts_end_list(Visitor *v, Error **errp)
 {
     OptsVisitor *ov = DO_UPCAST(OptsVisitor, visitor, v);
 
-    assert(ov->list_mode == LM_STARTED || ov->list_mode == LM_IN_PROGRESS);
+    assert(ov->list_mode == LM_STARTED ||
+           ov->list_mode == LM_IN_PROGRESS ||
+           ov->list_mode == LM_SIGNED_INTERVAL ||
+           ov->list_mode == LM_UNSIGNED_INTERVAL);
     ov->repeated_opts = NULL;
     ov->list_mode = LM_NONE;
 }
@@ -303,6 +356,11 @@ opts_type_int(Visitor *v, int64_t *obj, const char *name, Error **errp)
     long long val;
     char *endptr;
 
+    if (ov->list_mode == LM_SIGNED_INTERVAL) {
+        *obj = ov->range_next.s;
+        return;
+    }
+
     opt = lookup_scalar(ov, name, errp);
     if (!opt) {
         return;
@@ -328,6 +386,11 @@ opts_type_uint64(Visitor *v, uint64_t *obj, const char *name, Error **errp)
     const QemuOpt *opt;
     const char *str;
 
+    if (ov->list_mode == LM_UNSIGNED_INTERVAL) {
+        *obj = ov->range_next.u;
+        return;
+    }
+
     opt = lookup_scalar(ov, name, errp);
     if (!opt) {
         return;
-- 
1.7.1

[Qemu-devel] [PATCH 3/8] OptsVisitor: opts_type_int(): recognize intervals when LM_IN_PROGRESS

[Laszlo Ersek]

When a well-formed range value, bounded by signed integers, is encountered
while processing a repeated option, enter LM_SIGNED_INTERVAL and return
the low bound.

Signed-off-by: Laszlo Ersek <lersek@redhat.com>
---
 qapi/opts-visitor.c |   34 ++++++++++++++++++++++++++++------
 1 files changed, 28 insertions(+), 6 deletions(-)

diff --git a/qapi/opts-visitor.c b/qapi/opts-visitor.c
index c2a57bd..90be583 100644
--- a/qapi/opts-visitor.c
+++ b/qapi/opts-visitor.c
@@ -367,15 +367,37 @@ opts_type_int(Visitor *v, int64_t *obj, const char *name, Error **errp)
     }
     str = opt->str ? opt->str : "";
 
+    /* we've gotten past lookup_scalar() */
+    assert(ov->list_mode == LM_NONE || ov->list_mode == LM_IN_PROGRESS);
+
     errno = 0;
     val = strtoll(str, &endptr, 0);
-    if (*str != '\0' && *endptr == '\0' && errno == 0 && INT64_MIN <= val &&
-        val <= INT64_MAX) {
-        *obj = val;
-        processed(ov, name);
-        return;
+    if (errno == 0 && endptr > str && INT64_MIN <= val && val <= INT64_MAX) {
+        if (*endptr == '\0') {
+            *obj = val;
+            processed(ov, name);
+            return;
+        }
+        if (*endptr == '-' && ov->list_mode == LM_IN_PROGRESS) {
+            long long val2;
+
+            str = endptr + 1;
+            val2 = strtoll(str, &endptr, 0);
+            if (errno == 0 && endptr > str && *endptr == '\0' &&
+                INT64_MIN <= val2 && val2 <= INT64_MAX && val <= val2) {
+                ov->range_next.s = val;
+                ov->range_limit.s = val2;
+                ov->list_mode = LM_SIGNED_INTERVAL;
+
+                /* as if entering on the top */
+                *obj = ov->range_next.s;
+                return;
+            }
+        }
     }
-    error_set(errp, QERR_INVALID_PARAMETER_VALUE, opt->name, "an int64 value");
+    error_set(errp, QERR_INVALID_PARAMETER_VALUE, opt->name,
+              (ov->list_mode == LM_NONE) ? "an int64 value" :
+                                           "an int64 value or range");
 }
 
 
-- 
1.7.1

[Qemu-devel] [PATCH 4/8] OptsVisitor: rebase opts_type_uint64() to parse_uint_full()

[Laszlo Ersek]

Simplify the code in preparation for the next patch.

Signed-off-by: Laszlo Ersek <lersek@redhat.com>
---
 qapi/opts-visitor.c |   23 +++++------------------
 1 files changed, 5 insertions(+), 18 deletions(-)

diff --git a/qapi/opts-visitor.c b/qapi/opts-visitor.c
index 90be583..d8f9a0e 100644
--- a/qapi/opts-visitor.c
+++ b/qapi/opts-visitor.c
@@ -407,6 +407,7 @@ opts_type_uint64(Visitor *v, uint64_t *obj, const char *name, Error **errp)
     OptsVisitor *ov = DO_UPCAST(OptsVisitor, visitor, v);
     const QemuOpt *opt;
     const char *str;
+    unsigned long long val;
 
     if (ov->list_mode == LM_UNSIGNED_INTERVAL) {
         *obj = ov->range_next.u;
@@ -417,26 +418,12 @@ opts_type_uint64(Visitor *v, uint64_t *obj, const char *name, Error **errp)
     if (!opt) {
         return;
     }
-
     str = opt->str;
-    if (str != NULL) {
-        while (isspace((unsigned char)*str)) {
-            ++str;
-        }
-
-        if (*str != '-' && *str != '\0') {
-            unsigned long long val;
-            char *endptr;
 
-            /* non-empty, non-negative subject sequence */
-            errno = 0;
-            val = strtoull(str, &endptr, 0);
-            if (*endptr == '\0' && errno == 0 && val <= UINT64_MAX) {
-                *obj = val;
-                processed(ov, name);
-                return;
-            }
-        }
+    if (parse_uint_full(str, &val, 0) == 0 && val <= UINT64_MAX) {
+        *obj = val;
+        processed(ov, name);
+        return;
     }
     error_set(errp, QERR_INVALID_PARAMETER_VALUE, opt->name,
               "an uint64 value");
-- 
1.7.1

[Qemu-devel] [PATCH 5/8] OptsVisitor: opts_type_uint64(): recognize intervals when LM_IN_PROGRESS

[Laszlo Ersek]

When a well-formed range value, bounded by unsigned integers, is
encountered while processing a repeated option, enter LM_UNSIGNED_INTERVAL
and return the low bound.

Signed-off-by: Laszlo Ersek <lersek@redhat.com>
---
 qapi/opts-visitor.c |   32 +++++++++++++++++++++++++++-----
 1 files changed, 27 insertions(+), 5 deletions(-)

diff --git a/qapi/opts-visitor.c b/qapi/opts-visitor.c
index d8f9a0e..d54d373 100644
--- a/qapi/opts-visitor.c
+++ b/qapi/opts-visitor.c
@@ -408,6 +408,7 @@ opts_type_uint64(Visitor *v, uint64_t *obj, const char *name, Error **errp)
     const QemuOpt *opt;
     const char *str;
     unsigned long long val;
+    char *endptr;
 
     if (ov->list_mode == LM_UNSIGNED_INTERVAL) {
         *obj = ov->range_next.u;
@@ -420,13 +421,34 @@ opts_type_uint64(Visitor *v, uint64_t *obj, const char *name, Error **errp)
     }
     str = opt->str;
 
-    if (parse_uint_full(str, &val, 0) == 0 && val <= UINT64_MAX) {
-        *obj = val;
-        processed(ov, name);
-        return;
+    /* we've gotten past lookup_scalar() */
+    assert(ov->list_mode == LM_NONE || ov->list_mode == LM_IN_PROGRESS);
+
+    if (parse_uint(str, &val, &endptr, 0) == 0 && val <= UINT64_MAX) {
+        if (*endptr == '\0') {
+            *obj = val;
+            processed(ov, name);
+            return;
+        }
+        if (*endptr == '-' && ov->list_mode == LM_IN_PROGRESS) {
+            unsigned long long val2;
+
+            str = endptr + 1;
+            if (parse_uint_full(str, &val2, 0) == 0 &&
+                val2 <= UINT64_MAX && val <= val2) {
+                ov->range_next.u = val;
+                ov->range_limit.u = val2;
+                ov->list_mode = LM_UNSIGNED_INTERVAL;
+
+                /* as if entering on the top */
+                *obj = ov->range_next.u;
+                return;
+            }
+        }
     }
     error_set(errp, QERR_INVALID_PARAMETER_VALUE, opt->name,
-              "an uint64 value");
+              (ov->list_mode == LM_NONE) ? "a uint64 value" :
+                                           "a uint64 value or range");
 }
 
 
-- 
1.7.1

[Qemu-devel] [PATCH 6/8] OptsVisitor: don't try to flatten overlong integer ranges

[Laszlo Ersek]

Prevent mistyped command line options from incurring high memory and CPU
usage at startup. 64K elements in a range should be enough for everyone
(TM).

The OPTS_VISITOR_RANGE_MAX macro is public so that unit tests can
construct corner cases with it.

Signed-off-by: Laszlo Ersek <lersek@redhat.com>
---
 include/qapi/opts-visitor.h |    6 ++++++
 qapi/opts-visitor.c         |    7 +++++--
 2 files changed, 11 insertions(+), 2 deletions(-)

diff --git a/include/qapi/opts-visitor.h b/include/qapi/opts-visitor.h
index 5939eee..fd48c14 100644
--- a/include/qapi/opts-visitor.h
+++ b/include/qapi/opts-visitor.h
@@ -16,6 +16,12 @@
 #include "qapi/visitor.h"
 #include "qemu/option.h"
 
+/* Inclusive upper bound on the size of any flattened range. This is a safety
+ * (= anti-annoyance) measure; wrong ranges should not cause long startup
+ * delays nor exhaust virtual memory.
+ */
+#define OPTS_VISITOR_RANGE_MAX 65536
+
 typedef struct OptsVisitor OptsVisitor;
 
 /* Contrarily to qemu-option.c::parse_option_number(), OptsVisitor's "int"
diff --git a/qapi/opts-visitor.c b/qapi/opts-visitor.c
index d54d373..96ed858 100644
--- a/qapi/opts-visitor.c
+++ b/qapi/opts-visitor.c
@@ -384,7 +384,9 @@ opts_type_int(Visitor *v, int64_t *obj, const char *name, Error **errp)
             str = endptr + 1;
             val2 = strtoll(str, &endptr, 0);
             if (errno == 0 && endptr > str && *endptr == '\0' &&
-                INT64_MIN <= val2 && val2 <= INT64_MAX && val <= val2) {
+                INT64_MIN <= val2 && val2 <= INT64_MAX && val <= val2 &&
+                (val > INT64_MAX - OPTS_VISITOR_RANGE_MAX ||
+                 val2 < val + OPTS_VISITOR_RANGE_MAX)) {
                 ov->range_next.s = val;
                 ov->range_limit.s = val2;
                 ov->list_mode = LM_SIGNED_INTERVAL;
@@ -435,7 +437,8 @@ opts_type_uint64(Visitor *v, uint64_t *obj, const char *name, Error **errp)
 
             str = endptr + 1;
             if (parse_uint_full(str, &val2, 0) == 0 &&
-                val2 <= UINT64_MAX && val <= val2) {
+                val2 <= UINT64_MAX && val <= val2 &&
+                val2 - val < OPTS_VISITOR_RANGE_MAX) {
                 ov->range_next.u = val;
                 ov->range_limit.u = val2;
                 ov->list_mode = LM_UNSIGNED_INTERVAL;
-- 
1.7.1

[Qemu-devel] [PATCH 7/8] add "test-int128" to .gitignore

[Laszlo Ersek]

Probably missed in commit 6046c620
("int128: optimize and add test cases").

Signed-off-by: Laszlo Ersek <lersek@redhat.com>
---
 .gitignore |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/.gitignore b/.gitignore
index 0fe114d..388cb45 100644
--- a/.gitignore
+++ b/.gitignore
@@ -46,6 +46,7 @@ qemu-monitor.texi
 vscclient
 QMP/qmp-commands.txt
 test-coroutine
+test-int128
 test-qmp-input-visitor
 test-qmp-output-visitor
 test-string-input-visitor
-- 
1.7.1

[Qemu-devel] [PATCH 8/8] OptsVisitor: introduce unit tests, with test cases for range flattening

[Laszlo Ersek]

Signed-off-by: Laszlo Ersek <lersek@redhat.com>
---
 tests/Makefile            |    6 +-
 qapi-schema-test.json     |   15 +++
 tests/test-opts-visitor.c |  275 +++++++++++++++++++++++++++++++++++++++++++++
 .gitignore                |    1 +
 4 files changed, 296 insertions(+), 1 deletions(-)
 create mode 100644 tests/test-opts-visitor.c

diff --git a/tests/Makefile b/tests/Makefile
index 425a9a8..8bb290e 100644
--- a/tests/Makefile
+++ b/tests/Makefile
@@ -23,6 +23,8 @@ check-unit-y += tests/test-string-input-visitor$(EXESUF)
 gcov-files-test-string-input-visitor-y = qapi/string-input-visitor.c
 check-unit-y += tests/test-string-output-visitor$(EXESUF)
 gcov-files-test-string-output-visitor-y = qapi/string-output-visitor.c
+check-unit-y += tests/test-opts-visitor$(EXESUF)
+gcov-files-test-opts-visitor-y = qapi/opts-visitor.c
 check-unit-y += tests/test-coroutine$(EXESUF)
 gcov-files-test-coroutine-y = coroutine-$(CONFIG_COROUTINE_BACKEND).c
 check-unit-y += tests/test-visitor-serialization$(EXESUF)
@@ -81,7 +83,8 @@ test-obj-y = tests/check-qint.o tests/check-qstring.o tests/check-qdict.o \
 	tests/test-string-input-visitor.o tests/test-qmp-output-visitor.o \
 	tests/test-qmp-input-visitor.o tests/test-qmp-input-strict.o \
 	tests/test-qmp-commands.o tests/test-visitor-serialization.o \
-	tests/test-x86-cpuid.o tests/test-mul64.o tests/test-int128.o
+	tests/test-x86-cpuid.o tests/test-mul64.o tests/test-int128.o \
+	tests/test-opts-visitor.o
 
 test-qapi-obj-y = tests/test-qapi-visit.o tests/test-qapi-types.o
 
@@ -123,6 +126,7 @@ tests/test-qmp-input-visitor$(EXESUF): tests/test-qmp-input-visitor.o $(test-qap
 tests/test-qmp-input-strict$(EXESUF): tests/test-qmp-input-strict.o $(test-qapi-obj-y) libqemuutil.a libqemustub.a
 tests/test-qmp-commands$(EXESUF): tests/test-qmp-commands.o tests/test-qmp-marshal.o $(test-qapi-obj-y) qapi-types.o qapi-visit.o libqemuutil.a libqemustub.a
 tests/test-visitor-serialization$(EXESUF): tests/test-visitor-serialization.o $(test-qapi-obj-y) libqemuutil.a libqemustub.a
+tests/test-opts-visitor$(EXESUF): tests/test-opts-visitor.o $(test-qapi-obj-y) libqemuutil.a libqemustub.a
 
 tests/test-mul64$(EXESUF): tests/test-mul64.o libqemuutil.a
 
diff --git a/qapi-schema-test.json b/qapi-schema-test.json
index 4434fa3..fe5af75 100644
--- a/qapi-schema-test.json
+++ b/qapi-schema-test.json
@@ -51,3 +51,18 @@
 { 'command': 'user_def_cmd', 'data': {} }
 { 'command': 'user_def_cmd1', 'data': {'ud1a': 'UserDefOne'} }
 { 'command': 'user_def_cmd2', 'data': {'ud1a': 'UserDefOne', 'ud1b': 'UserDefOne'}, 'returns': 'UserDefTwo' }
+
+# For testing integer range flattening in opts-visitor. The following schema
+# corresponds to the option format:
+#
+# -userdef i64=3-6,i64=-5--1,u64=2,u16=1,u16=7-12
+#
+# For simplicity, this example doesn't use [type=]discriminator nor optargs
+# specific to discriminator values.
+{ 'type': 'UserDefOptions',
+  'data': {
+    '*i64' : [ 'int'    ],
+    '*u64' : [ 'uint64' ],
+    '*u16' : [ 'uint16' ],
+    '*i64x':   'int'     ,
+    '*u64x':   'uint64'  } }
diff --git a/tests/test-opts-visitor.c b/tests/test-opts-visitor.c
new file mode 100644
index 0000000..9f902b5
--- /dev/null
+++ b/tests/test-opts-visitor.c
@@ -0,0 +1,275 @@
+/*
+ * Options Visitor unit-tests.
+ *
+ * Copyright (C) 2013 Red Hat, Inc.
+ *
+ * Authors:
+ *   Laszlo Ersek <lersek@redhat.com> (based on test-string-output-visitor)
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or later.
+ * See the COPYING file in the top-level directory.
+ */
+
+#include <glib.h>
+
+#include "qemu/config-file.h"     /* qemu_add_opts() */
+#include "qemu/option.h"          /* qemu_opts_parse() */
+#include "qapi/opts-visitor.h"    /* opts_visitor_new() */
+#include "test-qapi-visit.h"      /* visit_type_UserDefOptions() */
+#include "qapi/dealloc-visitor.h" /* qapi_dealloc_visitor_new() */
+
+static QemuOptsList userdef_opts = {
+    .name = "userdef",
+    .head = QTAILQ_HEAD_INITIALIZER(userdef_opts.head),
+    .desc = { { 0 } } /* validated with OptsVisitor */
+};
+
+/* fixture (= glib test case context) and test case manipulation */
+
+typedef struct OptsVisitorFixture {
+    UserDefOptions *userdef;
+    Error *err;
+} OptsVisitorFixture;
+
+
+static void
+setup_fixture(OptsVisitorFixture *f, gconstpointer test_data)
+{
+    const char *opts_string = test_data;
+    QemuOpts *opts;
+    OptsVisitor *ov;
+
+    opts = qemu_opts_parse(qemu_find_opts("userdef"), opts_string, 0);
+    g_assert(opts != NULL);
+
+    ov = opts_visitor_new(opts);
+    visit_type_UserDefOptions(opts_get_visitor(ov), &f->userdef, NULL,
+                              &f->err);
+    opts_visitor_cleanup(ov);
+    qemu_opts_del(opts);
+}
+
+
+static void
+teardown_fixture(OptsVisitorFixture *f, gconstpointer test_data)
+{
+    if (f->userdef != NULL) {
+        QapiDeallocVisitor *dv;
+
+        dv = qapi_dealloc_visitor_new();
+        visit_type_UserDefOptions(qapi_dealloc_get_visitor(dv), &f->userdef,
+                                  NULL, NULL);
+        qapi_dealloc_visitor_cleanup(dv);
+    }
+    error_free(f->err);
+}
+
+
+static void
+add_test(const char *testpath,
+         void (*test_func)(OptsVisitorFixture *f, gconstpointer test_data),
+         gconstpointer test_data)
+{
+    g_test_add(testpath, OptsVisitorFixture, test_data, setup_fixture,
+               test_func, teardown_fixture);
+}
+
+/* test output evaluation */
+
+static void
+expect_ok(OptsVisitorFixture *f, gconstpointer test_data)
+{
+    g_assert(f->err == NULL);
+    g_assert(f->userdef != NULL);
+}
+
+
+static void
+expect_fail(OptsVisitorFixture *f, gconstpointer test_data)
+{
+    g_assert(f->err != NULL);
+
+    /* The error message is printed when this test utility is invoked directly
+     * (ie. without gtester) and the --verbose flag is passed:
+     *
+     * tests/test-opts-visitor --verbose
+     */
+    g_test_message("'%s': %s", (const char *)test_data,
+                   error_get_pretty(f->err));
+}
+
+
+static void
+test_value(OptsVisitorFixture *f, gconstpointer test_data)
+{
+    uint64_t magic, bitval;
+    intList *i64;
+    uint64List *u64;
+    uint16List *u16;
+
+    expect_ok(f, test_data);
+
+    magic = 0;
+    for (i64 = f->userdef->i64; i64 != NULL; i64 = i64->next) {
+        g_assert(-16 <= i64->value && i64->value < 64-16);
+        bitval = 1ull << (i64->value + 16);
+        g_assert((magic & bitval) == 0);
+        magic |= bitval;
+    }
+    g_assert(magic == 0xDEADBEEF);
+
+    magic = 0;
+    for (u64 = f->userdef->u64; u64 != NULL; u64 = u64->next) {
+        g_assert(u64->value < 64);
+        bitval = 1ull << u64->value;
+        g_assert((magic & bitval) == 0);
+        magic |= bitval;
+    }
+    g_assert(magic == 0xBADC0FFEE0DDF00D);
+
+    magic = 0;
+    for (u16 = f->userdef->u16; u16 != NULL; u16 = u16->next) {
+        g_assert(u16->value < 64);
+        bitval = 1ull << u16->value;
+        g_assert((magic & bitval) == 0);
+        magic |= bitval;
+    }
+    g_assert(magic == 0xD15EA5E);
+}
+
+
+static void
+expect_i64_min(OptsVisitorFixture *f, gconstpointer test_data)
+{
+    expect_ok(f, test_data);
+    g_assert(f->userdef->has_i64);
+    g_assert(f->userdef->i64->next == NULL);
+    g_assert(f->userdef->i64->value == INT64_MIN);
+}
+
+
+static void
+expect_i64_max(OptsVisitorFixture *f, gconstpointer test_data)
+{
+    expect_ok(f, test_data);
+    g_assert(f->userdef->has_i64);
+    g_assert(f->userdef->i64->next == NULL);
+    g_assert(f->userdef->i64->value == INT64_MAX);
+}
+
+
+static void
+expect_zero(OptsVisitorFixture *f, gconstpointer test_data)
+{
+    expect_ok(f, test_data);
+    g_assert(f->userdef->has_u64);
+    g_assert(f->userdef->u64->next == NULL);
+    g_assert(f->userdef->u64->value == 0);
+}
+
+
+static void
+expect_u64_max(OptsVisitorFixture *f, gconstpointer test_data)
+{
+    expect_ok(f, test_data);
+    g_assert(f->userdef->has_u64);
+    g_assert(f->userdef->u64->next == NULL);
+    g_assert(f->userdef->u64->value == UINT64_MAX);
+}
+
+/* test cases */
+
+int
+main(int argc, char **argv)
+{
+    g_test_init(&argc, &argv, NULL);
+
+    qemu_add_opts(&userdef_opts);
+
+    /* Three hexadecimal magic numbers, "dead beef", "bad coffee, odd food" and
+     * "disease", from
+     * <http://en.wikipedia.org/wiki/Magic_number_%28programming%29>, were
+     * converted to binary and dissected into bit ranges. Each magic number is
+     * going to be recomposed using the lists called "i64", "u64" and "u16",
+     * respectively.
+     *
+     * (Note that these types pertain to the individual bit shift counts, not
+     * the magic numbers themselves; the intent is to exercise opts_type_int()
+     * and opts_type_uint64().)
+     *
+     * The "i64" shift counts have been decreased by 16 (decimal) in order to
+     * test negative values as well. Finally, the full list of QemuOpt elements
+     * has been permuted with "shuf".
+     *
+     * Both "i64" and "u64" have some (distinct) single-element ranges
+     * represented as both "a" and "a-a". "u16" is a special case of "i64" (see
+     * visit_type_uint16()), so it wouldn't add a separate test in this regard.
+     */
+
+    add_test("/visitor/opts/flatten/value", &test_value,
+             "i64=-1-0,u64=12-16,u64=2-3,i64=-11--9,u64=57,u16=9,i64=5-5,"
+             "u16=1-4,u16=20,u64=63-63,i64=-16--13,u64=50-52,i64=14-15,u16=11,"
+             "i64=7,u16=18,i64=2-3,u16=6,u64=54-55,u64=0,u64=18-20,u64=33-43,"
+             "i64=9-12,u16=26-27,u64=59-61,u16=13-16,u64=29-31,u64=22-23,"
+             "u16=24,i64=-7--3");
+
+    add_test("/visitor/opts/i64/val1/errno",    &expect_fail,
+             "i64=0x8000000000000000");
+    add_test("/visitor/opts/i64/val1/empty",    &expect_fail, "i64=");
+    add_test("/visitor/opts/i64/val1/trailing", &expect_fail, "i64=5z");
+    add_test("/visitor/opts/i64/nonlist",       &expect_fail, "i64x=5-6");
+    add_test("/visitor/opts/i64/val2/errno",    &expect_fail,
+             "i64=0x7fffffffffffffff-0x8000000000000000");
+    add_test("/visitor/opts/i64/val2/empty",    &expect_fail, "i64=5-");
+    add_test("/visitor/opts/i64/val2/trailing", &expect_fail, "i64=5-6z");
+    add_test("/visitor/opts/i64/range/empty",   &expect_fail, "i64=6-5");
+    add_test("/visitor/opts/i64/range/minval",  &expect_i64_min,
+             "i64=-0x8000000000000000--0x8000000000000000");
+    add_test("/visitor/opts/i64/range/maxval",  &expect_i64_max,
+             "i64=0x7fffffffffffffff-0x7fffffffffffffff");
+
+    add_test("/visitor/opts/u64/val1/errno",    &expect_fail, "u64=-1");
+    add_test("/visitor/opts/u64/val1/empty",    &expect_fail, "u64=");
+    add_test("/visitor/opts/u64/val1/trailing", &expect_fail, "u64=5z");
+    add_test("/visitor/opts/u64/nonlist",       &expect_fail, "u64x=5-6");
+    add_test("/visitor/opts/u64/val2/errno",    &expect_fail,
+             "u64=0xffffffffffffffff-0x10000000000000000");
+    add_test("/visitor/opts/u64/val2/empty",    &expect_fail, "u64=5-");
+    add_test("/visitor/opts/u64/val2/trailing", &expect_fail, "u64=5-6z");
+    add_test("/visitor/opts/u64/range/empty",   &expect_fail, "u64=6-5");
+    add_test("/visitor/opts/u64/range/minval",  &expect_zero, "u64=0-0");
+    add_test("/visitor/opts/u64/range/maxval",  &expect_u64_max,
+             "u64=0xffffffffffffffff-0xffffffffffffffff");
+
+    /* Test maximum range sizes. The macro value is open-coded here
+     * *intentionally*; the test case must use concrete values by design. If
+     * OPTS_VISITOR_RANGE_MAX is changed, the following values need to be
+     * recalculated as well. The assert and this comment should help with it.
+     */
+    g_assert(OPTS_VISITOR_RANGE_MAX == 65536);
+
+    /* The unsigned case is simple, a u64-u64 difference can always be
+     * represented as a u64.
+     */
+    add_test("/visitor/opts/u64/range/max",  &expect_ok,   "u64=0-65535");
+    add_test("/visitor/opts/u64/range/2big", &expect_fail, "u64=0-65536");
+
+    /* The same cannot be said about an i64-i64 difference. */
+    add_test("/visitor/opts/i64/range/max/pos/a", &expect_ok,
+             "i64=0x7fffffffffff0000-0x7fffffffffffffff");
+    add_test("/visitor/opts/i64/range/max/pos/b", &expect_ok,
+             "i64=0x7ffffffffffeffff-0x7ffffffffffffffe");
+    add_test("/visitor/opts/i64/range/2big/pos",  &expect_fail,
+             "i64=0x7ffffffffffeffff-0x7fffffffffffffff");
+    add_test("/visitor/opts/i64/range/max/neg/a", &expect_ok,
+             "i64=-0x8000000000000000--0x7fffffffffff0001");
+    add_test("/visitor/opts/i64/range/max/neg/b", &expect_ok,
+             "i64=-0x7fffffffffffffff--0x7fffffffffff0000");
+    add_test("/visitor/opts/i64/range/2big/neg",  &expect_fail,
+             "i64=-0x8000000000000000--0x7fffffffffff0000");
+    add_test("/visitor/opts/i64/range/2big/full", &expect_fail,
+             "i64=-0x8000000000000000-0x7fffffffffffffff");
+
+    g_test_run();
+    return 0;
+}
diff --git a/.gitignore b/.gitignore
index 388cb45..60a0363 100644
--- a/.gitignore
+++ b/.gitignore
@@ -47,6 +47,7 @@ vscclient
 QMP/qmp-commands.txt
 test-coroutine
 test-int128
+test-opts-visitor
 test-qmp-input-visitor
 test-qmp-output-visitor
 test-string-input-visitor
-- 
1.7.1

Re: [Qemu-devel] [PATCH 8/8] OptsVisitor: introduce unit tests, with test cases for range flattening

[Eric Blake]

[-- Attachment #1: Type: text/plain, Size: 1610 bytes --]

On 07/22/2013 03:07 PM, Laszlo Ersek wrote:
> Signed-off-by: Laszlo Ersek <lersek@redhat.com>
> ---
>  tests/Makefile            |    6 +-
>  qapi-schema-test.json     |   15 +++
>  tests/test-opts-visitor.c |  275 +++++++++++++++++++++++++++++++++++++++++++++
>  .gitignore                |    1 +
>  4 files changed, 296 insertions(+), 1 deletions(-)
>  create mode 100644 tests/test-opts-visitor.c

> +    add_test("/visitor/opts/i64/val1/errno",    &expect_fail,
> +             "i64=0x8000000000000000");
> +    add_test("/visitor/opts/i64/val1/empty",    &expect_fail, "i64=");
> +    add_test("/visitor/opts/i64/val1/trailing", &expect_fail, "i64=5z");
> +    add_test("/visitor/opts/i64/nonlist",       &expect_fail, "i64x=5-6");
> +    add_test("/visitor/opts/i64/val2/errno",    &expect_fail,
> +             "i64=0x7fffffffffffffff-0x8000000000000000");
> +    add_test("/visitor/opts/i64/val2/empty",    &expect_fail, "i64=5-");
> +    add_test("/visitor/opts/i64/val2/trailing", &expect_fail, "i64=5-6z");
> +    add_test("/visitor/opts/i64/range/empty",   &expect_fail, "i64=6-5");
> +    add_test("/visitor/opts/i64/range/minval",  &expect_i64_min,
> +             "i64=-0x8000000000000000--0x8000000000000000");
> +    add_test("/visitor/opts/i64/range/maxval",  &expect_i64_max,
> +             "i64=0x7fffffffffffffff-0x7fffffffffffffff");

Pretty thorough, although I thought of a couple other ideas to test:
i64=5z-6 should fail; i64=5-6-7 should fail

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 621 bytes --]

Re: [Qemu-devel] [PATCH 8/8] OptsVisitor: introduce unit tests, with test cases for range flattening

[Laszlo Ersek]

On 07/23/13 00:04, Eric Blake wrote:
> On 07/22/2013 03:07 PM, Laszlo Ersek wrote:
>> Signed-off-by: Laszlo Ersek <lersek@redhat.com>
>> ---
>>  tests/Makefile            |    6 +-
>>  qapi-schema-test.json     |   15 +++
>>  tests/test-opts-visitor.c |  275 +++++++++++++++++++++++++++++++++++++++++++++
>>  .gitignore                |    1 +
>>  4 files changed, 296 insertions(+), 1 deletions(-)
>>  create mode 100644 tests/test-opts-visitor.c
> 
>> +    add_test("/visitor/opts/i64/val1/errno",    &expect_fail,
>> +             "i64=0x8000000000000000");
>> +    add_test("/visitor/opts/i64/val1/empty",    &expect_fail, "i64=");
>> +    add_test("/visitor/opts/i64/val1/trailing", &expect_fail, "i64=5z");
>> +    add_test("/visitor/opts/i64/nonlist",       &expect_fail, "i64x=5-6");
>> +    add_test("/visitor/opts/i64/val2/errno",    &expect_fail,
>> +             "i64=0x7fffffffffffffff-0x8000000000000000");
>> +    add_test("/visitor/opts/i64/val2/empty",    &expect_fail, "i64=5-");
>> +    add_test("/visitor/opts/i64/val2/trailing", &expect_fail, "i64=5-6z");
>> +    add_test("/visitor/opts/i64/range/empty",   &expect_fail, "i64=6-5");
>> +    add_test("/visitor/opts/i64/range/minval",  &expect_i64_min,
>> +             "i64=-0x8000000000000000--0x8000000000000000");
>> +    add_test("/visitor/opts/i64/range/maxval",  &expect_i64_max,
>> +             "i64=0x7fffffffffffffff-0x7fffffffffffffff");
> 
> Pretty thorough, although I thought of a couple other ideas to test:
> i64=5z-6 should fail; i64=5-6-7 should fail

I can add them if you insist, but I wrote (and single-stepped all of)
the test cases so that all branches added by patches 3, 5 and 6 would be
covered. (Some of the final tests in this function are actually
redundant, but I liked how they looked :))

For example, "i64=5z-6" is no different from "i64=5z", in patch 3 both
the first added (*endptr == '\0') condition and the (*endptr == '-')
fail the same way for both input strings: we never look past the "z".

Likewise, "i64=5-6-7" is the same case as "i64=5-6z": both characters
after the "6" (ie. "-" and "z") violate the second added (*endptr ==
'\0') condition in patch 3 the same way.

Do you accept this argument? :)

Thanks!
Laszlo

Re: [Qemu-devel] [PATCH 8/8] OptsVisitor: introduce unit tests, with test cases for range flattening

[Eric Blake]

[-- Attachment #1: Type: text/plain, Size: 1455 bytes --]

On 07/22/2013 04:24 PM, Laszlo Ersek wrote:
>> Pretty thorough, although I thought of a couple other ideas to test:
>> i64=5z-6 should fail; i64=5-6-7 should fail
> 
> I can add them if you insist, but I wrote (and single-stepped all of)
> the test cases so that all branches added by patches 3, 5 and 6 would be
> covered. (Some of the final tests in this function are actually
> redundant, but I liked how they looked :))
> 
> For example, "i64=5z-6" is no different from "i64=5z", in patch 3 both
> the first added (*endptr == '\0') condition and the (*endptr == '-')
> fail the same way for both input strings: we never look past the "z".
> 
> Likewise, "i64=5-6-7" is the same case as "i64=5-6z": both characters
> after the "6" (ie. "-" and "z") violate the second added (*endptr ==
> '\0') condition in patch 3 the same way.
> 
> Do you accept this argument? :)

Yes, I can agree you have 100% code coverage as currently coded.  Adding
what currently forms redundant cases may avoid future patch-writers from
breaking 100% coverage while actually triggering different paths between
the cases; but at the same time, we can assume such a future
patch-writer would be adding some new feature to the parser, and could
expand the testsuite accordingly as part of their efforts.  So no, I
won't insist on a respin :)

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 621 bytes --]

Re: [Qemu-devel] [PATCH 8/8] OptsVisitor: introduce unit tests, with test cases for range flattening

[Laszlo Ersek]

On 07/23/13 00:26, Eric Blake wrote:
> On 07/22/2013 04:24 PM, Laszlo Ersek wrote:
>>> Pretty thorough, although I thought of a couple other ideas to test:
>>> i64=5z-6 should fail; i64=5-6-7 should fail
>>
>> I can add them if you insist, but I wrote (and single-stepped all of)
>> the test cases so that all branches added by patches 3, 5 and 6 would be
>> covered. (Some of the final tests in this function are actually
>> redundant, but I liked how they looked :))
>>
>> For example, "i64=5z-6" is no different from "i64=5z", in patch 3 both
>> the first added (*endptr == '\0') condition and the (*endptr == '-')
>> fail the same way for both input strings: we never look past the "z".
>>
>> Likewise, "i64=5-6-7" is the same case as "i64=5-6z": both characters
>> after the "6" (ie. "-" and "z") violate the second added (*endptr ==
>> '\0') condition in patch 3 the same way.
>>
>> Do you accept this argument? :)
> 
> Yes, I can agree you have 100% code coverage as currently coded.  Adding
> what currently forms redundant cases may avoid future patch-writers from
> breaking 100% coverage while actually triggering different paths between
> the cases; but at the same time, we can assume such a future
> patch-writer would be adding some new feature to the parser, and could
> expand the testsuite accordingly as part of their efforts.

Agreed!

> So no, I
> won't insist on a respin :)

Thank you very much :)

/me bows and scrapes
Laszlo

Re: [Qemu-devel] [PATCH 0/8] OptsVisitor: support / flatten integer ranges for repeating options

[Laszlo Ersek]

On 07/22/13 23:07, Laszlo Ersek wrote:
> rfc->v1:
> - addressed Paolo's comments for patches 1 and 2,
> - patches 7 and 8 are new (unit tests),
> - updated the cover letter to take native lists into account, plus
>   cleaned it up.

Will this be considered for 1.7?

I'm not sure how Wanlong's NUMA stuff is scheduled, but I think it
shouldn't wait for my series' acceptance. (Rebasing -numa range parsing
to rely on the new OptsVisitor "capability" shouldn't be hard -- it's
mostly removing "client" code.)

Thanks
Laszlo

Re: [Qemu-devel] [PATCH 0/8] OptsVisitor: support / flatten integer ranges for repeating options

[Paolo Bonzini]

Il 29/07/2013 11:47, Laszlo Ersek ha scritto:
> On 07/22/13 23:07, Laszlo Ersek wrote:
>> rfc->v1:
>> - addressed Paolo's comments for patches 1 and 2,
>> - patches 7 and 8 are new (unit tests),
>> - updated the cover letter to take native lists into account, plus
>>   cleaned it up.
> 
> Will this be considered for 1.7?

Yes, why not? :)

Paolo

> I'm not sure how Wanlong's NUMA stuff is scheduled, but I think it
> shouldn't wait for my series' acceptance. (Rebasing -numa range parsing
> to rely on the new OptsVisitor "capability" shouldn't be hard -- it's
> mostly removing "client" code.)
> 
> Thanks
> Laszlo
> 

Re: [Qemu-devel] [PATCH 0/8] OptsVisitor: support / flatten integer ranges for repeating options

[Wanlong Gao]

On 07/29/2013 07:01 PM, Paolo Bonzini wrote:
> Il 29/07/2013 11:47, Laszlo Ersek ha scritto:
>> On 07/22/13 23:07, Laszlo Ersek wrote:
>>> rfc->v1:
>>> - addressed Paolo's comments for patches 1 and 2,
>>> - patches 7 and 8 are new (unit tests),
>>> - updated the cover letter to take native lists into account, plus
>>>   cleaned it up.
>>
>> Will this be considered for 1.7?
> 
> Yes, why not? :)

Nice, I'm rebasing my NUMA patch set on this series.

Thanks,
Wanlong Gao

> 
> Paolo
> 
>> I'm not sure how Wanlong's NUMA stuff is scheduled, but I think it
>> shouldn't wait for my series' acceptance. (Rebasing -numa range parsing
>> to rely on the new OptsVisitor "capability" shouldn't be hard -- it's
>> mostly removing "client" code.)
>>
>> Thanks
>> Laszlo
>>
> 
> 

Re: [Qemu-devel] [PATCH 0/8] OptsVisitor: support / flatten integer ranges for repeating options

[Laszlo Ersek]

On 07/29/13 13:11, Wanlong Gao wrote:
> On 07/29/2013 07:01 PM, Paolo Bonzini wrote:
>> Il 29/07/2013 11:47, Laszlo Ersek ha scritto:
>>> On 07/22/13 23:07, Laszlo Ersek wrote:
>>>> rfc->v1:
>>>> - addressed Paolo's comments for patches 1 and 2,
>>>> - patches 7 and 8 are new (unit tests),
>>>> - updated the cover letter to take native lists into account, plus
>>>>   cleaned it up.
>>>
>>> Will this be considered for 1.7?
>>
>> Yes, why not? :)
> 
> Nice, I'm rebasing my NUMA patch set on this series.

Thank you, that's most appreciated. I didn't want to ask for it, but I
*have* learned that the best way to sneak in utility stuff is to base
new features on them :)

(This is how OptsVisitor had gotten in originally BTW; Stefan was
working on new network stuff (VLANs vs. hubs IIRC) and he (supposedly)
didn't want to wait too long for my series that introduced OptsVisitor
and also rebased net option parsing to it (his series was modifying net
code too, obviously), so he just applied my stuff :))

Laszlo

Re: [Qemu-devel] [PATCH 0/8] OptsVisitor: support / flatten integer ranges for repeating options

[Laszlo Ersek]

Ping then, for 1.7 :)

(Don't stone me, I don't have the slightest clue about the 1.7 backlog.
I'm fine with this being tacked to the end, just don't let it fall
through the cracks.)

Thanks
Laszlo

On 07/22/13 23:07, Laszlo Ersek wrote:
> rfc->v1:
> - addressed Paolo's comments for patches 1 and 2,
> - patches 7 and 8 are new (unit tests),
> - updated the cover letter to take native lists into account, plus
>   cleaned it up.
> 
> Consider the following QAPI schema fragment, for the purpose of command
> line parsing with OptsVisitor:
> 
>   { 'union': 'NumaOptions',
>     'data': {
>       'node': 'NumaNodeOptions',
>       'mem' : 'NumaMemOptions' }}
> 
>   { 'type': 'NumaNodeOptions',
>     'data': {
>       '*nodeid': 'int',
>       '*cpus'  : ['uint16'] }}
> 
>   { 'type': 'NumaMemOptions',
>     'data': {
>       '*nodeid': 'int',
>       '*size'  : 'size' }}
> 
> (Commit eb7ee2cb ("qapi: introduce OptsVisitor") had originally
> documented OptsVisitor's general schema requirements for parsing
> repeated options such that the list element type had to be a struct with
> one mandatory scalar field. Accordingly, the RFC version of this series
> required for interval flattening that this underlying scalar type be an
> integer type. However, since commit a678e26c ("qapi: pad GenericList
> value fields to 64 bits") we've had reliable native lists; OptsVisitor
> turns out to support them automatically.)
> 
> OptsVisitor already accepts the following command line with the above
> schema:
> 
>   -numa node,nodeid=3,cpus=0,cpus=1,cpus=2,cpus=6,cpus=7,cpus=8
> 
> Paolo suggested in
> <http://thread.gmane.org/gmane.comp.emulators.qemu/222589/focus=222732>
> that OptsVisitor should allow the following shortcut:
> 
>   -numa node,nodeid=3,cpus=0-2,cpus=6-8
> 
> and that the code processing the "cpus" list should encounter all six
> elements (0, 1, 2, 6, 7, 8) individually.
> 
> The series implements this feature. Both signed and unsigned values and
> intervals are supported in general:
> 
>   * 0     (zero)
>   * 1-5   (one to five)
>   * 4-4   (four to four, range with one element)
>   * -2    (minus two)
>   * -5-8  (minus five to plus eight)
>   * -9--6 (minus nine to minus six)
> 
> The restrictions imposed by the native list element's signedness and
> size (in the above schema example, 'uint16') are enforced element-wise
> as usual. That is, for 'uint16', the command line option
> 
>   -numa node,nodeid=3,cpus=65534-65537
> 
> is equivalent to
> 
>   -numa node,nodeid=3,cpus=65534,cpus=65535,cpus=65536,cpus=65537
> 
> and visit_type_uint16() [qapi/qapi-visit-core.c] will catch the first
> element (= 65536) that has been parsed by opts_type_int() but cannot be
> represented as 'uint16'.
> 
> Laszlo Ersek (8):
>   OptsVisitor: introduce basic list modes
>   OptsVisitor: introduce list modes for interval flattening
>   OptsVisitor: opts_type_int(): recognize intervals when LM_IN_PROGRESS
>   OptsVisitor: rebase opts_type_uint64() to parse_uint_full()
>   OptsVisitor: opts_type_uint64(): recognize intervals when
>     LM_IN_PROGRESS
>   OptsVisitor: don't try to flatten overlong integer ranges
>   add "test-int128" to .gitignore
>   OptsVisitor: introduce unit tests, with test cases for range
>     flattening
> 
>  tests/Makefile              |    6 +-
>  qapi-schema-test.json       |   15 +++
>  include/qapi/opts-visitor.h |    6 +
>  qapi/opts-visitor.c         |  184 ++++++++++++++++++++++++-----
>  tests/test-opts-visitor.c   |  275 +++++++++++++++++++++++++++++++++++++++++++
>  .gitignore                  |    2 +
>  6 files changed, 456 insertions(+), 32 deletions(-)
>  create mode 100644 tests/test-opts-visitor.c
> 
> 

Re: [Qemu-devel] [PATCH 8/8] OptsVisitor: introduce unit tests, with test cases for range flattening

[Luiz Capitulino]

On Mon, 22 Jul 2013 23:07:36 +0200
Laszlo Ersek <lersek@redhat.com> wrote:

> 
> Signed-off-by: Laszlo Ersek <lersek@redhat.com>

This patch now conflicts, can you respin please?

> ---
>  tests/Makefile            |    6 +-
>  qapi-schema-test.json     |   15 +++
>  tests/test-opts-visitor.c |  275 +++++++++++++++++++++++++++++++++++++++++++++
>  .gitignore                |    1 +
>  4 files changed, 296 insertions(+), 1 deletions(-)
>  create mode 100644 tests/test-opts-visitor.c
> 
> diff --git a/tests/Makefile b/tests/Makefile
> index 425a9a8..8bb290e 100644
> --- a/tests/Makefile
> +++ b/tests/Makefile
> @@ -23,6 +23,8 @@ check-unit-y += tests/test-string-input-visitor$(EXESUF)
>  gcov-files-test-string-input-visitor-y = qapi/string-input-visitor.c
>  check-unit-y += tests/test-string-output-visitor$(EXESUF)
>  gcov-files-test-string-output-visitor-y = qapi/string-output-visitor.c
> +check-unit-y += tests/test-opts-visitor$(EXESUF)
> +gcov-files-test-opts-visitor-y = qapi/opts-visitor.c
>  check-unit-y += tests/test-coroutine$(EXESUF)
>  gcov-files-test-coroutine-y = coroutine-$(CONFIG_COROUTINE_BACKEND).c
>  check-unit-y += tests/test-visitor-serialization$(EXESUF)
> @@ -81,7 +83,8 @@ test-obj-y = tests/check-qint.o tests/check-qstring.o tests/check-qdict.o \
>  	tests/test-string-input-visitor.o tests/test-qmp-output-visitor.o \
>  	tests/test-qmp-input-visitor.o tests/test-qmp-input-strict.o \
>  	tests/test-qmp-commands.o tests/test-visitor-serialization.o \
> -	tests/test-x86-cpuid.o tests/test-mul64.o tests/test-int128.o
> +	tests/test-x86-cpuid.o tests/test-mul64.o tests/test-int128.o \
> +	tests/test-opts-visitor.o
>  
>  test-qapi-obj-y = tests/test-qapi-visit.o tests/test-qapi-types.o
>  
> @@ -123,6 +126,7 @@ tests/test-qmp-input-visitor$(EXESUF): tests/test-qmp-input-visitor.o $(test-qap
>  tests/test-qmp-input-strict$(EXESUF): tests/test-qmp-input-strict.o $(test-qapi-obj-y) libqemuutil.a libqemustub.a
>  tests/test-qmp-commands$(EXESUF): tests/test-qmp-commands.o tests/test-qmp-marshal.o $(test-qapi-obj-y) qapi-types.o qapi-visit.o libqemuutil.a libqemustub.a
>  tests/test-visitor-serialization$(EXESUF): tests/test-visitor-serialization.o $(test-qapi-obj-y) libqemuutil.a libqemustub.a
> +tests/test-opts-visitor$(EXESUF): tests/test-opts-visitor.o $(test-qapi-obj-y) libqemuutil.a libqemustub.a
>  
>  tests/test-mul64$(EXESUF): tests/test-mul64.o libqemuutil.a
>  
> diff --git a/qapi-schema-test.json b/qapi-schema-test.json
> index 4434fa3..fe5af75 100644
> --- a/qapi-schema-test.json
> +++ b/qapi-schema-test.json
> @@ -51,3 +51,18 @@
>  { 'command': 'user_def_cmd', 'data': {} }
>  { 'command': 'user_def_cmd1', 'data': {'ud1a': 'UserDefOne'} }
>  { 'command': 'user_def_cmd2', 'data': {'ud1a': 'UserDefOne', 'ud1b': 'UserDefOne'}, 'returns': 'UserDefTwo' }
> +
> +# For testing integer range flattening in opts-visitor. The following schema
> +# corresponds to the option format:
> +#
> +# -userdef i64=3-6,i64=-5--1,u64=2,u16=1,u16=7-12
> +#
> +# For simplicity, this example doesn't use [type=]discriminator nor optargs
> +# specific to discriminator values.
> +{ 'type': 'UserDefOptions',
> +  'data': {
> +    '*i64' : [ 'int'    ],
> +    '*u64' : [ 'uint64' ],
> +    '*u16' : [ 'uint16' ],
> +    '*i64x':   'int'     ,
> +    '*u64x':   'uint64'  } }
> diff --git a/tests/test-opts-visitor.c b/tests/test-opts-visitor.c
> new file mode 100644
> index 0000000..9f902b5
> --- /dev/null
> +++ b/tests/test-opts-visitor.c
> @@ -0,0 +1,275 @@
> +/*
> + * Options Visitor unit-tests.
> + *
> + * Copyright (C) 2013 Red Hat, Inc.
> + *
> + * Authors:
> + *   Laszlo Ersek <lersek@redhat.com> (based on test-string-output-visitor)
> + *
> + * This work is licensed under the terms of the GNU GPL, version 2 or later.
> + * See the COPYING file in the top-level directory.
> + */
> +
> +#include <glib.h>
> +
> +#include "qemu/config-file.h"     /* qemu_add_opts() */
> +#include "qemu/option.h"          /* qemu_opts_parse() */
> +#include "qapi/opts-visitor.h"    /* opts_visitor_new() */
> +#include "test-qapi-visit.h"      /* visit_type_UserDefOptions() */
> +#include "qapi/dealloc-visitor.h" /* qapi_dealloc_visitor_new() */
> +
> +static QemuOptsList userdef_opts = {
> +    .name = "userdef",
> +    .head = QTAILQ_HEAD_INITIALIZER(userdef_opts.head),
> +    .desc = { { 0 } } /* validated with OptsVisitor */
> +};
> +
> +/* fixture (= glib test case context) and test case manipulation */
> +
> +typedef struct OptsVisitorFixture {
> +    UserDefOptions *userdef;
> +    Error *err;
> +} OptsVisitorFixture;
> +
> +
> +static void
> +setup_fixture(OptsVisitorFixture *f, gconstpointer test_data)
> +{
> +    const char *opts_string = test_data;
> +    QemuOpts *opts;
> +    OptsVisitor *ov;
> +
> +    opts = qemu_opts_parse(qemu_find_opts("userdef"), opts_string, 0);
> +    g_assert(opts != NULL);
> +
> +    ov = opts_visitor_new(opts);
> +    visit_type_UserDefOptions(opts_get_visitor(ov), &f->userdef, NULL,
> +                              &f->err);
> +    opts_visitor_cleanup(ov);
> +    qemu_opts_del(opts);
> +}
> +
> +
> +static void
> +teardown_fixture(OptsVisitorFixture *f, gconstpointer test_data)
> +{
> +    if (f->userdef != NULL) {
> +        QapiDeallocVisitor *dv;
> +
> +        dv = qapi_dealloc_visitor_new();
> +        visit_type_UserDefOptions(qapi_dealloc_get_visitor(dv), &f->userdef,
> +                                  NULL, NULL);
> +        qapi_dealloc_visitor_cleanup(dv);
> +    }
> +    error_free(f->err);
> +}
> +
> +
> +static void
> +add_test(const char *testpath,
> +         void (*test_func)(OptsVisitorFixture *f, gconstpointer test_data),
> +         gconstpointer test_data)
> +{
> +    g_test_add(testpath, OptsVisitorFixture, test_data, setup_fixture,
> +               test_func, teardown_fixture);
> +}
> +
> +/* test output evaluation */
> +
> +static void
> +expect_ok(OptsVisitorFixture *f, gconstpointer test_data)
> +{
> +    g_assert(f->err == NULL);
> +    g_assert(f->userdef != NULL);
> +}
> +
> +
> +static void
> +expect_fail(OptsVisitorFixture *f, gconstpointer test_data)
> +{
> +    g_assert(f->err != NULL);
> +
> +    /* The error message is printed when this test utility is invoked directly
> +     * (ie. without gtester) and the --verbose flag is passed:
> +     *
> +     * tests/test-opts-visitor --verbose
> +     */
> +    g_test_message("'%s': %s", (const char *)test_data,
> +                   error_get_pretty(f->err));
> +}
> +
> +
> +static void
> +test_value(OptsVisitorFixture *f, gconstpointer test_data)
> +{
> +    uint64_t magic, bitval;
> +    intList *i64;
> +    uint64List *u64;
> +    uint16List *u16;
> +
> +    expect_ok(f, test_data);
> +
> +    magic = 0;
> +    for (i64 = f->userdef->i64; i64 != NULL; i64 = i64->next) {
> +        g_assert(-16 <= i64->value && i64->value < 64-16);
> +        bitval = 1ull << (i64->value + 16);
> +        g_assert((magic & bitval) == 0);
> +        magic |= bitval;
> +    }
> +    g_assert(magic == 0xDEADBEEF);
> +
> +    magic = 0;
> +    for (u64 = f->userdef->u64; u64 != NULL; u64 = u64->next) {
> +        g_assert(u64->value < 64);
> +        bitval = 1ull << u64->value;
> +        g_assert((magic & bitval) == 0);
> +        magic |= bitval;
> +    }
> +    g_assert(magic == 0xBADC0FFEE0DDF00D);
> +
> +    magic = 0;
> +    for (u16 = f->userdef->u16; u16 != NULL; u16 = u16->next) {
> +        g_assert(u16->value < 64);
> +        bitval = 1ull << u16->value;
> +        g_assert((magic & bitval) == 0);
> +        magic |= bitval;
> +    }
> +    g_assert(magic == 0xD15EA5E);
> +}
> +
> +
> +static void
> +expect_i64_min(OptsVisitorFixture *f, gconstpointer test_data)
> +{
> +    expect_ok(f, test_data);
> +    g_assert(f->userdef->has_i64);
> +    g_assert(f->userdef->i64->next == NULL);
> +    g_assert(f->userdef->i64->value == INT64_MIN);
> +}
> +
> +
> +static void
> +expect_i64_max(OptsVisitorFixture *f, gconstpointer test_data)
> +{
> +    expect_ok(f, test_data);
> +    g_assert(f->userdef->has_i64);
> +    g_assert(f->userdef->i64->next == NULL);
> +    g_assert(f->userdef->i64->value == INT64_MAX);
> +}
> +
> +
> +static void
> +expect_zero(OptsVisitorFixture *f, gconstpointer test_data)
> +{
> +    expect_ok(f, test_data);
> +    g_assert(f->userdef->has_u64);
> +    g_assert(f->userdef->u64->next == NULL);
> +    g_assert(f->userdef->u64->value == 0);
> +}
> +
> +
> +static void
> +expect_u64_max(OptsVisitorFixture *f, gconstpointer test_data)
> +{
> +    expect_ok(f, test_data);
> +    g_assert(f->userdef->has_u64);
> +    g_assert(f->userdef->u64->next == NULL);
> +    g_assert(f->userdef->u64->value == UINT64_MAX);
> +}
> +
> +/* test cases */
> +
> +int
> +main(int argc, char **argv)
> +{
> +    g_test_init(&argc, &argv, NULL);
> +
> +    qemu_add_opts(&userdef_opts);
> +
> +    /* Three hexadecimal magic numbers, "dead beef", "bad coffee, odd food" and
> +     * "disease", from
> +     * <http://en.wikipedia.org/wiki/Magic_number_%28programming%29>, were
> +     * converted to binary and dissected into bit ranges. Each magic number is
> +     * going to be recomposed using the lists called "i64", "u64" and "u16",
> +     * respectively.
> +     *
> +     * (Note that these types pertain to the individual bit shift counts, not
> +     * the magic numbers themselves; the intent is to exercise opts_type_int()
> +     * and opts_type_uint64().)
> +     *
> +     * The "i64" shift counts have been decreased by 16 (decimal) in order to
> +     * test negative values as well. Finally, the full list of QemuOpt elements
> +     * has been permuted with "shuf".
> +     *
> +     * Both "i64" and "u64" have some (distinct) single-element ranges
> +     * represented as both "a" and "a-a". "u16" is a special case of "i64" (see
> +     * visit_type_uint16()), so it wouldn't add a separate test in this regard.
> +     */
> +
> +    add_test("/visitor/opts/flatten/value", &test_value,
> +             "i64=-1-0,u64=12-16,u64=2-3,i64=-11--9,u64=57,u16=9,i64=5-5,"
> +             "u16=1-4,u16=20,u64=63-63,i64=-16--13,u64=50-52,i64=14-15,u16=11,"
> +             "i64=7,u16=18,i64=2-3,u16=6,u64=54-55,u64=0,u64=18-20,u64=33-43,"
> +             "i64=9-12,u16=26-27,u64=59-61,u16=13-16,u64=29-31,u64=22-23,"
> +             "u16=24,i64=-7--3");
> +
> +    add_test("/visitor/opts/i64/val1/errno",    &expect_fail,
> +             "i64=0x8000000000000000");
> +    add_test("/visitor/opts/i64/val1/empty",    &expect_fail, "i64=");
> +    add_test("/visitor/opts/i64/val1/trailing", &expect_fail, "i64=5z");
> +    add_test("/visitor/opts/i64/nonlist",       &expect_fail, "i64x=5-6");
> +    add_test("/visitor/opts/i64/val2/errno",    &expect_fail,
> +             "i64=0x7fffffffffffffff-0x8000000000000000");
> +    add_test("/visitor/opts/i64/val2/empty",    &expect_fail, "i64=5-");
> +    add_test("/visitor/opts/i64/val2/trailing", &expect_fail, "i64=5-6z");
> +    add_test("/visitor/opts/i64/range/empty",   &expect_fail, "i64=6-5");
> +    add_test("/visitor/opts/i64/range/minval",  &expect_i64_min,
> +             "i64=-0x8000000000000000--0x8000000000000000");
> +    add_test("/visitor/opts/i64/range/maxval",  &expect_i64_max,
> +             "i64=0x7fffffffffffffff-0x7fffffffffffffff");
> +
> +    add_test("/visitor/opts/u64/val1/errno",    &expect_fail, "u64=-1");
> +    add_test("/visitor/opts/u64/val1/empty",    &expect_fail, "u64=");
> +    add_test("/visitor/opts/u64/val1/trailing", &expect_fail, "u64=5z");
> +    add_test("/visitor/opts/u64/nonlist",       &expect_fail, "u64x=5-6");
> +    add_test("/visitor/opts/u64/val2/errno",    &expect_fail,
> +             "u64=0xffffffffffffffff-0x10000000000000000");
> +    add_test("/visitor/opts/u64/val2/empty",    &expect_fail, "u64=5-");
> +    add_test("/visitor/opts/u64/val2/trailing", &expect_fail, "u64=5-6z");
> +    add_test("/visitor/opts/u64/range/empty",   &expect_fail, "u64=6-5");
> +    add_test("/visitor/opts/u64/range/minval",  &expect_zero, "u64=0-0");
> +    add_test("/visitor/opts/u64/range/maxval",  &expect_u64_max,
> +             "u64=0xffffffffffffffff-0xffffffffffffffff");
> +
> +    /* Test maximum range sizes. The macro value is open-coded here
> +     * *intentionally*; the test case must use concrete values by design. If
> +     * OPTS_VISITOR_RANGE_MAX is changed, the following values need to be
> +     * recalculated as well. The assert and this comment should help with it.
> +     */
> +    g_assert(OPTS_VISITOR_RANGE_MAX == 65536);
> +
> +    /* The unsigned case is simple, a u64-u64 difference can always be
> +     * represented as a u64.
> +     */
> +    add_test("/visitor/opts/u64/range/max",  &expect_ok,   "u64=0-65535");
> +    add_test("/visitor/opts/u64/range/2big", &expect_fail, "u64=0-65536");
> +
> +    /* The same cannot be said about an i64-i64 difference. */
> +    add_test("/visitor/opts/i64/range/max/pos/a", &expect_ok,
> +             "i64=0x7fffffffffff0000-0x7fffffffffffffff");
> +    add_test("/visitor/opts/i64/range/max/pos/b", &expect_ok,
> +             "i64=0x7ffffffffffeffff-0x7ffffffffffffffe");
> +    add_test("/visitor/opts/i64/range/2big/pos",  &expect_fail,
> +             "i64=0x7ffffffffffeffff-0x7fffffffffffffff");
> +    add_test("/visitor/opts/i64/range/max/neg/a", &expect_ok,
> +             "i64=-0x8000000000000000--0x7fffffffffff0001");
> +    add_test("/visitor/opts/i64/range/max/neg/b", &expect_ok,
> +             "i64=-0x7fffffffffffffff--0x7fffffffffff0000");
> +    add_test("/visitor/opts/i64/range/2big/neg",  &expect_fail,
> +             "i64=-0x8000000000000000--0x7fffffffffff0000");
> +    add_test("/visitor/opts/i64/range/2big/full", &expect_fail,
> +             "i64=-0x8000000000000000-0x7fffffffffffffff");
> +
> +    g_test_run();
> +    return 0;
> +}
> diff --git a/.gitignore b/.gitignore
> index 388cb45..60a0363 100644
> --- a/.gitignore
> +++ b/.gitignore
> @@ -47,6 +47,7 @@ vscclient
>  QMP/qmp-commands.txt
>  test-coroutine
>  test-int128
> +test-opts-visitor
>  test-qmp-input-visitor
>  test-qmp-output-visitor
>  test-string-input-visitor

Re: [Qemu-devel] [PATCH 8/8] OptsVisitor: introduce unit tests, with test cases for range flattening

[Laszlo Ersek]

On 08/19/13 21:26, Luiz Capitulino wrote:
> On Mon, 22 Jul 2013 23:07:36 +0200
> Laszlo Ersek <lersek@redhat.com> wrote:
> 
>>
>> Signed-off-by: Laszlo Ersek <lersek@redhat.com>
> 
> This patch now conflicts, can you respin please?

Can you retry with "git am -3"?

"git rebase -i" didn't ask me to do anything manually, so I'm guessing
if you let "git am" to do a 3-way merge, it should work.

(Of course I don't have anything against posting a v2, this is just an
attempt to keep the traffic low.)

Thanks!
Laszlo

Re: [Qemu-devel] [PATCH 8/8] OptsVisitor: introduce unit tests, with test cases for range flattening

[Laszlo Ersek]

On 08/19/13 21:55, Laszlo Ersek wrote:
> On 08/19/13 21:26, Luiz Capitulino wrote:
>> On Mon, 22 Jul 2013 23:07:36 +0200
>> Laszlo Ersek <lersek@redhat.com> wrote:
>>
>>>
>>> Signed-off-by: Laszlo Ersek <lersek@redhat.com>
>>
>> This patch now conflicts, can you respin please?
> 
> Can you retry with "git am -3"?
> 
> "git rebase -i" didn't ask me to do anything manually, so I'm guessing
> if you let "git am" to do a 3-way merge, it should work.
> 
> (Of course I don't have anything against posting a v2, this is just an
> attempt to keep the traffic low.)

... yeah it looks like:

- a trivial context change in "tests/Makefile" (due to commit 3464700f),

- "qapi-schema-test.json" has been renamed to
"tests/qapi-schema/qapi-schema-test.json" (commit 4f193e34):

diff --git a/qapi-schema-test.json b/tests/qapi-schema/qapi-schema-test.json
similarity index 100%
rename from qapi-schema-test.json
rename to tests/qapi-schema/qapi-schema-test.json

These should be no problem for "git am -3".

Thanks,
Laszlo