From bec00335248f53fdfe7c731e4f4ff72272ca7cc1 Mon Sep 17 00:00:00 2001
Message-Id: <bec00335248f53fdfe7c731e4f4ff72272ca7cc1.1389183249.git.jdenemar@redhat.com>
From: Peter Krempa <pkrempa@redhat.com>
Date: Mon, 6 Jan 2014 17:02:25 +0100
Subject: [PATCH] virBitmapParse: Fix behavior in case of error and fix up
 callers

https://bugzilla.redhat.com/show_bug.cgi?id=1047234

Re-arrange the code so that the returned bitmap is always initialized to
NULL even on early failures and return an error message as some callers
are already expecting it. Fix up the rest not to shadow the error.

(cherry picked from commit 106a2ddaa7b5ef7d69242333e48d77fda0b7ccf3)

Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
---
 po/POTFILES.in          |  1 +
 src/conf/domain_conf.c  |  5 +----
 src/conf/network_conf.c |  3 ---
 src/nodeinfo.c          |  5 +----
 src/qemu/qemu_driver.c  |  2 --
 src/util/virbitmap.c    | 18 +++++++++---------
 src/xenxs/xen_sxpr.c    |  5 +----
 tests/cpuset            |  2 +-
 8 files changed, 14 insertions(+), 27 deletions(-)

diff --git a/po/POTFILES.in b/po/POTFILES.in
index 8afd015..0e4aa76 100644
--- a/po/POTFILES.in
+++ b/po/POTFILES.in
@@ -146,6 +146,7 @@ src/util/viralloc.c
 src/util/viraudit.c
 src/util/virauth.c
 src/util/virauthconfig.c
+src/util/virbitmap.c
 src/util/vircgroup.c
 src/util/virclosecallbacks.c
 src/util/vircommand.c
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index d4161a5..bc53a89 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -11081,11 +11081,8 @@ virDomainDefParseXML(xmlDocPtr xml,
         tmp = virXPathString("string(./vcpu[1]/@cpuset)", ctxt);
         if (tmp) {
             if (virBitmapParse(tmp, 0, &def->cpumask,
-                               VIR_DOMAIN_CPUMASK_LEN) < 0) {
-                virReportError(VIR_ERR_XML_ERROR,
-                               "%s", _("topology cpuset syntax error"));
+                               VIR_DOMAIN_CPUMASK_LEN) < 0)
                 goto error;
-            }
             VIR_FREE(tmp);
         }
     }
diff --git a/src/conf/network_conf.c b/src/conf/network_conf.c
index b358393..8846675 100644
--- a/src/conf/network_conf.c
+++ b/src/conf/network_conf.c
@@ -2873,9 +2873,6 @@ virNetworkLoadState(virNetworkObjListPtr nets,
         if ((class_id = virXPathString("string(./class_id[1]/@bitmap)", ctxt))) {
             if (virBitmapParse(class_id, 0, &class_id_map,
                                CLASS_ID_BITMAP_SIZE) < 0) {
-                virReportError(VIR_ERR_INTERNAL_ERROR,
-                               _("Malformed 'class_id' attribute: %s"),
-                               class_id);
                 VIR_FREE(class_id);
                 goto error;
             }
diff --git a/src/nodeinfo.c b/src/nodeinfo.c
index 4df4851..33a79b7 100644
--- a/src/nodeinfo.c
+++ b/src/nodeinfo.c
@@ -1547,11 +1547,8 @@ virNodeGetSiblingsList(const char *dir, int cpu_id)
     if (virFileReadAll(path, SYSFS_THREAD_SIBLINGS_LIST_LENGTH_MAX, &buf) < 0)
         goto cleanup;
 
-    if (virBitmapParse(buf, 0, &ret, NUMA_MAX_N_CPUS) < 0) {
-        virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
-                       _("Failed to parse thread siblings"));
+    if (virBitmapParse(buf, 0, &ret, NUMA_MAX_N_CPUS) < 0)
         goto cleanup;
-    }
 
 cleanup:
     VIR_FREE(buf);
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index d79bd02..5216e44 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -8484,8 +8484,6 @@ qemuDomainSetNumaParameters(virDomainPtr dom,
             if (virBitmapParse(params[i].value.s,
                                0, &nodeset,
                                VIR_DOMAIN_CPUMASK_LEN) < 0) {
-                virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
-                               _("Failed to parse nodeset"));
                 ret = -1;
                 continue;
             }
diff --git a/src/util/virbitmap.c b/src/util/virbitmap.c
index 47c678e..870c8fe 100644
--- a/src/util/virbitmap.c
+++ b/src/util/virbitmap.c
@@ -298,23 +298,21 @@ virBitmapParse(const char *str,
                size_t bitmapSize)
 {
     bool neg = false;
-    const char *cur;
+    const char *cur = str;
     char *tmp;
     size_t i;
     int start, last;
 
-    if (!str)
+    if (!(*bitmap = virBitmapNew(bitmapSize)))
         return -1;
 
-    cur = str;
-    virSkipSpaces(&cur);
+    if (!str)
+        goto error;
 
-    if (*cur == 0)
-        return -1;
+    virSkipSpaces(&cur);
 
-    *bitmap = virBitmapNew(bitmapSize);
-    if (!*bitmap)
-        return -1;
+    if (*cur == '\0')
+        goto error;
 
     while (*cur != 0 && *cur != terminator) {
         /*
@@ -384,6 +382,8 @@ virBitmapParse(const char *str,
     return virBitmapCountBits(*bitmap);
 
 error:
+    virReportError(VIR_ERR_INVALID_ARG,
+                   _("Failed to parse bitmap '%s'"), str);
     virBitmapFree(*bitmap);
     *bitmap = NULL;
     return -1;
diff --git a/src/xenxs/xen_sxpr.c b/src/xenxs/xen_sxpr.c
index fb01e0d..c61336c 100644
--- a/src/xenxs/xen_sxpr.c
+++ b/src/xenxs/xen_sxpr.c
@@ -1160,11 +1160,8 @@ xenParseSxpr(const struct sexpr *root,
 
     if (cpus != NULL) {
         if (virBitmapParse(cpus, 0, &def->cpumask,
-                           VIR_DOMAIN_CPUMASK_LEN) < 0) {
-            virReportError(VIR_ERR_INTERNAL_ERROR,
-                           _("invalid CPU mask %s"), cpus);
+                           VIR_DOMAIN_CPUMASK_LEN) < 0)
             goto error;
-        }
     }
 
     def->maxvcpus = sexpr_int(root, "domain/vcpus");
diff --git a/tests/cpuset b/tests/cpuset
index b617d6f..35803be 100755
--- a/tests/cpuset
+++ b/tests/cpuset
@@ -42,7 +42,7 @@ sed "s/vcpu placement='static'>/vcpu cpuset='aaa'>/" xml > xml-invalid || fail=1
 $abs_top_builddir/tools/virsh --connect test:///default define xml-invalid > out 2>&1 && fail=1
 cat <<\EOF > exp || fail=1
 error: Failed to define domain from xml-invalid
-error: XML error: topology cpuset syntax error
+error: invalid argument: Failed to parse bitmap 'aaa'
 
 EOF
 compare exp out || fail=1
-- 
1.8.5.2