[2086] use raw Name data in LabelSequence

instead of reference to Name
- moved actual name comparison code to labelsequence
- removed Name reference
- replaced by data, offsets, and offset_size values
- added 'raw' constructor for LabelSequence

src/lib/dns/labelsequence.cc

@@ -26,7 +26,7 @@ namespace dns {
 const uint8_t*
 LabelSequence::getData(size_t *len) const {
     *len = getDataLength();
-    return (&name_.ndata_[name_.offsets_[first_label_]]);
+    return &(data_[offsets_[first_label_]]);
 }
 
 size_t
@@ -37,10 +37,10 @@ LabelSequence::getDataLength() const {
     // the length for the 'previous' label (the root label) plus
     // one (for the root label zero octet)
     if (isAbsolute()) {
-        return (name_.offsets_[last_label_ - 1] -
-                name_.offsets_[first_label_] + 1);
+        return (offsets_[last_label_ - 1] -
+                offsets_[first_label_] + 1);
     } else {
-        return (name_.offsets_[last_label_] - name_.offsets_[first_label_]);
+        return (offsets_[last_label_] - offsets_[first_label_]);
     }
 }
 
@@ -78,12 +78,87 @@ LabelSequence::compare(const LabelSequence& other,
         return (NameComparisonResult(0, 0, NameComparisonResult::NONE));
     }
 
-    return (name_.compare(other.name_,
-                          first_label_,
-                          other.first_label_,
-                          last_label_,
-                          other.last_label_,
-                          case_sensitive));
+    // Determine the relative ordering under the DNSSEC order relation of
+    // 'this' and 'other', and also determine the hierarchical relationship
+    // of the names.
+
+    if ((first_label_ > last_label_) ||
+        (other.first_label_ > other.last_label_)) {
+        isc_throw(BadValue, "Bad label index ranges were passed");
+    }
+
+    unsigned int nlabels = 0;
+    int l1 = last_label_ - first_label_;
+    int l2 = other.last_label_ - other.first_label_;
+    int ldiff = (int)l1 - (int)l2;
+    unsigned int l = (ldiff < 0) ? l1 : l2;
+
+    while (l > 0) {
+        --l;
+        --l1;
+        --l2;
+        size_t pos1 = offsets_[l1 + first_label_];
+        size_t pos2 = other.offsets_[l2 + other.first_label_];
+        unsigned int count1 = data_[pos1++];
+        unsigned int count2 = other.data_[pos2++];
+
+        // We don't support any extended label types including now-obsolete
+        // bitstring labels.
+        assert(count1 <= Name::MAX_LABELLEN && count2 <= Name::MAX_LABELLEN);
+
+        int cdiff = (int)count1 - (int)count2;
+        unsigned int count = (cdiff < 0) ? count1 : count2;
+
+        while (count > 0) {
+            uint8_t label1 = data_[pos1];
+            uint8_t label2 = other.data_[pos2];
+            int chdiff;
+
+            if (case_sensitive) {
+                chdiff = (int)label1 - (int)label2;
+            } else {
+                chdiff = (int)isc::dns::name::internal::maptolower[label1] - (int)isc::dns::name::internal::maptolower[label2];
+            }
+
+            if (chdiff != 0) {
+                if ((nlabels == 0) &&
+                     (!isAbsolute() ||
+                     ((last_label_ < getLabelCount()) ||
+                      (other.last_label_ < other.getLabelCount())))) {
+                    return (NameComparisonResult(0, 0,
+                                                 NameComparisonResult::NONE));
+                } else {
+                    return (NameComparisonResult(chdiff, nlabels,
+                                                 NameComparisonResult::COMMONANCESTOR));
+                }
+            }
+            --count;
+            ++pos1;
+            ++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));
+            }
+        }
+        ++nlabels;
+    }
+
+    if (ldiff < 0) {
+        return (NameComparisonResult(ldiff, nlabels,
+                                     NameComparisonResult::SUPERDOMAIN));
+    } else if (ldiff > 0) {
+        return (NameComparisonResult(ldiff, nlabels,
+                                     NameComparisonResult::SUBDOMAIN));
+    }
+
+    return (NameComparisonResult(ldiff, nlabels, NameComparisonResult::EQUAL));
 }
 
 void
