Hi Neel,
I have tested saImmOmClassDescriptionMemoryFree_2() and it's ack.
The rest of the code I just did a review without testing, and here are my
comments:
- reorder expressions in if statement to "if(rc == SA_AIS_ERR_LIBRARY &&
returnParams && *returnParams != NULL)" for efficient comparison.
@@ -3645,6 +3645,24 @@ static SaAisErrorT admin_op_invoke_commo
+ lock_fail1:
+ if(returnParams && *returnParams != NULL && rc == SA_AIS_ERR_LIBRARY) {
- change attrDefinition into *attrDefinition and reorder if statement to: "if
(rc == SA_AIS_ERR_LIBRARY && *attrDefinition)"
@@ -4779,6 +4797,24 @@ SaAisErrorT saImmOmClassDescriptionGet_2
+ lock_fail1:
+
+ if (attrDefinition && rc == SA_AIS_ERR_LIBRARY) {
Best regards,
Zoran
-----Original Message-----
From: reddy.neelaka...@oracle.com [mailto:reddy.neelaka...@oracle.com]
Sent: den 11 september 2013 14:44
To: Anders Björnerstedt
Cc: opensaf-devel@lists.sourceforge.net
Subject: [devel] [PATCH 1 of 1] IMM: fix the memory leaks when ncs_lock failed
in IMM agent
osaf/libs/agents/saf/imma/imma_oi_api.c | 6 +-
osaf/libs/agents/saf/imma/imma_om_api.c | 84 +++++++++++++++------------
2 files changed, 50 insertions(+), 40 deletions(-)
In the IMM API's if ncs_lock fails with SA_AIS_ERR_LIBRARY, memory if any
aloocated in the API must be freed.
In the saImmOmClassDescriptionMemoryFree_2 API is modified to use
imma_freeAttrValue3
diff --git a/osaf/libs/agents/saf/imma/imma_oi_api.c
b/osaf/libs/agents/saf/imma/imma_oi_api.c
--- a/osaf/libs/agents/saf/imma/imma_oi_api.c
+++ b/osaf/libs/agents/saf/imma/imma_oi_api.c
@@ -1457,7 +1457,7 @@ SaAisErrorT saImmOiImplementerClear(SaIm
rc = SA_AIS_ERR_LIBRARY;
/* Losing track of the pending reply count, but ERR_LIBRARY
dominates*/
TRACE_4("ERR_LIBRARY: LOCK failed");
- goto lock_fail;
+ goto lock_fail1;
}
locked = true;
@@ -1507,6 +1507,7 @@ SaAisErrorT saImmOiImplementerClear(SaIm
if (locked)
m_NCS_UNLOCK(&cb->cb_lock, NCS_LOCK_WRITE);
+ lock_fail1:
if (out_evt)
free(out_evt);
@@ -2465,7 +2466,7 @@ SaAisErrorT saImmOiRtObjectUpdate_2(SaIm
rc = SA_AIS_ERR_LIBRARY;
/* Losing track of the pending reply count, but ERR_LIBRARY
dominates*/
TRACE_4("ERR_LIBRARY: Lock failed");
- goto lock_fail;
+ goto lock_fail1;
}
locked = true;
@@ -2496,6 +2497,7 @@ SaAisErrorT saImmOiRtObjectUpdate_2(SaIm
skip_over_send:
bad_sync:
+ lock_fail1:
if (evt.info.immnd.info.objModify.objectName.buf) { /*free-1 */
free(evt.info.immnd.info.objModify.objectName.buf);
evt.info.immnd.info.objModify.objectName.buf = NULL;
diff --git a/osaf/libs/agents/saf/imma/imma_om_api.c
b/osaf/libs/agents/saf/imma/imma_om_api.c
--- a/osaf/libs/agents/saf/imma/imma_om_api.c
+++ b/osaf/libs/agents/saf/imma/imma_om_api.c
@@ -376,6 +376,7 @@ static SaAisErrorT initialize_common(SaI
imma_callback_ipc_destroy(cl_node);
ipc_init_fail:
+ lock_fail:
if (rc != SA_AIS_OK) {
free(cl_node);
cl_node=NULL;
@@ -384,7 +385,6 @@ static SaAisErrorT initialize_common(SaI
if (locked)
m_NCS_UNLOCK(&cb->cb_lock, NCS_LOCK_WRITE);
- lock_fail:
if (out_evt) {
free(out_evt);
@@ -3604,7 +3604,7 @@ static SaAisErrorT admin_op_invoke_commo
if (!locked && m_NCS_LOCK(&cb->cb_lock, NCS_LOCK_WRITE) !=
NCSCC_RC_SUCCESS) {
TRACE_4("ERR_LIBRARY: Lock failed");
rc = SA_AIS_ERR_LIBRARY;
- goto lock_fail;
+ goto lock_fail1;
}
locked = true;
@@ -3645,6 +3645,24 @@ static SaAisErrorT admin_op_invoke_commo
if (locked)
m_NCS_UNLOCK(&cb->cb_lock, NCS_LOCK_WRITE);
+ lock_fail1:
+ if(returnParams && *returnParams != NULL && rc == SA_AIS_ERR_LIBRARY) {
+ SaImmAdminOperationParamsT_2 **Params = *returnParams;
+ SaImmAdminOperationParamsT_2 *q = NULL;
+ unsigned int ix = 0;
+ while(Params[ix]) {
+ q = Params[ix];
+ imma_freeAttrValue3(q->paramBuffer, q->paramType);
+ free(q->paramName);
+ q->paramName = NULL;
+ free(q);
+ ++ix;
+ }
+
+ free(Params);
+ }
+
+
lock_fail:
done:
@@ -4754,7 +4772,7 @@ SaAisErrorT saImmOmClassDescriptionGet_2
rc = SA_AIS_ERR_LIBRARY;
TRACE_4("ERR_LIBRARY: Lock failed");
/* Losing track of the pending reply count, but ERR_LIBRARY
dominates*/
- goto lock_fail;
+ goto lock_fail1;
}
locked = true;
@@ -4779,6 +4797,24 @@ SaAisErrorT saImmOmClassDescriptionGet_2
if (locked)
m_NCS_UNLOCK(&cb->cb_lock, NCS_LOCK_WRITE);
+ lock_fail1:
+
+ if (attrDefinition && rc == SA_AIS_ERR_LIBRARY) {
+ SaImmAttrDefinitionT_2 **attr1 = *attrDefinition;
+ int i;
+ for (i = 0; attr1[i]; ++i) {
+ if (attr1[i]->attrDefaultValue) {
+ imma_freeAttrValue3(attr1[i]->attrDefaultValue,
attr1[i]->attrValueType); /* free-4, free-5 */
+ attr1[i]->attrDefaultValue = 0;
+ }
+ free(attr1[i]->attrName); /*free-3 */
+ attr1[i]->attrName = 0;
+ free(attr1[i]); /*free-2 */
+ attr1[i] = 0;
+ }
+ free(attr1); /*free-1 */
+ }
+
lock_fail:
TRACE_LEAVE();
@@ -4821,36 +4857,7 @@ SaAisErrorT saImmOmClassDescriptionMemor
int i;
for (i = 0; attrDefinition[i]; ++i) {
if (attrDefinition[i]->attrDefaultValue) {
- SaStringT *strp;
- SaAnyT *anyp;
- switch (attrDefinition[i]->attrValueType) {
- case SA_IMM_ATTR_SAINT32T:
//intended fall through
- case SA_IMM_ATTR_SAUINT32T:
//intended fall through
- case SA_IMM_ATTR_SAINT64T:
//intended fall through
- case SA_IMM_ATTR_SAUINT64T:
//intended fall through
- case SA_IMM_ATTR_SATIMET:
//intended fall through
- case SA_IMM_ATTR_SAFLOATT:
//intended fall through
- case SA_IMM_ATTR_SADOUBLET:
- case SA_IMM_ATTR_SANAMET:
-
free(attrDefinition[i]->attrDefaultValue); /*free-4 */
- break;
-
- case SA_IMM_ATTR_SASTRINGT:
- strp = (SaStringT
*)attrDefinition[i]->attrDefaultValue;
- free(*strp); /*free-5 */
- free(strp); /*free-4 */
- break;
-
- case SA_IMM_ATTR_SAANYT:
- anyp = (SaAnyT
*)attrDefinition[i]->attrDefaultValue;
- free(anyp->bufferAddr);
/*free-5 */
- anyp->bufferAddr = 0;
- free(anyp); /*free-4 */
- break;
-
- default:
- abort();
- }//switch
+
imma_freeAttrValue3(attrDefinition[i]->attrDefaultValue,
attrDefinition[i]->attrValueType); /* free-4, free-5 */
attrDefinition[i]->attrDefaultValue = 0;
}
free(attrDefinition[i]->attrName); /*free-3 */
@@ -5462,7 +5469,7 @@ mds_send_fail:
if (m_NCS_LOCK(&cb->cb_lock, NCS_LOCK_WRITE) != NCSCC_RC_SUCCESS) {
rc = SA_AIS_ERR_LIBRARY;
TRACE_4("ERR_LIBRARY: Lock error");
- goto release_cb;
+ goto lock_fail;
}
locked = true;
@@ -5500,6 +5507,8 @@ mds_send_fail:
m_NCS_UNLOCK(&cb->cb_lock, NCS_LOCK_WRITE);
}
+lock_fail:
+
if(out_evt) {
if(out_evt->info.imma.info.searchNextRsp) {
if(out_evt->info.imma.info.searchNextRsp->attrValuesList) {
@@ -6256,7 +6265,7 @@ SaAisErrorT saImmOmSearchInitialize_2(Sa
if (m_NCS_LOCK(&cb->cb_lock, NCS_LOCK_WRITE) != NCSCC_RC_SUCCESS) {
rc = SA_AIS_ERR_LIBRARY;
TRACE_4("ERR_LIBRARY: Lock error");
- goto release_cb;
+ goto lock_fail;
}
locked = true;
@@ -6326,7 +6335,7 @@ SaAisErrorT saImmOmSearchInitialize_2(Sa
if (locked) {
m_NCS_UNLOCK(&cb->cb_lock, NCS_LOCK_WRITE);
}
-
+lock_fail:
if(out_evt) {
free(out_evt);
out_evt=NULL;
@@ -6469,8 +6478,7 @@ SaAisErrorT saImmOmSearchNext_2(SaImmSea
if (m_NCS_LOCK(&cb->cb_lock, NCS_LOCK_WRITE) != NCSCC_RC_SUCCESS) {
TRACE_4("ERR_LIBRARY: Lock error");
- return SA_AIS_ERR_LIBRARY;
- /*Error case will leak memory. */
+ goto release_evt;
}
locked = true;
------------------------------------------------------------------------------
How ServiceNow helps IT people transform IT departments:
1. Consolidate legacy IT systems to a single system of record for IT
2. Standardize and globalize service processes across IT
3. Implement zero-touch automation to replace manual, redundant tasks
http://pubads.g.doubleclick.net/gampad/clk?id=51271111&iu=/4140/ostg.clktrk
_______________________________________________
Opensaf-devel mailing list
Opensaf-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/opensaf-devel
------------------------------------------------------------------------------
LIMITED TIME SALE - Full Year of Microsoft Training For Just $49.99!
1,500+ hours of tutorials including VisualStudio 2012, Windows 8, SharePoint
2013, SQL 2012, MVC 4, more. BEST VALUE: New Multi-Library Power Pack includes
Mobile, Cloud, Java, and UX Design. Lowest price ever! Ends 9/20/13.
http://pubads.g.doubleclick.net/gampad/clk?id=58041151&iu=/4140/ostg.clktrk
_______________________________________________
Opensaf-devel mailing list
Opensaf-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/opensaf-devel
