Hi zoran,

will add the error code.

/Neel.
On Friday 20 September 2013 11:36 AM, Zoran Milinkovic wrote:
> In
>
> @@ -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;
>
> you forgot to set return error code.... "error = SA_AIS_ERR_LIBRARY;"
>
> Regrards,
> Zoran
> ________________________________________
> From: reddy.neelaka...@oracle.com [reddy.neelaka...@oracle.com]
> Sent: Wednesday, 11 September 2013 2:43 PM
> 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

Reply via email to