Hi Zoran,

Thank you for your comment.
I am going to apply your idea in the new commit.

 Best regards,
/Danh

-----Original Message-----
From: Zoran Milinkovic <[email protected]> 
Sent: Tuesday, May 8, 2018 3:47 PM
To: Danh Cong Vo <[email protected]>; [email protected];
Hans Nordebäck <[email protected]>; Anders Widell
<[email protected]>; Lennart Lund <[email protected]>; Vu
Minh Nguyen <[email protected]>
Cc: [email protected]; Danh Cong Vo
<[email protected]>
Subject: RE: [PATCH 1/1] imm: sPbeRtMutations is updated even when
validation for duplicate values fails [#2422]

Hi Danh,

The block below can be rewritten to check only once and skip the check in
the second loop:

The block:
+            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);
             }

New block:

            if (doIt) {
               multiattr->setExtraValue(tmpos);
             } else 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
            }

The same code can be applied for other changes.

BR,
Zoran

-----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

Reply via email to