* The short commit message should describe what the patch is doing * Wherever free is used instead of delete a comment should added explaining why * Do not add error handling for out of memory errors, in case of malloc just do osafassert(p != NULL);
Hans N: any comments? Thanks, HansF On 13 January 2014 14:46, Gary Lee <[email protected]> wrote: > osaf/services/saf/amf/amfnd/amfnd.cc | 4 +- > osaf/services/saf/amf/amfnd/cbq.cc | 4 +- > osaf/services/saf/amf/amfnd/ckpt_updt.cc | 4 +- > osaf/services/saf/amf/amfnd/comp.cc | 14 +- > 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 | 15 +- > osaf/services/saf/amf/amfnd/susm.cc | 2 +- > osaf/services/saf/amf/amfnd/util.cc | 145 > ++++++++++++++++++++++- > 10 files changed, 169 insertions(+), 33 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 create > 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 > @@ -220,7 +220,7 @@ > } > /* free the csi attributes */ > if (csi_rec->attrs.list) > - delete csi_rec->attrs.list; > + free(csi_rec->attrs.list); > > /* finally free this record */ > delete csi_rec; > @@ -994,7 +994,7 @@ > } > /* free the csi attributes */ > if (csi_rec->attrs.list) > - delete > csi_rec->attrs.list; > + > free(csi_rec->attrs.list); > > /* finally free this record */ > delete 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 > @@ -1892,10 +1892,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) { > @@ -1960,7 +1957,12 @@ > /* 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))); > + if (!attr.list) { > + LOG_ER("%s: calloc FAILED", > __FUNCTION__); > + return NCSCC_RC_FAILURE; > + } > > memcpy(attr.list, curr_csi->attrs.list, > sizeof(AVSV_ATTR_NAME_VAL) * > curr_csi->attrs.number); > @@ -2022,7 +2024,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 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); > > @@ -974,9 +977,11 @@ > while ((curr = siq->info.list) != 0) { > siq->info.list = curr->next; > if (curr->attrs.list) > - delete curr->attrs.list; > + free(curr->attrs.list); > > - delete curr; > + // 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 > @@ -1032,7 +1032,7 @@ > osafassert(t_csi); > /* free the csi attributes */ > if (t_csi->attrs.list) > - delete t_csi->attrs.list; > + 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); > 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,7 @@ > 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); > + free(compcsi_info->attrs.list); > compcsi_info->attrs.list = NULL; > } > delete compcsi_info; > @@ -324,7 +324,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 +420,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 +433,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 +496,103 @@ > > /* 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.numberOfItems]; > + > + 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))); > + if ((*o_dcbk)->param.csi_set.attrs.list == NULL) { > + LOG_ER("%s: calloc FAILED", __FUNCTION__); > + return NCSCC_RC_FAILURE; > + } > + > + 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,13 +619,13 @@ > 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; > + 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); > > ------------------------------------------------------------------------------ > CenturyLink Cloud: The Leader in Enterprise Cloud Services. > Learn Why More Businesses Are Choosing CenturyLink Cloud For > Critical Workloads, Development Environments & Everything In Between. > Get a Quote or Start a Free Trial Today. > http://pubads.g.doubleclick.net/gampad/clk?id=119420431&iu=/4140/ostg.clktrk > _______________________________________________ > Opensaf-devel mailing list > [email protected] > https://lists.sourceforge.net/lists/listinfo/opensaf-devel ------------------------------------------------------------------------------ CenturyLink Cloud: The Leader in Enterprise Cloud Services. Learn Why More Businesses Are Choosing CenturyLink Cloud For Critical Workloads, Development Environments & Everything In Between. Get a Quote or Start a Free Trial Today. http://pubads.g.doubleclick.net/gampad/clk?id=119420431&iu=/4140/ostg.clktrk _______________________________________________ Opensaf-devel mailing list [email protected] https://lists.sourceforge.net/lists/listinfo/opensaf-devel
