From 8ee8409eff5643c677d6e5dce6620355a89c80f8 Mon Sep 17 00:00:00 2001
From: Markus Armbruster <armbru@redhat.com>
Date: Tue, 21 Feb 2017 21:13:46 +0100
Subject: option: Assert value string isn't null

Plenty of code relies on QemuOpt member @str not being null, including
qemu_opts_print(), qemu_opts_to_qdict(), and callbacks passed to
qemu_opt_foreach().

Begs the question whether it can be null.  Only opt_set() creates
QemuOpt.  It sets member @str to its argument @value.  Passing null
for @value would plant a time bomb.  Callers:

* opts_do_parse() can't pass null.

* qemu_opt_set() passes its argument @value.  Callers:

  - qemu_opts_from_qdict_1() can't pass null

  - qemu_opts_set() passes its argument @value, but none of its
    callers pass null.

  - Many more outside qemu-option.c, but they shouldn't pass null,
    either.

Assert member @str isn't null, so that misuse is caught right away.

Simplify parse_option_bool(), parse_option_number() and
parse_option_size() accordingly.  Best viewed with whitespace changes
ignored.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Message-Id: <1487708048-2131-3-git-send-email-armbru@redhat.com>
---
 util/qemu-option.c | 89 ++++++++++++++++++++++++------------------------------
 1 file changed, 39 insertions(+), 50 deletions(-)

(limited to 'util')

diff --git a/util/qemu-option.c b/util/qemu-option.c
index d611946..9708668 100644
--- a/util/qemu-option.c
+++ b/util/qemu-option.c
@@ -128,17 +128,13 @@ int get_param_value(char *buf, int buf_size,
 static void parse_option_bool(const char *name, const char *value, bool *ret,
                               Error **errp)
 {
-    if (value != NULL) {
-        if (!strcmp(value, "on")) {
-            *ret = 1;
-        } else if (!strcmp(value, "off")) {
-            *ret = 0;
-        } else {
-            error_setg(errp, QERR_INVALID_PARAMETER_VALUE,
-                       name, "'on' or 'off'");
-        }
-    } else {
+    if (!strcmp(value, "on")) {
         *ret = 1;
+    } else if (!strcmp(value, "off")) {
+        *ret = 0;
+    } else {
+        error_setg(errp, QERR_INVALID_PARAMETER_VALUE,
+                   name, "'on' or 'off'");
     }
 }
 
@@ -148,16 +144,12 @@ static void parse_option_number(const char *name, const char *value,
     char *postfix;
     uint64_t number;
 
-    if (value != NULL) {
-        number = strtoull(value, &postfix, 0);
-        if (*postfix != '\0') {
-            error_setg(errp, QERR_INVALID_PARAMETER_VALUE, name, "a number");
-            return;
-        }
-        *ret = number;
-    } else {
+    number = strtoull(value, &postfix, 0);
+    if (*postfix != '\0') {
         error_setg(errp, QERR_INVALID_PARAMETER_VALUE, name, "a number");
+        return;
     }
+    *ret = number;
 }
 
 static const QemuOptDesc *find_desc_by_name(const QemuOptDesc *desc,
@@ -180,39 +172,35 @@ void parse_option_size(const char *name, const char *value,
     char *postfix;
     double sizef;
 
-    if (value != NULL) {
-        sizef = strtod(value, &postfix);
-        if (sizef < 0 || sizef > UINT64_MAX) {
-            error_setg(errp, QERR_INVALID_PARAMETER_VALUE, name,
-                             "a non-negative number below 2^64");
-            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;
-        }
-    } else {
+    sizef = strtod(value, &postfix);
+    if (sizef < 0 || sizef > UINT64_MAX) {
+        error_setg(errp, QERR_INVALID_PARAMETER_VALUE, name,
+                   "a non-negative number below 2^64");
+        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;
     }
 }
 
@@ -566,6 +554,7 @@ static void opt_set(QemuOpts *opts, const char *name, const char *value,
     }
     opt->desc = desc;
     opt->str = g_strdup(value);
+    assert(opt->str);
     qemu_opt_parse(opt, &local_err);
     if (local_err) {
         error_propagate(errp, local_err);
-- 
cgit v1.1