Ack, not tested. -Nagu
> -----Original Message----- > From: Gary Lee [mailto:[email protected]] > Sent: 28 January 2014 12:02 > To: [email protected]; [email protected]; > [email protected] > Cc: [email protected] > Subject: [devel] [PATCH 1 of 1] amfnd: fix errors reported by valgrind [#716] > > osaf/services/saf/amf/amfnd/amfnd.cc | 4 +- > osaf/services/saf/amf/amfnd/cbq.cc | 4 +- > osaf/services/saf/amf/amfnd/ckpt_updt.cc | 18 +- > osaf/services/saf/amf/amfnd/comp.cc | 11 +- > osaf/services/saf/amf/amfnd/compdb.cc | 6 +- > osaf/services/saf/amf/amfnd/include/avnd_util.h | 2 + > osaf/services/saf/amf/amfnd/pg.cc | 6 +- > osaf/services/saf/amf/amfnd/sidb.cc | 22 ++- > osaf/services/saf/amf/amfnd/susm.cc | 9 +- > osaf/services/saf/amf/amfnd/util.cc | 151 > ++++++++++++++++++++++- > 10 files changed, 190 insertions(+), 43 deletions(-) > > > * replace call to malloc(AVSV_AMF_CBK_INFO) with new() in comp.cc, > so it is in line with the rest of amfnd when allocating AVSV_AMF_CBK_INFO. > This eliminates the mixing of malloc/new/delete. > * add functions amf_cbk_copy() and amf_csi_attr_list_copy(), > which are based on the avsv equivalent but "C++ aware". > This is done to avoid mixing of malloc/new/delete when dealing > with 'SaAmfProtectionGroupNotificationT' in 'AVSV_AMF_CBK_INFO'. > * malloc/free are now used whenever a AVSV_ATTR_NAME_VAL structure is > created > or deleted. Previously, there were mixed usages of malloc/new/free/delete > resulting in mismatches. > * fix a few places where delete was used, instead of delete []. > * in pg.cc, don't set cbk_info to NULL before deleting it. > > diff --git a/osaf/services/saf/amf/amfnd/amfnd.cc > b/osaf/services/saf/amf/amfnd/amfnd.cc > --- a/osaf/services/saf/amf/amfnd/amfnd.cc > +++ b/osaf/services/saf/amf/amfnd/amfnd.cc > @@ -297,7 +297,7 @@ > TRACE_ENTER2("Type:%u, Hdl:%llu, Inv:%llu", cbk_info->type, > cbk_info->hdl, cbk_info->inv); > > /* Create a callback record for storing purpose. */ > - rc = avsv_amf_cbk_copy(&cbk_rec, cbk_info); > + rc = amf_cbk_copy(&cbk_rec, cbk_info); > > if (NCSCC_RC_SUCCESS != rc) > goto done; > @@ -317,7 +317,7 @@ > LOG_ER("Couldn't find comp %s in Inter/Ext Comp > DB",cbk_info->param.hc.comp_name.value); > /* free the callback info */ > if (cbk_rec) > - avsv_amf_cbk_free(cbk_rec); > + amf_cbk_free(cbk_rec); > > goto done; > } > diff --git a/osaf/services/saf/amf/amfnd/cbq.cc > b/osaf/services/saf/amf/amfnd/cbq.cc > --- a/osaf/services/saf/amf/amfnd/cbq.cc > +++ b/osaf/services/saf/amf/amfnd/cbq.cc > @@ -704,7 +704,7 @@ > /* populate the msg */ > msg.type = AVND_MSG_AVA; > msg.info.ava->type = AVSV_AVND_AMF_CBK_MSG; > - rc = avsv_amf_cbk_copy(&msg.info.ava->info.cbk_info, rec- > >cbk_info); > + rc = amf_cbk_copy(&msg.info.ava->info.cbk_info, rec->cbk_info); > if (NCSCC_RC_SUCCESS != rc) > goto done; > > @@ -930,7 +930,7 @@ > > /* free the callback info */ > if (rec->cbk_info) > - avsv_amf_cbk_free(rec->cbk_info); > + amf_cbk_free(rec->cbk_info); > > /* free the record */ > delete rec; > diff --git a/osaf/services/saf/amf/amfnd/ckpt_updt.cc > b/osaf/services/saf/amf/amfnd/ckpt_updt.cc > --- a/osaf/services/saf/amf/amfnd/ckpt_updt.cc > +++ b/osaf/services/saf/amf/amfnd/ckpt_updt.cc > @@ -219,9 +219,12 @@ > return NCSCC_RC_FAILURE; > } > /* free the csi attributes */ > - if (csi_rec->attrs.list) > - delete csi_rec->attrs.list; > - > + if (csi_rec->attrs.list) { > + // AVSV_ATTR_NAME_VAL > variables > + // are malloc'ed, use free() > + free(csi_rec->attrs.list); > + } > + > /* finally free this record */ > delete csi_rec; > } /* while ( 0 != csi_rec) */ > @@ -993,9 +996,12 @@ > return > NCSCC_RC_FAILURE; > } > /* free the csi attributes */ > - if (csi_rec->attrs.list) > - delete csi_rec- > >attrs.list; > - > + if (csi_rec->attrs.list) { > + // > AVSV_ATTR_NAME_VAL variables > + // are malloc'ed, use > free() > + free(csi_rec- > >attrs.list); > + } > + > /* finally free this record */ > delete csi_rec; > } /* while ( 0 != csi_rec) */ > diff --git a/osaf/services/saf/amf/amfnd/comp.cc > b/osaf/services/saf/amf/amfnd/comp.cc > --- a/osaf/services/saf/amf/amfnd/comp.cc > +++ b/osaf/services/saf/amf/amfnd/comp.cc > @@ -1890,10 +1890,7 @@ > goto done; > > /* allocate cbk-info memory */ > - if ((0 == (cbk_info = static_cast<AVSV_AMF_CBK_INFO*>(calloc(1, > sizeof(AVSV_AMF_CBK_INFO)))))) { > - rc = NCSCC_RC_FAILURE; > - goto done; > - } > + cbk_info = new AVSV_AMF_CBK_INFO(); > > /* fill the callback params */ > switch (type) { > @@ -1958,7 +1955,9 @@ > /* copy the attributes */ > memset(&attr, 0, sizeof(AVSV_CSI_ATTRS)); > if ((SA_AMF_CSI_ADD_ONE == csi_desc.csiFlags) && > (curr_csi->attrs.number != 0)) { > - attr.list = new > AVSV_ATTR_NAME_VAL[curr_csi->attrs.number]; > + attr.list = > static_cast<AVSV_ATTR_NAME_VAL*> > + (calloc(curr_csi->attrs.number, > sizeof(AVSV_ATTR_NAME_VAL))); > + osafassert(attr.list != NULL); > > memcpy(attr.list, curr_csi->attrs.list, > sizeof(AVSV_ATTR_NAME_VAL) * curr_csi- > >attrs.number); @@ -2020,7 +2019,7 @@ > > done: > if ((NCSCC_RC_SUCCESS != rc) && cbk_info) > - avsv_amf_cbk_free(cbk_info); > + amf_cbk_free(cbk_info); > > TRACE_LEAVE2("%u", rc); > return rc; > diff --git a/osaf/services/saf/amf/amfnd/compdb.cc > b/osaf/services/saf/amf/amfnd/compdb.cc > --- a/osaf/services/saf/amf/amfnd/compdb.cc > +++ b/osaf/services/saf/amf/amfnd/compdb.cc > @@ -903,21 +903,21 @@ > delete [] argv; > delete [] compt->saAmfCtDefCleanupCmdArgv; > > - delete compt->saAmfCtRelPathAmStartCmd; > + delete [] compt->saAmfCtRelPathAmStartCmd; > /* Free saAmfCtDefAmStartCmdArgv[i] before freeing > saAmfCtDefAmStartCmdArgv */ > arg_counter = 0; > while ((argv = compt->saAmfCtDefAmStartCmdArgv[arg_counter++]) > != NULL) > delete [] argv; > delete [] compt->saAmfCtDefAmStartCmdArgv; > > - delete compt->saAmfCtRelPathAmStopCmd; > + delete [] compt->saAmfCtRelPathAmStopCmd; > /* Free saAmfCtDefAmStopCmdArgv[i] before freeing > saAmfCtDefAmStopCmdArgv */ > arg_counter = 0; > while ((argv = compt->saAmfCtDefAmStopCmdArgv[arg_counter++]) > != NULL) > delete [] argv; > delete [] compt->saAmfCtDefAmStopCmdArgv; > > - delete compt->osafAmfCtRelPathHcCmd; > + delete [] compt->osafAmfCtRelPathHcCmd; > arg_counter = 0; > while ((argv = compt->osafAmfCtDefHcCmdArgv[arg_counter++]) != > NULL) > delete [] argv; > diff --git a/osaf/services/saf/amf/amfnd/include/avnd_util.h > b/osaf/services/saf/amf/amfnd/include/avnd_util.h > --- a/osaf/services/saf/amf/amfnd/include/avnd_util.h > +++ b/osaf/services/saf/amf/amfnd/include/avnd_util.h > @@ -51,7 +51,9 @@ > void dnd_msg_free(AVSV_DND_MSG *msg); > void nda_ava_msg_free(AVSV_NDA_AVA_MSG *msg); void > nda_ava_msg_content_free(AVSV_NDA_AVA_MSG *msg); > +void amf_csi_attr_list_copy(SaAmfCSIAttributeListT *dattr, const > +SaAmfCSIAttributeListT *sattr); > void amf_csi_attr_list_free(SaAmfCSIAttributeListT *attrs); > +uint32_t amf_cbk_copy(AVSV_AMF_CBK_INFO **o_dcbk, const > +AVSV_AMF_CBK_INFO *scbk); > void amf_cbk_free(AVSV_AMF_CBK_INFO* cbk_info); void > nd2nd_avnd_msg_free(AVSV_ND2ND_AVND_MSG *msg); > > diff --git a/osaf/services/saf/amf/amfnd/pg.cc > b/osaf/services/saf/amf/amfnd/pg.cc > --- a/osaf/services/saf/amf/amfnd/pg.cc > +++ b/osaf/services/saf/amf/amfnd/pg.cc > @@ -769,7 +769,7 @@ > > if (chg_mem && m_AVND_PG_TRK_IS_CHANGES_ONLY(trk)) { > /*** include only the modified member ***/ > - pg_param->buf.notification = new > SaAmfProtectionGroupNotificationT; > + pg_param->buf.notification = new > +SaAmfProtectionGroupNotificationT[1]; > > *pg_param->buf.notification = chg_mem->info; > pg_param->buf.numberOfItems = 1; > @@ -780,14 +780,12 @@ > > /* now send the cbk msg */ > rc = avnd_pg_cbk_msg_send(cb, trk, cbk_info); > - /* we will free the ckb info both in success/failure case */ > - cbk_info = NULL; > > /* reset the is_syn flag */ > trk->info.is_syn = false; > > if ((NCSCC_RC_SUCCESS != rc) && cbk_info) > - avsv_amf_cbk_free(cbk_info); > + amf_cbk_free(cbk_info); > > TRACE_LEAVE2("rc '%u'", rc); > return rc; > diff --git a/osaf/services/saf/amf/amfnd/sidb.cc > b/osaf/services/saf/amf/amfnd/sidb.cc > --- a/osaf/services/saf/amf/amfnd/sidb.cc > +++ b/osaf/services/saf/amf/amfnd/sidb.cc > @@ -852,9 +852,12 @@ > * Free the memory alloced for this record. > */ > /* free the csi attributes */ > - if (csi_rec->attrs.list) > - delete csi_rec->attrs.list; > - > + if (csi_rec->attrs.list) { > + // use of free() is required as it was > + // malloc'ed (eg. in avsv_edp_susi_asgn()) > + free(csi_rec->attrs.list); > + } > + > /* free the pg list TBD */ > TRACE_1("Comp-CSI record deletion success, Comp=%s : > CSI=%s",csi_rec->comp->name.value,csi_rec->name.value); > > @@ -973,10 +976,15 @@ > /* delete the comp-csi info */ > while ((curr = siq->info.list) != 0) { > siq->info.list = curr->next; > - if (curr->attrs.list) > - delete curr->attrs.list; > - > - delete curr; > + if (curr->attrs.list) { > + // AVSV_ATTR_NAME_VAL variables > + // are malloc'ed, use free() > + free(curr->attrs.list); > + } > + > + // use of free() is required as it was > + // malloc'ed in avsv_edp_susi_asgn() > + free(curr); > } > > /* free the rec */ > diff --git a/osaf/services/saf/amf/amfnd/susm.cc > b/osaf/services/saf/amf/amfnd/susm.cc > --- a/osaf/services/saf/amf/amfnd/susm.cc > +++ b/osaf/services/saf/amf/amfnd/susm.cc > @@ -1031,9 +1031,12 @@ > } > osafassert(t_csi); > /* free the csi attributes */ > - if (t_csi->attrs.list) > - delete t_csi->attrs.list; > - > + if (t_csi->attrs.list) { > + // AVSV_ATTR_NAME_VAL variables > + // are malloc'ed, use free() > + free(t_csi->attrs.list); > + } > + > m_AVND_SU_SI_CSI_REC_REM(*si, *t_csi); > m_AVND_COMPDB_REC_CSI_REM(*(t_csi->comp), *t_csi); > delete t_csi; > diff --git a/osaf/services/saf/amf/amfnd/util.cc > b/osaf/services/saf/amf/amfnd/util.cc > --- a/osaf/services/saf/amf/amfnd/util.cc > +++ b/osaf/services/saf/amf/amfnd/util.cc > @@ -298,7 +298,9 @@ > compcsi_info = susi_msg->msg_info.d2n_su_si_assign.list; > susi_msg->msg_info.d2n_su_si_assign.list = compcsi_info- > >next; > if (compcsi_info->attrs.list != NULL) { > - delete(compcsi_info->attrs.list); > + // AVSV_ATTR_NAME_VAL variables > + // are malloc'ed, use free() > + free(compcsi_info->attrs.list); > compcsi_info->attrs.list = NULL; > } > delete compcsi_info; > @@ -324,7 +326,7 @@ > AVSV_D2N_PG_TRACK_ACT_RSP_MSG_INFO *info = &pg_msg- > >msg_info.d2n_pg_track_act_rsp; > > if (info->mem_list.numberOfItems) > - delete info->mem_list.notification; > + delete [] info->mem_list.notification; > > info->mem_list.notification = 0; > info->mem_list.numberOfItems = 0; > @@ -420,7 +422,7 @@ > > case AVSV_AVND_AMF_CBK_MSG: > if (msg->info.cbk_info) { > - avsv_amf_cbk_free(msg->info.cbk_info); > + amf_cbk_free(msg->info.cbk_info); > msg->info.cbk_info = 0; > } > break; > @@ -433,6 +435,50 @@ > } > > > /************************************************************ > **************** > + Name : amf_csi_attr_list_copy > + > + Description : This routine copies the csi attribute list. > + > + Arguments : dattr - ptr to the destination csi-attr list > + sattr - ptr to the src csi-attr list > + > + Return Values : NCSCC_RC_SUCCESS / NCSCC_RC_FAILURE > + > + Notes : None. > +************************************************************ > *********** > +*******/ void amf_csi_attr_list_copy(SaAmfCSIAttributeListT *dattr, > + const SaAmfCSIAttributeListT *sattr) > +{ > + uint32_t cnt; > + > + if (!dattr || !sattr) > + goto done; > + > + dattr->attr = new SaAmfCSIAttributeT[sattr->number]; > + > + for (cnt = 0; cnt < sattr->number; cnt++) { > + /* alloc memory for attr name & value */ > + size_t attrNameSize = strlen((char*)sattr->attr[cnt].attrName) > + 1; > + dattr->attr[cnt].attrName = new SaUint8T[attrNameSize]; > + > + size_t attrValueSize = > strlen((char*)sattr->attr[cnt].attrValue) + > 1; > + dattr->attr[cnt].attrValue = new SaUint8T[attrNameSize]; > + > + /* copy the attr name & value */ > + strncpy((char*)dattr->attr[cnt].attrName, > + (char*)sattr->attr[cnt].attrName, attrNameSize); > + strncpy((char*)dattr->attr[cnt].attrValue, > + (char*)sattr->attr[cnt].attrValue, attrValueSize); > + > + /* increment the attr name-val pair cnt that is copied */ > + dattr->number++; > + } > + > +done: > + return; > +} > + > +/*********************************************************** > *********** > +****** > Name : amf_csi_attr_list_free > > Description : This routine frees the csi attribute list. > @@ -452,18 +498,100 @@ > > /* free the attr name-val pair */ > for (cnt = 0; cnt < attrs->number; cnt++) { > - delete attrs->attr[cnt].attrName; > - delete attrs->attr[cnt].attrValue; > + delete [] attrs->attr[cnt].attrName; > + delete [] attrs->attr[cnt].attrValue; > } /* for */ > > /* finally free the attr list ptr */ > if (attrs->attr) > - delete attrs->attr; > + delete [] attrs->attr; > > return; > } > > > /************************************************************ > **************** > + Name : amf_cbk_copy > + > + Description : This routine copies the AMF callback info message. > + > + Arguments : o_dcbk - double ptr to the dest cbk-info (o/p) > + scbk - ptr to the source cbk-info > + > + Return Values : NCSCC_RC_SUCCESS/NCSCC_RC_FAILURE. > + > + Notes : None > +************************************************************ > *********** > +*******/ uint32_t amf_cbk_copy(AVSV_AMF_CBK_INFO **o_dcbk, const > +AVSV_AMF_CBK_INFO *scbk) { > + uint32_t rc = NCSCC_RC_SUCCESS; > + > + if (!o_dcbk || !scbk) > + return NCSCC_RC_FAILURE; > + > + /* allocate the dest cbk-info */ > + *o_dcbk = new AVSV_AMF_CBK_INFO(); > + > + /* copy the common fields */ > + memcpy(*o_dcbk, scbk, sizeof(AVSV_AMF_CBK_INFO)); > + > + switch (scbk->type) { > + case AVSV_AMF_HC: > + case AVSV_AMF_COMP_TERM: > + case AVSV_AMF_CSI_REM: > + case AVSV_AMF_PXIED_COMP_INST: > + case AVSV_AMF_PXIED_COMP_CLEAN: > + break; > + > + case AVSV_AMF_PG_TRACK: > + /* memset notify buffer */ > + memset(&(*o_dcbk)->param.pg_track.buf, 0, > +sizeof(SaAmfProtectionGroupNotificationBufferT)); > + > + /* copy the notify buffer, if any */ > + if (scbk->param.pg_track.buf.numberOfItems) { > + (*o_dcbk)->param.pg_track.buf.notification = > + new > +SaAmfProtectionGroupNotificationT[scbk- > >param.pg_track.buf.numberOfItem > +s]; > + > + for (SaUint32T i = 0; i < scbk- > >param.pg_track.buf.numberOfItems; ++i) { > + (*o_dcbk)->param.pg_track.buf.notification[i] > = > + scbk- > >param.pg_track.buf.notification[i]; > + } > + (*o_dcbk)->param.pg_track.buf.numberOfItems = > scbk->param.pg_track.buf.numberOfItems; > + } > + break; > + > + case AVSV_AMF_CSI_SET: > + /* memset avsv & amf csi attr lists */ > + memset(&(*o_dcbk)->param.csi_set.attrs, 0, > sizeof(AVSV_CSI_ATTRS)); > + memset(&(*o_dcbk)->param.csi_set.csi_desc.csiAttr, 0, > +sizeof(SaAmfCSIAttributeListT)); > + > + /* copy the avsv csi attr list */ > + if (scbk->param.csi_set.attrs.number) { > + (*o_dcbk)->param.csi_set.attrs.list = > static_cast<AVSV_ATTR_NAME_VAL*> > + (calloc(scbk->param.csi_set.attrs.number, > sizeof(AVSV_ATTR_NAME_VAL))); > + osafassert((*o_dcbk)->param.csi_set.attrs.list != > NULL); > + > + for (uint32_t i = 0; i < scbk- > >param.csi_set.attrs.number; ++i) { > + (*o_dcbk)->param.csi_set.attrs.list[i] = > + scbk->param.csi_set.attrs.list[i]; > + } > + (*o_dcbk)->param.csi_set.attrs.number = scbk- > >param.csi_set.attrs.number; > + } > + > + /* copy the amf csi attr list */ > + if (scbk->param.csi_set.csi_desc.csiAttr.number) { > + amf_csi_attr_list_copy(&(*o_dcbk)- > >param.csi_set.csi_desc.csiAttr, > + &scbk->param.csi_set.csi_desc.csiAttr); > + } > + break; > + > + default: > + osafassert(0); > + } > + > + return rc; > +} > + > +/*********************************************************** > *********** > +****** > Name : amf_cbk_free > > Description : This routine frees callback information. > @@ -490,14 +618,17 @@ > case AVSV_AMF_PG_TRACK: > /* free the notify buffer */ > if (cbk_info->param.pg_track.buf.numberOfItems) > - delete cbk_info->param.pg_track.buf.notification; > + delete [] cbk_info->param.pg_track.buf.notification; > break; > > case AVSV_AMF_CSI_SET: > /* free the avsv csi attr list */ > - if (cbk_info->param.csi_set.attrs.number) > - delete cbk_info->param.csi_set.attrs.list; > - > + if (cbk_info->param.csi_set.attrs.number) { > + // AVSV_ATTR_NAME_VAL variables > + // are malloc'ed, use free() > + free(cbk_info->param.csi_set.attrs.list); > + } > + > /* free the amf csi attr list */ > amf_csi_attr_list_free(&cbk_info- > >param.csi_set.csi_desc.csiAttr); > break; > > ------------------------------------------------------------------------------ > WatchGuard Dimension instantly turns raw network data into actionable > security intelligence. It gives you real-time visual feedback on key security > issues > and trends. Skip the complicated setup - simply import a virtual appliance > and > go from zero to informed in seconds. > http://pubads.g.doubleclick.net/gampad/clk?id=123612991&iu=/4140/ostg.clk > trk > _______________________________________________ > Opensaf-devel mailing list > [email protected] > https://lists.sourceforge.net/lists/listinfo/opensaf-devel ------------------------------------------------------------------------------ Managing the Performance of Cloud-Based Applications Take advantage of what the Cloud has to offer - Avoid Common Pitfalls. Read the Whitepaper. http://pubads.g.doubleclick.net/gampad/clk?id=121051231&iu=/4140/ostg.clktrk _______________________________________________ Opensaf-devel mailing list [email protected] https://lists.sourceforge.net/lists/listinfo/opensaf-devel
