ack from me too (review only) > On 25 Sep 2015, at 8:13 pm, Nagendra Kumar <[email protected]> wrote: > > Ack, please push it. > > Thanks > -Nagu >> -----Original Message----- >> From: Hans Nordeback [mailto:[email protected]] >> Sent: 25 September 2015 14:39 >> To: Praveen Malviya; Nagendra Kumar; [email protected] >> Cc: [email protected] >> Subject: [PATCH 1 of 1] amfd: segv in get_comp_capability [#1502] >> >> osaf/services/saf/amf/amfd/comp.cc | 2 +- >> osaf/services/saf/amf/amfd/csi.cc | 6 +++--- >> osaf/services/saf/amf/amfd/include/comp.h | 5 ++--- >> osaf/services/saf/amf/amfd/include/csi.h | 2 +- >> osaf/services/saf/amf/amfd/sgproc.cc | 24 ++++++++++++------------ >> osaf/services/saf/amf/amfd/su.cc | 29 +++++++++++++++++++--------- >> - >> 6 files changed, 38 insertions(+), 30 deletions(-) >> >> >> diff --git a/osaf/services/saf/amf/amfd/comp.cc >> b/osaf/services/saf/amf/amfd/comp.cc >> --- a/osaf/services/saf/amf/amfd/comp.cc >> +++ b/osaf/services/saf/amf/amfd/comp.cc >> @@ -75,7 +75,7 @@ void AVD_COMP::initialize() { >> saAmfCompRestartCount = {}; >> saAmfCompCurrProxyName = {}; >> saAmfCompCurrProxiedNames = {}; >> - assign_flag = {}; >> + assign_flag = false; >> comp_type = {}; >> comp_type_list_comp_next = {}; >> su = {}; >> diff --git a/osaf/services/saf/amf/amfd/csi.cc >> b/osaf/services/saf/amf/amfd/csi.cc >> --- a/osaf/services/saf/amf/amfd/csi.cc >> +++ b/osaf/services/saf/amf/amfd/csi.cc >> @@ -52,7 +52,7 @@ AVD_COMP* AVD_CSI::find_assigned_comp(co >> } >> } >> if (iter == list_of_comp.end()) { >> - return NULL; >> + return nullptr; >> } else { >> return comp; >> } >> @@ -511,7 +511,7 @@ static SaAisErrorT csi_ccb_completed_cre >> >> compcsi = t_sisu->list_of_csicomp; >> while (compcsi != NULL) { >> - compcsi->comp->set_assigned(); >> + compcsi->comp->set_assigned(true); >> compcsi = compcsi- >>> susi_csicomp_next; >> } >> >> @@ -993,7 +993,7 @@ SaAisErrorT csi_assign_hdlr(AVD_CSI *csi >> >> compcsi = t_sisu->list_of_csicomp; >> while (compcsi != NULL) { >> - compcsi->comp->set_assigned(); >> + compcsi->comp->set_assigned(true); >> compcsi = compcsi->susi_csicomp_next; >> } >> >> diff --git a/osaf/services/saf/amf/amfd/include/comp.h >> b/osaf/services/saf/amf/amfd/include/comp.h >> --- a/osaf/services/saf/amf/amfd/include/comp.h >> +++ b/osaf/services/saf/amf/amfd/include/comp.h >> @@ -123,9 +123,8 @@ class AVD_COMP { >> AVD_SU *su; /* SU to which this component belongs */ >> AVD_ADMIN_OPER_CBK admin_pend_cbk; /* holds callback invocation for >> admin operation */ >> >> - void set_unassigned() {assign_flag = false;} >> - void set_assigned() {assign_flag = true;} >> - bool is_assigned() {return assign_flag;} >> + void set_assigned(bool assigned) {assign_flag = assigned;} bool >> + assigned() const {return assign_flag;} >> private: >> void initialize(); >> // disallow copy and assign >> diff --git a/osaf/services/saf/amf/amfd/include/csi.h >> b/osaf/services/saf/amf/amfd/include/csi.h >> --- a/osaf/services/saf/amf/amfd/include/csi.h >> +++ b/osaf/services/saf/amf/amfd/include/csi.h >> @@ -82,7 +82,7 @@ class AVD_CSI { >> uint32_t compcsi_cnt {}; /* no of comp-csi rels */ >> AVD_CSI *csi_list_cs_type_next {}; >> AVD_CS_TYPE *cstype {}; >> - bool assign_flag {}; /* Flag used while assigning. to mark this csi has >> been >> assigned a Comp >> + bool assign_flag = false; /* Flag used while assigning. to mark this >> csi has >> been assigned a Comp >> from * current SI being assigned */ >> >> static AVD_COMP* find_assigned_comp(const SaNameT *cstype, const >> AVD_SU_SI_REL *sisu, const std::vector<AVD_COMP*> &list_of_comp); diff >> --git a/osaf/services/saf/amf/amfd/sgproc.cc >> b/osaf/services/saf/amf/amfd/sgproc.cc >> --- a/osaf/services/saf/amf/amfd/sgproc.cc >> +++ b/osaf/services/saf/amf/amfd/sgproc.cc >> @@ -62,7 +62,7 @@ static void verify_csi_deps_and_delete_i >> compcsi->comp- >>> comp_info.name.value, >> compcsi->csi- >>> name.value); >> compcsi->csi->assign_flag = false; >> - compcsi->comp->set_unassigned(); >> + compcsi->comp- >>> set_assigned(false); >> >> avd_compcsi_from_csi_and_susi_delete(susi, compcsi, true); >> //Delete compcsi of dependents. >> >> verify_csi_deps_and_delete_invalid_compcsi(susi); >> @@ -134,7 +134,7 @@ uint32_t avd_new_assgn_susi(AVD_CL_CB *c >> /* find a component that can be assigned this CSI */ >> l_comp = su- >>> find_unassigned_comp_that_provides_cstype(&l_csi->saAmfCSType); >> >> - if (l_comp == NULL) { >> + if (l_comp == nullptr) { >> /* This means either - 1. l_csi cann't be assigned to >> any comp or 2. some comp got assigned >> and the rest cann't be assigned.*/ >> l_csi = l_csi->si_list_of_csi_next; >> @@ -148,7 +148,7 @@ uint32_t avd_new_assgn_susi(AVD_CL_CB *c >> continue; >> } >> >> - l_comp->set_assigned(); >> + l_comp->set_assigned(true); >> l_csi->assign_flag = true; >> l_csi = l_csi->si_list_of_csi_next; >> } /* while(l_csi != AVD_CSI_NULL) */ >> @@ -165,17 +165,17 @@ uint32_t avd_new_assgn_susi(AVD_CL_CB *c >> Here, policy for assigning more than 1 csi to components is : Assign >> to max to the deserving comps and then >> assign the rest csi to others. We are taking advantage of Specs >> defining implementation specific csi >> assigiment.*/ >> - TRACE("Now assiging more than one csi per comp"); >> + TRACE("Now assigning more than one csi per comp"); >> l_csi = si->list_of_csi; >> while (NULL != l_csi) { >> if (false == l_csi->assign_flag) { >> /* Assign to only those comps, which have >> assignment. Those comps, which could not have assignment >> before, cann't find compcsi here also.*/ >> - for (const auto& l_comp : su->list_of_comp) { >> - AVD_COMP_TYPE *comptype = >> comptype_db->find(Amf::to_string(&l_comp->saAmfCompType)); >> + for (const auto& comp_ : su->list_of_comp) { >> + AVD_COMP_TYPE *comptype = >> +comptype_db->find(Amf::to_string(&comp_->saAmfCompType)); >> osafassert(comptype); >> - if ((true == l_comp->is_assigned()) && >> (comptype->saAmfCtCompCategory != SA_AMF_COMP_LOCAL)) { >> - if (NULL != (cst = >> avd_compcstype_find_match(&l_csi->saAmfCSType, l_comp))) { >> + if ((true == comp_->assigned()) && >> (comptype->saAmfCtCompCategory != SA_AMF_COMP_LOCAL)) { >> + if (NULL != (cst = >> avd_compcstype_find_match(&l_csi->saAmfCSType, >> +comp_))) { >> if (SA_AMF_HA_ACTIVE == >> ha_state) { >> if (cst- >>> saAmfCompNumCurrActiveCSIs < cst->saAmfCompNumMaxActiveCSIs) { >> } else { /* We cann't >> assign this csi to this comp, so check for another comp */ @@ -187,7 +187,7 >> @@ uint32_t avd_new_assgn_susi(AVD_CL_CB *c >> continue ; >> } >> } >> - if ((compcsi = >> avd_compcsi_create(susi, l_csi, l_comp, true)) == NULL) { >> + if ((compcsi = >> avd_compcsi_create(susi, l_csi, comp_, true)) == >> +NULL) { >> /* free all the CSI >> assignments and end this loop */ >> >> avd_compcsi_delete(cb, susi, true); >> continue; >> @@ -195,9 +195,9 @@ uint32_t avd_new_assgn_susi(AVD_CL_CB *c >> l_csi->assign_flag = true; >> /* If one csi has been >> assigned to a comp, then look for another csi. */ >> break; >> - }/* if (NULL != (cst = >> avd_compcstype_find_match(&l_csi->saAmfCSType, l_comp))) */ >> - }/* if (true == l_comp->assign_flag) */ >> - }/* for (const auto& l_comp : su->list_of_comp) */ >> + }/* if (NULL != (cst = >> avd_compcstype_find_match(&l_csi->saAmfCSType, comp_))) */ >> + }/* if (true == comp_->assigned()) */ >> + }/* for (const auto& comp_ : su->list_of_comp) */ >> }/* if (false == l_csi->assign_flag)*/ >> l_csi = l_csi->si_list_of_csi_next; >> }/* while (l_csi != NULL) */ >> diff --git a/osaf/services/saf/amf/amfd/su.cc >> b/osaf/services/saf/amf/amfd/su.cc >> --- a/osaf/services/saf/amf/amfd/su.cc >> +++ b/osaf/services/saf/amf/amfd/su.cc >> @@ -122,9 +122,13 @@ void AVD_SU::remove_comp(AVD_COMP *comp) >> osafassert(su_ref != NULL); >> >> if (comp->su != nullptr) { >> - comp->su->list_of_comp.erase(std::remove(comp->su- >>> list_of_comp.begin(), >> - comp->su->list_of_comp.end(), >> comp), comp->su- >>> list_of_comp.end()); >> - } >> + su_ref->list_of_comp.erase(std::remove(su_ref- >>> list_of_comp.begin(), >> + su_ref->list_of_comp.end(), >> +comp), su_ref->list_of_comp.end()); >> + >> + /* Marking SU referance pointer to NULL, please dont use >> further in the routine */ >> + comp->su = nullptr; >> + } >> + >> bool old_preinst_value = saAmfSUPreInstantiable; >> bool curr_preinst_value = saAmfSUPreInstantiable; >> >> @@ -152,8 +156,8 @@ void AVD_SU::remove_comp(AVD_COMP *comp) } >> >> void AVD_SU::add_comp(AVD_COMP *comp) { >> - comp->su->list_of_comp.push_back(comp); >> - std::sort(comp->su->list_of_comp.begin(), comp->su- >>> list_of_comp.end(), [] (const AVD_COMP *c1, const AVD_COMP *c2) -> >> bool { >> + list_of_comp.push_back(comp); >> + std::sort(list_of_comp.begin(), list_of_comp.end(), [] (const >> AVD_COMP >> +*c1, const AVD_COMP *c2) -> bool { >> if (c1->comp_info.inst_level < c2- >>> comp_info.inst_level) return true; >> if (c1->comp_info.inst_level == c2- >>> comp_info.inst_level) { >> if (m_CMP_HORDER_SANAMET(c1- >>> comp_info.name, c2->comp_info.name) < 0) @@ -2011,7 +2015,7 @@ >> void AVD_SU::set_saAmfSUPreInstantiable( >> * resets the assign flag for all contained components >> */ >> void AVD_SU::reset_all_comps_assign_flag() { >> - std::for_each(list_of_comp.begin(), list_of_comp.end(), >> [](AVD_COMP *comp ) {comp->set_unassigned();}); >> + std::for_each(list_of_comp.begin(), list_of_comp.end(), >> [](AVD_COMP >> +*comp ) {comp->set_assigned(false);}); >> } >> >> /** >> @@ -2021,22 +2025,27 @@ void AVD_SU::reset_all_comps_assign_flag >> */ >> AVD_COMP *AVD_SU::find_unassigned_comp_that_provides_cstype(const >> SaNameT *cstype) { >> AVD_COMP *l_comp = nullptr; >> - for (const auto& comp : list_of_comp) { >> - l_comp = comp; >> + auto iter = list_of_comp.begin(); >> + for (; iter != list_of_comp.end(); ++iter) { >> + l_comp = *iter; >> bool npi_is_assigned = false; >> AVD_COMP_TYPE *comptype = comptype_db- >>> find(Amf::to_string(&l_comp->saAmfCompType)); >> osafassert(comptype); >> if ((comptype->saAmfCtCompCategory == >> SA_AMF_COMP_LOCAL) && is_comp_assigned_any_csi(l_comp)) >> npi_is_assigned = true; >> >> - if ((l_comp->is_assigned() == false) && (npi_is_assigned == >> false)) { >> + if ((l_comp->assigned() == false) && (npi_is_assigned == >> false)) { >> AVD_COMPCS_TYPE *cst = >> avd_compcstype_find_match(cstype, l_comp); >> if (cst != NULL) >> break; >> } >> } >> >> - return l_comp; >> + if (iter == list_of_comp.end()) { >> + return nullptr; >> + } else { >> + return l_comp; >> + } >> } >> >> /**
------------------------------------------------------------------------------ _______________________________________________ Opensaf-devel mailing list [email protected] https://lists.sourceforge.net/lists/listinfo/opensaf-devel
