virBitmapParse: Fix behavior in case of error and fix up callers

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.

po/POTFILES.in

@@ -147,6 +147,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

src/conf/domain_conf.c

@@ -10981,11 +10981,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);
         }
     }

src/conf/network_conf.c

@@ -2897,9 +2897,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;
             }

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);

src/qemu/qemu_driver.c

@@ -8357,8 +8357,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;
             }

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;
+    if (!str)
+        goto error;
+
     virSkipSpaces(&cur);
 
-    if (*cur == 0)
-        return -1;
-
-    *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;

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");

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