parse_option_size()'s checking for overflow and trailing crap is
wrong.  Has always been that way.  qemu_strtosz() gets it right, so
use that.

This adds support for size suffixes 'P', 'E', and ignores case for all
suffixes, not just 'k'.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 tests/test-qemu-opts.c | 21 +++++++++------------
 util/qemu-option.c     | 41 +++++++++++++----------------------------
 2 files changed, 22 insertions(+), 40 deletions(-)

diff --git a/tests/test-qemu-opts.c b/tests/test-qemu-opts.c
index 6a9d3c5..b9d5b7e 100644
--- a/tests/test-qemu-opts.c
+++ b/tests/test-qemu-opts.c
@@ -667,10 +667,9 @@ static void test_opts_parse_size(void)
     g_assert(!opts);
     opts = qemu_opts_parse(&opts_list_02,
                            "size1=18446744073709550592", /* fffffffffffffc00 */
-                           false, &error_abort);
-    /* BUG: should reject */
-    g_assert_cmpuint(opts_count(opts), ==, 1);
-    g_assert_cmpuint(qemu_opt_get_size(opts, "size1", 1), ==, 0);
+                           false, &err);
+    error_free_or_abort(&err);
+    g_assert(!opts);
 
     /* Suffixes */
     opts = qemu_opts_parse(&opts_list_02, "size1=8b,size2=1.5k,size3=2M",
@@ -688,19 +687,17 @@ static void test_opts_parse_size(void)
 
     /* Beyond limit with suffix */
     opts = qemu_opts_parse(&opts_list_02, "size1=16777216T",
-                           false, &error_abort);
-    /* BUG: should reject */
-    g_assert_cmpuint(opts_count(opts), ==, 1);
-    g_assert_cmpuint(qemu_opt_get_size(opts, "size1", 1), ==, 0);
+                           false, &err);
+    error_free_or_abort(&err);
+    g_assert(!opts);
 
     /* Trailing crap */
     opts = qemu_opts_parse(&opts_list_02, "size1=16E", false, &err);
     error_free_or_abort(&err);
     g_assert(!opts);
-    opts = qemu_opts_parse(&opts_list_02, "size1=16Gi", false, &error_abort);
-    /* BUG: should reject */
-    g_assert_cmpuint(opts_count(opts), ==, 1);
-    g_assert_cmpuint(qemu_opt_get_size(opts, "size1", 1), ==, 16 * G_BYTE);
+    opts = qemu_opts_parse(&opts_list_02, "size1=16Gi", false, &err);
+    error_free_or_abort(&err);
+    g_assert(!opts);
 
     qemu_opts_reset(&opts_list_02);
 }
diff --git a/util/qemu-option.c b/util/qemu-option.c
index 5d82327..c11ce93 100644
--- a/util/qemu-option.c
+++ b/util/qemu-option.c
@@ -174,39 +174,24 @@ static const QemuOptDesc *find_desc_by_name(const QemuOptDesc *desc,
 void parse_option_size(const char *name, const char *value,
                        uint64_t *ret, Error **errp)
 {
-    char *postfix;
-    double sizef;
+    uint64_t size;
+    int err;
 
-    sizef = strtod(value, &postfix);
-    if (sizef < 0 || sizef > UINT64_MAX) {
+    err = qemu_strtosz(value, NULL, &size);
+    if (err == -ERANGE) {
+        error_setg(errp, "Value '%s' is too large for parameter '%s'",
+                   value, name);
+        return;
+    }
+    if (err) {
         error_setg(errp, QERR_INVALID_PARAMETER_VALUE, name,
                    "a non-negative number below 2^64");
+        error_append_hint(errp, "Optional suffix k, M, G, T, P or E means"
+                          " kilo-, mega-, giga-, tera-, peta-\n"
+                          "and exabytes, respectively.\n");
         return;
     }
-    switch (*postfix) {
-    case 'T':
-        sizef *= 1024;
-        /* fall through */
-    case 'G':
-        sizef *= 1024;
-        /* fall through */
-    case 'M':
-        sizef *= 1024;
-        /* fall through */
-    case 'K':
-    case 'k':
-        sizef *= 1024;
-        /* fall through */
-    case 'b':
-    case '\0':
-        *ret = (uint64_t) sizef;
-        break;
-    default:
-        error_setg(errp, QERR_INVALID_PARAMETER_VALUE, name, "a size");
-        error_append_hint(errp, "You may use k, M, G or T suffixes for "
-                          "kilobytes, megabytes, gigabytes and terabytes.\n");
-        return;
-    }
+    *ret = size;
 }
 
 bool has_help_option(const char *param)
-- 
2.7.4

On 02/14/2017 04:26 AM, Markus Armbruster wrote:
> parse_option_size()'s checking for overflow and trailing crap is
> wrong.  Has always been that way.  qemu_strtosz() gets it right, so
> use that.
> 
> This adds support for size suffixes 'P', 'E', and ignores case for all
> suffixes, not just 'k'.

Yay! Be liberal in what you accept!  (I've always been annoyed that '-m
1g' complains, forcing me to us '-m 1G').

Okay, we're at the end of the series, and I've pointed out other
possible improvements.  As mentioned elsewhere, it might be nice to add
a 25/24 to match coreutils, which allows 'm' and 'mib' to mean
1024*1024, vs. 'mb' to mean 1000*1000.  Right now, all clients of
qemu_strtosz() are stuck to one scale, vs. letting the user choose a
scale by their choice of suffix.  But it does not hold up this series if
you don't like the idea.

> 
> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> ---
>  tests/test-qemu-opts.c | 21 +++++++++------------
>  util/qemu-option.c     | 41 +++++++++++++----------------------------
>  2 files changed, 22 insertions(+), 40 deletions(-)
> 

Reviewed-by: Eric Blake <eblake@redhat.com>

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