Parsing of the cpu affinity list was using virParseNumber. Modernize it
to get rid of the virParseNumber call.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
---
 src/vmx/vmx.c | 56 ++++++++++++++++++---------------------------------
 1 file changed, 20 insertions(+), 36 deletions(-)

diff --git a/src/vmx/vmx.c b/src/vmx/vmx.c
index 429630faaf..70d9443766 100644
--- a/src/vmx/vmx.c
+++ b/src/vmx/vmx.c
@@ -1500,43 +1500,35 @@ virVMXParseConfig(virVMXContext *ctx,
     }

     if (sched_cpu_affinity != NULL && STRCASENEQ(sched_cpu_affinity, "all")) {
-        const char *current = sched_cpu_affinity;
-        int number, count = 0;
+        VIR_AUTOSTRINGLIST afflist = NULL;
+        char **aff;
+        size_t naffs;

         def->cpumask = virBitmapNew(VIR_DOMAIN_CPUMASK_LEN);
         if (!def->cpumask)
             goto cleanup;

-        while (*current != '\0') {
-            virSkipSpaces(&current);
-
-            number = virParseNumber(&current);
-
-            if (number < 0) {
-                virReportError(VIR_ERR_INTERNAL_ERROR,
-                               _("Expecting VMX entry 'sched.cpu.affinity' to be "
-                                 "a comma separated list of unsigned integers but "
-                                 "found '%s'"), sched_cpu_affinity);
-                goto cleanup;
-            }
+        if (!(afflist = virStringSplitCount(sched_cpu_affinity, ",", 0, &naffs)))
+            goto cleanup;

-            if (number >= VIR_DOMAIN_CPUMASK_LEN) {
-                virReportError(VIR_ERR_INTERNAL_ERROR,
-                               _("VMX entry 'sched.cpu.affinity' contains a %d, "
-                                 "this value is too large"), number);
-                goto cleanup;
-            }
+        if (naffs < numvcpus) {
+            virReportError(VIR_ERR_INTERNAL_ERROR,
+                           _("Expecting VMX entry 'sched.cpu.affinity' to contain "
+                             "at least as many values as 'numvcpus' (%lld) but "
+                             "found only %zu value(s)"), numvcpus, naffs);
+            goto cleanup;
+        }

-            ignore_value(virBitmapSetBit(def->cpumask, number));
-            ++count;
+        for (aff = afflist; *aff; aff++) {
+            const char *current = *aff;
+            unsigned int number;
+            int rc;

+            virSkipSpaces(&current);
+            rc = virStrToLong_uip(current, (char **) &current, 10, &number);
             virSkipSpaces(&current);

-            if (*current == ',') {
-                ++current;
-            } else if (*current == '\0') {
-                break;
-            } else {
+            if (rc < 0 || *current != '\0') {
                 virReportError(VIR_ERR_INTERNAL_ERROR,
                                _("Expecting VMX entry 'sched.cpu.affinity' to be "
                                  "a comma separated list of unsigned integers but "
@@ -1544,15 +1536,7 @@ virVMXParseConfig(virVMXContext *ctx,
                 goto cleanup;
             }

-            virSkipSpaces(&current);
-        }
-
-        if (count < numvcpus) {
-            virReportError(VIR_ERR_INTERNAL_ERROR,
-                           _("Expecting VMX entry 'sched.cpu.affinity' to contain "
-                             "at least as many values as 'numvcpus' (%lld) but "
-                             "found only %d value(s)"), numvcpus, count);
-            goto cleanup;
+            ignore_value(virBitmapSetBit(def->cpumask, number));
         }
     }

-- 
2.20.1

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

On Wed, Apr 03, 2019 at 02:44:52PM +0200, Peter Krempa wrote:
>Parsing of the cpu affinity list was using virParseNumber. Modernize it
>to get rid of the virParseNumber call.
>
>Signed-off-by: Peter Krempa <pkrempa@redhat.com>
>---
> src/vmx/vmx.c | 56 ++++++++++++++++++---------------------------------
> 1 file changed, 20 insertions(+), 36 deletions(-)
>
>diff --git a/src/vmx/vmx.c b/src/vmx/vmx.c
>index 429630faaf..70d9443766 100644
>--- a/src/vmx/vmx.c
>+++ b/src/vmx/vmx.c
>@@ -1500,43 +1500,35 @@ virVMXParseConfig(virVMXContext *ctx,
>+        if (!(afflist = virStringSplitCount(sched_cpu_affinity, ",", 0, &naffs)))
>+            goto cleanup;
>

>-            if (number >= VIR_DOMAIN_CPUMASK_LEN) {
>-                virReportError(VIR_ERR_INTERNAL_ERROR,
>-                               _("VMX entry 'sched.cpu.affinity' contains a %d, "
>-                                 "this value is too large"), number);
>-                goto cleanup;
>-            }

Even though an entry greater than VIR_DOMAIN_CPUMASK_LEN is unlikely, I
don't think a refactor should remove this check

>+        if (naffs < numvcpus) {
>+            virReportError(VIR_ERR_INTERNAL_ERROR,
>+                           _("Expecting VMX entry 'sched.cpu.affinity' to contain "
>+                             "at least as many values as 'numvcpus' (%lld) but "
>+                             "found only %zu value(s)"), numvcpus, naffs);
>+            goto cleanup;
>+        }
>
>-            ignore_value(virBitmapSetBit(def->cpumask, number));
>-            ++count;
>+        for (aff = afflist; *aff; aff++) {
>+            const char *current = *aff;
>+            unsigned int number;

With the check left in:
Reviewed-by: Ján Tomko <jtomko@redhat.com>

Jano
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list