[libvirt] [libvirt-php] free xml resources in get_string_from_xpath

Michal Privoznik mprivozn at redhat.com
Wed Sep 30 13:23:44 UTC 2015


On 30.09.2015 00:23, Vasiliy Tolstov wrote:
> free as much as possible on return from get_string_from_xpath
> 
> Signed-off-by: Vasiliy Tolstov <v.tolstov at selfip.ru>
> ---
>  src/libvirt-php.c | 13 +++++++++++--
>  1 file changed, 11 insertions(+), 2 deletions(-)
> 
> diff --git a/src/libvirt-php.c b/src/libvirt-php.c
> index 8588128..18499a6 100644
> --- a/src/libvirt-php.c
> +++ b/src/libvirt-php.c
> @@ -2597,6 +2597,7 @@ char *get_string_from_xpath(char *xml, char *xpath, zval **val, int *retVal)
>      if (!doc) {
>          if (retVal)
>              *retVal = -2;
> +        xmlFreeParserCtxt(xp);
>          xmlCleanupParser();
>          return NULL;
>      }
> @@ -2605,6 +2606,8 @@ char *get_string_from_xpath(char *xml, char *xpath, zval **val, int *retVal)
>      if (!context) {
>          if (retVal)
>              *retVal = -3;
> +        xmlFreeDoc(doc);
> +        xmlFreeParserCtxt(xp);
>          xmlCleanupParser();
>          return NULL;
>      }
> @@ -2614,6 +2617,8 @@ char *get_string_from_xpath(char *xml, char *xpath, zval **val, int *retVal)
>          if (retVal)
>              *retVal = -4;
>          xmlXPathFreeContext(context);
> +        xmlFreeParserCtxt(xp);
> +        xmlFreeDoc(doc);
>          xmlCleanupParser();
>          return NULL;
>      }
> @@ -2621,6 +2626,8 @@ char *get_string_from_xpath(char *xml, char *xpath, zval **val, int *retVal)
>      if(xmlXPathNodeSetIsEmpty(result->nodesetval)){
>          xmlXPathFreeObject(result);
>          xmlXPathFreeContext(context);
> +        xmlFreeParserCtxt(xp);
> +        xmlFreeDoc(doc);
>          xmlCleanupParser();
>          if (retVal)
>              *retVal = 0;
> @@ -2632,8 +2639,9 @@ char *get_string_from_xpath(char *xml, char *xpath, zval **val, int *retVal)
>  
>      if (ret == 0) {
>          xmlXPathFreeObject(result);
> -        xmlFreeDoc(doc);
>          xmlXPathFreeContext(context);
> +        xmlFreeParserCtxt(xp);
> +        xmlFreeDoc(doc);
>          xmlCleanupParser();
>          if (retVal)
>              *retVal = 0;
> @@ -2658,8 +2666,9 @@ char *get_string_from_xpath(char *xml, char *xpath, zval **val, int *retVal)
>              value = (char *)xmlNodeListGetString(doc, nodeset->nodeTab[0]->xmlChildrenNode, 1);
>      }
>  
> -    xmlXPathFreeContext(context);
>      xmlXPathFreeObject(result);
> +    xmlXPathFreeContext(context);
> +    xmlFreeParserCtxt(xp);
>      xmlFreeDoc(doc);
>      xmlCleanupParser();
>  
> 

What if we instead of adding more or less the same lines everywhere we invent cleanup label and do the cleanup work there?

In fact, I'm squashing this in, ACKing and pushing:

diff --git a/src/libvirt-php.c b/src/libvirt-php.c
index 18499a6..e85443f 100644
--- a/src/libvirt-php.c
+++ b/src/libvirt-php.c
@@ -2582,71 +2582,41 @@ char *get_string_from_xpath(char *xml, char *xpath, zval **val, int *retVal)
     char *value = NULL;
     char key[8] = { 0 };
 
-    if ((xpath == NULL) || (xml == NULL))
-    {
+    if (!xpath || !xml)
         return NULL;
-    }
 
     xp = xmlCreateDocParserCtxt( (xmlChar *)xml );
     if (!xp) {
-        if (retVal)
-            *retVal = -1;
-        return NULL;
+        ret = -1;
+        goto cleanup;
     }
+
     doc = xmlCtxtReadDoc(xp, (xmlChar *)xml, NULL, NULL, 0);
     if (!doc) {
-        if (retVal)
-            *retVal = -2;
-        xmlFreeParserCtxt(xp);
-        xmlCleanupParser();
-        return NULL;
+        ret = -2;
+        goto cleanup;
     }
 
     context = xmlXPathNewContext(doc);
     if (!context) {
-        if (retVal)
-            *retVal = -3;
-        xmlFreeDoc(doc);
-        xmlFreeParserCtxt(xp);
-        xmlCleanupParser();
-        return NULL;
+        ret = -3;
+        goto cleanup;
     }
 
     result = xmlXPathEvalExpression( (xmlChar *)xpath, context);
     if (!result) {
-        if (retVal)
-            *retVal = -4;
-        xmlXPathFreeContext(context);
-        xmlFreeParserCtxt(xp);
-        xmlFreeDoc(doc);
-        xmlCleanupParser();
-        return NULL;
+        ret = -4;
+        goto cleanup;
     }
-    if(xmlXPathNodeSetIsEmpty(result->nodesetval)){
-        xmlXPathFreeObject(result);
-        xmlXPathFreeContext(context);
-        xmlFreeParserCtxt(xp);
-        xmlFreeDoc(doc);
-        xmlCleanupParser();
-        if (retVal)
-            *retVal = 0;
-        return NULL;
-    }
+    if(xmlXPathNodeSetIsEmpty(result->nodesetval))
+        goto cleanup;
 
     nodeset = result->nodesetval;
     ret = nodeset->nodeNr;
 
-    if (ret == 0) {
-        xmlXPathFreeObject(result);
-        xmlXPathFreeContext(context);
-        xmlFreeParserCtxt(xp);
-        xmlFreeDoc(doc);
-        xmlCleanupParser();
-        if (retVal)
-            *retVal = 0;
-        return NULL;
-    }
+    if (!ret)
+        goto cleanup;
 
     if (val != NULL) {
         ret = 0;
@@ -2666,15 +2636,14 @@ char *get_string_from_xpath(char *xml, char *xpath, zval **val, int *retVal)
             value = (char *)xmlNodeListGetString(doc, nodeset->nodeTab[0]->xmlChildrenNode, 1);
     }
 
+ cleanup:
+    if (retVal)
+        *retVal = ret;
     xmlXPathFreeObject(result);
     xmlXPathFreeContext(context);
     xmlFreeParserCtxt(xp);
     xmlFreeDoc(doc);
     xmlCleanupParser();
-
-    if (retVal)
-        *retVal = ret;
-
     return (value != NULL) ? strdup(value) : NULL;
 }
 

Michal




More information about the libvir-list mailing list