Fixed multiple memory leaks & make test suite check for leaks with valgrind

ChangeLog

+Fri Oct  6 10:33:20 EDT 2006 Daniel P. Berrange <berrange@redhat.com>
+
+	* 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 <veillard@redhat.com>
 
 	* src/xen_internal.c: Daniel Berrange fixed some mlock size problem

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;
 }
 
 

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) {
@@ -752,24 +756,25 @@ virDomainParseXMLOSDescHVM(xmlNodePtr node, virBufferPtr buf, xmlXPathContextPtr
        if ((obj != NULL) && (obj->type == XPATH_NODESET) &&
            (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)) {
            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)) {
            virBufferAdd(buf, "(pae 1)", 7);
+       }
+       if (obj)
            xmlXPathFreeObject(obj);
        obj = NULL;
     }
-    }
 
     obj = xmlXPathEval(BAD_CAST "count(domain/devices/console) > 0", ctxt);
     if ((obj == NULL) || (obj->type != XPATH_BOOLEAN)) {
@@ -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);
 

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 = \

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) {

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;
+    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) {