[libvirt-php][PATCH 2/3] Fix some leaks related to get_string_from_xpath

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



This function not only did not free return value of
xmlNodeListGetString() it strdup()-ed its return value. Therefore
plenty of memory has been lost definitely upon return from this
function.

Signed-off-by: Michal Privoznik <mprivozn@xxxxxxxxxx>
---
 src/libvirt-php.c | 110 ++++++++++++++++++++++++++++++------------------------
 1 file changed, 62 insertions(+), 48 deletions(-)

diff --git a/src/libvirt-php.c b/src/libvirt-php.c
index 308e764..fb0679b 100644
--- a/src/libvirt-php.c
+++ b/src/libvirt-php.c
@@ -3232,19 +3232,17 @@ char *get_string_from_xpath(char *xml, char *xpath, zval **val, int *retVal)
     if (val != NULL) {
         ret = 0;
         for (i = 0; i < nodeset->nodeNr; i++) {
-            if (xmlNodeListGetString(doc, nodeset->nodeTab[i]->xmlChildrenNode, 1) != NULL) {
-                value = (char *)xmlNodeListGetString(doc, nodeset->nodeTab[i]->xmlChildrenNode, 1);
-
+            if ((value = xmlNodeListGetString(doc, nodeset->nodeTab[i]->xmlChildrenNode, 1))) {
                 snprintf(key, sizeof(key), "%d", i);
                 VIRT_ADD_ASSOC_STRING(*val, key, value);
+                free(value);
+                value = NULL;
                 ret++;
             }
         }
         add_assoc_long(*val, "num", (long)ret);
-    }
-    else {
-        if (xmlNodeListGetString(doc, nodeset->nodeTab[0]->xmlChildrenNode, 1) != NULL)
-            value = (char *)xmlNodeListGetString(doc, nodeset->nodeTab[0]->xmlChildrenNode, 1);
+    } else {
+        value = xmlNodeListGetString(doc, nodeset->nodeTab[0]->xmlChildrenNode, 1);
     }
 
  cleanup:
@@ -3255,7 +3253,7 @@ char *get_string_from_xpath(char *xml, char *xpath, zval **val, int *retVal)
     xmlFreeParserCtxt(xp);
     xmlFreeDoc(doc);
     xmlCleanupParser();
-    return (value != NULL) ? strdup(value) : NULL;
+    return value;
 }
 
 /*
@@ -3327,11 +3325,8 @@ char **get_array_from_xpath(char *xml, char *xpath, int *num)
     ret = 0;
     val = (char **)malloc( nodeset->nodeNr  * sizeof(char *) );
     for (i = 0; i < nodeset->nodeNr; i++) {
-        if (xmlNodeListGetString(doc, nodeset->nodeTab[i]->xmlChildrenNode, 1) != NULL) {
-            value = (char *)xmlNodeListGetString(doc, nodeset->nodeTab[i]->xmlChildrenNode, 1);
-
-            val[ret++] = strdup(value);
-        }
+        if ((value = xmlNodeListGetString(doc, nodeset->nodeTab[i]->xmlChildrenNode, 1)))
+            val[ret++] = value;
     }
 
     xmlXPathFreeContext(context);
@@ -3506,20 +3501,23 @@ long get_next_free_numeric_value(virDomainPtr domain, char *xpath)
  */
 char *connection_get_domain_type(virConnectPtr conn, char *arch TSRMLS_DC)
 {
-    int retval = -1;
+    char *ret = NULL;
     char *tmp = NULL;
     char *caps = NULL;
+    char *tmpArch = NULL;
     char xpath[1024] = { 0 };
+    int retval = -1;
 
     caps = virConnectGetCapabilities(conn);
     if (caps == NULL)
         return NULL;
 
     if (arch == NULL) {
-        arch = get_string_from_xpath(caps, "//capabilities/host/cpu/arch", NULL, &retval);
-        DPRINTF("%s: No architecture defined, got '%s' from capabilities XML\n", __FUNCTION__, arch);
-        if ((arch == NULL) || (retval < 0))
-            return NULL;
+        tmpArch = get_string_from_xpath(caps, "//capabilities/host/cpu/arch", NULL, &retval);
+        DPRINTF("%s: No architecture defined, got '%s' from capabilities XML\n", __FUNCTION__, tmpArch);
+        if (!tmpArch || retval < 0)
+            goto cleanup;
+        arch = tmpArch;
     }
 
     DPRINTF("%s: Requested domain type for arch '%s'\n",  __FUNCTION__, arch);
@@ -3529,12 +3527,17 @@ char *connection_get_domain_type(virConnectPtr conn, char *arch TSRMLS_DC)
     tmp = get_string_from_xpath(caps, xpath, NULL, &retval);
     if ((tmp == NULL) || (retval < 0)) {
         DPRINTF("%s: No domain type found in XML...\n", __FUNCTION__);
-        return NULL;
+        goto cleanup;
     }
 
-    DPRINTF("%s: Domain type is '%s'\n",  __FUNCTION__, tmp);
-
-    return tmp;
+    ret = tmp;
+    tmp = NULL;
+    DPRINTF("%s: Domain type is '%s'\n",  __FUNCTION__, ret);
+ cleanup:
+    free(tmpArch);
+    free(caps);
+    free(tmp);
+    return ret;
 }
 
 /*
@@ -3547,20 +3550,23 @@ char *connection_get_domain_type(virConnectPtr conn, char *arch TSRMLS_DC)
  */
 char *connection_get_emulator(virConnectPtr conn, char *arch TSRMLS_DC)
 {
-    int retval = -1;
+    char *ret = NULL;
     char *tmp = NULL;
     char *caps = NULL;
+    char *tmpArch = NULL;
     char xpath[1024] = { 0 };
+    int retval = -1;
 
     caps = virConnectGetCapabilities(conn);
     if (caps == NULL)
         return NULL;
 
     if (arch == NULL) {
-        arch = get_string_from_xpath(caps, "//capabilities/host/cpu/arch", NULL, &retval);
-        DPRINTF("%s: No architecture defined, got '%s' from capabilities XML\n", __FUNCTION__, arch);
-        if ((arch == NULL) || (retval < 0))
-            return NULL;
+        tmpArch = get_string_from_xpath(caps, "//capabilities/host/cpu/arch", NULL, &retval);
+        DPRINTF("%s: No architecture defined, got '%s' from capabilities XML\n", __FUNCTION__, tmpArch);
+        if (!tmpArch || retval < 0)
+            goto cleanup;
+        arch = tmpArch;
     }
 
     DPRINTF("%s: Requested emulator for arch '%s'\n",  __FUNCTION__, arch);
@@ -3568,26 +3574,30 @@ char *connection_get_emulator(virConnectPtr conn, char *arch TSRMLS_DC)
     snprintf(xpath, sizeof(xpath), "//capabilities/guest/arch[@name='%s']/domain/emulator", arch);
     DPRINTF("%s: Applying xPath '%s' to capabilities XML output\n", __FUNCTION__, xpath);
     tmp = get_string_from_xpath(caps, xpath, NULL, &retval);
-    if ((tmp == NULL) || (retval < 0)) {
-        DPRINTF("%s: No emulator found. Trying next location ...\n", __FUNCTION__);
-        snprintf(xpath, sizeof(xpath), "//capabilities/guest/arch[@name='%s']/emulator", arch);
-    }
-    else {
+    if (tmp && retval >= 0) {
         DPRINTF("%s: Emulator is '%s'\n",  __FUNCTION__, tmp);
-        return tmp;
+        goto done;
     }
 
+    DPRINTF("%s: No emulator found. Trying next location ...\n", __FUNCTION__);
+    snprintf(xpath, sizeof(xpath), "//capabilities/guest/arch[@name='%s']/emulator", arch);
     DPRINTF("%s: Applying xPath '%s' to capabilities XML output\n",  __FUNCTION__, xpath);
-
+    free(tmp);
     tmp = get_string_from_xpath(caps, xpath, NULL, &retval);
-    if ((tmp == NULL) || (retval < 0)) {
-        DPRINTF("%s: Emulator is '%s'\n",  __FUNCTION__, tmp);
-        return NULL;
+    if (!tmp || retval < 0) {
+        DPRINTF("%s: None emulator found\n",  __FUNCTION__);
+        goto cleanup;
     }
 
-    DPRINTF("%s: Emulator is '%s'\n",  __FUNCTION__, tmp);
-
-    return tmp;
+ done:
+    ret = tmp;
+    tmp = NULL;
+    DPRINTF("%s: Emulator is '%s'\n",  __FUNCTION__, ret);
+ cleanup:
+    free(tmpArch);
+    free(caps);
+    free(tmp);
+    return ret;
 }
 
 /*
@@ -3599,26 +3609,29 @@ char *connection_get_emulator(virConnectPtr conn, char *arch TSRMLS_DC)
  */
 char *connection_get_arch(virConnectPtr conn TSRMLS_DC)
 {
-    int retval = -1;
+    char *ret = NULL;
     char *tmp = NULL;
     char *caps = NULL;
-    // char xpath[1024] = { 0 };
+    int retval = -1;
 
     caps = virConnectGetCapabilities(conn);
     if (caps == NULL)
         return NULL;
 
     tmp = get_string_from_xpath(caps, "//capabilities/host/cpu/arch", NULL, &retval);
-    free(caps);
-
     if ((tmp == NULL) || (retval < 0)) {
         DPRINTF("%s: Cannot get host CPU architecture from capabilities XML\n", __FUNCTION__);
-        return NULL;
+        goto cleanup;
     }
 
-    DPRINTF("%s: Host CPU architecture is '%s'\n",  __FUNCTION__, tmp);
+    ret = tmp;
+    tmp = NULL;
+    DPRINTF("%s: Host CPU architecture is '%s'\n",  __FUNCTION__, ret);
 
-    return tmp;
+ cleanup:
+    free(caps);
+    free(tmp);
+    return ret;
 }
 
 /*
@@ -7130,7 +7143,8 @@ PHP_FUNCTION(libvirt_domain_xml_xpath)
 
     array_init(return_value);
 
-    if ((tmp = get_string_from_xpath(xml, (char *)zpath, &return_value, &rc)) == NULL) {
+    free(get_string_from_xpath(xml, (char *)zpath, &return_value, &rc));
+    if (return_value < 0) {
         free(xml);
         RETURN_FALSE;
     }
-- 
2.7.3

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list



[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]