6:23 a.m.
free as much as possible on return from get_string_from_xpath
Signed-off-by: Vasiliy Tolstov <v.tolstov(a)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();
--
2.5.0
9:23 p.m.
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(a)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
2:14 a.m.
30 сент. 2015 г. 16:23 пользователь "Michal Privoznik"
<mprivozn(a)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(a)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
10:19 p.m.
On 30.09.2015 20:14, Vasiliy Tolstov wrote:
30 сент. 2015 г. 16:23 пользователь "Michal Privoznik"
<mprivozn(a)redhat.com>
написал:
>
> <snip/>
>
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?
CC'ing Michal Novotny, the original author and maintainer of
libvirt-php. Michal, can we do that?
Michal
3304
days inactive
3306
days old
3 comments
2 participants
participants (2)
-
Michal Privoznik -
Vasiliy Tolstov