On 05/12/2011 03:47 PM, Cole Robinson wrote:
And update callers to actually respect the error
Signed-off-by: Cole Robinson <crobinso@redhat.com> --- 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 | 7 +++++- 9 files changed, 28 insertions(+), 61 deletions(-)
+++ b/src/util/xml.c @@ -589,7 +589,7 @@ virXPathNodeSet(const char *xpath,
if ((ctxt == NULL) || (xpath == NULL)) { virXMLError(VIR_ERR_INTERNAL_ERROR, - "%s", _("Invalid parameter to virXPathNodeSet()")); + _("Invalid parameter to %s"), __FUNCTION__);
Pre-existing, but virXMLError already outputs __FUNCTION__ earlier in the line. Any error message that lists its own name (whether explicitly as in pre-patch or implicitly with __FUNCTION__ or __func__ as in post-patch) looks silly.
@@ -601,10 +601,15 @@ 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' passed to %s"), + xpath, __FUNCTION__);
Likewise, and newly introduced. Here, you could get by with: _("Incorrect xpath '%s'"), xpath However, since the problem with __FUNCTION__ is more widespread than your patch, I'm okay whether you squash in a change to tweak those messages or save it for a a separate cleanup. ACK. -- Eric Blake eblake@redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org