[Qemu-devel] [PATCH v1 1/7] qapi: correctly parse uint64_t values from strings

[David Hildenbrand]

Right now, we parse uint64_t values just like int64_t values, resulting
in negative values getting accepted and certain valid large numbers only
being representable as negative numbers. Also, reported errors indicate
that an int64_t is expected.

Parse uin64_t separately. Implementation inspired by original
parse_str() implementation.

E.g. we can now specify
    -device nvdimm,memdev=mem1,id=nv1,addr=0xFFFFFFFFC0000000
Instead of going via negative values
    -device nvdimm,memdev=mem1,id=nv1,addr=-0x40000000

Resulting in the same values

(qemu) info memory-devices
Memory device [nvdimm]: "nv1"
  addr: 0xffffffffc0000000
  slot: 0
  node: 0

Signed-off-by: David Hildenbrand <address@hidden>
---
 qapi/string-input-visitor.c | 117 ++++++++++++++++++++++++++++++++----
 1 file changed, 106 insertions(+), 11 deletions(-)

diff --git a/qapi/string-input-visitor.c b/qapi/string-input-visitor.c
index b3fdd0827d..af0a841152 100644
--- a/qapi/string-input-visitor.c
+++ b/qapi/string-input-visitor.c
@@ -19,6 +19,7 @@
 #include "qapi/qmp/qnull.h"
 #include "qemu/option.h"
 #include "qemu/queue.h"
+#include "qemu/cutils.h"
 #include "qemu/range.h"
 
 
@@ -44,7 +45,8 @@ static void free_range(void *range, void *dummy)
     g_free(range);
 }
 
-static int parse_str(StringInputVisitor *siv, const char *name, Error **errp)
+static int parse_str_int64(StringInputVisitor *siv, const char *name,
+                           Error **errp)
 {
     char *str = (char *) siv->string;
     long long start, end;
@@ -118,6 +120,75 @@ error:
     return -1;
 }
 
+static int parse_str_uint64(StringInputVisitor *siv, const char *name,
+                            Error **errp)
+{
+    const char *str = (char *) siv->string;
+    uint64_t start, end;
+    const char *endptr;
+    Range *cur;
+
+    if (siv->ranges) {
+        return 0;
+    }
+
+    if (!*str) {
+        return 0;
+    }
+
+    do {
+        if (!qemu_strtou64(str, &endptr, 0, &start)) {
+            if (*endptr == '\0') {
+                cur = g_malloc0(sizeof(*cur));
+                range_set_bounds(cur, start, start);
+                siv->ranges = range_list_insert(siv->ranges, cur);
+                cur = NULL;
+                str = NULL;
+            } else if (*endptr == '-') {
+                str = endptr + 1;
+                if (!qemu_strtou64(str, &endptr, 0, &end) && start <= end) {
+                    if (*endptr == '\0') {
+                        cur = g_malloc0(sizeof(*cur));
+                        range_set_bounds(cur, start, end);
+                        siv->ranges = range_list_insert(siv->ranges, cur);
+                        cur = NULL;
+                        str = NULL;
+                    } else if (*endptr == ',') {
+                        str = endptr + 1;
+                        cur = g_malloc0(sizeof(*cur));
+                        range_set_bounds(cur, start, end);
+                        siv->ranges = range_list_insert(siv->ranges, cur);
+                        cur = NULL;
+                    } else {
+                        goto error;
+                    }
+                } else {
+                    goto error;
+                }
+            } else if (*endptr == ',') {
+                str = endptr + 1;
+                cur = g_malloc0(sizeof(*cur));
+                range_set_bounds(cur, start, start);
+                siv->ranges = range_list_insert(siv->ranges, cur);
+                cur = NULL;
+            } else {
+                goto error;
+            }
+        } else {
+            goto error;
+        }
+    } while (str);
+
+    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 uint64 value or range");
+    return -1;
+}
+
 static void
 start_list(Visitor *v, const char *name, GenericList **list, size_t size,
            Error **errp)
@@ -128,7 +199,7 @@ start_list(Visitor *v, const char *name, GenericList 
**list, size_t size,
     assert(list);
     siv->list = list;
 
-    if (parse_str(siv, name, errp) < 0) {
+    if (parse_str_int64(siv, name, errp) < 0) {
         *list = NULL;
         return;
     }
@@ -216,7 +287,7 @@ static void parse_type_int64(Visitor *v, const char *name, 
int64_t *obj,
 {
     StringInputVisitor *siv = to_siv(v);
 
-    if (parse_str(siv, name, errp) < 0) {
+    if (parse_str_int64(siv, name, errp) < 0) {
         return;
     }
 
@@ -252,15 +323,39 @@ error:
 static void parse_type_uint64(Visitor *v, const char *name, uint64_t *obj,
                               Error **errp)
 {
-    /* FIXME: parse_type_int64 mishandles values over INT64_MAX */
-    int64_t i;
-    Error *err = NULL;
-    parse_type_int64(v, name, &i, &err);
-    if (err) {
-        error_propagate(errp, err);
-    } else {
-        *obj = i;
+    StringInputVisitor *siv = to_siv(v);
+
+    if (parse_str_uint64(siv, name, errp) < 0) {
+        return;
+    }
+
+    if (!siv->ranges) {
+        goto error;
+    }
+
+    if (!siv->cur_range) {
+        Range *r;
+
+        siv->cur_range = g_list_first(siv->ranges);
+        if (!siv->cur_range) {
+            goto error;
+        }
+
+        r = siv->cur_range->data;
+        if (!r) {
+            goto error;
+        }
+
+        siv->cur = range_lob(r);
     }
+
+    *obj = siv->cur;
+    siv->cur++;
+    return;
+
+error:
+    error_setg(errp, QERR_INVALID_PARAMETER_VALUE, name ? name : "null",
+               "an uint64 value or range");
 }
 
 static void parse_type_size(Visitor *v, const char *name, uint64_t *obj,
-- 
2.17.1