30 сент. 2015 г. 16:23 пользователь "Michal Privoznik" <mprivozn@xxxxxxxxxx> написал:
>
> 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@xxxxxxxxx>
> > ---
> > 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
-- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list