Hi Daniel, I'm sending you some memory leak fix we have found and fixed on modules/utils/xcap_ auth.c.
The fixed memory leak is in pres_watcher_allowed (), and it is about the "xmlFreeDoc" that it's never called for the "xcap_tree" variable.
We downloaded the source file from master branch and then we fixed it. You can find here the diff from original and new code and in attach both the original and the new files.
diff -u a/modules/utils/xcap_auth.c b/modules/utils/xcap_auth.c --- a/modules/utils/xcap_auth.c +++ b/modules/utils/xcap_auth.c @@ -280,13 +280,15 @@ }
node= get_rule_node(subs, xcap_tree); - if (node== NULL) + if (node== NULL){ + xmlFreeDoc(xcap_tree); return 0; - + } /* process actions */ actions_node = xmlNodeGetChildByName(node, "actions"); if (actions_node == NULL) { LM_DBG("actions_node NULL\n"); + xmlFreeDoc(xcap_tree); return 0; } LM_DBG("actions_node->name= %s\n", actions_node->name); @@ -294,6 +296,7 @@ sub_handling_node = xmlNodeGetChildByName(actions_node, "sub-handling"); if (sub_handling_node== NULL) { LM_DBG("sub_handling_node NULL\n"); + xmlFreeDoc(xcap_tree); return 0; } sub_handling = (char*)xmlNodeGetContent(sub_handling_node); @@ -302,6 +305,7 @@
if (sub_handling == NULL) { LM_ERR("Couldn't get sub-handling content\n"); + xmlFreeDoc(xcap_tree); return -1; } if (strncmp((char*)sub_handling, "block", 5) == 0) { @@ -325,10 +329,12 @@ else { LM_ERR("unknown subscription handling action\n"); xmlFree(sub_handling); + xmlFreeDoc(xcap_tree); return -1; }
xmlFree(sub_handling); + xmlFreeDoc(xcap_tree);
return 0;
Kind Regards, laura
Hi Laura,
thanks for the patch. I wonder how an xcap related function landed in utils module, perhaps Juha felt more comfortable adding it there.
At first sight, patch looks ok. I cc-ed Juha in case he wants to have a look and integrate it. If not, I will commit before 3.2.2.
Cheers, Daniel
On 1/30/12 1:01 PM, laura testi wrote:
Hi Daniel, I'm sending you some memory leak fix we have found and fixed on modules/utils/xcap_ auth.c.
The fixed memory leak is in pres_watcher_allowed (), and it is about the "xmlFreeDoc" that it's never called for the "xcap_tree" variable.
We downloaded the source file from master branch and then we fixed it. You can find here the diff from original and new code and in attach both the original and the new files.
diff -u a/modules/utils/xcap_auth.c b/modules/utils/xcap_auth.c --- a/modules/utils/xcap_auth.c +++ b/modules/utils/xcap_auth.c @@ -280,13 +280,15 @@ }
node= get_rule_node(subs, xcap_tree);
- if (node== NULL)
- if (node== NULL){
- xmlFreeDoc(xcap_tree); return 0;
} /* process actions */ actions_node = xmlNodeGetChildByName(node, "actions"); if (actions_node == NULL) { LM_DBG("actions_node NULL\n");
xmlFreeDoc(xcap_tree); return 0; } LM_DBG("actions_node->name= %s\n", actions_node->name);
@@ -294,6 +296,7 @@ sub_handling_node = xmlNodeGetChildByName(actions_node, "sub-handling"); if (sub_handling_node== NULL) { LM_DBG("sub_handling_node NULL\n");
xmlFreeDoc(xcap_tree); return 0; } sub_handling = (char*)xmlNodeGetContent(sub_handling_node);
@@ -302,6 +305,7 @@
if (sub_handling == NULL) { LM_ERR("Couldn't get sub-handling content\n");
xmlFreeDoc(xcap_tree); return -1; } if (strncmp((char*)sub_handling, "block", 5) == 0) {
@@ -325,10 +329,12 @@ else { LM_ERR("unknown subscription handling action\n"); xmlFree(sub_handling);
xmlFreeDoc(xcap_tree); return -1; } xmlFree(sub_handling);
xmlFreeDoc(xcap_tree);
return 0;
Kind Regards, laura
SIP Express Router (SER) and Kamailio (OpenSER) - sr-users mailing list sr-users@lists.sip-router.org http://lists.sip-router.org/cgi-bin/mailman/listinfo/sr-users
Hi Daniel,
I remember looking at this file in the past. It seemed, to me at the time, to do a lot of the same stuff as the file with the same name in presence_xml but always with direct access to the xcap_server database.
My thought at the time was that it could be used to run things like pres_auth_status() (xcap_auth_status() in utils) for blocking of IMs and calls based on the contents of XML documents without having to run the full set of presence modules.
It looks like that fix is pretty similar to one I checked in for presence_xml/xcap_auth.c last week.
Peter
On Mon, 2012-01-30 at 14:03 +0100, Daniel-Constantin Mierla wrote:
Hi Laura,
thanks for the patch. I wonder how an xcap related function landed in utils module, perhaps Juha felt more comfortable adding it there.
At first sight, patch looks ok. I cc-ed Juha in case he wants to have a look and integrate it. If not, I will commit before 3.2.2.
Cheers, Daniel
On 1/30/12 1:01 PM, laura testi wrote:
Hi Daniel, I'm sending you some memory leak fix we have found and fixed on modules/utils/xcap_ auth.c.
The fixed memory leak is in pres_watcher_allowed (), and it is about the "xmlFreeDoc" that it's never called for the "xcap_tree" variable.
We downloaded the source file from master branch and then we fixed it. You can find here the diff from original and new code and in attach both the original and the new files.
diff -u a/modules/utils/xcap_auth.c b/modules/utils/xcap_auth.c --- a/modules/utils/xcap_auth.c +++ b/modules/utils/xcap_auth.c @@ -280,13 +280,15 @@ }
node= get_rule_node(subs, xcap_tree);
- if (node== NULL)
- if (node== NULL){
- xmlFreeDoc(xcap_tree); return 0;
/* process actions */ actions_node = xmlNodeGetChildByName(node, "actions"); if (actions_node == NULL) { LM_DBG("actions_node NULL\n");}
} LM_DBG("actions_node->name= %s\n", actions_node->name);xmlFreeDoc(xcap_tree); return 0;
@@ -294,6 +296,7 @@ sub_handling_node = xmlNodeGetChildByName(actions_node, "sub-handling"); if (sub_handling_node== NULL) { LM_DBG("sub_handling_node NULL\n");
} sub_handling = (char*)xmlNodeGetContent(sub_handling_node);xmlFreeDoc(xcap_tree); return 0;
@@ -302,6 +305,7 @@
if (sub_handling == NULL) { LM_ERR("Couldn't get sub-handling content\n");
} if (strncmp((char*)sub_handling, "block", 5) == 0) {xmlFreeDoc(xcap_tree); return -1;
@@ -325,10 +329,12 @@ else { LM_ERR("unknown subscription handling action\n"); xmlFree(sub_handling);
xmlFreeDoc(xcap_tree); return -1; }
xmlFree(sub_handling);
xmlFreeDoc(xcap_tree);
return 0;
Kind Regards, laura
SIP Express Router (SER) and Kamailio (OpenSER) - sr-users mailing list sr-users@lists.sip-router.org http://lists.sip-router.org/cgi-bin/mailman/listinfo/sr-users
-- Daniel-Constantin Mierla -- http://www.asipto.com http://linkedin.com/in/miconda -- http://twitter.com/miconda _______________________________________________ sr-dev mailing list sr-dev@lists.sip-router.org http://lists.sip-router.org/cgi-bin/mailman/listinfo/sr-dev
Peter Dunkley writes:
My thought at the time was that it could be used to run things like pres_auth_status() (xcap_auth_status() in utils) for blocking of IMs and calls based on the contents of XML documents without having to run the full set of presence modules.
that was exactly the reason i wrote the function. it is ok with me to apply the memory leak fix.
-- juha