[PATCH 4/5] treat all NULL returns from virXMLNodeContentString() as an error
Laine Stump
laine at redhat.com
Mon Jul 20 20:48:42 UTC 2020
and stop erroneously equating NULL with "". The latter means that the
element has empty content, while the former means there was an error
during parsing (either internal with the parser, or the content of the
XML was bad).
Signed-off-by: Laine Stump <laine at redhat.com>
---
src/conf/domain_conf.c | 68 ++++++++++++++++++++++---------------
src/conf/network_conf.c | 5 ++-
src/conf/node_device_conf.c | 6 ++--
3 files changed, 48 insertions(+), 31 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index cb69c97a8e..c377fd74aa 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -1643,7 +1643,8 @@ virDomainBlkioDeviceParseXML(xmlNodePtr root,
if (node->type != XML_ELEMENT_NODE)
continue;
- c = virXMLNodeContentString(node);
+ if (!(c = virXMLNodeContentString(node)))
+ return -1;
if (virXMLNodeNameEqual(node, "path")) {
/* To avoid the need for explicit cleanup on failure,
@@ -9373,10 +9374,12 @@ virDomainLeaseDefParseXML(xmlNodePtr node)
while (cur != NULL) {
if (cur->type == XML_ELEMENT_NODE) {
if (!key && virXMLNodeNameEqual(cur, "key")) {
- key = virXMLNodeContentString(cur);
+ if (!(key = virXMLNodeContentString(cur)))
+ goto error;
} else if (!lockspace &&
virXMLNodeNameEqual(cur, "lockspace")) {
- lockspace = virXMLNodeContentString(cur);
+ if (!(lockspace = virXMLNodeContentString(cur)))
+ goto error;
} else if (!path &&
virXMLNodeNameEqual(cur, "target")) {
path = virXMLPropString(cur, "path");
@@ -10595,16 +10598,19 @@ virDomainDiskDefParseXML(virDomainXMLOptionPtr xmlopt,
} else if (!serial &&
virXMLNodeNameEqual(cur, "serial")) {
- serial = virXMLNodeContentString(cur);
+ if (!(serial = virXMLNodeContentString(cur)))
+ goto error;
} else if (!wwn &&
virXMLNodeNameEqual(cur, "wwn")) {
- wwn = virXMLNodeContentString(cur);
+ if (!(wwn = virXMLNodeContentString(cur)))
+ goto error;
if (!virValidateWWN(wwn))
goto error;
} else if (!vendor &&
virXMLNodeNameEqual(cur, "vendor")) {
- vendor = virXMLNodeContentString(cur);
+ if (!(vendor = virXMLNodeContentString(cur)))
+ goto error;
if (strlen(vendor) > VENDOR_LEN) {
virReportError(VIR_ERR_XML_ERROR, "%s",
@@ -10619,7 +10625,8 @@ virDomainDiskDefParseXML(virDomainXMLOptionPtr xmlopt,
}
} else if (!product &&
virXMLNodeNameEqual(cur, "product")) {
- product = virXMLNodeContentString(cur);
+ if (!(product = virXMLNodeContentString(cur)))
+ goto error;
if (strlen(product) > PRODUCT_LEN) {
virReportError(VIR_ERR_XML_ERROR, "%s",
@@ -13513,20 +13520,16 @@ virDomainSmartcardDefParseXML(virDomainXMLOptionPtr xmlopt,
"exactly three certificates"));
goto error;
}
- def->data.cert.file[i] = virXMLNodeContentString(cur);
- if (!def->data.cert.file[i]) {
- virReportOOMError();
+ if (!(def->data.cert.file[i] = virXMLNodeContentString(cur)))
goto error;
- }
+
i++;
} else if (cur->type == XML_ELEMENT_NODE &&
virXMLNodeNameEqual(cur, "database") &&
!def->data.cert.database) {
- def->data.cert.database = virXMLNodeContentString(cur);
- if (!def->data.cert.database) {
- virReportOOMError();
+ if (!(def->data.cert.database = virXMLNodeContentString(cur)))
goto error;
- }
+
if (*def->data.cert.database != '/') {
virReportError(VIR_ERR_XML_ERROR,
_("expecting absolute path: %s"),
@@ -15638,8 +15641,10 @@ virSysinfoOEMStringsParseXML(xmlNodePtr node,
goto cleanup;
def->nvalues = nstrings;
- for (i = 0; i < nstrings; i++)
- def->values[i] = virXMLNodeContentString(strings[i]);
+ for (i = 0; i < nstrings; i++) {
+ if (!(def->values[i] = virXMLNodeContentString(strings[i])))
+ goto cleanup;
+ }
*oem = g_steal_pointer(&def);
ret = 0;
@@ -15767,7 +15772,9 @@ virSysinfoParseFWCfgDef(virSysinfoDefPtr def,
return -1;
}
- value = virXMLNodeContentString(nodes[i]);
+ if (!(value = virXMLNodeContentString(nodes[i])))
+ return -1;
+
file = virXMLPropString(nodes[i], "file");
if (virStringIsEmpty(value))
@@ -19875,8 +19882,10 @@ virDomainLoaderDefParseXML(xmlNodePtr node,
if (!fwAutoSelect) {
readonly_str = virXMLPropString(node, "readonly");
type_str = virXMLPropString(node, "type");
- loader->path = virXMLNodeContentString(node);
- if (STREQ_NULLABLE(loader->path, ""))
+ if (!(loader->path = virXMLNodeContentString(node)))
+ return -1;
+
+ if (STREQ(loader->path, ""))
VIR_FREE(loader->path);
}
@@ -20093,14 +20102,15 @@ virDomainVcpuParse(virDomainDefPtr def,
vcpus = maxvcpus = 1;
if ((vcpuNode = virXPathNode("./vcpu[1]", ctxt))) {
- if ((tmp = virXMLNodeContentString(vcpuNode))) {
- if (virStrToLong_ui(tmp, NULL, 10, &maxvcpus) < 0) {
- virReportError(VIR_ERR_XML_ERROR, "%s",
- _("maximum vcpus count must be an integer"));
- return -1;
- }
- VIR_FREE(tmp);
+ if (!(tmp = virXMLNodeContentString(vcpuNode)))
+ return -1;
+
+ if (virStrToLong_ui(tmp, NULL, 10, &maxvcpus) < 0) {
+ virReportError(VIR_ERR_XML_ERROR, "%s",
+ _("maximum vcpus count must be an integer"));
+ return -1;
}
+ VIR_FREE(tmp);
if ((tmp = virXMLPropString(vcpuNode, "current"))) {
if (virStrToLong_ui(tmp, NULL, 10, &vcpus) < 0) {
@@ -20361,7 +20371,9 @@ virDomainDefParseBootOptions(virDomainDefPtr def,
if (STREQ_NULLABLE(tmp, "slic")) {
VIR_FREE(tmp);
- tmp = virXMLNodeContentString(nodes[0]);
+ if (!(tmp = virXMLNodeContentString(nodes[0])))
+ return -1;
+
def->os.slic_table = virFileSanitizePath(tmp);
} else {
virReportError(VIR_ERR_XML_ERROR,
diff --git a/src/conf/network_conf.c b/src/conf/network_conf.c
index 0a32f57188..54a723be64 100644
--- a/src/conf/network_conf.c
+++ b/src/conf/network_conf.c
@@ -722,7 +722,10 @@ virNetworkDNSHostDefParseXML(const char *networkName,
if (cur->children != NULL) {
g_autofree char *name = virXMLNodeContentString(cur);
- if (!name) {
+ if (!name)
+ goto error;
+
+ if (!name[0]) {
virReportError(VIR_ERR_XML_DETAIL,
_("Missing hostname in network '%s' DNS HOST record"),
networkName);
diff --git a/src/conf/node_device_conf.c b/src/conf/node_device_conf.c
index 7bbe709c5d..31c81a3b85 100644
--- a/src/conf/node_device_conf.c
+++ b/src/conf/node_device_conf.c
@@ -1988,10 +1988,12 @@ virNodeDeviceDefParseXML(xmlXPathContextPtr ctxt,
switch ((virNodeDevDevnodeType)val) {
case VIR_NODE_DEV_DEVNODE_DEV:
- def->devnode = virXMLNodeContentString(node);
+ if (!(def->devnode = virXMLNodeContentString(node)))
+ goto error;
break;
case VIR_NODE_DEV_DEVNODE_LINK:
- def->devlinks[m++] = virXMLNodeContentString(node);
+ if (!(def->devlinks[m++] = virXMLNodeContentString(node)))
+ goto error;
break;
case VIR_NODE_DEV_DEVNODE_LAST:
break;
--
2.25.4
More information about the libvir-list
mailing list