Re: [devel] [PATCH 1 of 1] amfd: validate NG admin state at the time of creation [#1560]

[Hans N. <han...@er...>]

ack, code review only/Thanks Hans

On 10/29/2015 06:24 AM, pra...@or... wrote:
>   osaf/services/saf/amf/amfd/nodegroup.cc |  10 ++++++++--
>   osaf/services/saf/amf/amfd/util.cc      |   2 +-
>   2 files changed, 9 insertions(+), 3 deletions(-)
>
>
> AMF allows creation of NG with invalid admin state.
>
> Check is missing in CCB completed callback.
>
> Patch adds the required check.
>
> diff --git a/osaf/services/saf/amf/amfd/nodegroup.cc b/osaf/services/saf/amf/amfd/nodegroup.cc
> --- a/osaf/services/saf/amf/amfd/nodegroup.cc
> +++ b/osaf/services/saf/amf/amfd/nodegroup.cc
> @@ -105,6 +105,13 @@ static int is_config_valid(const SaNameT
>   		delete tmp_ng;
>   		return 0;
>   	}
> +	//Check if admin state is valid or not.
> +	if (!avd_admin_state_is_valid(tmp_ng->saAmfNGAdminState)) {
> +		LOG_ER("Incorrect saAmfNGAdminState:'%u' for '%s'",tmp_ng->saAmfNGAdminState,
> +				tmp_ng->name.value);
> +		delete tmp_ng;
> +		return 0;
> +	}
>   	delete tmp_ng;
>   	return 1;
>   }
> @@ -159,9 +166,8 @@ static AVD_AMF_NG *ng_create(SaNameT *dn
>   	if (immutil_getAttr(const_cast<SaImmAttrNameT>("saAmfNGAdminState"),
>   				attributes, 0, &ng->saAmfNGAdminState) != SA_AIS_OK) {
>                   ng->saAmfNGAdminState = SA_AMF_ADMIN_UNLOCKED;
> -		LOG_NO("Setting saAmfNGAdminState to :'%u'",ng->saAmfNGAdminState);
> +		TRACE("Setting saAmfNGAdminState to :'%u'",ng->saAmfNGAdminState);
>   	}
> -	//TODO_NG: Add protection against shutting down state and lock-in state.
>   	rc = 0;
>   done:
>   	if (rc != 0) {
> diff --git a/osaf/services/saf/amf/amfd/util.cc b/osaf/services/saf/amf/amfd/util.cc
> --- a/osaf/services/saf/amf/amfd/util.cc
> +++ b/osaf/services/saf/amf/amfd/util.cc
> @@ -1285,7 +1285,7 @@ uint32_t avd_snd_comp_validation_resp(AV
>   
>   int avd_admin_state_is_valid(SaAmfAdminStateT state)
>   {
> -	return ((state >= SA_AMF_ADMIN_UNLOCKED) && (state <= SA_AMF_ADMIN_SHUTTING_DOWN));
> +	return ((state >= SA_AMF_ADMIN_UNLOCKED) && (state < SA_AMF_ADMIN_SHUTTING_DOWN));
>   }
>   
>   /*****************************************************************************