keyval: Fix parsing of ',' in value of implied key

The previous commit demonstrated documentation and code disagree on
parsing of ',' in the value of an implied key.  Fix the code to match
the documentation.

This breaks uses of keyval_parse() that pass an implied key and accept
a value containing ','.  None of the existing uses does:

* audiodev: implied key "driver" is enum AudiodevDriver, none of the
  values contains ','

* display: implied key "type" is enum DisplayType, none of the values
  contains ','

* blockdev: implied key "driver is enum BlockdevDriver, none of the
  values contains ','

* export: implied key "type" is enum BlockExportType, none of the
  values contains ','

* monitor: implied key "mode" is enum MonitorMode, none of the values
  contains ','

* nbd-server: no implied key.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
Message-Id: <20201011073505.1185335-4-armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>

diff --git a/tests/test-keyval.c b/tests/test-keyval.c
index f02bdf70293c459c27992a31b8dcf691ba9db5ec..04c62cf04562bb5f47ce482a2a75d0f5c519fc3e 100644 (file)
--- a/tests/test-keyval.c
+++ b/tests/test-keyval.c
@@ -183,11 +183,9 @@ static void test_keyval_parse(void)
     g_assert(!qdict);
 
     /* Implied key's value can't have comma (qemu_opts_parse(): it can) */
-    /* BUG: it can */
-    qdict = keyval_parse("val,,ue", "implied", &error_abort);
-    g_assert_cmpuint(qdict_size(qdict), ==, 1);
-    g_assert_cmpstr(qdict_get_try_str(qdict, "implied"), ==, "val,ue");
-    qobject_unref(qdict);
+    qdict = keyval_parse("val,,ue", "implied", &err);
+    error_free_or_abort(&err);
+    g_assert(!qdict);
 
     /* Empty key is not an implied key */
     qdict = keyval_parse("=val", "implied", &err);

diff --git a/util/keyval.c b/util/keyval.c
index 82d8497c71de62207b762c79117a6c965b2d1418..8f33a36a7cc956629b91175ad54de58c7b3f382e 100644 (file)
--- a/util/keyval.c
+++ b/util/keyval.c
@@ -173,7 +173,7 @@ static const char *keyval_parse_one(QDict *qdict, const char *params,
                                     const char *implied_key,
                                     Error **errp)
 {
-    const char *key, *key_end, *s, *end;
+    const char *key, *key_end, *val_end, *s, *end;
     size_t len;
     char key_in_cur[128];
     QDict *cur;
@@ -182,10 +182,12 @@ static const char *keyval_parse_one(QDict *qdict, const char *params,
     QString *val;
 
     key = params;
+    val_end = NULL;
     len = strcspn(params, "=,");
     if (implied_key && len && key[len] != '=') {
         /* Desugar implied key */
         key = implied_key;
+        val_end = params + len;
         len = strlen(implied_key);
     }
     key_end = key + len;
@@ -241,7 +243,11 @@ static const char *keyval_parse_one(QDict *qdict, const char *params,
 
     if (key == implied_key) {
         assert(!*s);
-        s = params;
+        val = qstring_from_substr(params, 0, val_end - params);
+        s = val_end;
+        if (*s == ',') {
+            s++;
+        }
     } else {
         if (*s != '=') {
             error_setg(errp, "Expected '=' after parameter '%.*s'",
@@ -249,19 +255,19 @@ static const char *keyval_parse_one(QDict *qdict, const char *params,
             return NULL;
         }
         s++;
-    }
 
-    val = qstring_new();
-    for (;;) {
-        if (!*s) {
-            break;
-        } else if (*s == ',') {
-            s++;
-            if (*s != ',') {
+        val = qstring_new();
+        for (;;) {
+            if (!*s) {
                 break;
+            } else if (*s == ',') {
+                s++;
+                if (*s != ',') {
+                    break;
+                }
             }
+            qstring_append_chr(val, *s++);
         }
-        qstring_append_chr(val, *s++);
     }
 
     if (!keyval_parse_put(cur, key_in_cur, val, key, key_end, errp)) {