Parcourir la source

[2091a] updated LabelSequence::compare() for the NONE-relationship cases.

the originally intended behavior was to return non 0 order for these cases,
so this point was adjusted.
the code was also simplified by removing unnecessary conditions like:
-                     ((last_label_ < getLabelCount()) ||
-                      (other.last_label_ < other.getLabelCount())))) {

this is meaningless because getLabelCount() is last_label_ - first_label_,
so the codntion is actually first_label_ < 0, which is always false.
whatever the real intent of this condition was, such additional checks
didn't seem to be necessary for the intended behavior of compare() anyway,
so I simply removed them.
some test cases were adjusted for the change, and some were added to confirm
related cases that were not tested before.
JINMEI Tatuya il y a 12 ans
Parent
commit
0c5ac6b64b
2 fichiers modifiés avec 27 ajouts et 28 suppressions
  1. 6 14
      src/lib/dns/labelsequence.cc
  2. 21 14
      src/lib/dns/tests/labelsequence_unittest.cc

+ 6 - 14
src/lib/dns/labelsequence.cc

@@ -157,11 +157,8 @@ LabelSequence::compare(const LabelSequence& other,
             }
 
             if (chdiff != 0) {
-                if ((nlabels == 0) &&
-                     (!isAbsolute() ||
-                     ((last_label_ < getLabelCount()) ||
-                      (other.last_label_ < other.getLabelCount())))) {
-                    return (NameComparisonResult(0, 0,
+                if (nlabels == 0 && !isAbsolute()) {
+                    return (NameComparisonResult(chdiff, 0,
                                                  NameComparisonResult::NONE));
                 } else {
                     return (NameComparisonResult(chdiff, nlabels,
@@ -173,15 +170,10 @@ LabelSequence::compare(const LabelSequence& other,
             ++pos2;
         }
         if (cdiff != 0) {
-            if ((nlabels == 0) &&
-                ((last_label_ < getLabelCount()) ||
-                 (other.last_label_ < other.getLabelCount()))) {
-                return (NameComparisonResult(0, 0,
-                                             NameComparisonResult::NONE));
-            } else {
-                return (NameComparisonResult(cdiff, nlabels,
-                                             NameComparisonResult::COMMONANCESTOR));
-            }
+            return (NameComparisonResult(
+                        cdiff, nlabels,
+                        nlabels == 0 ? NameComparisonResult::NONE :
+                        NameComparisonResult::COMMONANCESTOR));
         }
         ++nlabels;
     }

+ 21 - 14
src/lib/dns/tests/labelsequence_unittest.cc

@@ -192,8 +192,7 @@ TEST_F(LabelSequenceTest, compare) {
     // in-sensitive
     lsb.stripRight(1);
     result = lsc.compare(lsb);
-    EXPECT_EQ(isc::dns::NameComparisonResult::NONE,
-              result.getRelation());
+    EXPECT_EQ(isc::dns::NameComparisonResult::NONE, result.getRelation());
     EXPECT_EQ(0, result.getOrder());
     EXPECT_EQ(0, result.getCommonLabels());
 
@@ -298,14 +297,23 @@ TEST_F(LabelSequenceTest, compare) {
     EXPECT_LT(0, result.getOrder());
     EXPECT_EQ(2, result.getCommonLabels());
 
-    // "a.b.c" (not absolute) and
-    // "w.x.y" (not absolute), case in-sensitive
+    // lsf: "a.b.c" (not absolute) and
+    // lsg: "w.x.y" (not absolute), case in-sensitive; a.b.c < w.x.y;
+    // no common labels.
     lsf.stripRight(2);
     lsg.stripRight(2);
-    result = lsg.compare(lsf);
-    EXPECT_EQ(isc::dns::NameComparisonResult::NONE,
-              result.getRelation());
-    EXPECT_EQ(0, result.getOrder());
+    result = lsf.compare(lsg);
+    EXPECT_EQ(isc::dns::NameComparisonResult::NONE, result.getRelation());
+    EXPECT_GT(0, result.getOrder());
+    EXPECT_EQ(0, result.getCommonLabels());
+
+    // lsf2: a.b.cc (not absolute); a.b.c < a.b.cc, no common labels.
+    const Name nf2("a.b.cc");
+    LabelSequence lsf2(nf2);
+    lsf2.stripRight(1);
+    result = lsf.compare(lsf2);
+    EXPECT_EQ(isc::dns::NameComparisonResult::NONE, result.getRelation());
+    EXPECT_GT(0, result.getOrder());
     EXPECT_EQ(0, result.getCommonLabels());
 
     Name nh("aexample.org");
@@ -324,13 +332,13 @@ TEST_F(LabelSequenceTest, compare) {
     EXPECT_EQ(1, result.getCommonLabels());
 
     // "aexample" (not absolute) and
-    // "bexample" (not absolute), case in-sensitive
+    // "bexample" (not absolute), case in-sensitive;
+    // aexample < bexample; no common labels.
     lsh.stripRight(1);
     lsi.stripRight(1);
     result = lsh.compare(lsi);
-    EXPECT_EQ(isc::dns::NameComparisonResult::NONE,
-              result.getRelation());
-    EXPECT_EQ(0, result.getOrder());
+    EXPECT_EQ(isc::dns::NameComparisonResult::NONE, result.getRelation());
+    EXPECT_GT(0, result.getOrder());
     EXPECT_EQ(0, result.getCommonLabels());
 
     Name nj("example.org");
@@ -806,8 +814,7 @@ TEST(LabelSequence, rawConstruction) {
 
     data[9] = 'f';
     result = s1.compare(s3);
-    EXPECT_EQ(isc::dns::NameComparisonResult::NONE,
-              result.getRelation());
+    EXPECT_EQ(isc::dns::NameComparisonResult::NONE, result.getRelation());
     EXPECT_EQ(0, result.getCommonLabels());
 }