<p dir="ltr"><br>
30 сент. 2015 г. 16:23 пользователь "Michal Privoznik" <<a href="mailto:mprivozn@redhat.com">mprivozn@redhat.com</a>> написал:<br>
><br>
> On 30.09.2015 00:23, Vasiliy Tolstov wrote:<br>
> > free as much as possible on return from get_string_from_xpath<br>
> ><br>
> > Signed-off-by: Vasiliy Tolstov <<a href="mailto:v.tolstov@selfip.ru">v.tolstov@selfip.ru</a>><br>
> > ---<br>
> >  src/libvirt-php.c | 13 +++++++++++--<br>
> >  1 file changed, 11 insertions(+), 2 deletions(-)<br>
> ><br>
> > diff --git a/src/libvirt-php.c b/src/libvirt-php.c<br>
> > index 8588128..18499a6 100644<br>
> > --- a/src/libvirt-php.c<br>
> > +++ b/src/libvirt-php.c<br>
> > @@ -2597,6 +2597,7 @@ char *get_string_from_xpath(char *xml, char *xpath, zval **val, int *retVal)<br>
> >      if (!doc) {<br>
> >          if (retVal)<br>
> >              *retVal = -2;<br>
> > +        xmlFreeParserCtxt(xp);<br>
> >          xmlCleanupParser();<br>
> >          return NULL;<br>
> >      }<br>
> > @@ -2605,6 +2606,8 @@ char *get_string_from_xpath(char *xml, char *xpath, zval **val, int *retVal)<br>
> >      if (!context) {<br>
> >          if (retVal)<br>
> >              *retVal = -3;<br>
> > +        xmlFreeDoc(doc);<br>
> > +        xmlFreeParserCtxt(xp);<br>
> >          xmlCleanupParser();<br>
> >          return NULL;<br>
> >      }<br>
> > @@ -2614,6 +2617,8 @@ char *get_string_from_xpath(char *xml, char *xpath, zval **val, int *retVal)<br>
> >          if (retVal)<br>
> >              *retVal = -4;<br>
> >          xmlXPathFreeContext(context);<br>
> > +        xmlFreeParserCtxt(xp);<br>
> > +        xmlFreeDoc(doc);<br>
> >          xmlCleanupParser();<br>
> >          return NULL;<br>
> >      }<br>
> > @@ -2621,6 +2626,8 @@ char *get_string_from_xpath(char *xml, char *xpath, zval **val, int *retVal)<br>
> >      if(xmlXPathNodeSetIsEmpty(result->nodesetval)){<br>
> >          xmlXPathFreeObject(result);<br>
> >          xmlXPathFreeContext(context);<br>
> > +        xmlFreeParserCtxt(xp);<br>
> > +        xmlFreeDoc(doc);<br>
> >          xmlCleanupParser();<br>
> >          if (retVal)<br>
> >              *retVal = 0;<br>
> > @@ -2632,8 +2639,9 @@ char *get_string_from_xpath(char *xml, char *xpath, zval **val, int *retVal)<br>
> ><br>
> >      if (ret == 0) {<br>
> >          xmlXPathFreeObject(result);<br>
> > -        xmlFreeDoc(doc);<br>
> >          xmlXPathFreeContext(context);<br>
> > +        xmlFreeParserCtxt(xp);<br>
> > +        xmlFreeDoc(doc);<br>
> >          xmlCleanupParser();<br>
> >          if (retVal)<br>
> >              *retVal = 0;<br>
> > @@ -2658,8 +2666,9 @@ char *get_string_from_xpath(char *xml, char *xpath, zval **val, int *retVal)<br>
> >              value = (char *)xmlNodeListGetString(doc, nodeset->nodeTab[0]->xmlChildrenNode, 1);<br>
> >      }<br>
> ><br>
> > -    xmlXPathFreeContext(context);<br>
> >      xmlXPathFreeObject(result);<br>
> > +    xmlXPathFreeContext(context);<br>
> > +    xmlFreeParserCtxt(xp);<br>
> >      xmlFreeDoc(doc);<br>
> >      xmlCleanupParser();<br>
> ><br>
> ><br>
><br>
> What if we instead of adding more or less the same lines everywhere we invent cleanup label and do the cleanup work there?<br>
><br>
> In fact, I'm squashing this in, ACKing and pushing:<br>
><br></p>
<p dir="ltr">Big thanks! If XML parser does not panic when cleaning null variables this is more preferred.<br>
Can somebody regenerate site docs for this binding and tag new release?<br>
> diff --git a/src/libvirt-php.c b/src/libvirt-php.c<br>
> index 18499a6..e85443f 100644<br>
> --- a/src/libvirt-php.c<br>
> +++ b/src/libvirt-php.c<br>
> @@ -2582,71 +2582,41 @@ char *get_string_from_xpath(char *xml, char *xpath, zval **val, int *retVal)<br>
>      char *value = NULL;<br>
>      char key[8] = { 0 };<br>
><br>
> -    if ((xpath == NULL) || (xml == NULL))<br>
> -    {<br>
> +    if (!xpath || !xml)<br>
>          return NULL;<br>
> -    }<br>
><br>
>      xp = xmlCreateDocParserCtxt( (xmlChar *)xml );<br>
>      if (!xp) {<br>
> -        if (retVal)<br>
> -            *retVal = -1;<br>
> -        return NULL;<br>
> +        ret = -1;<br>
> +        goto cleanup;<br>
>      }<br>
> +<br>
>      doc = xmlCtxtReadDoc(xp, (xmlChar *)xml, NULL, NULL, 0);<br>
>      if (!doc) {<br>
> -        if (retVal)<br>
> -            *retVal = -2;<br>
> -        xmlFreeParserCtxt(xp);<br>
> -        xmlCleanupParser();<br>
> -        return NULL;<br>
> +        ret = -2;<br>
> +        goto cleanup;<br>
>      }<br>
><br>
>      context = xmlXPathNewContext(doc);<br>
>      if (!context) {<br>
> -        if (retVal)<br>
> -            *retVal = -3;<br>
> -        xmlFreeDoc(doc);<br>
> -        xmlFreeParserCtxt(xp);<br>
> -        xmlCleanupParser();<br>
> -        return NULL;<br>
> +        ret = -3;<br>
> +        goto cleanup;<br>
>      }<br>
><br>
>      result = xmlXPathEvalExpression( (xmlChar *)xpath, context);<br>
>      if (!result) {<br>
> -        if (retVal)<br>
> -            *retVal = -4;<br>
> -        xmlXPathFreeContext(context);<br>
> -        xmlFreeParserCtxt(xp);<br>
> -        xmlFreeDoc(doc);<br>
> -        xmlCleanupParser();<br>
> -        return NULL;<br>
> +        ret = -4;<br>
> +        goto cleanup;<br>
>      }<br>
> -    if(xmlXPathNodeSetIsEmpty(result->nodesetval)){<br>
> -        xmlXPathFreeObject(result);<br>
> -        xmlXPathFreeContext(context);<br>
> -        xmlFreeParserCtxt(xp);<br>
> -        xmlFreeDoc(doc);<br>
> -        xmlCleanupParser();<br>
> -        if (retVal)<br>
> -            *retVal = 0;<br>
> -        return NULL;<br>
> -    }<br>
> +    if(xmlXPathNodeSetIsEmpty(result->nodesetval))<br>
> +        goto cleanup;<br>
><br>
>      nodeset = result->nodesetval;<br>
>      ret = nodeset->nodeNr;<br>
><br>
> -    if (ret == 0) {<br>
> -        xmlXPathFreeObject(result);<br>
> -        xmlXPathFreeContext(context);<br>
> -        xmlFreeParserCtxt(xp);<br>
> -        xmlFreeDoc(doc);<br>
> -        xmlCleanupParser();<br>
> -        if (retVal)<br>
> -            *retVal = 0;<br>
> -        return NULL;<br>
> -    }<br>
> +    if (!ret)<br>
> +        goto cleanup;<br>
><br>
>      if (val != NULL) {<br>
>          ret = 0;<br>
> @@ -2666,15 +2636,14 @@ char *get_string_from_xpath(char *xml, char *xpath, zval **val, int *retVal)<br>
>              value = (char *)xmlNodeListGetString(doc, nodeset->nodeTab[0]->xmlChildrenNode, 1);<br>
>      }<br>
><br>
> + cleanup:<br>
> +    if (retVal)<br>
> +        *retVal = ret;<br>
>      xmlXPathFreeObject(result);<br>
>      xmlXPathFreeContext(context);<br>
>      xmlFreeParserCtxt(xp);<br>
>      xmlFreeDoc(doc);<br>
>      xmlCleanupParser();<br>
> -<br>
> -    if (retVal)<br>
> -        *retVal = ret;<br>
> -<br>
>      return (value != NULL) ? strdup(value) : NULL;<br>
>  }<br>
><br>
><br>
> Michal<br>
</p>