ack, a few comments:

ndproc.cc:730
su->oper_state_set(static_cast<SaAmfOperationalStateT>(l_val)); can be 
changed to su->oper_state_set(l_val);

and overload oper_state_set in su.hh:

void oper_state_set(uint32_t state);

su.cc
void AVD_SU::oper_state_set(uint32_t oper_state) {
oper_state_set(static_cast<SaAmfOperationalStateT>(oper_state));
}

If oper_state_set is called from many places then there is only one cast 
needed, and the cast is placed where it belongs.

In method oper_state_set the use of this-> in several places is not 
needed and could be removed.

/BR Hans


On 04/10/14 14:53, Hans Feldt wrote:
>   osaf/services/saf/amf/amfd/include/su.h |   3 +-
>   osaf/services/saf/amf/amfd/ndfsm.cc     |   2 +-
>   osaf/services/saf/amf/amfd/ndproc.cc    |   2 +-
>   osaf/services/saf/amf/amfd/sgproc.cc    |  19 ++++++++---------
>   osaf/services/saf/amf/amfd/su.cc        |  36 
> ++++++++++++++++++--------------
>   5 files changed, 33 insertions(+), 29 deletions(-)
>
>
> Change function avd_su_oper_state_set into a method, use it and remove the old
> function.
>
> diff --git a/osaf/services/saf/amf/amfd/include/su.h 
> b/osaf/services/saf/amf/amfd/include/su.h
> --- a/osaf/services/saf/amf/amfd/include/su.h
> +++ b/osaf/services/saf/amf/amfd/include/su.h
> @@ -109,6 +109,8 @@ class AVD_SU {
>       AVD_SU *avnd_list_su_next;      /* the next SU in the AvND */
>       struct avd_sutype *su_type;
>       AVD_SU *su_list_su_type_next;
> +
> +     void oper_state_set(SaAmfOperationalStateT state);
>   };
>   
>   typedef struct {
> @@ -202,7 +204,6 @@ void avd_su_del_avnd_list(AVD_CL_CB *cb,
>   extern SaAisErrorT avd_su_config_get(const SaNameT *sg_name, struct 
> avd_sg_tag *sg);
>   
>   extern void avd_su_pres_state_set(AVD_SU *su, SaAmfPresenceStateT 
> pres_state);
> -extern void avd_su_oper_state_set(AVD_SU *su, SaAmfOperationalStateT 
> oper_state);
>   extern void avd_su_readiness_state_set(AVD_SU *su, SaAmfReadinessStateT 
> readiness_state);
>   extern void avd_su_admin_state_set(AVD_SU *su, SaAmfAdminStateT 
> admin_state);
>   
> diff --git a/osaf/services/saf/amf/amfd/ndfsm.cc 
> b/osaf/services/saf/amf/amfd/ndfsm.cc
> --- a/osaf/services/saf/amf/amfd/ndfsm.cc
> +++ b/osaf/services/saf/amf/amfd/ndfsm.cc
> @@ -207,7 +207,7 @@ void avd_nd_ncs_su_assigned(AVD_CL_CB *c
>   
>               /* Make application SUs operational state ENABLED */
>               for (su = avnd->list_of_su; su != NULL; su = 
> su->avnd_list_su_next) {
> -                     avd_su_oper_state_set(su, SA_AMF_OPERATIONAL_ENABLED);
> +                     su->oper_state_set(SA_AMF_OPERATIONAL_ENABLED);
>                       AVD_COMP *comp;
>                       for (comp = su->list_of_comp; comp; comp = 
> comp->su_comp_next)
>                               avd_comp_oper_state_set(comp, 
> SA_AMF_OPERATIONAL_ENABLED);
> diff --git a/osaf/services/saf/amf/amfd/ndproc.cc 
> b/osaf/services/saf/amf/amfd/ndproc.cc
> --- a/osaf/services/saf/amf/amfd/ndproc.cc
> +++ b/osaf/services/saf/amf/amfd/ndproc.cc
> @@ -727,7 +727,7 @@ void avd_data_update_req_evh(AVD_CL_CB *
>                               TRACE("oper pres state");
>                               if 
> (n2d_msg->msg_info.n2d_data_req.param_info.value_len == sizeof(uint32_t)) {
>                                       l_val = ntohl(*((uint32_t 
> *)&n2d_msg->msg_info.n2d_data_req.param_info.value[0]));
> -                                     avd_su_oper_state_set(su, 
> static_cast<SaAmfOperationalStateT>(l_val));
> +                                     
> su->oper_state_set(static_cast<SaAmfOperationalStateT>(l_val));
>                               }
>                               break;
>                       case saAmfSUPresenceState_ID:
> 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
> @@ -290,7 +290,7 @@ static uint32_t sg_su_failover_func(AVD_
>               goto done;
>       }
>   
> -     avd_su_oper_state_set(su, SA_AMF_OPERATIONAL_DISABLED);
> +     su->oper_state_set(SA_AMF_OPERATIONAL_DISABLED);
>       avd_su_readiness_state_set(su, SA_AMF_READINESS_OUT_OF_SERVICE);
>       if (su->saAmfSUAdminState == SA_AMF_ADMIN_LOCKED)
>               su_complete_admin_op(su, SA_AIS_OK);
> @@ -449,7 +449,7 @@ void avd_su_oper_state_evh(AVD_CL_CB *cb
>           (n2d_msg->msg_info.n2d_opr_state.node_oper_state == 
> SA_AMF_OPERATIONAL_DISABLED) &&
>           (n2d_msg->msg_info.n2d_opr_state.rec_rcvr.saf_amf == 
> SA_AMF_NODE_FAILFAST)) {
>               /* as of now do the same opearation as ncs su failure */
> -             avd_su_oper_state_set(su, SA_AMF_OPERATIONAL_DISABLED);
> +             su->oper_state_set(SA_AMF_OPERATIONAL_DISABLED);
>               if ((node->type == AVSV_AVND_CARD_SYS_CON) && 
> (node->node_info.nodeId == cb->node_id_avd)) {
>                       TRACE("Component in %s requested FAILFAST", 
> su->name.value);
>               }
> @@ -470,7 +470,7 @@ void avd_su_oper_state_evh(AVD_CL_CB *cb
>               /* if the SU is NCS SU, call the node FSM routine to handle the 
> failure.
>                */
>               if (su->sg_of_su->sg_ncs_spec == true) {
> -                     avd_su_oper_state_set(su, SA_AMF_OPERATIONAL_DISABLED);
> +                     su->oper_state_set(SA_AMF_OPERATIONAL_DISABLED);
>                       avd_nd_ncs_su_failed(cb, node);
>                       goto done;
>               }
> @@ -480,7 +480,7 @@ void avd_su_oper_state_evh(AVD_CL_CB *cb
>                */
>   
>               if (cb->init_state == AVD_INIT_DONE) {
> -                     avd_su_oper_state_set(su, SA_AMF_OPERATIONAL_DISABLED);
> +                     su->oper_state_set(SA_AMF_OPERATIONAL_DISABLED);
>                       avd_su_readiness_state_set(su, 
> SA_AMF_READINESS_OUT_OF_SERVICE);
>                       if (n2d_msg->msg_info.n2d_opr_state.node_oper_state == 
> SA_AMF_OPERATIONAL_DISABLED) {
>                               /* Mark the node operational state as disable 
> and make all the
> @@ -496,7 +496,7 @@ void avd_su_oper_state_evh(AVD_CL_CB *cb
>                       }       /* if 
> (n2d_msg->msg_info.n2d_opr_state.node_oper_state == 
> SA_AMF_OPERATIONAL_DISABLED) */
>               } /* if(cb->init_state == AVD_INIT_DONE) */
>               else if (cb->init_state == AVD_APP_STATE) {
> -                     avd_su_oper_state_set(su, SA_AMF_OPERATIONAL_DISABLED);
> +                     su->oper_state_set(SA_AMF_OPERATIONAL_DISABLED);
>                       avd_su_readiness_state_set(su, 
> SA_AMF_READINESS_OUT_OF_SERVICE);
>                       if (n2d_msg->msg_info.n2d_opr_state.node_oper_state == 
> SA_AMF_OPERATIONAL_DISABLED) {
>                               /* Mark the node operational state as disable 
> and make all the
> @@ -646,7 +646,7 @@ void avd_su_oper_state_evh(AVD_CL_CB *cb
>               /* else if(cb->init_state == AVD_APP_STATE) */
>       } /* if (n2d_msg->msg_info.n2d_opr_state.su_oper_state == 
> SA_AMF_OPERATIONAL_DISABLED) */
>       else if (n2d_msg->msg_info.n2d_opr_state.su_oper_state == 
> SA_AMF_OPERATIONAL_ENABLED) {
> -             avd_su_oper_state_set(su, SA_AMF_OPERATIONAL_ENABLED);
> +             su->oper_state_set(SA_AMF_OPERATIONAL_ENABLED);
>               /* if the SU is NCS SU, mark the SU readiness state as in 
> service and call
>                * the SG FSM.
>                */
> @@ -1289,8 +1289,7 @@ void avd_sg_app_node_su_inst_func(AVD_CL
>                                       }
>   
>                               } else {
> -                                     /* mark the non preinstatiable as 
> enable. */
> -                                     avd_su_oper_state_set(i_su, 
> SA_AMF_OPERATIONAL_ENABLED);
> +                                     
> i_su->oper_state_set(SA_AMF_OPERATIONAL_ENABLED);
>   
>                                       m_AVD_GET_SU_NODE_PTR(cb, i_su, 
> su_node_ptr);
>   
> @@ -1550,7 +1549,7 @@ void avd_node_down_mw_susi_failover(AVD_
>       i_su = avnd->list_of_ncs_su;
>       osafassert(i_su != 0);
>       while (i_su != NULL) {
> -             avd_su_oper_state_set(i_su, SA_AMF_OPERATIONAL_DISABLED);
> +             i_su->oper_state_set(SA_AMF_OPERATIONAL_DISABLED);
>               avd_su_pres_state_set(i_su, SA_AMF_PRESENCE_UNINSTANTIATED);
>               avd_su_readiness_state_set(i_su, 
> SA_AMF_READINESS_OUT_OF_SERVICE);
>               su_complete_admin_op(i_su, SA_AIS_ERR_TIMEOUT);
> @@ -1601,7 +1600,7 @@ void avd_node_down_appl_susi_failover(AV
>        */
>       i_su = avnd->list_of_su;
>       while (i_su != NULL) {
> -             avd_su_oper_state_set(i_su, SA_AMF_OPERATIONAL_DISABLED);
> +             i_su->oper_state_set(SA_AMF_OPERATIONAL_DISABLED);
>               avd_su_pres_state_set(i_su, SA_AMF_PRESENCE_UNINSTANTIATED);
>               avd_su_readiness_state_set(i_su, 
> SA_AMF_READINESS_OUT_OF_SERVICE);
>   
> 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
> @@ -660,9 +660,9 @@ static void su_add_to_model(AVD_SU *su)
>                               goto done;
>                       }
>   
> -                     avd_su_oper_state_set(su, SA_AMF_OPERATIONAL_ENABLED);
> +                     su->oper_state_set(SA_AMF_OPERATIONAL_ENABLED);
>               } else
> -                     avd_su_oper_state_set(su, SA_AMF_OPERATIONAL_DISABLED);
> +                     su->oper_state_set(SA_AMF_OPERATIONAL_DISABLED);
>       }
>   
>   done:
> @@ -776,27 +776,31 @@ void avd_su_pres_state_set(AVD_SU *su, S
>       m_AVSV_SEND_CKPT_UPDT_ASYNC_UPDT(avd_cb, su, AVSV_CKPT_SU_PRES_STATE);
>   }
>   
> -void avd_su_oper_state_set(AVD_SU *su, SaAmfOperationalStateT oper_state)
> -{
> -     SaAmfOperationalStateT old_state = su->saAmfSUOperState;
> +void AVD_SU::oper_state_set(SaAmfOperationalStateT oper_state) {
> +     SaAmfOperationalStateT old_state = this->saAmfSUOperState;
>       
> -     if (su->saAmfSUOperState == oper_state)
> +     if (this->saAmfSUOperState == oper_state)
>               return;
> +
>       osafassert(oper_state <= SA_AMF_OPERATIONAL_DISABLED);
> -     TRACE_ENTER2("'%s' %s => %s", su->name.value, 
> avd_oper_state_name[su->saAmfSUOperState],
> -                     avd_oper_state_name[oper_state]);
> +     TRACE_ENTER2("'%s' %s => %s", this->name.value,
> +             avd_oper_state_name[this->saAmfSUOperState],
> +             avd_oper_state_name[oper_state]);
>   
> -     saflog(LOG_NOTICE, amfSvcUsrName, "%s OperState %s => %s", 
> su->name.value,
> -                avd_oper_state_name[su->saAmfSUOperState], 
> avd_oper_state_name[oper_state]);
> +     saflog(LOG_NOTICE, amfSvcUsrName, "%s OperState %s => %s", 
> this->name.value,
> +             avd_oper_state_name[this->saAmfSUOperState],
> +             avd_oper_state_name[oper_state]);
>   
> -     su->saAmfSUOperState = oper_state;
> +     this->saAmfSUOperState = oper_state;
>   
> -     /* alarm & notifications */
> -     avd_send_oper_chg_ntf(&su->name, SA_AMF_NTFID_SU_OP_STATE, old_state, 
> su->saAmfSUOperState);
> +     avd_send_oper_chg_ntf(&this->name, SA_AMF_NTFID_SU_OP_STATE, old_state,
> +             this->saAmfSUOperState);
>   
> -     avd_saImmOiRtObjectUpdate(&su->name, "saAmfSUOperState",
> -             SA_IMM_ATTR_SAUINT32T, &su->saAmfSUOperState);
> -     m_AVSV_SEND_CKPT_UPDT_ASYNC_UPDT(avd_cb, su, AVSV_CKPT_SU_OPER_STATE);
> +     avd_saImmOiRtObjectUpdate(&this->name, "saAmfSUOperState",
> +             SA_IMM_ATTR_SAUINT32T, &this->saAmfSUOperState);
> +     m_AVSV_SEND_CKPT_UPDT_ASYNC_UPDT(avd_cb, this, AVSV_CKPT_SU_OPER_STATE);
> +
> +     TRACE_LEAVE();
>   }
>   
>   /**


------------------------------------------------------------------------------
Put Bad Developers to Shame
Dominate Development with Jenkins Continuous Integration
Continuously Automate Build, Test & Deployment 
Start a new project now. Try Jenkins in the cloud.
http://p.sf.net/sfu/13600_Cloudbees
_______________________________________________
Opensaf-devel mailing list
Opensaf-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/opensaf-devel

Reply via email to