Hi Gary,
ack, one question, please see inline.
/Thanks HansN
/Thanks Hans
On 09/17/2015 07:22 AM, Gary Lee wrote:
> osaf/services/saf/amf/amfd/sg.cc | 179
> +++++++++++++++++++-------------------
> 1 files changed, 90 insertions(+), 89 deletions(-)
>
>
> diff --git a/osaf/services/saf/amf/amfd/sg.cc
> b/osaf/services/saf/amf/amfd/sg.cc
> --- a/osaf/services/saf/amf/amfd/sg.cc
> +++ b/osaf/services/saf/amf/amfd/sg.cc
> @@ -117,7 +117,6 @@
> sg_fsm_state(AVD_SG_FSM_STABLE),
> admin_si(NULL),
> sg_redundancy_model(SA_AMF_NO_REDUNDANCY_MODEL),
> - list_of_su(NULL),
> list_of_si(NULL),
> sg_type(NULL),
> sg_list_app_next(NULL),
> @@ -164,7 +163,7 @@
> void avd_sg_delete(AVD_SG *sg)
> {
> /* by now SU and SI should have been deleted */
> - osafassert(sg->list_of_su == NULL);
> + osafassert(sg->list_of_su.empty() == true);
> osafassert(sg->list_of_si == NULL);
> sg_remove_from_model(sg);
> delete sg;
> @@ -757,7 +756,6 @@
> */
> static void sg_nd_attribute_update(AVD_SG *sg, uint32_t attrib_id)
> {
> - AVD_SU *su = NULL;
> AVD_AVND *su_node_ptr = NULL;
> AVSV_PARAM_INFO param;
> memset(((uint8_t *)¶m), '\0', sizeof(AVSV_PARAM_INFO));
> @@ -809,8 +807,7 @@
> }
>
> /* This value has to be updated on each SU on this SG */
> - su = sg->list_of_su;
> - while (su) {
> + for (const auto& su : sg->list_of_su) {
> su_node_ptr = su->get_node_ptr();
>
> if ((su_node_ptr) && (su_node_ptr->node_state ==
> AVD_AVND_STATE_PRESENT)) {
> @@ -820,7 +817,6 @@
> LOG_ER("%s::failed for %s",__FUNCTION__,
> su_node_ptr->name.value);
> }
> }
> - su = su->sg_list_su_next;
> }
> TRACE_LEAVE();
> }
> @@ -1103,29 +1099,36 @@
>
> /**
> * Terminate SU in reverse order
> - * @param su
> * @return NCSCC_RC_SUCCESS/NCSCC_RC_FAILURE.
> */
> -static uint32_t avd_sg_su_term_in_reverse(AVD_SU *su)
> +uint32_t AVD_SG::term_su_list_in_reverse()
> {
> - uint32_t rc = NCSCC_RC_SUCCESS;
> - TRACE_ENTER2("su:'%s'", su ? su->name.value : NULL);
> - if (su->sg_list_su_next != NULL)
> - rc = avd_sg_su_term_in_reverse(su->sg_list_su_next);
> - if ((su->saAmfSUPreInstantiable == true) &&
> - (su->saAmfSUPresenceState != SA_AMF_PRESENCE_UNINSTANTIATED)
> &&
> - (su->saAmfSUPresenceState !=
> SA_AMF_PRESENCE_INSTANTIATION_FAILED) &&
> - (su->saAmfSUPresenceState !=
> SA_AMF_PRESENCE_TERMINATION_FAILED)) {
> + uint32_t rc = NCSCC_RC_SUCCESS;
> + std::vector<AVD_SU*>::const_reverse_iterator iter;
> + AVD_SU *su;
>
> - if (avd_snd_presence_msg(avd_cb, su, true) == NCSCC_RC_SUCCESS) {
> - su->set_term_state(true);
> - } else {
> - rc = NCSCC_RC_FAILURE;
> - LOG_WA("Failed Termination '%s'", su->name.value);
> - }
> - }
> - TRACE_LEAVE();
> - return rc ;
> + TRACE_ENTER2("sg: %s", this->name.value);
> + for (iter = list_of_su.rbegin(); iter != list_of_su.rend(); ++iter) {
[HansN] Is there a reason not to use, for (auto iter = .. and skip
const_reverse_iter declaration above?
> + su = *iter;
> + TRACE("terminate su:'%s'", su ? su->name.value : NULL);
> +
> + if ((su->saAmfSUPreInstantiable == true) &&
> + (su->saAmfSUPresenceState !=
> SA_AMF_PRESENCE_UNINSTANTIATED) &&
> + (su->saAmfSUPresenceState !=
> SA_AMF_PRESENCE_INSTANTIATION_FAILED) &&
> + (su->saAmfSUPresenceState !=
> SA_AMF_PRESENCE_TERMINATION_FAILED)) {
> +
> + if (avd_snd_presence_msg(avd_cb, su, true) ==
> NCSCC_RC_SUCCESS) {
> + su->set_term_state(true);
> + } else {
> + rc = NCSCC_RC_FAILURE;
> + LOG_WA("Failed Termination '%s'",
> su->name.value);
> + }
> + }
> + }
> +
> + TRACE_LEAVE();
> +
> + return rc ;
> }
> /**
> * perform lock-instantiation on a given SG
> @@ -1139,8 +1142,8 @@
> TRACE_ENTER2("%s", sg->name.value);
>
> /* terminate all the SUs on this Node */
> - if (sg->list_of_su != NULL)
> - rc = avd_sg_su_term_in_reverse(sg->list_of_su);
> + if (sg->list_of_su.empty() == false)
> + rc = sg->term_su_list_in_reverse();
>
> TRACE_LEAVE2("%u", rc);
> return rc;
> @@ -1154,13 +1157,14 @@
> */
> static void sg_app_sg_admin_unlock_inst(AVD_CL_CB *cb, AVD_SG *sg)
> {
> - AVD_SU *su;
> uint32_t su_try_inst;
>
> TRACE_ENTER2("%s", sg->name.value);
>
> /* Instantiate the SUs in this SG */
> - for (su = sg->list_of_su, su_try_inst = 0; su != NULL; su =
> su->sg_list_su_next) {
> + su_try_inst = 0;
> +
> + for (const auto& su : sg->list_of_su) {
> if ((su->saAmfSUAdminState !=
> SA_AMF_ADMIN_LOCKED_INSTANTIATION) &&
> (su->su_on_node->saAmfNodeAdminState !=
> SA_AMF_ADMIN_LOCKED_INSTANTIATION) &&
> (su->saAmfSUOperState ==
> SA_AMF_OPERATIONAL_ENABLED) &&
> @@ -1174,7 +1178,7 @@
>
> __FUNCTION__, su->name.value,
>
> su->su_on_node->node_info.nodeId);
> } else {
> - su_try_inst ++;
> + su_try_inst++;
> }
> }
> }
> @@ -1214,7 +1218,6 @@
> {
> AVD_SG *sg;
> SaAmfAdminStateT adm_state;
> - AVD_SU *su;
> AVD_AVND *node;
>
> TRACE_ENTER2("'%s', %llu", object_name->value, op_id);
> @@ -1233,7 +1236,7 @@
> }
>
> /* Avoid multiple admin operations on other SUs belonging to the same
> SG. */
> - for (su = sg->list_of_su; su != NULL; su = su->sg_list_su_next) {
> + for (const auto& su : sg->list_of_su) {
> node = su->get_node_ptr();
> if (su->pend_cbk.invocation != 0) {
> report_admin_op_error(immOiHandle, invocation,
> SA_AIS_ERR_TRY_AGAIN, NULL,
> @@ -1285,7 +1288,7 @@
> adm_state = sg->saAmfSGAdminState;
> avd_sg_admin_state_set(sg, SA_AMF_ADMIN_UNLOCKED);
> if (avd_cb->init_state == AVD_INIT_DONE) {
> - for (su = sg->list_of_su; su != NULL; su =
> su->sg_list_su_next) {
> + for (const auto& su : sg->list_of_su) {
> if (su->is_in_service() == true) {
>
> su->set_readiness_state(SA_AMF_READINESS_IN_SERVICE);
> }
> @@ -1318,7 +1321,7 @@
> avd_sg_admin_state_set(sg, SA_AMF_ADMIN_LOCKED);
>
> if (avd_cb->init_state == AVD_INIT_DONE) {
> - for (su = sg->list_of_su; su != NULL; su =
> su->sg_list_su_next) {
> + for (const auto& su : sg->list_of_su) {
>
> su->set_readiness_state(SA_AMF_READINESS_OUT_OF_SERVICE);
> }
> break;
> @@ -1398,7 +1401,7 @@
> /* If any su is in terminating state, that means lock-in op
> has not completed. Allow su to move into permanent state
> i.e. either in uninstanted or term failed state. */
> - for (su = sg->list_of_su; su != NULL; su = su->sg_list_su_next)
> {
> + for (const auto& su : sg->list_of_su) {
> if (su->saAmfSUPresenceState ==
> SA_AMF_PRESENCE_TERMINATING) {
> report_admin_op_error(immOiHandle, invocation,
> SA_AIS_ERR_TRY_AGAIN, NULL,
> @@ -1410,7 +1413,7 @@
>
> avd_sg_admin_state_set(sg, SA_AMF_ADMIN_LOCKED);
>
> - if ((sg->list_of_su != NULL) &&
> (sg->list_of_su->saAmfSUPreInstantiable == false)) {
> + if ((sg->list_of_su.empty() == false) &&
> (sg->first_su()->saAmfSUPreInstantiable == false)) {
> avd_saImmOiAdminOperationResult(immOiHandle,
> invocation, SA_AIS_OK);
> goto done;
> }
> @@ -1564,30 +1567,20 @@
>
> void avd_sg_remove_su(AVD_SU* su)
> {
> - AVD_SU *i_su = NULL;
> - AVD_SU *prev_su = NULL;
> AVD_SG *sg = su->sg_of_su;
>
> if (su->sg_of_su != NULL) {
> /* remove SU from SG */
> - i_su = su->sg_of_su->list_of_su;
> -
> - while ((i_su != NULL) && (i_su != su)) {
> - prev_su = i_su;
> - i_su = i_su->sg_list_su_next;
> + auto su_to_delete = std::find(sg->list_of_su.begin(),
> + sg->list_of_su.end(),
> + su);
> +
> + if (su_to_delete != sg->list_of_su.end()) {
> + sg->list_of_su.erase(su_to_delete);
> + } else {
> + LOG_ER("su cannot be found");
> }
>
> - if (i_su != su) {
> - /* Log a fatal error */
> - } else {
> - if (prev_su == NULL) {
> - su->sg_of_su->list_of_su = su->sg_list_su_next;
> - } else {
> - prev_su->sg_list_su_next = su->sg_list_su_next;
> - }
> - }
> -
> - su->sg_list_su_next = NULL;
> su->sg_of_su = NULL;
> } /* if (su->sg_of_su != AVD_SG_NULL) */
>
> @@ -1597,26 +1590,17 @@
>
> void avd_sg_add_su(AVD_SU* su)
> {
> - AVD_SU *i_su = NULL;
> - AVD_SU *prev_su = NULL;
> -
> if ((su == NULL) || (su->sg_of_su == NULL))
> return;
>
> - i_su = su->sg_of_su->list_of_su;
> + su->sg_of_su->list_of_su.push_back(su);
>
> - while ((i_su != NULL) && (i_su->saAmfSURank < su->saAmfSURank)) {
> - prev_su = i_su;
> - i_su = i_su->sg_list_su_next;
> - }
> + // in descending order of SU rank (lowest to
> + // highest integer). Note: the lower the integer
> + // value, the higher the rank.
> + std::sort(su->sg_of_su->list_of_su.begin(),
> su->sg_of_su->list_of_su.end(),
> + [](const AVD_SU *a, const AVD_SU *b) -> bool {return
> a->saAmfSURank < b->saAmfSURank;});
>
> - if (prev_su == NULL) {
> - su->sg_list_su_next = su->sg_of_su->list_of_su;
> - su->sg_of_su->list_of_su = su;
> - } else {
> - prev_su->sg_list_su_next = su;
> - su->sg_list_su_next = i_su;
> - }
> avd_verify_equal_ranked_su(su->sg_of_su);
> }
>
> @@ -1668,6 +1652,8 @@
> }
>
> void AVD_SG::set_fsm_state(AVD_SG_FSM_STATE state) {
> + TRACE_ENTER();
> +
> if (sg_fsm_state != state) {
> TRACE("%s sg_fsm_state %u => %u", name.value, sg_fsm_state,
> state);
> sg_fsm_state = state;
> @@ -1689,6 +1675,8 @@
> }
> }
> }
> +
> + TRACE_LEAVE();
> }
>
> void AVD_SG::set_adjust_state(SaAdjustState state) {
> @@ -1712,8 +1700,9 @@
> }
>
> void AVD_SG::for_all_su_set_readiness_state(SaAmfReadinessStateT state) {
> - for (AVD_SU *su = list_of_su; su != NULL; su = su->sg_list_su_next)
> + for (const auto& su : list_of_su) {
> su->set_readiness_state(state);
> + }
> }
>
> bool AVD_SG::in_su_oper_list(const AVD_SU *i_su) {
> @@ -1753,20 +1742,24 @@
> */
> static void avd_verify_equal_ranked_su(AVD_SG *avd_sg)
> {
> - AVD_SU *pre_temp_su = NULL, *temp_su;
> + uint32_t rank = 0;
> + bool valid_rank = false;
>
> TRACE_ENTER();
> - /* Check ranks are still equal or not. Mark true in the begining*/
> + /* Check ranks are still equal or not. Mark true in the beginning*/
> avd_sg->equal_ranked_su = true;
> - temp_su = avd_sg->list_of_su;
> - while(temp_su) {
> - if (pre_temp_su && temp_su->saAmfSURank !=
> pre_temp_su->saAmfSURank) {
> +
> + for (const auto& su : avd_sg->list_of_su) {
> + if (valid_rank && rank != su->saAmfSURank) {
> avd_sg->equal_ranked_su = false;
> break;
> }
> - pre_temp_su = temp_su;
> - temp_su = temp_su->sg_list_su_next;
> +
> + // store rank for comparison
> + rank = su->saAmfSURank;
> + valid_rank = true;
> }
> +
> TRACE_LEAVE2("avd_sg->equal_ranked_su '%u'", avd_sg->equal_ranked_su);
>
> return;
> @@ -1782,7 +1775,7 @@
> void avd_sg_adjust_config(AVD_SG *sg)
> {
> // SUs in an SG are equal, only need to look at the first one
> - if ((sg->list_of_su != NULL) &&
> !sg->list_of_su->saAmfSUPreInstantiable) {
> + if (sg->list_of_su.empty() == false &&
> !(sg->first_su()->saAmfSUPreInstantiable)) {
> // NPI SUs can only be assigned one SI, see 3.2.1.1
> if ((sg->saAmfSGMaxActiveSIsperSU !=
> static_cast<SaUint32T>(-1)) && (sg->saAmfSGMaxActiveSIsperSU > 1)) {
> LOG_WA("invalid saAmfSGMaxActiveSIsperSU (%u) for NPI
> SU '%s', adjusting...",
> @@ -1796,12 +1789,11 @@
> }
> sg->saAmfSGMaxStandbySIsperSU = 1;
> /* saAmfSUFailover must be true for a NPI SU sec 3.11.1.3.2
> AMF-B.04.01 spec */
> - for (AVD_SU *su = sg->list_of_su; su != NULL; su =
> su->sg_list_su_next) {
> + for (const auto& su : sg->list_of_su) {
> if (!su->saAmfSUFailover) {
> su->set_su_failover(true);
> }
> }
> -
> }
>
> /* adjust saAmfSGNumPrefAssignedSUs if not configured, only applicable
> for
> @@ -1825,11 +1817,10 @@
> */
> uint32_t sg_instantiated_su_count(const AVD_SG *sg)
> {
> - uint32_t inst_su_count;
> - AVD_SU *su;
> + uint32_t inst_su_count = 0;
>
> TRACE_ENTER();
> - for (su = sg->list_of_su, inst_su_count = 0; su != NULL; su =
> su->sg_list_su_next) {
> + for (const auto& su : sg->list_of_su) {
> TRACE_1("su'%s', pres state'%u', in_serv'%u', PrefIn'%u'",
> su->name.value,
> su->saAmfSUPresenceState,
> su->saAmfSuReadinessState, sg->saAmfSGNumPrefInserviceSUs);
> if (((su->saAmfSUPresenceState == SA_AMF_PRESENCE_INSTANTIATED)
> ||
> @@ -1838,6 +1829,7 @@
> inst_su_count ++;
> }
> }
> +
> TRACE_LEAVE2("%u", inst_su_count);
> return inst_su_count;
> }
> @@ -1868,7 +1860,7 @@
>
> switch (sg->adminOp) {
> case SA_AMF_ADMIN_LOCK_INSTANTIATION :
> - for (AVD_SU *su = sg->list_of_su; su; su = su->sg_list_su_next)
> {
> + for (const auto& su : sg->list_of_su) {
> if ((su->saAmfSUPresenceState !=
> SA_AMF_PRESENCE_UNINSTANTIATED) &&
> (su->saAmfSUPresenceState !=
> SA_AMF_PRESENCE_INSTANTIATION_FAILED) &&
> (su->saAmfSUPresenceState !=
> SA_AMF_PRESENCE_TERMINATION_FAILED))
> @@ -1877,10 +1869,10 @@
> break;
> case SA_AMF_ADMIN_UNLOCK_INSTANTIATION :
> /* Unlock-in of SG will not instantiate any component in NPI
> SU.*/
> - if ((sg->list_of_su != NULL) &&
> (sg->list_of_su->saAmfSUPreInstantiable == false))
> + if ((sg->list_of_su.empty() == false) &&
> (sg->first_su()->saAmfSUPreInstantiable == false))
> return true;
> -
> - for (AVD_SU *su = sg->list_of_su; su; su = su->sg_list_su_next)
> {
> +
> + for (const auto& su : sg->list_of_su) {
> node_admin_state = su->su_on_node->saAmfNodeAdminState;
>
> if ((su->saAmfSUPresenceState ==
> SA_AMF_PRESENCE_INSTANTIATION_FAILED) ||
> @@ -1935,7 +1927,7 @@
> */
> bool AVD_SG::is_sg_assigned_only_in_ng(const AVD_AMF_NG *ng)
> {
> - for (AVD_SU *su = list_of_su; su; su = su->sg_list_su_next) {
> + for (const auto& su : list_of_su) {
> if (su->list_of_susi == NULL)
> continue;
> //Return if this assigned su is not in this ng.
> @@ -1952,7 +1944,7 @@
> */
> bool AVD_SG::is_sg_serviceable_outside_ng(const AVD_AMF_NG *ng)
> {
> - for (AVD_SU *su = list_of_su; su; su = su->sg_list_su_next) {
> + for (const auto& su : list_of_su) {
> //Check if any su exists outside nodegroup for this sg.
> if (node_in_nodegroup(Amf::to_string(&su->su_on_node->name),
> ng) == false) {
> /*
> @@ -2005,3 +1997,12 @@
> return rc;
>
> }
> +
> +AVD_SU* AVD_SG::first_su()
> +{
> + if (!list_of_su.empty()) {
> + return *list_of_su.begin();
> + } else {
> + return NULL;
> + }
> +}
------------------------------------------------------------------------------
_______________________________________________
Opensaf-devel mailing list
Opensaf-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/opensaf-devel
