diff mbox series

[v3,11/44] qemu-option: Replace opt_set() by cleaner opt_validate()

Message ID 20200706080950.403087-12-armbru@redhat.com
State New
Headers show
Series Less clumsy error checking | expand

Commit Message

Markus Armbruster July 6, 2020, 8:09 a.m. UTC 
opt_set() frees its argument @value on failure.  Slightly unclean;
functions ideally do nothing on failure.

To tidy this up, move opt_create() from opt_set() into its callers,
along with the cleanup.  Rename opt_set() to opt_validate(), noting
its similarity to qemu_opts_validate().  Drop redundant parameter
@opts; use opt->opts instead.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
---
 util/qemu-option.c | 35 +++++++++++++++++++----------------
 1 file changed, 19 insertions(+), 16 deletions(-)
diff mbox series

Patch

diff --git a/util/qemu-option.c b/util/qemu-option.c
index 1023fe7527..d8233b3b35 100644
--- a/util/qemu-option.c
+++ b/util/qemu-option.c
@@ -516,36 +516,39 @@  static QemuOpt *opt_create(QemuOpts *opts, const char *name, char *value,
     return opt;
 }
 
-static void opt_set(QemuOpts *opts, const char *name, char *value,
-                    bool prepend, bool *help_wanted, Error **errp)
+static bool opt_validate(QemuOpt *opt, bool *help_wanted,
+                         Error **errp)
 {
-    QemuOpt *opt;
     const QemuOptDesc *desc;
     Error *local_err = NULL;
 
-    desc = find_desc_by_name(opts->list->desc, name);
-    if (!desc && !opts_accepts_any(opts)) {
-        g_free(value);
-        error_setg(errp, QERR_INVALID_PARAMETER, name);
-        if (help_wanted && is_help_option(name)) {
+    desc = find_desc_by_name(opt->opts->list->desc, opt->name);
+    if (!desc && !opts_accepts_any(opt->opts)) {
+        error_setg(errp, QERR_INVALID_PARAMETER, opt->name);
+        if (help_wanted && is_help_option(opt->name)) {
             *help_wanted = true;
         }
-        return;
+        return false;
     }
 
-    opt = opt_create(opts, name, value, prepend);
     opt->desc = desc;
     qemu_opt_parse(opt, &local_err);
     if (local_err) {
         error_propagate(errp, local_err);
-        qemu_opt_del(opt);
+        return false;
     }
+
+    return true;
 }
 
 void qemu_opt_set(QemuOpts *opts, const char *name, const char *value,
                   Error **errp)
 {
-    opt_set(opts, name, g_strdup(value), false, NULL, errp);
+    QemuOpt *opt = opt_create(opts, name, g_strdup(value), false);
+
+    if (!opt_validate(opt, NULL, errp)) {
+        qemu_opt_del(opt);
+    }
 }
 
 void qemu_opt_set_bool(QemuOpts *opts, const char *name, bool val,
@@ -817,9 +820,9 @@  static void opts_do_parse(QemuOpts *opts, const char *params,
                           const char *firstname, bool prepend,
                           bool *help_wanted, Error **errp)
 {
-    Error *local_err = NULL;
     char *option, *value;
     const char *p;
+    QemuOpt *opt;
 
     for (p = params; *p;) {
         p = get_opt_name_value(p, firstname, &option, &value);
@@ -831,10 +834,10 @@  static void opts_do_parse(QemuOpts *opts, const char *params,
             continue;
         }
 
-        opt_set(opts, option, value, prepend, help_wanted, &local_err);
+        opt = opt_create(opts, option, value, prepend);
         g_free(option);
-        if (local_err) {
-            error_propagate(errp, local_err);
+        if (!opt_validate(opt, help_wanted, errp)) {
+            qemu_opt_del(opt);
             return;
         }
     }