From 6129803b55553b90805aa5012077b21c6c6eacdc Mon Sep 17 00:00:00 2001 From: Markus Armbruster Date: Wed, 15 Apr 2020 09:49:20 +0200 Subject: qemu-options: Factor out get_opt_name_value() helper The next commits will put it to use. Signed-off-by: Markus Armbruster Reviewed-by: Eric Blake Reviewed-by: Kevin Wolf Message-Id: <20200415074927.19897-3-armbru@redhat.com> --- util/qemu-option.c | 102 +++++++++++++++++++++++++++++------------------------ 1 file changed, 56 insertions(+), 46 deletions(-) (limited to 'util/qemu-option.c') diff --git a/util/qemu-option.c b/util/qemu-option.c index 97172b5eaa..f08f4bc458 100644 --- a/util/qemu-option.c +++ b/util/qemu-option.c @@ -805,61 +805,71 @@ void qemu_opts_print(QemuOpts *opts, const char *separator) } } +static const char *get_opt_name_value(const char *params, + const char *firstname, + char **name, char **value) +{ + const char *p, *pe, *pc; + + pe = strchr(params, '='); + pc = strchr(params, ','); + + if (!pe || (pc && pc < pe)) { + /* found "foo,more" */ + if (firstname) { + /* implicitly named first option */ + *name = g_strdup(firstname); + p = get_opt_value(params, value); + } else { + /* option without value, must be a flag */ + p = get_opt_name(params, name, ','); + if (strncmp(*name, "no", 2) == 0) { + memmove(*name, *name + 2, strlen(*name + 2) + 1); + *value = g_strdup("off"); + } else { + *value = g_strdup("on"); + } + } + } else { + /* found "foo=bar,more" */ + p = get_opt_name(params, name, '='); + assert(*p == '='); + p++; + p = get_opt_value(p, value); + } + + assert(!*p || *p == ','); + if (*p == ',') { + p++; + } + return p; +} + static void opts_do_parse(QemuOpts *opts, const char *params, const char *firstname, bool prepend, bool *invalidp, Error **errp) { - char *option = NULL; - char *value = NULL; - const char *p,*pe,*pc; Error *local_err = NULL; + char *option, *value; + const char *p; - for (p = params; *p != '\0'; p++) { - pe = strchr(p, '='); - pc = strchr(p, ','); - if (!pe || (pc && pc < pe)) { - /* found "foo,more" */ - if (p == params && firstname) { - /* implicitly named first option */ - option = g_strdup(firstname); - p = get_opt_value(p, &value); - } else { - /* option without value, probably a flag */ - p = get_opt_name(p, &option, ','); - if (strncmp(option, "no", 2) == 0) { - memmove(option, option+2, strlen(option+2)+1); - value = g_strdup("off"); - } else { - value = g_strdup("on"); - } - } - } else { - /* found "foo=bar,more" */ - p = get_opt_name(p, &option, '='); - assert(*p == '='); - p++; - p = get_opt_value(p, &value); - } - if (strcmp(option, "id") != 0) { - /* store and parse */ - opt_set(opts, option, value, prepend, invalidp, &local_err); - value = NULL; - if (local_err) { - error_propagate(errp, local_err); - goto cleanup; - } - } - if (*p != ',') { - break; + for (p = params; *p;) { + p = get_opt_name_value(p, firstname, &option, &value); + firstname = NULL; + + if (!strcmp(option, "id")) { + g_free(option); + g_free(value); + continue; } + + opt_set(opts, option, value, prepend, invalidp, &local_err); g_free(option); - g_free(value); - option = value = NULL; + if (local_err) { + error_propagate(errp, local_err); + return; + } } - - cleanup: - g_free(option); - g_free(value); } /** -- cgit v1.2.3 From 933d1527785fe839300459abb486905094d192a7 Mon Sep 17 00:00:00 2001 From: Markus Armbruster Date: Wed, 15 Apr 2020 09:49:21 +0200 Subject: qemu-option: Fix sloppy recognition of "id=..." after ",," Signed-off-by: Markus Armbruster Reviewed-by: Eric Blake Reviewed-by: Kevin Wolf Message-Id: <20200415074927.19897-4-armbru@redhat.com> --- util/qemu-option.c | 27 +++++++++++++++++++-------- 1 file changed, 19 insertions(+), 8 deletions(-) (limited to 'util/qemu-option.c') diff --git a/util/qemu-option.c b/util/qemu-option.c index f08f4bc458..d2956082bd 100644 --- a/util/qemu-option.c +++ b/util/qemu-option.c @@ -872,6 +872,24 @@ static void opts_do_parse(QemuOpts *opts, const char *params, } } +static char *opts_parse_id(const char *params) +{ + const char *p; + char *name, *value; + + for (p = params; *p;) { + p = get_opt_name_value(p, NULL, &name, &value); + if (!strcmp(name, "id")) { + g_free(name); + return value; + } + g_free(name); + g_free(value); + } + + return NULL; +} + /** * Store options parsed from @params into @opts. * If @firstname is non-null, the first key=value in @params may omit @@ -889,20 +907,13 @@ static QemuOpts *opts_parse(QemuOptsList *list, const char *params, bool *invalidp, Error **errp) { const char *firstname; - char *id = NULL; - const char *p; + char *id = opts_parse_id(params); QemuOpts *opts; Error *local_err = NULL; assert(!permit_abbrev || list->implied_opt_name); firstname = permit_abbrev ? list->implied_opt_name : NULL; - if (strncmp(params, "id=", 3) == 0) { - get_opt_value(params + 3, &id); - } else if ((p = strstr(params, ",id=")) != NULL) { - get_opt_value(p + 4, &id); - } - /* * This code doesn't work for defaults && !list->merge_lists: when * params has no id=, and list has an element with !opts->id, it -- cgit v1.2.3 From 80a94855737622436a9b5cd25315b9c80d7e3ffa Mon Sep 17 00:00:00 2001 From: Markus Armbruster Date: Wed, 15 Apr 2020 09:49:22 +0200 Subject: qemu-option: Fix has_help_option()'s sloppy parsing has_help_option() uses its own parser. It's inconsistent with qemu_opts_parse(), as demonstrated by test-qemu-opts case /qemu-opts/has_help_option. Fix by reusing the common parser. Signed-off-by: Markus Armbruster Message-Id: <20200415074927.19897-5-armbru@redhat.com> Reviewed-by: Eric Blake --- util/qemu-option.c | 39 +++++++++++++++++++-------------------- 1 file changed, 19 insertions(+), 20 deletions(-) (limited to 'util/qemu-option.c') diff --git a/util/qemu-option.c b/util/qemu-option.c index d2956082bd..0abf26b61f 100644 --- a/util/qemu-option.c +++ b/util/qemu-option.c @@ -165,26 +165,6 @@ void parse_option_size(const char *name, const char *value, *ret = size; } -bool has_help_option(const char *param) -{ - const char *p = param; - bool result = false; - - while (*p && !result) { - char *value; - - p = get_opt_value(p, &value); - if (*p) { - p++; - } - - result = is_help_option(value); - g_free(value); - } - - return result; -} - bool is_valid_option_list(const char *p) { char *value = NULL; @@ -890,6 +870,25 @@ static char *opts_parse_id(const char *params) return NULL; } +bool has_help_option(const char *params) +{ + const char *p; + char *name, *value; + bool ret; + + for (p = params; *p;) { + p = get_opt_name_value(p, NULL, &name, &value); + ret = is_help_option(name); + g_free(name); + g_free(value); + if (ret) { + return true; + } + } + + return false; +} + /** * Store options parsed from @params into @opts. * If @firstname is non-null, the first key=value in @params may omit -- cgit v1.2.3 From 56a9efa199a603b77e7f2bd0e84e11e897bf7473 Mon Sep 17 00:00:00 2001 From: Markus Armbruster Date: Wed, 15 Apr 2020 09:49:24 +0200 Subject: qemu-option: Avoid has_help_option() in qemu_opts_parse_noisily() When opts_parse() sets @invalidp to true, qemu_opts_parse_noisily() uses has_help_option() to decide whether to print help. This parses the input string a second time. Easy to avoid: replace @invalidp by @help_wanted. Signed-off-by: Markus Armbruster Reviewed-by: Eric Blake Message-Id: <20200415074927.19897-7-armbru@redhat.com> --- util/qemu-option.c | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) (limited to 'util/qemu-option.c') diff --git a/util/qemu-option.c b/util/qemu-option.c index 0abf26b61f..2d0d24ee27 100644 --- a/util/qemu-option.c +++ b/util/qemu-option.c @@ -519,7 +519,7 @@ int qemu_opt_unset(QemuOpts *opts, const char *name) } static void opt_set(QemuOpts *opts, const char *name, char *value, - bool prepend, bool *invalidp, Error **errp) + bool prepend, bool *help_wanted, Error **errp) { QemuOpt *opt; const QemuOptDesc *desc; @@ -529,8 +529,8 @@ static void opt_set(QemuOpts *opts, const char *name, char *value, if (!desc && !opts_accepts_any(opts)) { g_free(value); error_setg(errp, QERR_INVALID_PARAMETER, name); - if (invalidp) { - *invalidp = true; + if (help_wanted && is_help_option(name)) { + *help_wanted = true; } return; } @@ -827,7 +827,7 @@ static const char *get_opt_name_value(const char *params, static void opts_do_parse(QemuOpts *opts, const char *params, const char *firstname, bool prepend, - bool *invalidp, Error **errp) + bool *help_wanted, Error **errp) { Error *local_err = NULL; char *option, *value; @@ -843,7 +843,7 @@ static void opts_do_parse(QemuOpts *opts, const char *params, continue; } - opt_set(opts, option, value, prepend, invalidp, &local_err); + opt_set(opts, option, value, prepend, help_wanted, &local_err); g_free(option); if (local_err) { error_propagate(errp, local_err); @@ -903,7 +903,7 @@ void qemu_opts_do_parse(QemuOpts *opts, const char *params, static QemuOpts *opts_parse(QemuOptsList *list, const char *params, bool permit_abbrev, bool defaults, - bool *invalidp, Error **errp) + bool *help_wanted, Error **errp) { const char *firstname; char *id = opts_parse_id(params); @@ -928,7 +928,7 @@ static QemuOpts *opts_parse(QemuOptsList *list, const char *params, return NULL; } - opts_do_parse(opts, params, firstname, defaults, invalidp, &local_err); + opts_do_parse(opts, params, firstname, defaults, help_wanted, &local_err); if (local_err) { error_propagate(errp, local_err); qemu_opts_del(opts); @@ -964,11 +964,11 @@ QemuOpts *qemu_opts_parse_noisily(QemuOptsList *list, const char *params, { Error *err = NULL; QemuOpts *opts; - bool invalidp = false; + bool help_wanted = false; - opts = opts_parse(list, params, permit_abbrev, false, &invalidp, &err); + opts = opts_parse(list, params, permit_abbrev, false, &help_wanted, &err); if (err) { - if (invalidp && has_help_option(params)) { + if (help_wanted) { qemu_opts_print_help(list, true); error_free(err); } else { -- cgit v1.2.3 From 80c710cb06ff40b45de033e4352528b3adcd2de9 Mon Sep 17 00:00:00 2001 From: Markus Armbruster Date: Wed, 15 Apr 2020 09:49:26 +0200 Subject: qemu-img: Move is_valid_option_list() to qemu-img.c and rewrite is_valid_option_list()'s purpose is ensuring qemu-img.c's can safely join multiple parameter strings separated by ',' like this: g_strdup_printf("%s,%s", params1, params2); How it does that is anything but obvious. A close reading of the code reveals that it fails exactly when its argument starts with ',' or ends with an odd number of ','. Makes sense, actually, because when the argument starts with ',', a separating ',' preceding it would get escaped, and when it ends with an odd number of ',', a separating ',' following it would get escaped. Move it to qemu-img.c and rewrite it the obvious way. Signed-off-by: Markus Armbruster Reviewed-by: Eric Blake Message-Id: <20200415074927.19897-9-armbru@redhat.com> --- util/qemu-option.c | 22 ---------------------- 1 file changed, 22 deletions(-) (limited to 'util/qemu-option.c') diff --git a/util/qemu-option.c b/util/qemu-option.c index 2d0d24ee27..9542988183 100644 --- a/util/qemu-option.c +++ b/util/qemu-option.c @@ -165,28 +165,6 @@ void parse_option_size(const char *name, const char *value, *ret = size; } -bool is_valid_option_list(const char *p) -{ - char *value = NULL; - bool result = false; - - while (*p) { - p = get_opt_value(p, &value); - if ((*p && !*++p) || - (!*value || *value == ',')) { - goto out; - } - - g_free(value); - value = NULL; - } - - result = true; -out: - g_free(value); - return result; -} - static const char *opt_type_to_string(enum QemuOptType type) { switch (type) { -- cgit v1.2.3