1:24 p.m.
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.
---
v2: fix virDomainParseMemory to detect parse errors on optional
arguments, rather than silently ignoring those arguments
src/conf/domain_conf.c | 24 ++++++++++++++----------
src/conf/storage_conf.c | 7 ++++---
src/util/xml.c | 36 ++++--------------------------------
3 files changed, 22 insertions(+), 45 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 5ab052a..8f352ea 100644
--- 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 2330fa1..7579327 100644
--- 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 79a9d27..7411968 100644
--- 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;
--
1.7.7.6