[Qemu-devel] [PATCH for-2.9 13/17] cpu: Support comma escaping when parsing -cpu

[Eduardo Habkost]

Currently it's impossible to use commas inside any option value
in -cpu due to the simple way the parser splits the options.

Change both cpu_common_parse_features() and
x86_cpu_parse_featurestr() to use get_opt_*() parsing options,
that can handle handle ",," escaping of commas.

The ideal solution is to use QemuOpts to parse the -cpu option.
But this will require changing the CPUClass::parse_features()
interface, so it will be done later.

Cc: Igor Mammedov <address@hidden>
Signed-off-by: Eduardo Habkost <address@hidden>
---
 tests/test-x86-cpuid-compat.c | 19 +++++++++++++
 qom/cpu.c                     | 32 +++++++++++++++-------
 target-i386/cpu.c             | 63 ++++++++++++++++++++++---------------------
 3 files changed, 74 insertions(+), 40 deletions(-)

diff --git a/tests/test-x86-cpuid-compat.c b/tests/test-x86-cpuid-compat.c
index 79a2e69..06caa5b 100644
--- a/tests/test-x86-cpuid-compat.c
+++ b/tests/test-x86-cpuid-compat.c
@@ -4,6 +4,7 @@
 #include "qapi/qmp/qdict.h"
 #include "qapi/qmp/qint.h"
 #include "qapi/qmp/qbool.h"
+#include "qapi/qmp/qstring.h"
 #include "libqtest.h"
 
 static char *get_cpu0_qom_path(void)
@@ -52,6 +53,22 @@ typedef struct CpuidTestArgs {
     int64_t expected_value;
 } CpuidTestArgs;
 
