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