diff --git a/ChangeLog b/ChangeLog index 56400f1050940f44fb92a9163337507d52ec51ab..87d347e28f9f21808e087bacadf4e7c22d2e510b 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,15 @@ +Fri Oct 6 10:33:20 EDT 2006 Daniel P. Berrange + + * src/xend_internal.c: Fixed memory leak in xend_get_config_version + routine. + * src/xml.c: Fixed memory leaks in XML parsing routines relating + to VNC port, HVM boot devices, HVM floppy & CDROM, HVM features, + disk device type. + * tests/Makefile.am: Use --leak-check=full when running valgrind + to detect all leaks, in addition to memory corruption checks + * tests/sexpr2xmltest.c, tests/xml2sexprtest.c: Fixed memory leaks + in test harness leading to valgrind false-positives. + Mon Oct 2 23:16:06 CEST 2006 Daniel Veillard * src/xen_internal.c: Daniel Berrange fixed some mlock size problem diff --git a/src/xend_internal.c b/src/xend_internal.c index 0bd4e2b1508d6d8fb702bda85b3d9484c2e36984..70b162fa9494dac85a544d16da220041fece5f23 100644 --- a/src/xend_internal.c +++ b/src/xend_internal.c @@ -1260,7 +1260,6 @@ xend_get_node(virConnectPtr xend) static int xend_get_config_version(virConnectPtr conn) { - int ret = -1; struct sexpr *root; const char *value; @@ -1276,15 +1275,16 @@ xend_get_config_version(virConnectPtr conn) { value = sexpr_node(root, "node/xend_config_format"); if (value) { - return strtol(value, NULL, 10); - } else { - /* Xen prior to 3.0.3 did not have the xend_config_format - field, and is implicitly version 1. */ - return 1; - } + int version = strtol(value, NULL, 10); + sexpr_free(root); + return version; + } sexpr_free(root); - return (ret); + + /* Xen prior to 3.0.3 did not have the xend_config_format + field, and is implicitly version 1. */ + return 1; } diff --git a/src/xml.c b/src/xml.c index e4f7365382cc16761e35c448c627c9ae2f8d8852..7ae89b3ccfd8aa6869060e4f39002371c863bcb7 100644 --- a/src/xml.c +++ b/src/xml.c @@ -598,18 +598,16 @@ static int virDomainParseXMLGraphicsDesc(xmlNodePtr node, virBufferPtr buf, int //virBufferAdd(buf, "(xauthority /root/.Xauthority)", 30); } else if (xmlStrEqual(graphics_type, BAD_CAST "vnc")) { - xmlChar *vncport = NULL; - long port; - virBufferAdd(buf, "(vnc 1)", 7); if (xendConfigVersion >= 2) { - vncport = xmlGetProp(node, BAD_CAST "port"); + xmlChar *vncport = xmlGetProp(node, BAD_CAST "port"); if (vncport != NULL) { - port = strtol((const char *)vncport, NULL, 10); + long port = strtol((const char *)vncport, NULL, 10); if (port == -1) virBufferAdd(buf, "(vncunused 1)", 13); else if (port > 5900) virBufferVSprintf(buf, "(vncdisplay %d)", port - 5900); + xmlFree(vncport); } } } @@ -638,9 +636,9 @@ virDomainParseXMLOSDescHVM(xmlNodePtr node, virBufferPtr buf, xmlXPathContextPtr { xmlXPathObjectPtr obj = NULL; xmlNodePtr cur, txt; - const xmlChar *type = NULL; - const xmlChar *loader = NULL; - const xmlChar *boot_dev = NULL; + xmlChar *type = NULL; + xmlChar *loader = NULL; + xmlChar *boot_dev = NULL; int res; cur = node->children; @@ -720,9 +718,12 @@ virDomainParseXMLOSDescHVM(xmlNodePtr node, virBufferPtr buf, xmlXPathContextPtr obj = xmlXPathEval(BAD_CAST "/domain/devices/disk[@device='floppy' and target/@dev='fdb']/source", ctxt); if ((obj != NULL) && (obj->type == XPATH_NODESET) && (obj->nodesetval != NULL) && (obj->nodesetval->nodeNr == 1)) { + xmlChar *fdfile = NULL; cur = obj->nodesetval->nodeTab[0]; + fdfile = xmlGetProp(cur, BAD_CAST "file"); virBufferVSprintf(buf, "(fdb '%s')", - (const char *) xmlGetProp(cur, BAD_CAST "file")); + (const char *) fdfile); + xmlFree(fdfile); cur = NULL; } if (obj) { @@ -737,9 +738,12 @@ virDomainParseXMLOSDescHVM(xmlNodePtr node, virBufferPtr buf, xmlXPathContextPtr obj = xmlXPathEval(BAD_CAST "/domain/devices/disk[@device='cdrom' and target/@dev='hdc']/source", ctxt); if ((obj != NULL) && (obj->type == XPATH_NODESET) && (obj->nodesetval != NULL) && (obj->nodesetval->nodeNr == 1)) { + xmlChar *cdfile = NULL; cur = obj->nodesetval->nodeTab[0]; + cdfile = xmlGetProp(cur, BAD_CAST "file"); virBufferVSprintf(buf, "(cdrom '%s')", - (const char *) xmlGetProp(cur, BAD_CAST "file")); + (const char *)cdfile); + xmlFree(cdfile); cur = NULL; } if (obj) { @@ -750,25 +754,26 @@ virDomainParseXMLOSDescHVM(xmlNodePtr node, virBufferPtr buf, xmlXPathContextPtr obj = xmlXPathEval(BAD_CAST "/domain/features/acpi", ctxt); if ((obj != NULL) && (obj->type == XPATH_NODESET) && - (obj->nodesetval != NULL) && (obj->nodesetval->nodeNr == 1)) { + (obj->nodesetval != NULL) && (obj->nodesetval->nodeNr == 1)) { virBufferAdd(buf, "(acpi 1)", 8); - xmlXPathFreeObject(obj); - obj = NULL; } + if (obj) + xmlXPathFreeObject(obj); obj = xmlXPathEval(BAD_CAST "/domain/features/apic", ctxt); if ((obj != NULL) && (obj->type == XPATH_NODESET) && - (obj->nodesetval != NULL) && (obj->nodesetval->nodeNr == 1)) { + (obj->nodesetval != NULL) && (obj->nodesetval->nodeNr == 1)) { virBufferAdd(buf, "(apic 1)", 8); - xmlXPathFreeObject(obj); - obj = NULL; } + if (obj) + xmlXPathFreeObject(obj); obj = xmlXPathEval(BAD_CAST "/domain/features/pae", ctxt); if ((obj != NULL) && (obj->type == XPATH_NODESET) && - (obj->nodesetval != NULL) && (obj->nodesetval->nodeNr == 1)) { + (obj->nodesetval != NULL) && (obj->nodesetval->nodeNr == 1)) { virBufferAdd(buf, "(pae 1)", 7); - xmlXPathFreeObject(obj); - obj = NULL; } + if (obj) + xmlXPathFreeObject(obj); + obj = NULL; } obj = xmlXPathEval(BAD_CAST "count(domain/devices/console) > 0", ctxt); @@ -795,8 +800,13 @@ virDomainParseXMLOSDescHVM(xmlNodePtr node, virBufferPtr buf, xmlXPathContextPtr virBufferAdd(buf, "))", 2); + if (boot_dev) + xmlFree(boot_dev); + return (0); error: + if (boot_dev) + xmlFree(boot_dev); if (obj != NULL) xmlXPathFreeObject(obj); return(-1); @@ -960,12 +970,16 @@ virDomainParseXMLDiskDesc(xmlNodePtr node, virBufferPtr buf, int hvm, int xendCo if (target != NULL) xmlFree(target); + if (device != NULL) + xmlFree(device); return (-1); } if (target == NULL) { virXMLError(VIR_ERR_NO_TARGET, (const char *) source, 0); if (source != NULL) xmlFree(source); + if (device != NULL) + xmlFree(device); return (-1); } @@ -975,7 +989,7 @@ virDomainParseXMLDiskDesc(xmlNodePtr node, virBufferPtr buf, int hvm, int xendCo if (hvm && device && !strcmp((const char *)device, "floppy")) { - return 0; + goto cleanup; } /* Xend <= 3.0.2 doesn't include cdrom config here */ @@ -983,7 +997,7 @@ virDomainParseXMLDiskDesc(xmlNodePtr node, virBufferPtr buf, int hvm, int xendCo device && !strcmp((const char *)device, "cdrom")) { if (xendConfigVersion == 1) - return 0; + goto cleanup; else cdrom = 1; } @@ -1021,6 +1035,9 @@ virDomainParseXMLDiskDesc(xmlNodePtr node, virBufferPtr buf, int hvm, int xendCo virBufferAdd(buf, ")", 1); virBufferAdd(buf, ")", 1); + + cleanup: + xmlFree(device); xmlFree(target); xmlFree(source); return (0); @@ -1302,6 +1319,8 @@ virDomainParseXMLDesc(const char *xmldesc, char **name, int xendConfigVersion) if (name != NULL) *name = nam; + else + free(nam); return (ret); diff --git a/tests/Makefile.am b/tests/Makefile.am index 72c68ebee5f47f0c8531ae156759d7e217a9883a..8ee3cf3217a8f2968b1fdedefa812d3200914114 100644 --- a/tests/Makefile.am +++ b/tests/Makefile.am @@ -25,7 +25,7 @@ noinst_PROGRAMS = xmlrpctest xml2sexprtest sexpr2xmltest virshtest conftest \ TESTS = xml2sexprtest sexpr2xmltest virshtest test_conf.sh reconnect valgrind: - $(MAKE) check TESTS_ENVIRONMENT="valgrind --quiet" + $(MAKE) check TESTS_ENVIRONMENT="valgrind --quiet --leak-check=full" # Note: xmlrpc.[c|h] is not in libvirt yet xmlrpctest_SOURCES = \ diff --git a/tests/sexpr2xmltest.c b/tests/sexpr2xmltest.c index 6055293af14d0478a537bd764628f84305898c0c..83e1ed98bb2a9bb7fd52973774294cf30f65b8f9 100644 --- a/tests/sexpr2xmltest.c +++ b/tests/sexpr2xmltest.c @@ -17,24 +17,30 @@ static int testCompareFiles(const char *xml, const char *sexpr, int xendConfigVe char *gotxml = NULL; char *xmlPtr = &(xmlData[0]); char *sexprPtr = &(sexprData[0]); + int ret = -1; if (virtTestLoadFile(xml, &xmlPtr, MAX_FILE) < 0) - return -1; + goto fail; if (virtTestLoadFile(sexpr, &sexprPtr, MAX_FILE) < 0) - return -1; + goto fail; if (!(gotxml = xend_parse_domain_sexp(NULL, sexprData, xendConfigVersion))) - return -1; + goto fail; if (getenv("DEBUG_TESTS")) { printf("Expect %d '%s'\n", (int)strlen(xmlData), xmlData); printf("Actual %d '%s'\n", (int)strlen(gotxml), gotxml); } if (strcmp(xmlData, gotxml)) - return -1; + goto fail; - return 0; + ret = 0; + + fail: + free(gotxml); + + return ret; } static int testComparePVversion1(void *data ATTRIBUTE_UNUSED) { diff --git a/tests/xml2sexprtest.c b/tests/xml2sexprtest.c index ef15b0060b2d3833cbed2e44526d5a88a19afbe7..c23b3d500486717e0f9e38d0631ab669f91edabe 100644 --- a/tests/xml2sexprtest.c +++ b/tests/xml2sexprtest.c @@ -17,27 +17,35 @@ static int testCompareFiles(const char *xml, const char *sexpr, const char *name char *gotsexpr = NULL; char *xmlPtr = &(xmlData[0]); char *sexprPtr = &(sexprData[0]); + int ret = -1; if (virtTestLoadFile(xml, &xmlPtr, MAX_FILE) < 0) - return -1; + goto fail; if (virtTestLoadFile(sexpr, &sexprPtr, MAX_FILE) < 0) - return -1; + goto fail; - if (!(gotsexpr = virDomainParseXMLDesc(xmlData, &gotname, xendConfigVersion))) - return -1; + if (!(gotsexpr = virDomainParseXMLDesc(xmlData, &gotname, xendConfigVersion))) + goto fail; if (getenv("DEBUG_TESTS")) { printf("Expect %d '%s'\n", (int)strlen(sexprData), sexprData); printf("Actual %d '%s'\n", (int)strlen(gotsexpr), gotsexpr); } if (strcmp(sexprData, gotsexpr)) - return -1; + goto fail; if (strcmp(name, gotname)) - return -1; + goto fail; - return 0; + ret = 0; + + fail: + + free(gotname); + free(gotsexpr); + + return ret; } static int testComparePVversion1(void *data ATTRIBUTE_UNUSED) {