conf: tighten up XML integer parsing

https://bugzilla.redhat.com/show_bug.cgi?id=617711 reported that
even with my recent patched to allow <memory unit='G'>1</memory>,
people can still get away with trying <memory>1G</memory> and
silently get <memory unit='KiB'>1</memory> instead.  While
virt-xml-validate catches the error, our C parser did not.

Not to mention that it's always fun to fix bugs while reducing
lines of code.  :)

* src/conf/domain_conf.c (virDomainParseMemory): Check for parse error.
(virDomainDefParseXML): Avoid strtoll.
* src/conf/storage_conf.c (virStorageDefParsePerms): Likewise.
* src/util/xml.c (virXPathLongBase, virXPathULongBase)
(virXPathULongLong, virXPathLongLong): Likewise.

diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 5ab052aff82cc6385908c4c3b064b8670c7a996a..8f352eaed89b510255648e80e34ddf41e0a02528 100644 (file)
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -7609,10 +7609,16 @@ virDomainParseMemory(const char *xpath, xmlXPathContextPtr ctxt,
         virReportOOMError();
         goto cleanup;
     }
-    if (virXPathULongLong(xpath_full, ctxt, &bytes) < 0) {
-        if (required)
+    ret = virXPathULongLong(xpath_full, ctxt, &bytes);
+    if (ret < 0) {
+        if (ret == -2)
             virDomainReportError(VIR_ERR_XML_ERROR,
-                                 "%s", _("missing memory element"));
+                                 _("could not parse memory element %s"),
+                                 xpath);
+        else if (required)
+            virDomainReportError(VIR_ERR_XML_ERROR,
+                                 _("missing memory element %s"),
+                                 xpath);
         else
             ret = 0;
         goto cleanup;
@@ -8086,12 +8092,11 @@ static virDomainDefPtr virDomainDefParseXML(virCapsPtr caps,
             if (STREQ(tmp, "reset")) {
                 def->clock.data.utc_reset = true;
             } else {
-                char *conv = NULL;
-                unsigned long long val;
-                val = strtoll(tmp, &conv, 10);
-                if (conv == tmp || *conv != '\0') {
-                    virDomainReportError(VIR_ERR_INTERNAL_ERROR,
-                                         _("unknown clock adjustment '%s'"), tmp);
+                if (virStrToLong_ll(tmp, NULL, 10,
+                                    &def->clock.data.variable.adjustment) < 0) {
+                    virDomainReportError(VIR_ERR_XML_ERROR,
+                                         _("unknown clock adjustment '%s'"),
+                                         tmp);
                     goto error;
                 }
                 switch (def->clock.offset) {
@@ -8103,7 +8108,6 @@ static virDomainDefPtr virDomainDefParseXML(virCapsPtr caps,
                     break;
                 }
                 def->clock.offset = VIR_DOMAIN_CLOCK_OFFSET_VARIABLE;
-                def->clock.data.variable.adjustment = val;
             }
             VIR_FREE(tmp);
         } else {

diff --git a/src/conf/storage_conf.c b/src/conf/storage_conf.c
index 2330fa1c6e18f4109436c952ec1939679eab64f9..75793277da9c9e0f346d45d818f9512f483b1460 100644 (file)
--- a/src/conf/storage_conf.c
+++ b/src/conf/storage_conf.c
@@ -570,14 +570,15 @@ virStorageDefParsePerms(xmlXPathContextPtr ctxt,
     if (!mode) {
         perms->mode = defaultmode;
     } else {
-        char *end = NULL;
-        perms->mode = strtol(mode, &end, 8);
-        if (*end || (perms->mode & ~0777)) {
+        int tmp;
+
+        if (virStrToLong_i(mode, NULL, 8, &tmp) < 0 || (tmp & ~0777)) {
             VIR_FREE(mode);
             virStorageReportError(VIR_ERR_XML_ERROR,
                                   "%s", _("malformed octal mode"));
             goto error;
         }
+        perms->mode = tmp;
         VIR_FREE(mode);
     }
 

diff --git a/src/util/xml.c b/src/util/xml.c
index 79a9d2781e517cff7cf8a94a04885d5145288aa1..741196837a51446bdad2329942b4b454185c7196 100644 (file)
--- a/src/util/xml.c
+++ b/src/util/xml.c
@@ -174,15 +174,8 @@ virXPathLongBase(const char *xpath,
     ctxt->node = relnode;
     if ((obj != NULL) && (obj->type == XPATH_STRING) &&
         (obj->stringval != NULL) && (obj->stringval[0] != 0)) {
-        char *conv = NULL;
-        long val;
-
-        val = strtol((const char *) obj->stringval, &conv, base);
-        if (conv == (const char *) obj->stringval) {
+        if (virStrToLong_l((char *) obj->stringval, NULL, base, value) < 0)
             ret = -2;
-        } else {
-            *value = val;
-        }
     } else if ((obj != NULL) && (obj->type == XPATH_NUMBER) &&
                (!(isnan(obj->floatval)))) {
         *value = (long) obj->floatval;
@@ -287,15 +280,8 @@ virXPathULongBase(const char *xpath,
     ctxt->node = relnode;
     if ((obj != NULL) && (obj->type == XPATH_STRING) &&
         (obj->stringval != NULL) && (obj->stringval[0] != 0)) {
-        char *conv = NULL;
-        long val;
-
-        val = strtoul((const char *) obj->stringval, &conv, base);
-        if (conv == (const char *) obj->stringval) {
+        if (virStrToLong_ul((char *) obj->stringval, NULL, base, value) < 0)
             ret = -2;
-        } else {
-            *value = val;
-        }
     } else if ((obj != NULL) && (obj->type == XPATH_NUMBER) &&
                (!(isnan(obj->floatval)))) {
         *value = (unsigned long) obj->floatval;
@@ -411,15 +397,8 @@ virXPathULongLong(const char *xpath,
     ctxt->node = relnode;
     if ((obj != NULL) && (obj->type == XPATH_STRING) &&
         (obj->stringval != NULL) && (obj->stringval[0] != 0)) {
-        char *conv = NULL;
-        unsigned long long val;
-
-        val = strtoull((const char *) obj->stringval, &conv, 10);
-        if (conv == (const char *) obj->stringval) {
+        if (virStrToLong_ull((char *) obj->stringval, NULL, 10, value) < 0)
             ret = -2;
-        } else {
-            *value = val;
-        }
     } else if ((obj != NULL) && (obj->type == XPATH_NUMBER) &&
                (!(isnan(obj->floatval)))) {
         *value = (unsigned long long) obj->floatval;
@@ -465,15 +444,8 @@ virXPathLongLong(const char *xpath,
     ctxt->node = relnode;
     if ((obj != NULL) && (obj->type == XPATH_STRING) &&
         (obj->stringval != NULL) && (obj->stringval[0] != 0)) {
-        char *conv = NULL;
-        unsigned long long val;
-
-        val = strtoll((const char *) obj->stringval, &conv, 10);
-        if (conv == (const char *) obj->stringval) {
+        if (virStrToLong_ll((char *) obj->stringval, NULL, 10, value) < 0)
             ret = -2;
-        } else {
-            *value = val;
-        }
     } else if ((obj != NULL) && (obj->type == XPATH_NUMBER) &&
                (!(isnan(obj->floatval)))) {
         *value = (long long) obj->floatval;