[libvirt] [PATCH 1/2] qemu: use strsep for CPU command line parsing

[Ján Tomko]

Copy the command line to allow modifying it, then get the tokens
by strsep. This removes the need to strdup every feature separately,
and simplifies the code.

Change the error label to a cleanup label to prevent memory leaks.
---
 src/qemu/qemu_command.c | 110 ++++++++++++++++++------------------------------
 1 file changed, 41 insertions(+), 69 deletions(-)

diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 575dce1..3a27512 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -9306,63 +9306,51 @@ qemuParseCommandLineCPU(virDomainDefPtr dom,
                         const char *val)
 {
     virCPUDefPtr cpu = NULL;
-    const char *p = val;
-    const char *next;
+    char *val_copy;
+    char *next;
     char *model = NULL;
+    char *token = NULL;
+    int ret = -1;
 
-    do {
-        if (*p == '\0' || *p == ',')
-            goto syntax;
+    if (!(val_copy = strdup(val)))
+        goto no_memory;
 
-        if ((next = strchr(p, ',')))
-            next++;
+    next = val_copy;
 
-        if (p == val) {
-            if (next)
-                model = strndup(p, next - p - 1);
-            else
-                model = strdup(p);
+    while ((token = strsep(&next, ","))) {
+        if (*token == '\0')
+            goto syntax;
 
-            if (!model)
+        if (token == val_copy) {
+            if (!(model = strdup(token)))
                 goto no_memory;
 
             if (!STREQ(model, "qemu32") && !STREQ(model, "qemu64")) {
                 if (!(cpu = qemuInitGuestCPU(dom)))
-                    goto error;
+                    goto cleanup;
 
                 cpu->model = model;
                 model = NULL;
             }
-        } else if (*p == '+' || *p == '-') {
-            char *feature;
+        } else if (*token == '+' || *token == '-') {
+            const char *feature = token + 1; /* '+' or '-' */
             int policy;
-            int ret = 0;
 
-            if (*p == '+')
+            if (*token == '+')
                 policy = VIR_CPU_FEATURE_REQUIRE;
             else
                 policy = VIR_CPU_FEATURE_DISABLE;
 
-            p++;
-            if (*p == '\0' || *p == ',')
+            if (*feature == '\0')
                 goto syntax;
 
-            if (next)
-                feature = strndup(p, next - p - 1);
-            else
-                feature = strdup(p);
-
-            if (!feature)
-                goto no_memory;
-
             if (STREQ(feature, "kvmclock")) {
                 bool present = (policy == VIR_CPU_FEATURE_REQUIRE);
                 int i;
 
                 for (i = 0; i < dom->clock.ntimers; i++) {
-                    if (dom->clock.timers[i]->name == VIR_DOMAIN_TIMER_NAME_KVMCLOCK) {
+                    if (dom->clock.timers[i]->name == VIR_DOMAIN_TIMER_NAME_KVMCLOCK)
                         break;
-                    }
                 }
 
                 if (i == dom->clock.ntimers) {
@@ -9370,19 +9358,16 @@ qemuParseCommandLineCPU(virDomainDefPtr dom,
                         VIR_ALLOC(dom->clock.timers[i]) < 0)
                         goto no_memory;
                     dom->clock.timers[i]->name = VIR_DOMAIN_TIMER_NAME_KVMCLOCK;
-                    dom->clock.timers[i]->present = -1;
+                    dom->clock.timers[i]->present = present;
                     dom->clock.timers[i]->tickpolicy = -1;
                     dom->clock.timers[i]->track = -1;
                     dom->clock.ntimers++;
-                }
-
-                if (dom->clock.timers[i]->present != -1 &&
+                } else if (dom->clock.timers[i]->present != -1 &&
                     dom->clock.timers[i]->present != present) {
                     virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
                                    _("conflicting occurrences of kvmclock feature"));
-                    goto error;
+                    goto cleanup;
                 }
-                dom->clock.timers[i]->present = present;
             } else if (STREQ(feature, "kvm_pv_eoi")) {
                 if (policy == VIR_CPU_FEATURE_REQUIRE)
                     dom->apic_eoi = VIR_DOMAIN_FEATURE_STATE_ON;
@@ -9391,41 +9376,29 @@ qemuParseCommandLineCPU(virDomainDefPtr dom,
             } else {
                 if (!cpu) {
                     if (!(cpu = qemuInitGuestCPU(dom)))
-                        goto error;
+                        goto cleanup;
 
                     cpu->model = model;
                     model = NULL;
                 }
 
-                ret = virCPUDefAddFeature(cpu, feature, policy);
+                if (virCPUDefAddFeature(cpu, feature, policy) < 0)
+                    goto cleanup;
             }
-
-            VIR_FREE(feature);
-            if (ret < 0)
-                goto error;
-        } else if (STRPREFIX(p, "hv_")) {
-            char *feature;
+        } else if (STRPREFIX(token, "hv_")) {
+            const char *feature = token + 3; /* "hv_" */
             int f;
-            p += 3; /* "hv_" */
 
-            if (*p == '\0' || *p == ',')
+            if (*feature == '\0')
                 goto syntax;
 
-            if (next)
-                feature = strndup(p, next - p - 1);
-            else
-                feature = strdup(p);
-
-            if (!feature)
-                goto no_memory;
-
             dom->features |= (1 << VIR_DOMAIN_FEATURE_HYPERV);
 
             if ((f = virDomainHypervTypeFromString(feature)) < 0) {
                 virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
                                _("unsupported HyperV Enlightenment feature "
                                  "'%s'"), feature);
-                goto error;
+                goto cleanup;
             }
 
             switch ((enum virDomainHyperv) f) {
@@ -9436,21 +9409,17 @@ qemuParseCommandLineCPU(virDomainDefPtr dom,
             case VIR_DOMAIN_HYPERV_LAST:
                 break;
             }
-
-            VIR_FREE(feature);
         }
-    } while ((p = next));
+    }
 
     if (dom->os.arch == VIR_ARCH_X86_64) {
         bool is_32bit = false;
         if (cpu) {
             union cpuData *cpuData = NULL;
-            int ret;
 
-            ret = cpuEncode(VIR_ARCH_X86_64, cpu, NULL, &cpuData,
-                            NULL, NULL, NULL, NULL);
-            if (ret < 0)
-                goto error;
+            if (cpuEncode(VIR_ARCH_X86_64, cpu, NULL, &cpuData,
+                          NULL, NULL, NULL, NULL) < 0)
+                goto cleanup;
 
             is_32bit = (cpuHasFeature(VIR_ARCH_X86_64, cpuData, "lm") != 1);
             cpuDataFree(VIR_ARCH_X86_64, cpuData);
@@ -9458,22 +9427,25 @@ qemuParseCommandLineCPU(virDomainDefPtr dom,
             is_32bit = STREQ(model, "qemu32");
         }
 
-        if (is_32bit) {
+        if (is_32bit)
             dom->os.arch = VIR_ARCH_I686;
-        }
     }
+
+    ret = 0;
+
+cleanup:
     VIR_FREE(model);
-    return 0;
+    VIR_FREE(val_copy);
+    return ret;
 
 syntax:
     virReportError(VIR_ERR_INTERNAL_ERROR,
                    _("unknown CPU syntax '%s'"), val);
-    goto error;
+    goto cleanup;
 
 no_memory:
     virReportOOMError();
-error:
-    return -1;
+    goto cleanup;
 }
 
 
-- 
1.8.1.5