[PULL,21/23] qapi: Fix string input visitor handling of invalid list

Commit Message

Markus Armbruster May 12, 2016, 7:59 a.m. UTC

From: Eric Blake <[e-mail]>

As shown in the previous commit, the string input visitor was
treating bogus input as an empty list rather than an error.
Fix parse_str() to set errp, then the callers to exit early if
an error was reported.

Meanwhile, fix the testsuite to use the generated
qapi_free_int16List() instead of rolling our own, and to
validate the fixed behavior, while at the same time documenting
one more change that we'd like to make in a later patch (a
failed visit_start_list should guarantee a NULL pointer,
regardless of what things were on input).

Signed-off-by: Eric Blake <[e-mail]>
Message-Id: <[e-mail]>
Signed-off-by: Markus Armbruster <[e-mail]>
---
 qapi/string-input-visitor.c       | 17 ++++++++++++-----
 tests/test-string-input-visitor.c | 12 +++++-------
 2 files changed, 17 insertions(+), 12 deletions(-)

Patch

diff --git a/qapi/string-input-visitor.c b/qapi/string-input-visitor.c
index b55d199..ad150dc 100644
--- a/qapi/string-input-visitor.c
+++ b/qapi/string-input-visitor.c
@@ -44,7 +44,7 @@  static void free_range(void *range, void *dummy)
     g_free(range);
 }
 
-static void parse_str(StringInputVisitor *siv, Error **errp)
+static int parse_str(StringInputVisitor *siv, const char *name, Error **errp)
 {
     char *str = (char *) siv->string;
     long long start, end;
@@ -52,7 +52,7 @@  static void parse_str(StringInputVisitor *siv, Error **errp)
     char *endptr;
 
     if (siv->ranges) {
-        return;
+        return 0;
     }
 
     do {
@@ -117,11 +117,14 @@  static void parse_str(StringInputVisitor *siv, Error **errp)
         }
     } while (str);
 
-    return;
+    return 0;
 error:
     g_list_foreach(siv->ranges, free_range, NULL);
     g_list_free(siv->ranges);
     siv->ranges = NULL;
+    error_setg(errp, QERR_INVALID_PARAMETER_VALUE, name ? name : "null",
+               "an int64 value or range");
+    return -1;
 }
 
 static void
@@ -129,7 +132,9 @@  start_list(Visitor *v, const char *name, Error **errp)
 {
     StringInputVisitor *siv = to_siv(v);
 
-    parse_str(siv, errp);
+    if (parse_str(siv, name, errp) < 0) {
+        return;
+    }
 
     siv->cur_range = g_list_first(siv->ranges);
     if (siv->cur_range) {
@@ -195,7 +200,9 @@  static void parse_type_int64(Visitor *v, const char *name, int64_t *obj,
         return;
     }
 
-    parse_str(siv, errp);
+    if (parse_str(siv, name, errp) < 0) {
+        return;
+    }
 
     if (!siv->ranges) {
         goto error;
diff --git a/tests/test-string-input-visitor.c b/tests/test-string-input-visitor.c
index 8114908..f99824d 100644
--- a/tests/test-string-input-visitor.c
+++ b/tests/test-string-input-visitor.c
@@ -92,19 +92,17 @@  static void test_visitor_in_intList(TestInputVisitorData *data,
     }
     g_assert(!tmp);
 
-    tmp = res;
-    while (tmp) {
-        res = res->next;
-        g_free(tmp);
-        tmp = res;
-    }
+    qapi_free_int16List(res);
 
     visitor_input_teardown(data, unused);
 
     v = visitor_input_test_init(data, "not an int list");
 
+    /* FIXME: res should be NULL on failure, regardless of starting value */
+    res = NULL;
     visit_type_int16List(v, NULL, &res, &err);
-    /* FIXME fix the visitor, then error_free_or_abort(&err) here */
+    error_free_or_abort(&err);
+    g_assert(!res);
 }
 
 static void test_visitor_in_bool(TestInputVisitorData *data,