+static void test_commas(void)
+{
+    char *path;
+    QString *value;
+
+    qtest_start("-cpu 'qemu64,fpu=on,model-id=A CPU with commas,,fpu=off'");
+    path = get_cpu0_qom_path();
+    value = qobject_to_qstring(qom_get(path, "model-id"));
+    g_assert_true(qom_get_bool(path, "fpu"));
+    g_assert_cmpstr(qstring_get_str(value), ==, "A CPU with commas,fpu=off");
+    qtest_end();
+
+    QDECREF(value);
+    g_free(path);
+}
+
 static void test_cpuid_prop(const void *data)
 {
     const CpuidTestArgs *args = data;
@@ -132,6 +149,8 @@ int main(int argc, char **argv)
     g_test_add_func("/x86/cpuid/parsing-plus-minus", test_plus_minus);
 #endif
 
+    g_test_add_func("/x86/parsing/commas", test_commas);
+
     /* Original level values for CPU models: */
     add_cpuid_test("x86/cpuid/phenom/level",
                    "-cpu phenom", "level", 5);
diff --git a/qom/cpu.c b/qom/cpu.c
index 03d9190..47d69f7 100644
--- a/qom/cpu.c
+++ b/qom/cpu.c
@@ -295,8 +295,7 @@ static ObjectClass *cpu_common_class_by_name(const char 
*cpu_model)
 static void cpu_common_parse_features(const char *typename, char *features,
                                       Error **errp)
 {
-    char *featurestr; /* Single "key=value" string being parsed */
-    char *val;
+    const char *featurestr;
     static bool cpu_globals_initialized;
 
     /* TODO: all callers of ->parse_features() need to be changed to
@@ -310,16 +309,26 @@ static void cpu_common_parse_features(const char 
*typename, char *features,
     }
     cpu_globals_initialized = true;
 
-    featurestr = features ? strtok(features, ",") : NULL;
+    if (!features) {
+        return;
+    }
 
-    while (featurestr) {
-        val = strchr(featurestr, '=');
-        if (val) {
+    /*TODO: Use QemuOpts to parse -cpu on main(), so we don't need
+     *      to manually call get_opt_*() here.
+     */
+    for (featurestr = features; *featurestr != '\0'; featurestr++) {
+        const char *pe = strchr(featurestr, '=');
+        const char *pc = strchr(featurestr, ',');
+        if (pe && (!pc || pc > pe)) {
+            char option[128], val[1024];
             GlobalProperty *prop = g_new0(typeof(*prop), 1);
-            *val = 0;
-            val++;
+
+            featurestr = get_opt_name(option, sizeof(option), featurestr, '=');
+            featurestr++;
+            featurestr = get_opt_value(val, sizeof(val), featurestr);
+
             prop->driver = typename;
-            prop->property = g_strdup(featurestr);
+            prop->property = g_strdup(option);
             prop->value = g_strdup(val);
             prop->errp = &error_fatal;
             qdev_prop_register_global(prop);
@@ -328,7 +337,10 @@ static void cpu_common_parse_features(const char 
*typename, char *features,
                        featurestr);
             return;
         }
-        featurestr = strtok(NULL, ",");
+
+        if (*featurestr != ',') {
+            break;
+        }
     }
 }
 
diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index 78b25af..3539419 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -1968,7 +1968,7 @@ static gint compare_string(gconstpointer a, gconstpointer 
b)
 static void x86_cpu_parse_featurestr(const char *typename, char *features,
                                      Error **errp)
 {
-    char *featurestr; /* Single 'key=value" string being parsed */
+    const char *featurestr;
     static bool cpu_globals_initialized;
     bool ambiguous = false;
 
@@ -1981,36 +1981,40 @@ static void x86_cpu_parse_featurestr(const char 
*typename, char *features,
         return;
     }
 
-    for (featurestr = strtok(features, ",");
-         featurestr;
-         featurestr = strtok(NULL, ",")) {
-        const char *name;
-        const char *val = NULL;
-        char *eq = NULL;
-        char num[32];
+    /*TODO: Use QemuOpts to parse -cpu on main(), so we don't need
+     *      to manually call get_opt_*() here.
+     */
+    for (featurestr = features;
+         *featurestr != '\0';
+         *featurestr == ',' ? featurestr++ : 0) {
+        char name[128], val[1024];
         GlobalProperty *prop;
-
-        /* Compatibility syntax: */
-        if (featurestr[0] == '+') {
-            plus_features = g_list_append(plus_features,
-                                          g_strdup(featurestr + 1));
-            continue;
-        } else if (featurestr[0] == '-') {
-            minus_features = g_list_append(minus_features,
-                                           g_strdup(featurestr + 1));
-            continue;
-        }
-
-        eq = strchr(featurestr, '=');
-        if (eq) {
-            *eq++ = 0;
-            val = eq;
+        const char *pe = strchr(featurestr, '=');
+        const char *pc = strchr(featurestr, ',');
+
+        if (pe && (!pc || pc > pe)) {
+            /* opt=value[,...] */
+            featurestr = get_opt_name(name, sizeof(name), featurestr, '=');
+            featurestr++;
+            featurestr = get_opt_value(val, sizeof(val), featurestr);
         } else {
-            val = "on";
+            /* opt[,...] */
+            featurestr = get_opt_name(name, sizeof(name), featurestr, ',');
+            pstrcpy(val, sizeof(val), "on");
+
+            /* Compatibility syntax: */
+            if (name[0] == '+') {
+                plus_features = g_list_append(plus_features,
+                                              g_strdup(name + 1));
+                continue;
+            } else if (name[0] == '-') {
+                minus_features = g_list_append(minus_features,
+                                               g_strdup(name + 1));
+                continue;
+            }
         }
 
-        feat2prop(featurestr);
-        name = featurestr;
+        feat2prop(name);
 
         if (g_list_find_custom(plus_features, name, compare_string)) {
             error_report("warning: Ambiguous CPU model string. "
@@ -2036,9 +2040,8 @@ static void x86_cpu_parse_featurestr(const char 
*typename, char *features,
                 error_setg(errp, "bad numerical value %s", val);
                 return;
             }
-            snprintf(num, sizeof(num), "%" PRId64, tsc_freq);
-            val = num;
-            name = "tsc-frequency";
+            snprintf(val, sizeof(val), "%" PRId64, tsc_freq);
+            pstrcpy(name, sizeof(name), "tsc-frequency");
         }
 
         prop = g_new0(typeof(*prop), 1);
-- 
2.7.4