From 0571c3afde30267536a80c0323efe3383e02cf37 Mon Sep 17 00:00:00 2001 From: Cole Robinson Date: Thu, 12 May 2011 15:45:22 -0400 Subject: [PATCH] xml: Make sure virXpathNodeSet always sets an error And update callers to actually respect the error --- src/conf/domain_conf.c | 38 ------------------------------ src/conf/interface_conf.c | 15 ++++++++---- src/conf/network_conf.c | 3 +++ src/conf/node_device_conf.c | 12 ++++------ src/conf/storage_conf.c | 3 +++ src/conf/storage_encryption_conf.c | 2 -- src/qemu/qemu_domain.c | 2 -- src/test/test_driver.c | 7 ------ src/util/xml.c | 4 ++++ 9 files changed, 26 insertions(+), 60 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index d3efec6249..9ab9a5a498 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -5228,8 +5228,6 @@ virDomainDefParseBootXML(xmlXPathContextPtr ctxt, /* analysis of the boot devices */ if ((n = virXPathNodeSet("./os/boot", ctxt, &nodes)) < 0) { - virDomainReportError(VIR_ERR_INTERNAL_ERROR, - "%s", _("cannot extract boot device")); goto cleanup; } @@ -5517,8 +5515,6 @@ static virDomainDefPtr virDomainDefParseXML(virCapsPtr caps, def->cputune.shares = 0; if ((n = virXPathNodeSet("./cputune/vcpupin", ctxt, &nodes)) < 0) { - virDomainReportError(VIR_ERR_INTERNAL_ERROR, - "%s", _("cannot extract vcpupin nodes")); goto error; } @@ -5613,8 +5609,6 @@ static virDomainDefPtr virDomainDefParseXML(virCapsPtr caps, } if ((n = virXPathNodeSet("./clock/timer", ctxt, &nodes)) < 0) { - virDomainReportError(VIR_ERR_INTERNAL_ERROR, - "%s", _("failed to parse timers")); goto error; } if (n && VIR_ALLOC_N(def->clock.timers, n) < 0) @@ -5742,8 +5736,6 @@ static virDomainDefPtr virDomainDefParseXML(virCapsPtr caps, /* analysis of the disk devices */ if ((n = virXPathNodeSet("./devices/disk", ctxt, &nodes)) < 0) { - virDomainReportError(VIR_ERR_INTERNAL_ERROR, - "%s", _("cannot extract disk devices")); goto error; } if (n && VIR_ALLOC_N(def->disks, n) < 0) @@ -5762,8 +5754,6 @@ static virDomainDefPtr virDomainDefParseXML(virCapsPtr caps, /* analysis of the controller devices */ if ((n = virXPathNodeSet("./devices/controller", ctxt, &nodes)) < 0) { - virDomainReportError(VIR_ERR_INTERNAL_ERROR, - "%s", _("cannot extract controller devices")); goto error; } if (n && VIR_ALLOC_N(def->controllers, n) < 0) @@ -5780,8 +5770,6 @@ static virDomainDefPtr virDomainDefParseXML(virCapsPtr caps, /* analysis of the filesystems */ if ((n = virXPathNodeSet("./devices/filesystem", ctxt, &nodes)) < 0) { - virDomainReportError(VIR_ERR_INTERNAL_ERROR, - "%s", _("cannot extract filesystem devices")); goto error; } if (n && VIR_ALLOC_N(def->fss, n) < 0) @@ -5798,8 +5786,6 @@ static virDomainDefPtr virDomainDefParseXML(virCapsPtr caps, /* analysis of the network devices */ if ((n = virXPathNodeSet("./devices/interface", ctxt, &nodes)) < 0) { - virDomainReportError(VIR_ERR_INTERNAL_ERROR, - "%s", _("cannot extract network devices")); goto error; } if (n && VIR_ALLOC_N(def->nets, n) < 0) @@ -5820,8 +5806,6 @@ static virDomainDefPtr virDomainDefParseXML(virCapsPtr caps, /* analysis of the smartcard devices */ if ((n = virXPathNodeSet("./devices/smartcard", ctxt, &nodes)) < 0) { - virDomainReportError(VIR_ERR_INTERNAL_ERROR, - "%s", _("cannot extract smartcard devices")); goto error; } if (n && VIR_ALLOC_N(def->smartcards, n) < 0) @@ -5840,8 +5824,6 @@ static virDomainDefPtr virDomainDefParseXML(virCapsPtr caps, /* analysis of the character devices */ if ((n = virXPathNodeSet("./devices/parallel", ctxt, &nodes)) < 0) { - virDomainReportError(VIR_ERR_INTERNAL_ERROR, - "%s", _("cannot extract parallel devices")); goto error; } if (n && VIR_ALLOC_N(def->parallels, n) < 0) @@ -5868,8 +5850,6 @@ static virDomainDefPtr virDomainDefParseXML(virCapsPtr caps, VIR_FREE(nodes); if ((n = virXPathNodeSet("./devices/serial", ctxt, &nodes)) < 0) { - virDomainReportError(VIR_ERR_INTERNAL_ERROR, - "%s", _("cannot extract serial devices")); goto error; } if (n && VIR_ALLOC_N(def->serials, n) < 0) @@ -5926,8 +5906,6 @@ static virDomainDefPtr virDomainDefParseXML(virCapsPtr caps, } if ((n = virXPathNodeSet("./devices/channel", ctxt, &nodes)) < 0) { - virDomainReportError(VIR_ERR_INTERNAL_ERROR, - "%s", _("cannot extract channel devices")); goto error; } if (n && VIR_ALLOC_N(def->channels, n) < 0) @@ -5967,8 +5945,6 @@ static virDomainDefPtr virDomainDefParseXML(virCapsPtr caps, /* analysis of the input devices */ if ((n = virXPathNodeSet("./devices/input", ctxt, &nodes)) < 0) { - virDomainReportError(VIR_ERR_INTERNAL_ERROR, - "%s", _("cannot extract input devices")); goto error; } if (n && VIR_ALLOC_N(def->inputs, n) < 0) @@ -6001,8 +5977,6 @@ static virDomainDefPtr virDomainDefParseXML(virCapsPtr caps, /* analysis of the graphics devices */ if ((n = virXPathNodeSet("./devices/graphics", ctxt, &nodes)) < 0) { - virDomainReportError(VIR_ERR_INTERNAL_ERROR, - "%s", _("cannot extract graphics devices")); goto error; } if (n && VIR_ALLOC_N(def->graphics, n) < 0) @@ -6044,8 +6018,6 @@ static virDomainDefPtr virDomainDefParseXML(virCapsPtr caps, /* analysis of the sound devices */ if ((n = virXPathNodeSet("./devices/sound", ctxt, &nodes)) < 0) { - virDomainReportError(VIR_ERR_INTERNAL_ERROR, - "%s", _("cannot extract sound devices")); goto error; } if (n && VIR_ALLOC_N(def->sounds, n) < 0) @@ -6062,8 +6034,6 @@ static virDomainDefPtr virDomainDefParseXML(virCapsPtr caps, /* analysis of the video devices */ if ((n = virXPathNodeSet("./devices/video", ctxt, &nodes)) < 0) { - virDomainReportError(VIR_ERR_INTERNAL_ERROR, - "%s", _("cannot extract video devices")); goto error; } if (n && VIR_ALLOC_N(def->videos, n) < 0) @@ -6102,8 +6072,6 @@ static virDomainDefPtr virDomainDefParseXML(virCapsPtr caps, /* analysis of the host devices */ if ((n = virXPathNodeSet("./devices/hostdev", ctxt, &nodes)) < 0) { - virDomainReportError(VIR_ERR_INTERNAL_ERROR, - "%s", _("cannot extract host devices")); goto error; } if (n && VIR_ALLOC_N(def->hostdevs, n) < 0) @@ -6122,8 +6090,6 @@ static virDomainDefPtr virDomainDefParseXML(virCapsPtr caps, /* analysis of the watchdog devices */ def->watchdog = NULL; if ((n = virXPathNodeSet("./devices/watchdog", ctxt, &nodes)) < 0) { - virDomainReportError(VIR_ERR_INTERNAL_ERROR, - "%s", _("cannot extract watchdog devices")); goto error; } if (n > 1) { @@ -6144,8 +6110,6 @@ static virDomainDefPtr virDomainDefParseXML(virCapsPtr caps, /* analysis of the memballoon devices */ def->memballoon = NULL; if ((n = virXPathNodeSet("./devices/memballoon", ctxt, &nodes)) < 0) { - virDomainReportError(VIR_ERR_INTERNAL_ERROR, - "%s", _("cannot extract memory balloon devices")); goto error; } if (n > 1) { @@ -6312,8 +6276,6 @@ static virDomainObjPtr virDomainObjParseXML(virCapsPtr caps, obj->pid = (pid_t)val; if ((n = virXPathNodeSet("./taint", ctxt, &nodes)) < 0) { - virDomainReportError(VIR_ERR_INTERNAL_ERROR, - "%s", _("failed to parse taint flags")); goto error; } for (i = 0 ; i < n ; i++) { diff --git a/src/conf/interface_conf.c b/src/conf/interface_conf.c index 2c051ea009..f3848bdaab 100644 --- a/src/conf/interface_conf.c +++ b/src/conf/interface_conf.c @@ -424,7 +424,10 @@ virInterfaceDefParseIfAdressing(virInterfaceDefPtr def, save = ctxt->node; nProtoNodes = virXPathNodeSet("./protocol", ctxt, &protoNodes); - if (nProtoNodes <= 0) { + if (nProtoNodes < 0) + goto error; + + if (nProtoNodes == 0) { /* no protocols is an acceptable outcome */ return 0; } @@ -495,8 +498,6 @@ virInterfaceDefParseBridge(virInterfaceDefPtr def, bridge = ctxt->node; nbItf = virXPathNodeSet("./interface", ctxt, &interfaces); if (nbItf < 0) { - virInterfaceReportError(VIR_ERR_XML_ERROR, - "%s", _("bridge interfaces")); ret = -1; goto error; } @@ -536,12 +537,18 @@ virInterfaceDefParseBondItfs(virInterfaceDefPtr def, int ret = 0; nbItf = virXPathNodeSet("./interface", ctxt, &interfaces); - if (nbItf <= 0) { + if (nbItf < 0) { + ret = -1; + goto error; + } + + if (nbItf == 0) { virInterfaceReportError(VIR_ERR_XML_ERROR, "%s", _("bond has no interfaces")); ret = -1; goto error; } + if (VIR_ALLOC_N(def->data.bond.itf, nbItf) < 0) { virReportOOMError(); ret = -1; diff --git a/src/conf/network_conf.c b/src/conf/network_conf.c index 4eb46faea1..e4765ea48c 100644 --- a/src/conf/network_conf.c +++ b/src/conf/network_conf.c @@ -642,6 +642,9 @@ virNetworkDefParseXML(xmlXPathContextPtr ctxt) } nIps = virXPathNodeSet("./ip", ctxt, &ipNodes); + if (nIps < 0) + goto error; + if (nIps > 0) { int ii; diff --git a/src/conf/node_device_conf.c b/src/conf/node_device_conf.c index e9b8978b2c..dde292106a 100644 --- a/src/conf/node_device_conf.c +++ b/src/conf/node_device_conf.c @@ -570,9 +570,6 @@ virNodeDevCapStorageParseXML(xmlXPathContextPtr ctxt, data->storage.serial = virXPathString("string(./serial[1])", ctxt); if ((n = virXPathNodeSet("./capability", ctxt, &nodes)) < 0) { - virNodeDeviceReportError(VIR_ERR_INTERNAL_ERROR, - _("error parsing storage capabilities for '%s'"), - def->name); goto out; } @@ -735,9 +732,6 @@ virNodeDevCapScsiHostParseXML(xmlXPathContextPtr ctxt, } if ((n = virXPathNodeSet("./capability", ctxt, &nodes)) < 0) { - virNodeDeviceReportError(VIR_ERR_INTERNAL_ERROR, - _("error parsing SCSI host capabilities for '%s'"), - def->name); goto out; } @@ -1170,7 +1164,11 @@ virNodeDeviceDefParseXML(xmlXPathContextPtr ctxt, int create) /* Parse device capabilities */ nodes = NULL; - if ((n = virXPathNodeSet("./capability", ctxt, &nodes)) <= 0) { + if ((n = virXPathNodeSet("./capability", ctxt, &nodes)) < 0) { + goto error; + } + + if (n == 0) { virNodeDeviceReportError(VIR_ERR_INTERNAL_ERROR, _("no device capabilities for '%s'"), def->name); diff --git a/src/conf/storage_conf.c b/src/conf/storage_conf.c index 9be4caea76..ed7d300654 100644 --- a/src/conf/storage_conf.c +++ b/src/conf/storage_conf.c @@ -438,6 +438,9 @@ virStoragePoolDefParseSource(xmlXPathContextPtr ctxt, source->initiator.iqn = virXPathString("string(./initiator/iqn/@name)", ctxt); nsource = virXPathNodeSet("./device", ctxt, &nodeset); + if (nsource < 0) + goto cleanup; + if (nsource > 0) { if (VIR_ALLOC_N(source->devices, nsource) < 0) { VIR_FREE(nodeset); diff --git a/src/conf/storage_encryption_conf.c b/src/conf/storage_encryption_conf.c index 9ec3dade3b..545efadf3d 100644 --- a/src/conf/storage_encryption_conf.c +++ b/src/conf/storage_encryption_conf.c @@ -159,8 +159,6 @@ virStorageEncryptionParseXML(xmlXPathContextPtr ctxt) n = virXPathNodeSet("./secret", ctxt, &nodes); if (n < 0){ - virStorageReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("cannot extract volume encryption secrets")); goto cleanup; } if (n != 0 && VIR_ALLOC_N(ret->secrets, n) < 0) { diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 19e1938018..f579439f4e 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -351,7 +351,6 @@ qemuDomainDefNamespaceParse(xmlDocPtr xml, /* first handle the extra command-line arguments */ n = virXPathNodeSet("./qemu:commandline/qemu:arg", ctxt, &nodes); if (n < 0) - /* virXPathNodeSet already set the error */ goto error; if (n && VIR_ALLOC_N(cmd->args, n) < 0) @@ -372,7 +371,6 @@ qemuDomainDefNamespaceParse(xmlDocPtr xml, /* now handle the extra environment variables */ n = virXPathNodeSet("./qemu:commandline/qemu:env", ctxt, &nodes); if (n < 0) - /* virXPathNodeSet already set the error */ goto error; if (n && VIR_ALLOC_N(cmd->env_name, n) < 0) diff --git a/src/test/test_driver.c b/src/test/test_driver.c index 0f0d77bc5e..8fdf916fc5 100644 --- a/src/test/test_driver.c +++ b/src/test/test_driver.c @@ -681,8 +681,6 @@ static int testOpenVolumesForPool(xmlDocPtr xml, ret = virXPathNodeSet(vol_xpath, ctxt, &vols); VIR_FREE(vol_xpath); if (ret < 0) { - testError(VIR_ERR_XML_ERROR, - _("node vol list for pool '%s'"), pool->def->name); goto error; } @@ -892,7 +890,6 @@ static int testOpenFromFile(virConnectPtr conn, ret = virXPathNodeSet("/node/domain", ctxt, &domains); if (ret < 0) { - testError(VIR_ERR_XML_ERROR, "%s", _("node domain list")); goto error; } @@ -936,7 +933,6 @@ static int testOpenFromFile(virConnectPtr conn, ret = virXPathNodeSet("/node/network", ctxt, &networks); if (ret < 0) { - testError(VIR_ERR_XML_ERROR, "%s", _("node network list")); goto error; } for (i = 0 ; i < ret ; i++) { @@ -972,7 +968,6 @@ static int testOpenFromFile(virConnectPtr conn, /* Parse interface definitions */ ret = virXPathNodeSet("/node/interface", ctxt, &ifaces); if (ret < 0) { - testError(VIR_ERR_XML_ERROR, "%s", _("node interface list")); goto error; } for (i = 0 ; i < ret ; i++) { @@ -1008,7 +1003,6 @@ static int testOpenFromFile(virConnectPtr conn, /* Parse Storage Pool list */ ret = virXPathNodeSet("/node/pool", ctxt, &pools); if (ret < 0) { - testError(VIR_ERR_XML_ERROR, "%s", _("node pool list")); goto error; } for (i = 0 ; i < ret ; i++) { @@ -1059,7 +1053,6 @@ static int testOpenFromFile(virConnectPtr conn, ret = virXPathNodeSet("/node/device", ctxt, &devs); if (ret < 0) { - testError(VIR_ERR_XML_ERROR, "%s", _("node device list")); goto error; } for (i = 0 ; i < ret ; i++) { diff --git a/src/util/xml.c b/src/util/xml.c index d2989e2a63..05317eabc3 100644 --- a/src/util/xml.c +++ b/src/util/xml.c @@ -601,10 +601,14 @@ virXPathNodeSet(const char *xpath, ctxt->node = relnode; if (obj == NULL) return(0); + if (obj->type != XPATH_NODESET) { + virXMLError(VIR_ERR_INTERNAL_ERROR, + _("Incorrect xpath '%s'"), xpath); xmlXPathFreeObject(obj); return (-1); } + if ((obj->nodesetval == NULL) || (obj->nodesetval->nodeNr < 0)) { xmlXPathFreeObject(obj); return (0); -- GitLab