Ack with minor comments:
* check with NULL
* check for greater than 0
(Design rules for amf should be created...)
See inline
Thanks,
Hans
On 01/28/2014 07:32 AM, Gary Lee 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 | 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) {
remove if, OK todo free(NULL)
> + // 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) {
remove if, OK todo free(NULL)
> + // 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) {
remove if, OK todo free(NULL)
> + // 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) {
remove if, OK todo free(NULL)
> + // 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) {
remove if, OK todo free(NULL)
> + // 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)
check for NULL
> + 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)
check for non NULL
> - 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)
check for NULL
> + 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) {
> 0
> + (*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) {
> 0
> + (*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) {
> 0
> +
> 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) {
> 0
> + // 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.clktrk
> _______________________________________________
> 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
