Hi Hans, In general lock will not fail. But when testing, one can tweek the code and reproduce the scenario.
To get clean reports, when static/dynamic memory usage is checked for all the error cases. /Neel. On Wednesday 11 September 2013 06:01 PM, Hans Feldt wrote: > Why would lock fail? > /Hans > >> -----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 ------------------------------------------------------------------------------ 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
