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 -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list