From 32c2dcf5e87bf05153df92e49de75f72d4466e4e Mon Sep 17 00:00:00 2001 From: Markus Armbruster Date: Wed, 15 Apr 2020 09:49:19 +0200 Subject: tests-qemu-opts: Cover has_help_option(), qemu_opt_has_help_opt() The two turn out to be inconsistent for "a,b,,help". Test case marked /* BUG */. Signed-off-by: Markus Armbruster Message-Id: <20200415074927.19897-2-armbru@redhat.com> Reviewed-by: Eric Blake --- tests/test-qemu-opts.c | 44 ++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 44 insertions(+) (limited to 'tests/test-qemu-opts.c') diff --git a/tests/test-qemu-opts.c b/tests/test-qemu-opts.c index ef96e84aed..88a3e7bdf4 100644 --- a/tests/test-qemu-opts.c +++ b/tests/test-qemu-opts.c @@ -728,6 +728,49 @@ static void test_opts_parse_size(void) qemu_opts_reset(&opts_list_02); } +static void test_has_help_option(void) +{ + static const struct { + const char *params; + /* expected value of has_help_option() */ + bool expect_has_help_option; + /* expected value of qemu_opt_has_help_opt() with implied=false */ + bool expect_opt_has_help_opt; + /* expected value of qemu_opt_has_help_opt() with implied=true */ + bool expect_opt_has_help_opt_implied; + } test[] = { + { "help", true, true, false }, + { "?", true, true, false }, + { "helpme", false, false, false }, + { "?me", false, false, false }, + { "a,help", true, true, true }, + { "a,?", true, true, true }, + { "a=0,help,b", true, true, true }, + { "a=0,?,b", true, true, true }, + { "help,b=1", true, true, false }, + { "?,b=1", true, true, false }, + { "a,b,,help", false /* BUG */, true, true }, + { "a,b,,?", false /* BUG */, true, true }, + }; + int i; + QemuOpts *opts; + + for (i = 0; i < ARRAY_SIZE(test); i++) { + g_assert_cmpint(has_help_option(test[i].params), + ==, test[i].expect_has_help_option); + opts = qemu_opts_parse(&opts_list_03, test[i].params, false, + &error_abort); + g_assert_cmpint(qemu_opt_has_help_opt(opts), + ==, test[i].expect_opt_has_help_opt); + qemu_opts_del(opts); + opts = qemu_opts_parse(&opts_list_03, test[i].params, true, + &error_abort); + g_assert_cmpint(qemu_opt_has_help_opt(opts), + ==, test[i].expect_opt_has_help_opt_implied); + qemu_opts_del(opts); + } +} + static void append_verify_list_01(QemuOptDesc *desc, bool with_overlapping) { int i = 0; @@ -990,6 +1033,7 @@ int main(int argc, char *argv[]) g_test_add_func("/qemu-opts/opts_parse/bool", test_opts_parse_bool); g_test_add_func("/qemu-opts/opts_parse/number", test_opts_parse_number); g_test_add_func("/qemu-opts/opts_parse/size", test_opts_parse_size); + g_test_add_func("/qemu-opts/has_help_option", test_has_help_option); g_test_add_func("/qemu-opts/append_to_null", test_opts_append_to_null); g_test_add_func("/qemu-opts/append", test_opts_append); g_test_add_func("/qemu-opts/to_qdict/basic", test_opts_to_qdict_basic); -- cgit v1.2.3-55-g7522 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> --- tests/test-qemu-opts.c | 4 ++-- util/qemu-option.c | 27 +++++++++++++++++++-------- 2 files changed, 21 insertions(+), 10 deletions(-) (limited to 'tests/test-qemu-opts.c') diff --git a/tests/test-qemu-opts.c b/tests/test-qemu-opts.c index 88a3e7bdf4..8ff97268d8 100644 --- a/tests/test-qemu-opts.c +++ b/tests/test-qemu-opts.c @@ -500,10 +500,10 @@ static void test_opts_parse(void) g_assert(!opts); /* TODO Cover .merge_lists = true */ - /* Buggy ID recognition */ + /* Buggy ID recognition (fixed) */ opts = qemu_opts_parse(&opts_list_03, "x=,,id=bar", false, &error_abort); g_assert_cmpuint(opts_count(opts), ==, 1); - g_assert_cmpstr(qemu_opts_id(opts), ==, "bar"); /* BUG */ + g_assert(!qemu_opts_id(opts)); g_assert_cmpstr(qemu_opt_get(opts, "x"), ==, ",id=bar"); /* Anti-social ID */ 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-55-g7522 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 --- tests/test-qemu-opts.c | 4 ++-- util/qemu-option.c | 39 +++++++++++++++++++-------------------- 2 files changed, 21 insertions(+), 22 deletions(-) (limited to 'tests/test-qemu-opts.c') diff --git a/tests/test-qemu-opts.c b/tests/test-qemu-opts.c index 8ff97268d8..77c944c4aa 100644 --- a/tests/test-qemu-opts.c +++ b/tests/test-qemu-opts.c @@ -749,8 +749,8 @@ static void test_has_help_option(void) { "a=0,?,b", true, true, true }, { "help,b=1", true, true, false }, { "?,b=1", true, true, false }, - { "a,b,,help", false /* BUG */, true, true }, - { "a,b,,?", false /* BUG */, true, true }, + { "a,b,,help", true, true, true }, + { "a,b,,?", true, true, true }, }; int i; QemuOpts *opts; 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-55-g7522 From 59d27ebc446baec4a972cc146df732910b5aa2de Mon Sep 17 00:00:00 2001 From: Markus Armbruster Date: Wed, 15 Apr 2020 09:49:23 +0200 Subject: test-qemu-opts: Simplify test_has_help_option() after bug fix Signed-off-by: Markus Armbruster Reviewed-by: Eric Blake Reviewed-by: Kevin Wolf Message-Id: <20200415074927.19897-6-armbru@redhat.com> --- tests/test-qemu-opts.c | 36 +++++++++++++++++------------------- 1 file changed, 17 insertions(+), 19 deletions(-) (limited to 'tests/test-qemu-opts.c') diff --git a/tests/test-qemu-opts.c b/tests/test-qemu-opts.c index 77c944c4aa..2a0f42a09b 100644 --- a/tests/test-qemu-opts.c +++ b/tests/test-qemu-opts.c @@ -732,41 +732,39 @@ static void test_has_help_option(void) { static const struct { const char *params; - /* expected value of has_help_option() */ - bool expect_has_help_option; /* expected value of qemu_opt_has_help_opt() with implied=false */ - bool expect_opt_has_help_opt; + bool expect; /* expected value of qemu_opt_has_help_opt() with implied=true */ - bool expect_opt_has_help_opt_implied; + bool expect_implied; } test[] = { - { "help", true, true, false }, - { "?", true, true, false }, - { "helpme", false, false, false }, - { "?me", false, false, false }, - { "a,help", true, true, true }, - { "a,?", true, true, true }, - { "a=0,help,b", true, true, true }, - { "a=0,?,b", true, true, true }, - { "help,b=1", true, true, false }, - { "?,b=1", true, true, false }, - { "a,b,,help", true, true, true }, - { "a,b,,?", true, true, true }, + { "help", true, false }, + { "?", true, false }, + { "helpme", false, false }, + { "?me", false, false }, + { "a,help", true, true }, + { "a,?", true, true }, + { "a=0,help,b", true, true }, + { "a=0,?,b", true, true }, + { "help,b=1", true, false }, + { "?,b=1", true, false }, + { "a,b,,help", true, true }, + { "a,b,,?", true, true }, }; int i; QemuOpts *opts; for (i = 0; i < ARRAY_SIZE(test); i++) { g_assert_cmpint(has_help_option(test[i].params), - ==, test[i].expect_has_help_option); + ==, test[i].expect); opts = qemu_opts_parse(&opts_list_03, test[i].params, false, &error_abort); g_assert_cmpint(qemu_opt_has_help_opt(opts), - ==, test[i].expect_opt_has_help_opt); + ==, test[i].expect); qemu_opts_del(opts); opts = qemu_opts_parse(&opts_list_03, test[i].params, true, &error_abort); g_assert_cmpint(qemu_opt_has_help_opt(opts), - ==, test[i].expect_opt_has_help_opt_implied); + ==, test[i].expect_implied); qemu_opts_del(opts); } } -- cgit v1.2.3-55-g7522