Some comments inlined with Nagu ....
Thanks
-Nagu
> -----Original Message-----
> From: Gary Lee [mailto:gary....@dektech.com.au]
> Sent: 08 May 2014 12:14
> To: hans.fe...@ericsson.com; hans.nordeb...@ericsson.com; Nagendra
> Kumar; Praveen Malviya
> Cc: opensaf-devel@lists.sourceforge.net
> Subject: [PATCH 1 of 1] amfd: Remove asserts from validation routines [#849]
>
> osaf/services/saf/amf/amfd/app.cc | 18 +++++++++++-------
> osaf/services/saf/amf/amfd/comp.cc | 5 +++--
> osaf/services/saf/amf/amfd/compcstype.cc | 5 +++--
> osaf/services/saf/amf/amfd/hlt.cc | 10 ++++++----
> osaf/services/saf/amf/amfd/sg.cc | 9 +++++----
> osaf/services/saf/amf/amfd/si.cc | 5 +++--
> osaf/services/saf/amf/amfd/su.cc | 5 +++--
> 7 files changed, 34 insertions(+), 23 deletions(-)
>
>
> When an unknown attribute in encountered in various callbacks, sometimes an
> assert is called.
> In other cases, the operation is rejected.
>
> This may create forward compatibility issues if new attributes are added in
> the
> future.
>
> In this patch, the asserts are replaced with a warning and the corresponding
> attribute change
> will be permitted but ignored.
>
> diff --git a/osaf/services/saf/amf/amfd/app.cc
> b/osaf/services/saf/amf/amfd/app.cc
> --- a/osaf/services/saf/amf/amfd/app.cc
> +++ b/osaf/services/saf/amf/amfd/app.cc
> @@ -243,12 +243,15 @@ static SaAisErrorT app_ccb_completed_cb(
> SaNameT dn = *((SaNameT*)attribute-
> >attrValues[0]);
> if (NULL == avd_apptype_get(&dn)) {
> report_ccb_validation_error(opdata,
> "saAmfAppType '%s' not found", dn.value);
> + rc = SA_AIS_ERR_BAD_OPERATION;
> goto done;
> }
> rc = SA_AIS_OK;
> break;
> - } else
> - osafassert(0);
> + } else {
> + LOG_WA("Ignoring unknown attribute '%s'",
> attribute->attrName);
> + rc = SA_AIS_OK;
> + }
[Nagu]:: Returning ok for bad attribute may not be appropriate.BAD_OP can be
returned.
> }
> break;
> case CCBUTIL_DELETE:
> @@ -294,10 +297,10 @@ static void app_ccb_apply_cb(CcbUtilOper
> app->saAmfAppType =
> *((SaNameT*)attribute->attrValues[0]);
> app->app_type = avd_apptype_get(&app-
> >saAmfAppType);
> avd_apptype_add_app(app);
> - break;
> }
> - else
> - osafassert(0);
> + else {
> + TRACE("Ignoring unknown attribute '%s'",
> attribute->attrName);
> + }
[Nagu]: If above comments are accepted, then this check is not required.
> }
> break;
> }
> @@ -396,8 +399,9 @@ static SaAisErrorT app_rt_attr_cb(SaImmO
> if (!strcmp(attributeName, "saAmfApplicationCurrNumSGs")) {
> avd_saImmOiRtObjectUpdate_sync(objectName,
> attributeName,
> SA_IMM_ATTR_SAUINT32T, &app-
> >saAmfApplicationCurrNumSGs);
> - } else
> - osafassert(0);
> + } else {
> + LOG_WA("Ignoring unknown attribute '%s'",
> attributeName);
> + }
> }
>
> return SA_AIS_OK;
> diff --git a/osaf/services/saf/amf/amfd/comp.cc
> b/osaf/services/saf/amf/amfd/comp.cc
> --- a/osaf/services/saf/amf/amfd/comp.cc
> +++ b/osaf/services/saf/amf/amfd/comp.cc
> @@ -830,8 +830,9 @@ static SaAisErrorT comp_rt_attr_cb(SaImm
> /* TODO */
> } else if (!strcmp("saAmfCompCurrProxiedNames",
> attributeName)) {
> /* TODO */
> - } else
> - osafassert(0);
> + } else {
> + LOG_WA("Ignoring unknown attribute '%s'",
> attributeName);
> + }
> }
>
> return SA_AIS_OK;
> diff --git a/osaf/services/saf/amf/amfd/compcstype.cc
> b/osaf/services/saf/amf/amfd/compcstype.cc
> --- a/osaf/services/saf/amf/amfd/compcstype.cc
> +++ b/osaf/services/saf/amf/amfd/compcstype.cc
> @@ -435,8 +435,9 @@ static SaAisErrorT compcstype_rt_attr_ca
> SA_IMM_ATTR_SAUINT32T, &cst-
> >saAmfCompNumCurrStandbyCSIs);
> } else if (!strcmp("saAmfCompAssignedCsi", attributeName)) {
> /* TODO */
> - } else
> - osafassert(0);
> + } else {
> + LOG_WA("Ignoring unknown attribute '%s'",
> attributeName);
> + }
[Nagu]: Same comment here.
> }
>
> return SA_AIS_OK;
> diff --git a/osaf/services/saf/amf/amfd/hlt.cc
> b/osaf/services/saf/amf/amfd/hlt.cc
> --- a/osaf/services/saf/amf/amfd/hlt.cc
> +++ b/osaf/services/saf/amf/amfd/hlt.cc
> @@ -48,8 +48,9 @@ static SaAisErrorT ccb_completed_modify_
> *value, opdata-
> >objectName.value);
> return SA_AIS_ERR_BAD_OPERATION;
> }
> - } else
> - osafassert(0);
> + } else {
> + LOG_WA("Ignoring unknown attribute '%s'", attribute-
> >attrName);
> + }
[Nagu]: Same comment here.
> }
>
> return SA_AIS_OK;
> @@ -144,8 +145,9 @@ static void ccb_apply_modify_hdlr(CcbUti
> TRACE("saAmfHealthcheckMaxDuration modified to
> '%llu' for Comp'%s'", *param_val,
> comp->comp_info.name.value);
> param.attr_id = saAmfHealthcheckMaxDuration_ID;
> - } else
> - osafassert(0);
> + } else {
> + TRACE("Ignoring unknown attribute '%s'", attribute-
> >attrName);
> + }
[Nagu]: Same comment here.
>
> avd_snd_op_req_msg(avd_cb, comp->su->su_on_node,
> ¶m);
> }
> 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
> @@ -627,7 +627,7 @@ static SaAisErrorT ccb_completed_modify_
> goto done;
> }
> } else {
> - osafassert(0);
> + LOG_WA("Ignoring unknown attribute '%s'",
> attribute->attrName);
[Nagu]: Same comment here.
> }
> } /* while (attr_mod != NULL) */
>
> @@ -933,7 +933,7 @@ static void ccb_apply_modify_hdlr(CcbUti
> } else if (!strcmp(attribute->attrName,
> "saAmfSGSuHostNodeGroup")) {
> sg->saAmfSGSuHostNodeGroup = *((SaNameT
> *)value);
> } else {
> - osafassert(0);
> + TRACE("Ignoring unknown attribute '%s'",
> attribute->attrName);
> }
>
> attr_mod = opdata->param.modify.attrMods[i++];
> @@ -1322,8 +1322,9 @@ static SaAisErrorT sg_rt_attr_cb(SaImmOi
> } else if (!strcmp("saAmfSGNumCurrInstantiatedSpareSUs",
> attributeName)) {
> avd_saImmOiRtObjectUpdate_sync(objectName,
> attributeName,
> SA_IMM_ATTR_SAUINT32T, &sg-
> >saAmfSGNumCurrInstantiatedSpareSUs);
> - } else
> - osafassert(0);
> + } else {
> + LOG_WA("Ignoring unknown attribute '%s'",
> attributeName);
> + }
> }
>
> return SA_AIS_OK;
> diff --git a/osaf/services/saf/amf/amfd/si.cc
> b/osaf/services/saf/amf/amfd/si.cc
> --- a/osaf/services/saf/amf/amfd/si.cc
> +++ b/osaf/services/saf/amf/amfd/si.cc
> @@ -973,8 +973,9 @@ static SaAisErrorT si_rt_attr_cb(SaImmOi
> } else if (!strcmp("saAmfSINumCurrStandbyAssignments",
> attributeName)) {
> avd_saImmOiRtObjectUpdate_sync(objectName,
> attributeName,
> SA_IMM_ATTR_SAUINT32T, &si-
> >saAmfSINumCurrStandbyAssignments);
> - } else
> - osafassert(0);
> + } else {
> + LOG_WA("Ignoring unknown attribute '%s'",
> attributeName);
> + }
> }
>
> return SA_AIS_OK;
> 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
> @@ -1197,8 +1197,9 @@ static SaAisErrorT su_rt_attr_cb(SaImmOi
> } else if (!strcmp("saAmfSURestartCount", attributeName)) {
> avd_saImmOiRtObjectUpdate_sync(objectName,
> attributeName,
> SA_IMM_ATTR_SAUINT32T, &su-
> >saAmfSURestartCount);
> - } else
> - osafassert(0);
> + } else {
> + LOG_WA("Ignoring unknown attribute '%s'",
> attributeName);
> + }
> }
>
> return SA_AIS_OK;
------------------------------------------------------------------------------
"Accelerate Dev Cycles with Automated Cross-Browser Testing - For FREE
Instantly run your Selenium tests across 300+ browser/OS combos.
Get unparalleled scalability from the best Selenium testing platform available
Simple to use. Nothing to install. Get started now for free."
http://p.sf.net/sfu/SauceLabs
_______________________________________________
Opensaf-devel mailing list
Opensaf-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/opensaf-devel
