good, I'll update the patch and then I'll  push it/Thanks HansN

On 09/15/2015 01:22 PM, praveen malviya wrote:
> Ack with one minor comment, code review only.
>
> Thanks,
> Praveen
>
> On 14-Sep-15 5:53 PM, Hans Nordeback wrote:
>>   osaf/services/saf/amf/amfd/ckpt_dec.cc |   4 +++-
>>   osaf/services/saf/amf/amfd/node.cc     |   2 +-
>>   osaf/services/saf/amf/amfd/sgproc.cc   |   4 +++-
>>   osaf/services/saf/amf/amfd/su.cc       |  20 ++++++++++----------
>>   4 files changed, 17 insertions(+), 13 deletions(-)
>>
>>
>> diff --git a/osaf/services/saf/amf/amfd/ckpt_dec.cc 
>> b/osaf/services/saf/amf/amfd/ckpt_dec.cc
>> --- a/osaf/services/saf/amf/amfd/ckpt_dec.cc
>> +++ b/osaf/services/saf/amf/amfd/ckpt_dec.cc
>> @@ -3043,9 +3043,11 @@ static uint32_t dec_ng_admin_state(AVD_C
>>           AVD_AVND *node = avd_node_get(*iter);
>>           AVD_SU *su = NULL;
>>           //If this node has any susi on it.
>> -        for (const auto& su : node->list_of_su)
>> +        for (const auto& tmp : node->list_of_su) {
>> +            su = tmp;
>>               if (su->list_of_susi != NULL)
>>                   break;
>> +        }
>>           if ((ng->saAmfNGAdminState == SA_AMF_ADMIN_SHUTTING_DOWN) 
>> && (su != NULL))
>>               /* Still some assignments are not removed on the node.
>>                  if ng in SHUTTING_DOWN state and contoller role 
>> changes then
>> diff --git a/osaf/services/saf/amf/amfd/node.cc 
>> b/osaf/services/saf/amf/amfd/node.cc
>> --- a/osaf/services/saf/amf/amfd/node.cc
>> +++ b/osaf/services/saf/amf/amfd/node.cc
>> @@ -88,7 +88,7 @@ bool AVD_AVND::is_node_lock() {
>>         for (curr_susi = su->list_of_susi;
>>              (curr_susi) && ((SA_AMF_HA_QUIESCING != 
>> curr_susi->state) ||
>>                              ((AVD_SU_SI_STATE_UNASGN == 
>> curr_susi->fsm)));
>> -           curr_susi = curr_susi->su_next); \
>> +           curr_susi = curr_susi->su_next);
>>         if (curr_susi)
>>           return false;
>>       }
>> 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
>> @@ -712,7 +712,9 @@ void avd_su_oper_state_evh(AVD_CL_CB *cb
>>                    */
>>                   avd_node_oper_state_set(node, 
>> SA_AMF_OPERATIONAL_DISABLED);
>>                   node->recvr_fail_sw = true;
>> -                for (const auto& su : node->list_of_su) {
>> +                // TODO (uabhano) is the below code correct?
>> +                for (const auto& i_su : node->list_of_su) {
>> +                    TRACE("Is this for loop correct with regard to 
>> i_su and su below? %s", i_su->name.value);
>> su->set_readiness_state(SA_AMF_READINESS_OUT_OF_SERVICE);\
> As node operational state is disabled, readiness state of all the 
> application SUs on that node will be OOS. So mark all the SUs OOS.
>
> Thanks
> Praveen
>>                   }
>>               }    /* if 
>> (n2d_msg->msg_info.n2d_opr_state.node_oper_state == 
>> SA_AMF_OPERATIONAL_DISABLED) */
>> 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
>> @@ -489,6 +489,8 @@ static AVD_AVND *map_su_to_node(AVD_SU *
>>       AVD_SU *su_temp = NULL;
>>       AVD_AVND *node = NULL;
>>       std::set<std::string>::const_iterator node_iter;
>> +    std::vector<AVD_SU*>::const_iterator su_iter;
>> +    std::vector<AVD_SU*> *su_list = nullptr;
>>
>>       TRACE_ENTER2("'%s'", su->name.value);
>>
>> @@ -517,20 +519,18 @@ static AVD_AVND *map_su_to_node(AVD_SU *
>>           osafassert(node);
>>
>>           if (su->sg_of_su->sg_ncs_spec == true) {
>> -            for (const auto& su_temp : node->list_of_ncs_su) {
>> -                if (su_temp->sg_of_su == su->sg_of_su)
>> -                    break;
>> -            }
>> +            su_list = &node->list_of_ncs_su;
>> +        } else {
>> +            su_list = &node->list_of_su;
>>           }
>>
>> -        if (su->sg_of_su->sg_ncs_spec == false) {
>> -            for (const auto& su_temp : node->list_of_su) {
>> -                if (su_temp->sg_of_su == su->sg_of_su)
>> -                    break;
>> -            }
>> +        for (su_iter = su_list->begin(); su_iter != su_list->end(); 
>> ++su_iter) {
>> +            su_temp = *su_iter;
>> +            if (su_temp->sg_of_su == su->sg_of_su)
>> +                break;
>>           }
>>
>> -        if (su_temp == NULL)
>> +        if (su_iter == su_list->end())
>>               goto done;
>>       }
>>
>>


------------------------------------------------------------------------------
_______________________________________________
Opensaf-devel mailing list
Opensaf-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/opensaf-devel

Reply via email to