Hi, Ack with a minor comment
Some of the added lines are too long Thanks Lennart > -----Original Message----- > From: Danh Vo [mailto:[email protected]] > Sent: den 15 mars 2018 11:54 > To: [email protected]; Hans Nordebäck > <[email protected]>; Zoran Milinkovic > <[email protected]>; Anders Widell > <[email protected]>; Lennart Lund <[email protected]>; > Vu Minh Nguyen <[email protected]> > Cc: [email protected]; Danh Cong Vo > <[email protected]> > Subject: [PATCH 1/1] imm: sPbeRtMutations is updated even when > validation for duplicate values fails [#2422] > > When immnd receives event to update persistent runtime attribute > from immd, there is 2 loops: the first loop validates the change, > the second one updates the afim object. The root cause comes from > the check (attr->mFlags & SA_IMM_ATTR_NO_DUPLICATES) which is > inside the second loop instead of the first loop. So the > validation is still passed even when the attribute does not allow > duplicate value. As a result, sPbeRtMutations is updated in the > second loop. > --- > src/imm/immnd/ImmModel.cc | 65 ++++++++++++++++++++++++++-------- > ------------- > 1 file changed, 36 insertions(+), 29 deletions(-) > > diff --git a/src/imm/immnd/ImmModel.cc b/src/imm/immnd/ImmModel.cc > index c539fda..9f81c6b 100644 > --- a/src/imm/immnd/ImmModel.cc > +++ b/src/imm/immnd/ImmModel.cc > @@ -18102,19 +18102,23 @@ SaAisErrorT ImmModel::rtObjectUpdate( > err = SA_AIS_ERR_INVALID_PARAM; > break; // out of switch > } > - if (doIt) { > - osafassert(attrValue->isMultiValued()); > - ImmAttrMultiValue* multiattr = (ImmAttrMultiValue*)attrValue; > - if ((attr->mFlags & SA_IMM_ATTR_NO_DUPLICATES) && > - (multiattr->hasMatchingValue(tmpos))) { > - LOG_NO( > - "ERR_INVALID_PARAM: multivalued attr '%s' with > NO_DUPLICATES " > - "yet duplicate values provided in rta-update call. > Object:'%s'.", > - attrName.c_str(), objectName.c_str()); > - err = SA_AIS_ERR_INVALID_PARAM; > - break; // out of for switch > - } > > + osafassert(attrValue->isMultiValued()); > + ImmAttrMultiValue* multiattr = (ImmAttrMultiValue*)attrValue; > + eduAtValToOs(&tmpos, &(p->attrValue.attrValue), > + > (SaImmValueTypeT)p->attrValue.attrValueType); > + > + if ((attr->mFlags & SA_IMM_ATTR_NO_DUPLICATES) && > + (multiattr->hasMatchingValue(tmpos))) { > + LOG_NO( > + "ERR_INVALID_PARAM: multivalued attr '%s' with > NO_DUPLICATES " > + "yet duplicate values provided in rta-update call. > Object:'%s'.", > + attrName.c_str(), objectName.c_str()); > + err = SA_AIS_ERR_INVALID_PARAM; > + break; // out of for switch > + } > + > + if (doIt) { > multiattr->setExtraValue(tmpos); > } > } > @@ -18128,27 +18132,30 @@ SaAisErrorT ImmModel::rtObjectUpdate( > attrName.c_str()); > err = SA_AIS_ERR_INVALID_PARAM; > break; // out of switch > - } else if (doIt) { > - osafassert(attrValue->isMultiValued()); > - ImmAttrMultiValue* multiattr = (ImmAttrMultiValue*)attrValue; > + } > > - IMMSV_EDU_ATTR_VAL_LIST* al = p->attrValue.attrMoreValues; > - while (al) { > - eduAtValToOs(&tmpos, &(al->n), > - (SaImmValueTypeT)p->attrValue.attrValueType); > - if ((attr->mFlags & SA_IMM_ATTR_NO_DUPLICATES) && > - (multiattr->hasMatchingValue(tmpos))) { > - LOG_NO( > - "ERR_INVALID_PARAM: multivalued attr '%s' with > NO_DUPLICATES " > - "yet duplicate values provided in rta-update call. > Object:'%s'.", > - attrName.c_str(), objectName.c_str()); > - err = SA_AIS_ERR_INVALID_PARAM; > - break; // out of loop > - } > + osafassert(attrValue->isMultiValued()); > + ImmAttrMultiValue* multiattr = (ImmAttrMultiValue*)attrValue; > + IMMSV_EDU_ATTR_VAL_LIST* al = p->attrValue.attrMoreValues; > > + while (al) { > + eduAtValToOs(&tmpos, &(al->n), > + (SaImmValueTypeT)p->attrValue.attrValueType); > + if ((attr->mFlags & SA_IMM_ATTR_NO_DUPLICATES) && > + (multiattr->hasMatchingValue(tmpos))) { > + LOG_NO( > + "ERR_INVALID_PARAM: multivalued attr '%s' with > NO_DUPLICATES " > + "yet duplicate values provided in rta-update call. > Object:'%s'.", > + attrName.c_str(), objectName.c_str()); > + err = SA_AIS_ERR_INVALID_PARAM; > + break; // out of loop > + } > + > + if (doIt) { > multiattr->setExtraValue(tmpos); > - al = al->next; > } > + > + al = al->next; > } > } > break; > -- > 2.7.4 ------------------------------------------------------------------------------ Check out the vibrant tech community on one of the world's most engaging tech sites, Slashdot.org! http://sdm.link/slashdot _______________________________________________ Opensaf-devel mailing list [email protected] https://lists.sourceforge.net/lists/listinfo/opensaf-devel
