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

[Vasiliy Tolstov]

30 сент. 2015 г. 16:23 пользователь "Michal Privoznik" <mprivozn at redhat.com>
написал:
>
> 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:
>

Big thanks! If XML parser does not panic when cleaning null variables this
is more preferred.
Can somebody regenerate site docs for this binding and tag new release?
> 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
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20150930/a2df8d40/attachment-0001.htm>