Hi Gary,
I think this should work:
for (auto iter = list_of_su.rbegin(); iter != list_of_su.rend(); ++iter) {
but the original code is fine, without the auto declaration.
/Thanks HansN
On 09/18/2015 04:34 PM, Gary Lee wrote:
> Hi Hans
>
> Please see [GL].
>
> Quoting Hans Nordebäck <[email protected]>:
>
>> 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?
>>
>
> [GL] I think I would need to write a reverse adaptor like boost has,
> in order to use a range-based for loop.
> So I just did it quickly with a reverse iterator.
>
>>> + 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
[email protected]
https://lists.sourceforge.net/lists/listinfo/opensaf-devel
