diff mbox

[v18,11/14] qapi: make string input visitor parse int list

Message ID 2b9b6745ac6f7a5b697f479d684b226505952701.1392794450.git.hutao@cn.fujitsu.com
State New
Headers show

Commit Message

Hu Tao Feb. 19, 2014, 7:54 a.m. UTC
Cc: Laszlo Ersek <lersek@redhat.com>
Signed-off-by: Hu Tao <hutao@cn.fujitsu.com>
---
 qapi/string-input-visitor.c       | 160 ++++++++++++++++++++++++++++++++++++--
 tests/test-string-input-visitor.c |  22 ++++++
 2 files changed, 176 insertions(+), 6 deletions(-)

Comments

Hu Tao Feb. 19, 2014, 8:17 a.m. UTC | #1
On Wed, Feb 19, 2014 at 03:54:02PM +0800, Hu Tao wrote:
> Cc: Laszlo Ersek <lersek@redhat.com>
> Signed-off-by: Hu Tao <hutao@cn.fujitsu.com>
> ---
>  qapi/string-input-visitor.c       | 160 ++++++++++++++++++++++++++++++++++++--
>  tests/test-string-input-visitor.c |  22 ++++++
>  2 files changed, 176 insertions(+), 6 deletions(-)
> 
> diff --git a/qapi/string-input-visitor.c b/qapi/string-input-visitor.c
> index a152f5d..4540ca3 100644
> --- a/qapi/string-input-visitor.c
> +++ b/qapi/string-input-visitor.c
> @@ -15,30 +15,175 @@
>  #include "qapi/visitor-impl.h"
>  #include "qapi/qmp/qerror.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 StringInputVisitor
>  {
>      Visitor visitor;
> +
> +    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_next, range_limit;
> +
>      const char *string;
>  };
>  
> +static void
> +start_list(Visitor *v, const char *name, Error **errp)
> +{
> +    StringInputVisitor *siv = DO_UPCAST(StringInputVisitor, visitor, v);
> +
> +    /* we can't traverse a list in a list */
> +    assert(siv->list_mode == LM_NONE);
> +    siv->list_mode = LM_STARTED;
> +}
> +
> +static GenericList *
> +next_list(Visitor *v, GenericList **list, Error **errp)
> +{
> +    StringInputVisitor *siv = DO_UPCAST(StringInputVisitor, visitor, v);
> +    GenericList **link;
> +
> +    switch (siv->list_mode) {
> +    case LM_STARTED:
> +        siv->list_mode = LM_IN_PROGRESS;
> +        link = list;
> +        break;
> +
> +    case LM_SIGNED_INTERVAL:
> +    case LM_UNSIGNED_INTERVAL:
> +        link = &(*list)->next;
> +
> +        if (siv->list_mode == LM_SIGNED_INTERVAL) {
> +            if (siv->range_next.s < siv->range_limit.s) {
> +                ++siv->range_next.s;
> +                break;
> +            }
> +        } else if (siv->range_next.u < siv->range_limit.u) {
> +            ++siv->range_next.u;
> +            break;
> +        }
> +        siv->list_mode = LM_END;
> +        /* range has been completed, fall through */
> +
> +    case LM_END:
> +        return NULL;
> +
> +    case LM_IN_PROGRESS:
> +        link = &(*list)->next;
> +        break;
> +
> +    default:
> +        abort();
> +    }
> +
> +    *link = g_malloc0(sizeof **link);
> +    return *link;
> +}
> +
> +static void
> +end_list(Visitor *v, Error **errp)
> +{
> +    StringInputVisitor *siv = DO_UPCAST(StringInputVisitor, visitor, v);
> +
> +    assert(siv->list_mode == LM_STARTED ||
> +           siv->list_mode == LM_END ||
> +           siv->list_mode == LM_IN_PROGRESS ||
> +           siv->list_mode == LM_SIGNED_INTERVAL ||
> +           siv->list_mode == LM_UNSIGNED_INTERVAL);
> +    siv->list_mode = LM_NONE;
> +}
> +
>  static void parse_type_int(Visitor *v, int64_t *obj, const char *name,
>                             Error **errp)
>  {
>      StringInputVisitor *siv = DO_UPCAST(StringInputVisitor, visitor, v);
> -    char *endp = (char *) siv->string;
> +    char *str = (char *) siv->string;
>      long long val;
> +    char *endptr;
>  
> -    errno = 0;
> -    if (siv->string) {
> -        val = strtoll(siv->string, &endp, 0);
> +    if (siv->list_mode == LM_SIGNED_INTERVAL) {
> +        *obj = siv->range_next.s;
> +        return;
>      }
> -    if (!siv->string || errno || endp == siv->string || *endp) {
> +
> +    if (!siv->string) {
>          error_set(errp, QERR_INVALID_PARAMETER_TYPE, name ? name : "null",
>                    "integer");
>          return;
>      }
>  
> -    *obj = val;
> +    errno = 0;
> +    val = strtoll(siv->string, &endptr, 0);
> +
> +    if (errno == 0 && endptr > str && INT64_MIN <= val && val <= INT64_MAX) {
> +        if (*endptr == '\0') {
> +            *obj = val;
> +            siv->list_mode = LM_END;
> +            return;
> +        }
> +        if (*endptr == '-' && siv->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 &&
> +                (val > INT64_MAX - 65536 ||
> +                 val2 < val + 65536)) {
> +                siv->range_next.s = val;
> +                siv->range_limit.s = val2;
> +                siv->list_mode = LM_SIGNED_INTERVAL;
> +
> +                /* as if entering on the top */
> +                *obj = siv->range_next.s;
> +                return;
> +            }
> +        }
> +    }
> +    error_set(errp, QERR_INVALID_PARAMETER_VALUE, name,
> +              (siv->list_mode == LM_NONE) ? "an int64 value" :
> +                                           "an int64 value or range");
>  }

Two problems:

1. the code is mostly copied from OptsVisitor. maybe we can share the
   code?

2. int list is not implemented in string outout visitor. but there is
   currently no user of it. Should we implement it or not?
Paolo Bonzini Feb. 19, 2014, 8:42 a.m. UTC | #2
Il 19/02/2014 09:17, Hu Tao ha scritto:
> Two problems:
>
> 1. the code is mostly copied from OptsVisitor. maybe we can share the
>    code?

I think it's not a huge problem.  Maybe OptsVisitor could be made to use 
a StringInputVisitor internally.

> 2. int list is not implemented in string outout visitor. but there is
>    currently no user of it. Should we implement it or not?

Yes, please.  We probably will add sooner or later a qom-get/qom-set 
pair of HMP commands and these will use the string output visitor.

Paolo
diff mbox

Patch

diff --git a/qapi/string-input-visitor.c b/qapi/string-input-visitor.c
index a152f5d..4540ca3 100644
--- a/qapi/string-input-visitor.c
+++ b/qapi/string-input-visitor.c
@@ -15,30 +15,175 @@ 
 #include "qapi/visitor-impl.h"
 #include "qapi/qmp/qerror.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 StringInputVisitor
 {
     Visitor visitor;
+
+    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_next, range_limit;
+
     const char *string;
 };
 
+static void
+start_list(Visitor *v, const char *name, Error **errp)
+{
+    StringInputVisitor *siv = DO_UPCAST(StringInputVisitor, visitor, v);
+
+    /* we can't traverse a list in a list */
+    assert(siv->list_mode == LM_NONE);
+    siv->list_mode = LM_STARTED;
+}
+
+static GenericList *
+next_list(Visitor *v, GenericList **list, Error **errp)
+{
+    StringInputVisitor *siv = DO_UPCAST(StringInputVisitor, visitor, v);
+    GenericList **link;
+
+    switch (siv->list_mode) {
+    case LM_STARTED:
+        siv->list_mode = LM_IN_PROGRESS;
+        link = list;
+        break;
+
+    case LM_SIGNED_INTERVAL:
+    case LM_UNSIGNED_INTERVAL:
+        link = &(*list)->next;
+
+        if (siv->list_mode == LM_SIGNED_INTERVAL) {
+            if (siv->range_next.s < siv->range_limit.s) {
+                ++siv->range_next.s;
+                break;
+            }
+        } else if (siv->range_next.u < siv->range_limit.u) {
+            ++siv->range_next.u;
+            break;
+        }
+        siv->list_mode = LM_END;
+        /* range has been completed, fall through */
+
+    case LM_END:
+        return NULL;
+
+    case LM_IN_PROGRESS:
+        link = &(*list)->next;
+        break;
+
+    default:
+        abort();
+    }
+
+    *link = g_malloc0(sizeof **link);
+    return *link;
+}
+
+static void
+end_list(Visitor *v, Error **errp)
+{
+    StringInputVisitor *siv = DO_UPCAST(StringInputVisitor, visitor, v);
+
+    assert(siv->list_mode == LM_STARTED ||
+           siv->list_mode == LM_END ||
+           siv->list_mode == LM_IN_PROGRESS ||
+           siv->list_mode == LM_SIGNED_INTERVAL ||
+           siv->list_mode == LM_UNSIGNED_INTERVAL);
+    siv->list_mode = LM_NONE;
+}
+
 static void parse_type_int(Visitor *v, int64_t *obj, const char *name,
                            Error **errp)
 {
     StringInputVisitor *siv = DO_UPCAST(StringInputVisitor, visitor, v);
-    char *endp = (char *) siv->string;
+    char *str = (char *) siv->string;
     long long val;
+    char *endptr;
 
-    errno = 0;
-    if (siv->string) {
-        val = strtoll(siv->string, &endp, 0);
+    if (siv->list_mode == LM_SIGNED_INTERVAL) {
+        *obj = siv->range_next.s;
+        return;
     }
-    if (!siv->string || errno || endp == siv->string || *endp) {
+
+    if (!siv->string) {
         error_set(errp, QERR_INVALID_PARAMETER_TYPE, name ? name : "null",
                   "integer");
         return;
     }
 
-    *obj = val;
+    errno = 0;
+    val = strtoll(siv->string, &endptr, 0);
+
+    if (errno == 0 && endptr > str && INT64_MIN <= val && val <= INT64_MAX) {
+        if (*endptr == '\0') {
+            *obj = val;
+            siv->list_mode = LM_END;
+            return;
+        }
+        if (*endptr == '-' && siv->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 &&
+                (val > INT64_MAX - 65536 ||
+                 val2 < val + 65536)) {
+                siv->range_next.s = val;
+                siv->range_limit.s = val2;
+                siv->list_mode = LM_SIGNED_INTERVAL;
+
+                /* as if entering on the top */
+                *obj = siv->range_next.s;
+                return;
+            }
+        }
+    }
+    error_set(errp, QERR_INVALID_PARAMETER_VALUE, name,
+              (siv->list_mode == LM_NONE) ? "an int64 value" :
+                                           "an int64 value or range");
 }
 
 static void parse_type_bool(Visitor *v, bool *obj, const char *name,
@@ -149,6 +294,9 @@  StringInputVisitor *string_input_visitor_new(const char *str)
     v->visitor.type_str = parse_type_str;
     v->visitor.type_number = parse_type_number;
     v->visitor.type_size = parse_type_size;
+    v->visitor.start_list = start_list;
+    v->visitor.next_list = next_list;
+    v->visitor.end_list = end_list;
     v->visitor.start_optional = parse_start_optional;
 
     v->string = str;
diff --git a/tests/test-string-input-visitor.c b/tests/test-string-input-visitor.c
index 5989f81..3b47ddf 100644
--- a/tests/test-string-input-visitor.c
+++ b/tests/test-string-input-visitor.c
@@ -64,6 +64,26 @@  static void test_visitor_in_int(TestInputVisitorData *data,
     g_assert_cmpint(res, ==, value);
 }
 
+static void test_visitor_in_intList(TestInputVisitorData *data,
+                                    const void *unused)
+{
+    int64_t value[] = {-2, -1, 0, 1, 2, 3, 4};
+    int16List *res = NULL;
+    Error *errp = NULL;
+    Visitor *v;
+    int i = 0;
+
+    v = visitor_input_test_init(data, "-2-4");
+
+    visit_type_int16List(v, &res, NULL, &errp);
+    g_assert(!error_is_set(&errp));
+    while (res && i < sizeof(value) / sizeof(value[0])) {
+        printf("%d\n", res->value);
+        g_assert_cmpint(res->value, ==, value[i++]);
+        res = res->next;
+    }
+}
+
 static void test_visitor_in_bool(TestInputVisitorData *data,
                                  const void *unused)
 {
@@ -228,6 +248,8 @@  int main(int argc, char **argv)
 
     input_visitor_test_add("/string-visitor/input/int",
                            &in_visitor_data, test_visitor_in_int);
+    input_visitor_test_add("/string-visitor/input/intList",
+                           &in_visitor_data, test_visitor_in_intList);
     input_visitor_test_add("/string-visitor/input/bool",
                            &in_visitor_data, test_visitor_in_bool);
     input_visitor_test_add("/string-visitor/input/number",