[for-5.1,3/8] qemu-option: Fix sloppy recognition of "id=..." after ", , "

Commit Message

Markus Armbruster April 9, 2020, 3:30 p.m. UTC

Signed-off-by: Markus Armbruster <[e-mail]>
---
 tests/test-qemu-opts.c |  4 ++--
 util/qemu-option.c     | 27 +++++++++++++++++++--------
 2 files changed, 21 insertions(+), 10 deletions(-)

Comments

Eric Blake April 9, 2020, 6:05 p.m. UTC | #1

On 4/9/20 10:30 AM, Markus Armbruster wrote:
> Signed-off-by: Markus Armbruster <[e-mail]>
> ---
>   tests/test-qemu-opts.c |  4 ++--
>   util/qemu-option.c     | 27 +++++++++++++++++++--------
>   2 files changed, 21 insertions(+), 10 deletions(-)
> 
> diff --git a/tests/test-qemu-opts.c b/tests/test-qemu-opts.c
> index 0efe93b45e..27c24bb1a2 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");

Yay - that matches my intuition better about ,, handling.

Reviewed-by: Eric Blake <[e-mail]>

Kevin Wolf April 14, 2020, 2:44 p.m. UTC | #2

Am 09.04.2020 um 17:30 hat Markus Armbruster geschrieben:
> Signed-off-by: Markus Armbruster <[e-mail]>

Reviewed-by: Kevin Wolf <[e-mail]>

Patch

diff --git a/tests/test-qemu-opts.c b/tests/test-qemu-opts.c
index 0efe93b45e..27c24bb1a2 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