Hi Praveen
ack (review only) with some comments below.
On 8/9/16 7:07 pm, praveen.malv...@oracle.com wrote:
> osaf/libs/common/amf/n2avamsg.c | 5 +-
> osaf/services/saf/amf/amfd/csiattr.cc | 1 +
> osaf/services/saf/amf/amfd/util.cc | 49
> ++++++++++++++++++++----
> osaf/services/saf/amf/amfnd/comp.cc | 7 +--
> osaf/services/saf/amf/amfnd/include/avnd_util.h | 2 +
> osaf/services/saf/amf/amfnd/mds.cc | 6 +++
> osaf/services/saf/amf/amfnd/su.cc | 2 +-
> osaf/services/saf/amf/amfnd/util.cc | 27 +++++++++++++
> 8 files changed, 83 insertions(+), 16 deletions(-)
>
>
> Patch uses osaf_extended_name_alloc() APIs for manipulation of CSI attributes
> in SUSI and COMPCSI assignment messages.
>
> diff --git a/osaf/libs/common/amf/n2avamsg.c b/osaf/libs/common/amf/n2avamsg.c
> --- a/osaf/libs/common/amf/n2avamsg.c
> +++ b/osaf/libs/common/amf/n2avamsg.c
> @@ -358,7 +358,8 @@ uint32_t avsv_amf_cbk_copy(AVSV_AMF_CBK_
>
> osaf_extended_name_alloc(osaf_extended_name_borrow(&scbk->param.csi_attr_change.csi_name),
> &(*o_dcbk)->param.csi_attr_change.csi_name);
> /* memset avsv & amf csi attr lists */
> - memset(&(*o_dcbk)->param.csi_attr_change.attrs, 0,
> sizeof(AVSV_CSI_ATTRS));
> + memset(&(*o_dcbk)->param.csi_attr_change.attrs, 0,
> sizeof(AVSV_CSI_ATTRS));
> + memset(&(*o_dcbk)->param.csi_attr_change.csiAttr, 0,
> sizeof(SaAmfCSIAttributeListT));
> /* copy the avsv csi attr list */
> if (scbk->param.csi_attr_change.attrs.number > 0) {
> (*o_dcbk)->param.csi_attr_change.attrs.list =
> @@ -371,7 +372,7 @@ uint32_t avsv_amf_cbk_copy(AVSV_AMF_CBK_
> memcpy((*o_dcbk)->param.csi_attr_change.attrs.list,
> scbk->param.csi_attr_change.attrs.list,
> sizeof(AVSV_ATTR_NAME_VAL) *
> scbk->param.csi_attr_change.attrs.number);
> - for (i = 0; i < scbk->param.csi_set.attrs.number; i++) {
> + for (i = 0; i <
> scbk->param.csi_attr_change.attrs.number; i++) {
>
> osaf_extended_name_alloc(osaf_extended_name_borrow(&scbk->param.csi_attr_change.attrs.list[i].name),
>
> &(*o_dcbk)->param.csi_attr_change.attrs.list[i].name);
>
> osaf_extended_name_alloc(osaf_extended_name_borrow(&scbk->param.csi_attr_change.attrs.list[i].value),
> diff --git a/osaf/services/saf/amf/amfd/csiattr.cc
> b/osaf/services/saf/amf/amfd/csiattr.cc
> --- a/osaf/services/saf/amf/amfd/csiattr.cc
> +++ b/osaf/services/saf/amf/amfd/csiattr.cc
> @@ -577,6 +577,7 @@ static void csiattr_modify_apply(CcbUtil
>
> osaf_extended_name_alloc(csi_attr_name.c_str(), &csiattr->name_value.name);
> csiattr->name_value.string_ptr = new
> char[strlen(value)+1]();
> memcpy(csiattr->name_value.string_ptr,
> value, strlen(value)+1 );
> +
> osaf_extended_name_alloc(csiattr->name_value.string_ptr,
> &csiattr->name_value.value);
> } /* for */
> }
> /* add the modified csiattr values to parent csi */
> diff --git a/osaf/services/saf/amf/amfd/util.cc
> b/osaf/services/saf/amf/amfd/util.cc
> --- a/osaf/services/saf/amf/amfd/util.cc
> +++ b/osaf/services/saf/amf/amfd/util.cc
> @@ -660,11 +660,21 @@ static uint32_t avd_prep_csi_attr_info(A
> /* Scan the list of attributes for the CSI and add it to the message */
> while ((attr_ptr != nullptr) && (compcsi_info->attrs.number <
> compcsi->csi->num_attributes)) {
> memcpy(i_ptr, &attr_ptr->name_value,
> sizeof(AVSV_ATTR_NAME_VAL));
> +
> osaf_extended_name_alloc(osaf_extended_name_borrow(&attr_ptr->name_value.name),
> + &i_ptr->name);
> +
> osaf_extended_name_alloc(osaf_extended_name_borrow(&attr_ptr->name_value.value),
> + &i_ptr->value);
> + if (attr_ptr->name_value.string_ptr != nullptr) {
> + i_ptr->string_ptr = new
> char[strlen(attr_ptr->name_value.string_ptr)+1];
> + memcpy(i_ptr->string_ptr,
> attr_ptr->name_value.string_ptr,
> +
> strlen(attr_ptr->name_value.string_ptr)+1);
> + } else {
> + i_ptr->string_ptr = nullptr;
> + }
> compcsi_info->attrs.number++;
> i_ptr = i_ptr + 1;
> attr_ptr = attr_ptr->attr_next;
> }
> -
> TRACE_LEAVE();
> return NCSCC_RC_SUCCESS;
> }
> @@ -1684,6 +1694,7 @@ static void free_d2n_susi_msg_info(AVSV_
> {
> TRACE_ENTER();
> AVSV_SUSI_ASGN *compcsi_info;
[GL] reduce scope; declare i inside for loop
> + uint16_t i;
>
> osaf_extended_name_free(&susi_msg->msg_info.d2n_su_si_assign.si_name);
> osaf_extended_name_free(&susi_msg->msg_info.d2n_su_si_assign.su_name);
> @@ -1692,8 +1703,12 @@ static void free_d2n_susi_msg_info(AVSV_
> 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 != nullptr) {
> -
> osaf_extended_name_free(&compcsi_info->attrs.list->name);
> -
> osaf_extended_name_free(&compcsi_info->attrs.list->value);
> + for (i = 0; i < compcsi_info->attrs.number; i++) {
> +
> osaf_extended_name_free(&compcsi_info->attrs.list[i].name);
> +
> osaf_extended_name_free(&compcsi_info->attrs.list[i].value);
[GL] should be delete [] below
> + delete
> compcsi_info->attrs.list[i].string_ptr;
> + compcsi_info->attrs.list[i].string_ptr =
> nullptr;
> + }
> delete [] (compcsi_info->attrs.list);
> compcsi_info->attrs.list = nullptr;
> }
> @@ -1733,16 +1748,24 @@ static void free_d2n_pg_msg_info(AVSV_DN
> }
>
> static void free_d2n_compcsi_info(AVSV_DND_MSG *compcsi_msg) {
[GL] reduce scope; declare i inside for loop
> + uint16_t i;
> AVSV_D2N_COMPCSI_ASSIGN_MSG_INFO *compcsi =
> &compcsi_msg->msg_info.d2n_compcsi_assign_msg_info;
>
> osaf_extended_name_free(&compcsi->comp_name);
> osaf_extended_name_free(&compcsi->csi_name);
>
> if (compcsi->info.attrs.list != nullptr) {
> + for (i = 0; i < compcsi->info.attrs.number; i++) {
> + osaf_extended_name_free(&compcsi->info.attrs.list[i].name);
> + osaf_extended_name_free(&compcsi->info.attrs.list[i].value);
[GL] should be delete [] below
> + delete compcsi->info.attrs.list[i].string_ptr;
> + compcsi->info.attrs.list[i].string_ptr = nullptr;
> + }
> delete [] (compcsi->info.attrs.list);
> compcsi->info.attrs.list = nullptr;
> }
> }
> +
>
> /****************************************************************************
> Name : d2n_msg_free
>
> @@ -2030,11 +2053,21 @@ uint32_t avd_snd_compcsi_msg(AVD_COMP *c
>
> /* Scan the list of attributes for the CSI and add it to the message
> */
> while ((attr_ptr_db != nullptr) &&
> - (ptr_csiattr_msg->number < compcsi->csi->num_attributes))
> {
> - memcpy(i_ptr_msg, &attr_ptr_db->name_value,
> sizeof(AVSV_ATTR_NAME_VAL));
> - ptr_csiattr_msg->number++;
> - i_ptr_msg = i_ptr_msg + 1;
> - attr_ptr_db = attr_ptr_db->attr_next;
> + (ptr_csiattr_msg->number < compcsi->csi->num_attributes)) {
> +
> osaf_extended_name_alloc(osaf_extended_name_borrow(&attr_ptr_db->name_value.name),
> + &i_ptr_msg->name);
> +
> osaf_extended_name_alloc(osaf_extended_name_borrow(&attr_ptr_db->name_value.value),
> + &i_ptr_msg->value);
> + if (attr_ptr_db->name_value.string_ptr != nullptr) {
> + i_ptr_msg->string_ptr = new
> char[strlen(attr_ptr_db->name_value.string_ptr)+1];
> + memcpy(i_ptr_msg->string_ptr, attr_ptr_db->name_value.string_ptr,
> + strlen(attr_ptr_db->name_value.string_ptr)+1);
> + } else {
> + i_ptr_msg->string_ptr = nullptr;
> + }
> + ptr_csiattr_msg->number++;
> + i_ptr_msg = i_ptr_msg + 1;
> + attr_ptr_db = attr_ptr_db->attr_next;
> }
> break;
> }
> 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
> @@ -1911,8 +1911,7 @@ static void set_params_for_csi_attr_chan
> attr.list =
> static_cast<AVSV_ATTR_NAME_VAL*>(calloc(csi_rec->attrs.number,
> sizeof(AVSV_ATTR_NAME_VAL)));
> osafassert(attr.list != nullptr);
> - memcpy(attr.list, csi_rec->attrs.list,
> - sizeof(AVSV_ATTR_NAME_VAL)*csi_rec->attrs.number);
> + amfnd_copy_csi_attrs(&csi_rec->attrs, &attr);
> attr.number = csi_rec->attrs.number;
> }
> /* fill the callback params */
> @@ -2034,9 +2033,7 @@ uint32_t avnd_comp_cbk_send(AVND_CB *cb,
> attr.list = static_cast<AVSV_ATTR_NAME_VAL*>
> (calloc(curr_csi->attrs.number,
> sizeof(AVSV_ATTR_NAME_VAL)));
> osafassert(attr.list != nullptr);
> -
> - memcpy(attr.list, curr_csi->attrs.list,
> - sizeof(AVSV_ATTR_NAME_VAL) *
> curr_csi->attrs.number);
> + amfnd_copy_csi_attrs(&csi_rec->attrs, &attr);
> attr.number = curr_csi->attrs.number;
> }
>
> 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
> @@ -78,4 +78,6 @@ SaAisErrorT amf_saImmOmAccessorGet_o2(Sa
> const std::string& objectName,
> const SaImmAttrNameT * attributeNames,
> SaImmAttrValuesT_2 *** attributes);
> +void amfnd_free_csi_attr_list(AVSV_CSI_ATTRS *attrs);
> +void amfnd_copy_csi_attrs(AVSV_CSI_ATTRS *src_attrs, AVSV_CSI_ATTRS
> *dest_attrs);
> #endif /* !AVND_UTIL_H */
> diff --git a/osaf/services/saf/amf/amfnd/mds.cc
> b/osaf/services/saf/amf/amfnd/mds.cc
> --- a/osaf/services/saf/amf/amfnd/mds.cc
> +++ b/osaf/services/saf/amf/amfnd/mds.cc
> @@ -842,6 +842,12 @@ static uint32_t enc_csi_attr_change_cbk(
> ncs_encode_16bit(&p8, len);
> ncs_enc_claim_space(uba, 2);
> rc = ncs_encode_n_octets_in_uba(uba, (uint8_t *)value, (len + 1));
> + // free value
> + if (value != nullptr && value !=
> csi_attr_change->attrs.list[i].string_ptr) {
> + delete[] value;
> + value = nullptr;
> + }
> +
> if (NCSCC_RC_SUCCESS != rc)
> goto done;
> }
> diff --git a/osaf/services/saf/amf/amfnd/su.cc
> b/osaf/services/saf/amf/amfnd/su.cc
> --- a/osaf/services/saf/amf/amfnd/su.cc
> +++ b/osaf/services/saf/amf/amfnd/su.cc
> @@ -922,7 +922,7 @@ static uint32_t avnd_process_comp_csi_ms
> case AVSV_COMPCSI_ATTR_CHANGE_AND_NO_ACK: {
> //Free ealrliar allocated memory by EDP utils.
> if (csi_rec->attrs.list != nullptr)
> - free (csi_rec->attrs.list);
> + amfnd_free_csi_attr_list(&csi_rec->attrs);
> csi_rec->attrs.number = param->info.attrs.number;
> csi_rec->attrs.list = param->info.attrs.list;
> param->info.attrs.number = 0;
> 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
> @@ -397,3 +397,30 @@ SaAisErrorT amf_saImmOmAccessorGet_o2(Sa
>
> return rc;
> }
> +
> +void amfnd_free_csi_attr_list(AVSV_CSI_ATTRS *attrs) {
> + if (attrs == nullptr)
> + return;
> + for (uint16_t i = 0; i < attrs->number; i++) {
> + osaf_extended_name_free(&attrs->list[i].name);
> + osaf_extended_name_free(&attrs->list[i].value);
> + free(attrs->list[i].string_ptr);
> + attrs->list[i].string_ptr = nullptr;
> + }
> + free(attrs->list);
> + attrs->list = nullptr;
> +}
> +
> +void amfnd_copy_csi_attrs(AVSV_CSI_ATTRS *src_attrs, AVSV_CSI_ATTRS
> *dest_attrs) {
> + for (uint16_t i = 0; i < src_attrs->number; i++) {
> +
> osaf_extended_name_alloc(osaf_extended_name_borrow(&src_attrs->list[i].name),
> + &dest_attrs->list[i].name);
> +
> osaf_extended_name_alloc(osaf_extended_name_borrow(&src_attrs->list[i].value),
> + &dest_attrs->list[i].value);
> + //Let string.ptr points to original one, we never free it. Also
> + //encode callback takes care of encoding it.
> + if (src_attrs->list[i].string_ptr != nullptr)
> + dest_attrs->list[i].string_ptr = src_attrs->list[i].string_ptr;
> + }
> +}
> +
------------------------------------------------------------------------------
_______________________________________________
Opensaf-devel mailing list
Opensaf-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/opensaf-devel
