Re: [PATCH] Fix the check of <cpumap> in virsh vcpupin

["Richard W.M. Jones" <rjones@xxxxxxxxxx>]

Masayuki Sunou wrote:
> Hi
>
> Because virsh vcpupin does not check a form of <cpumap>,
> when non-numerical letters are set, it does not become an error.
>
> This patch fixes it so that it become an error when non-numerical
> letters are set.

You mean cpulist instead of cpumap?  Your patch doesn't deal with 
strings like "1,,,,3".

<rant> It's 2007 - we should not be writing any more software in C. </rant>

There seem to be another problem with the documentation of virsh vcpupin 
too.  It's documented as 'virsh vcpupin <domain>' yet takes two other 
parameters, so should really be 'virsh vcpupin <domain> <vcpu> <cpulist>'.

The attached patch (not applied) should fix all of the above.

Rich.

--
Emerging Technologies, Red Hat - http://et.redhat.com/~rjones/
Registered Address: Red Hat UK Ltd, Amberley Place, 107-111 Peascod
Street, Windsor, Berkshire, SL4 1TE, United Kingdom.  Registered in
England and Wales under Company Registration No. 03798903

Index: src/virsh.c
===================================================================
RCS file: /data/cvs/libvirt/src/virsh.c,v
retrieving revision 1.85
diff -u -r1.85 virsh.c
--- src/virsh.c	18 Jun 2007 08:33:08 -0000	1.85
+++ src/virsh.c	18 Jun 2007 11:00:59 -0000
@@ -1505,7 +1505,7 @@
  * "vcpupin" command
  */
 static vshCmdInfo info_vcpupin[] = {
-    {"syntax", "vcpupin <domain>"},
+    {"syntax", "vcpupin <domain> <vcpu> <cpulist>"},
     {"help", gettext_noop("control domain vcpu affinity")},
     {"desc", gettext_noop("Pin domain VCPUs to host physical CPUs.")},
     {NULL, NULL}
@@ -1530,6 +1530,8 @@
     int vcpufound = 0;
     unsigned char *cpumap;
     int cpumaplen;
+    int i;
+    enum { expect_num, expect_num_or_comma } state;
 
     if (!vshConnectionUsability(ctl, ctl->conn, TRUE))
         return FALSE;
@@ -1563,6 +1565,42 @@
         return FALSE;
     }
 
+    /* Check that the cpulist parameter is a comma-separated list of
+     * numbers and give an intelligent error message if not.
+     */
+    if (cpulist[0] == '\0') {
+        vshError(ctl, FALSE, _("cpulist: Invalid format. Empty string."));
+        virDomainFree (dom);
+        return FALSE;
+    }
+
+    state = expect_num;
+    for (i = 0; cpulist[i]; i++) {
+        switch (state) {
+        case expect_num:
+            if (!isdigit (cpulist[i])) {
+                vshError( ctl, FALSE, _("cpulist: %s: Invalid format. Expecting digit at position %d (near '%c')."), cpulist, i, cpulist[i]);
+                virDomainFree (dom);
+                return FALSE;
+            }
+            state = expect_num_or_comma;
+            break;
+        case expect_num_or_comma:
+            if (cpulist[i] == ',')
+                state = expect_num;
+            else if (!isdigit (cpulist[i])) {
+                vshError(ctl, FALSE, _("cpulist: %s: Invalid format. Expecting digit or comma at position %d (near '%c')."), cpulist, i, cpulist[i]);
+                virDomainFree (dom);
+                return FALSE;
+            }
+        }
+    }
+    if (state == expect_num) {
+        vshError(ctl, FALSE, _("cpulist: %s: Invalid format. Trailing comma at position %d."), cpulist, i);
+        virDomainFree (dom);
+        return FALSE;
+    }
+
     cpumaplen = VIR_CPU_MAPLEN(VIR_NODEINFO_MAXCPUS(nodeinfo));
     cpumap = vshCalloc(ctl, 1, cpumaplen);

Attachment: smime.p7s
Description: S/MIME Cryptographic Signature