@@ -106,7 +181,7 @@ LabelSequence::stripRight(size_t i) {
 
 bool
 LabelSequence::isAbsolute() const {
-    return (last_label_ == name_.offsets_.size());
+    return (last_label_ == offsets_size_);
 }
 
 size_t
@@ -129,9 +204,9 @@ LabelSequence::getHash(bool case_sensitive) const {
 
 std::string
 LabelSequence::toText(bool omit_final_dot) const {
-    Name::NameString::const_iterator np = name_.ndata_.begin() +
-        name_.offsets_[first_label_];
-    const Name::NameString::const_iterator np_end = np + getDataLength();
+    const uint8_t* np = &data_[offsets_[first_label_]];
+    const uint8_t* np_end = np + getDataLength();
+
     // use for integrity check
     unsigned int labels = last_label_ - first_label_;
     // init with an impossible value to catch error cases in the end:

src/lib/dns/labelsequence.h

@@ -53,11 +53,35 @@ public:
     /// to the labels in the Name object).
     ///
     /// \param name The Name to construct a LabelSequence for
-    LabelSequence(const Name& name): name_(name),
+    explicit LabelSequence(const Name& name): /*name_(name),*/
+                                     data_(&name.ndata_[0]),
+                                     offsets_(&name.offsets_[0]),
+                                     offsets_size_(name.offsets_.size()),
                                      first_label_(0),
                                      last_label_(name.getLabelCount())
     {}
 
+    /// \brief Constructs a LabelSequence for the given data
+    ///
+    /// \note The associated data MUST remain in scope during the lifetime
+    /// of this LabelSequence, since only the pointers are copied.
+    ///
+    /// \note No validation is done on the given data upon construction;
+    ///       use with care.
+    ///
+    /// \param data The Name data, in wire format
+    /// \param offsets The offsets of the labels in the Name, as given
+    ///        by the Name object or another LabelSequence
+    /// \param offsets_size The size of the offsets data
+    LabelSequence(const uint8_t* data,
+                  const uint8_t* offsets,
+                  size_t offsets_size) : data_(data),
+                                         offsets_(offsets),
+                                         offsets_size_(offsets_size),
+                                         first_label_(0),
+                                         last_label_(offsets_size_)
+    {}
+
     /// \brief Return the wire-format data for this LabelSequence
     ///
     /// The data, is returned as a pointer to the original wireformat
@@ -201,7 +225,9 @@ public:
     bool isAbsolute() const;
 
 private:
-    const Name& name_;
+    const uint8_t* data_;
+    const uint8_t* offsets_;
+    size_t offsets_size_;
     size_t first_label_;
     size_t last_label_;
 };

src/lib/dns/name.cc

@@ -127,7 +127,7 @@ typedef enum {
 
     // Unused at this moment.  We'll revisit this when we support master file
     // parser where @ is used to mean an origin name.
-    ft_at                  
+    ft_at
 } ft_state;
 }
 
@@ -435,101 +435,7 @@ Name::toText(bool omit_final_dot) const {
 
 NameComparisonResult
 Name::compare(const Name& other) const {
-    return (compare(other, 0, 0, labelcount_, other.labelcount_));
-}
-
-NameComparisonResult
-Name::compare(const Name& other,
-              unsigned int first_label,
-              unsigned int first_label_other,
-              unsigned int last_label,
-              unsigned int last_label_other,
-              bool case_sensitive) const {
-    // Determine the relative ordering under the DNSSEC order relation of
-    // 'this' and 'other', and also determine the hierarchical relationship
-    // of the names.
-
-    if ((first_label > last_label) ||
-        (first_label_other > last_label_other)) {
-        isc_throw(BadValue, "Bad label index ranges were passed");
-    }
-
-    if ((first_label > labelcount_) ||
-        (first_label_other > other.labelcount_)) {
-        isc_throw(BadValue, "Bad first label indices were passed");
-    }
-
-    unsigned int nlabels = 0;
-    int l1 = last_label - first_label;
-    int l2 = last_label_other - first_label_other;
-    int ldiff = (int)l1 - (int)l2;
-    unsigned int l = (ldiff < 0) ? l1 : l2;
-
-    while (l > 0) {
-        --l;
-        --l1;
-        --l2;
-        size_t pos1 = offsets_[l1 + first_label];
-        size_t pos2 = other.offsets_[l2 + first_label_other];
-        unsigned int count1 = ndata_[pos1++];
-        unsigned int count2 = other.ndata_[pos2++];
-
-        // We don't support any extended label types including now-obsolete
-        // bitstring labels.
-        assert(count1 <= MAX_LABELLEN && count2 <= MAX_LABELLEN);
-
-        int cdiff = (int)count1 - (int)count2;
-        unsigned int count = (cdiff < 0) ? count1 : count2;
-
-        while (count > 0) {
-            uint8_t label1 = ndata_[pos1];
-            uint8_t label2 = other.ndata_[pos2];
-            int chdiff;
-
-            if (case_sensitive) {
-                chdiff = (int)label1 - (int)label2;
-            } else {
-                chdiff = (int)maptolower[label1] - (int)maptolower[label2];
-            }
-
-            if (chdiff != 0) {
-                if ((nlabels == 0) &&
-                    ((last_label < labelcount_) ||
-                     (last_label_other < other.labelcount_))) {
-                    return (NameComparisonResult(0, 0,
-                                                 NameComparisonResult::NONE));
-                } else {
-                    return (NameComparisonResult(chdiff, nlabels,
-                                                 NameComparisonResult::COMMONANCESTOR));
-                }
-            }
-            --count;
-            ++pos1;
-            ++pos2;
-        }
-        if (cdiff != 0) {
-            if ((nlabels == 0) &&
-                ((last_label < labelcount_) ||
-                 (last_label_other < other.labelcount_))) {
-                return (NameComparisonResult(0, 0,
-                                             NameComparisonResult::NONE));
-            } else {
-                return (NameComparisonResult(cdiff, nlabels,
-                                             NameComparisonResult::COMMONANCESTOR));
-            }
-        }
-        ++nlabels;
-    }
-
-    if (ldiff < 0) {
-        return (NameComparisonResult(ldiff, nlabels,
-                                     NameComparisonResult::SUPERDOMAIN));
-    } else if (ldiff > 0) {
-        return (NameComparisonResult(ldiff, nlabels,
-                                     NameComparisonResult::SUBDOMAIN));
-    }
-
-    return (NameComparisonResult(ldiff, nlabels, NameComparisonResult::EQUAL));
+    return LabelSequence(*this).compare(LabelSequence(other));
 }
 
 bool
@@ -581,7 +487,7 @@ Name::gthan(const Name& other) const {
 
 bool
 Name::isWildcard() const {
-    return (length_ >= 2 && ndata_[0] == 1 && ndata_[1] == '*'); 
+    return (length_ >= 2 && ndata_[0] == 1 && ndata_[1] == '*');
 }
 
 Name
@@ -729,5 +635,6 @@ operator<<(std::ostream& os, const Name& name) {
     os << name.toText();
     return (os);
 }
+
 }
 }

src/lib/dns/name.h

@@ -137,7 +137,7 @@ public:
     /// want to distinguish "com" and "com.", and the current definition would
     /// be more compatible for that purpose.
     /// If, on the other hand, we finally decide we really don't need that
-    /// notion, we'll probably reconsider the design here, too. 
+    /// notion, we'll probably reconsider the design here, too.
     enum NameRelation {
         SUPERDOMAIN = 0,
         SUBDOMAIN = 1,
@@ -427,12 +427,14 @@ private:
     /// \param case_sensitive If true, comparison is case-insensitive
     /// \return a <code>NameComparisonResult</code> object representing the
     /// comparison result.
+/*
     NameComparisonResult compare(const Name& other,
                                  unsigned int first_label,
                                  unsigned int first_label_other,
                                  unsigned int last_label,
                                  unsigned int last_label_other,
                                  bool case_sensitive = false) const;
+*/
 
 public:
     /// \brief Return true iff two names are equal.
@@ -551,7 +553,7 @@ public:
     /// \param first The start position (in labels) of the extracted name
     /// \param n Number of labels of the extracted name
     /// \return A new Name object based on the Name containing <code>n</code>
-    /// labels including and following the <code>first</code> label.  
+    /// labels including and following the <code>first</code> label.
     Name split(unsigned int first, unsigned int n) const;
 
     /// \brief Extract a specified super domain name of Name.
@@ -623,7 +625,7 @@ public:
     /// \brief Reverse the labels of a name
     ///
     /// This method reverses the labels of a name.  For example, if
-    /// \c this is "www.example.com.", this method will return 
+    /// \c this is "www.example.com.", this method will return
     /// "com.example.www."  (This is useful because DNSSEC sort order
     /// is equivalent to a lexical sort of label-reversed names.)
     Name reverse() const;
@@ -743,10 +745,11 @@ Name::ROOT_NAME() {
 /// parameter \c os after the insertion operation.
 std::ostream&
 operator<<(std::ostream& os, const Name& name);
+
 }
 }
 #endif // __NAME_H
 
-// Local Variables: 
+// Local Variables:
 // mode: c++
-// End: 
+// End:

src/lib/dns/tests/labelsequence_unittest.cc

@@ -674,4 +674,46 @@ TEST_F(LabelSequenceTest, LeftShiftOperator) {
     oss << ls1;
     EXPECT_EQ(ls1.toText(), oss.str());
 }
+
+// Test different ways of construction, and see if they compare
+TEST(LabelSequence, rawConstruction) {
+    Name n("example.org");
+
+    uint8_t data[] = { 0x07, 'e', 'x', 'a', 'm', 'p', 'l', 'e',
+                       0x03, 'o', 'r', 'g',
+                       0x00 };
+    uint8_t offsets[] = { 0, 8, 12 };
+    size_t offsets_size = 3;
+
+    LabelSequence s1(n);
+    LabelSequence s2(s1);
+    LabelSequence s3(data, offsets, offsets_size);
+
+    // Assuming equality is transitive, so only comparing 1 to 2 and 1 to 3
+    NameComparisonResult result = s1.compare(s2);
+    EXPECT_EQ(isc::dns::NameComparisonResult::EQUAL,
+              result.getRelation());
+    EXPECT_EQ(0, result.getOrder());
+    EXPECT_EQ(3, result.getCommonLabels());
+
+    result = s1.compare(s3);
+    EXPECT_EQ(isc::dns::NameComparisonResult::EQUAL,
+              result.getRelation());
+    EXPECT_EQ(0, result.getOrder());
+    EXPECT_EQ(3, result.getCommonLabels());
+
+    // Modify the data and make sure it's not equal anymore
+    data[2] = 'f';
+    result = s1.compare(s3);
+    EXPECT_EQ(isc::dns::NameComparisonResult::COMMONANCESTOR,
+              result.getRelation());
+
+    s1.stripRight(1);
+    s2.stripRight(1);
+    data[9] = 'f';
+    result = s1.compare(s3);
+    EXPECT_EQ(isc::dns::NameComparisonResult::NONE,
+              result.getRelation());
+}
+
 }