[Qemu-devel] [PATCH v15 21/23] qapi: Fix string input visitor handling of invalid list

[Eric Blake]

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.  Also, make sure the error message
for visit_type_uint64() gracefully handles a NULL 'name' when
called from the top level or a list context.

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 <address@hidden>

---
v15: new patch
---
 qapi/string-input-visitor.c       | 19 +++++++++++++------
 tests/test-string-input-visitor.c | 12 +++++-------
 2 files changed, 18 insertions(+), 13 deletions(-)

diff --git a/qapi/string-input-visitor.c b/qapi/string-input-visitor.c
index 797973a..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;
@@ -222,7 +229,7 @@ static void parse_type_int64(Visitor *v, const char *name, 
int64_t *obj,
     return;

 error:
-    error_setg(errp, QERR_INVALID_PARAMETER_VALUE, name,
+    error_setg(errp, QERR_INVALID_PARAMETER_VALUE, name ? name : "null",
                "an int64 value or range");
 }

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,
-- 
2.5.5