[1602] get rid of internal buffer

src/lib/dns/labelsequence.cc

@@ -24,65 +24,56 @@ namespace dns {
 
 LabelSequence::LabelSequence(const Name& name) : name_(name),
                              first_label_(0) {
-    isc::util::OutputBuffer buf(0);
-    name.toWire(buf);
-    size_t buflen = buf.getLength();
-    data_ = new char[buflen];
-    memcpy(data_, buf.getData(), buflen);
-    data_[buflen-1] = '\0';
-
     size_t label_count_ = name.getLabelCount();
     last_label_ = label_count_ - 1;
     offsets_ = new size_t[label_count_];
     offsets_[0] = 0;
     for (size_t i = 1; i < label_count_; ++i) {
-        offsets_[i] = offsets_[i - 1] + data_[offsets_[i - 1]] + 1;
+        offsets_[i] = offsets_[i - 1] + name.at(offsets_[i - 1]) + 1;
     }
 }
 
 LabelSequence::~LabelSequence() {
-    delete[] data_;
     delete[] offsets_;
 }
 
 const char*
-LabelSequence::getData() const {
-    return &data_[offsets_[first_label_]];
-}
-
-const char*
 LabelSequence::getData(size_t *len) const {
-    std::cout << "[XX]" << std::endl;
-    std::cout << "[XX] at: " << offsets_[first_label_] << std::endl;
-    std::cout << "[XX] value: " << name_.at(offsets_[first_label_]) << std::endl;
-    std::cout << "[XX] ptr: " << name_.at_p(offsets_[first_label_]) << std::endl;
     *len = offsets_[last_label_] - offsets_[first_label_];
-    return name_.at_p(offsets_[first_label_]);
+    return (name_.at_p(offsets_[first_label_]));
 }
 
 bool
 LabelSequence::equals(const LabelSequence& other, bool case_sensitive) const {
+    size_t len, other_len;
+    const char* data = getData(&len);
+    const char* other_data = other.getData(&other_len);
+
+    if (len != other_len) {
+        return (false);
+    }
     if (case_sensitive) {
-        return (strcasecmp(getData(), other.getData()) == 0);
+        return (strncasecmp(data, other_data, len) == 0);
     } else {
-        return (strcmp(getData(), other.getData()) == 0);
+        return (strncmp(data, other_data, len) == 0);
     }
 }
 
 void
 LabelSequence::split(int i) {
     if (i > 0) {
-        if (first_label_ + i > last_label_) {
-            isc_throw(Exception, "split(" << i <<") but labelcount: " << getLabelCount());
+        if (i > getLabelCount()) {
+            isc_throw(OutOfRange, "Label " << i << " out of range (" <<
+                                  getLabelCount() << ")");
         } else {
             first_label_ += i;
         }
     } else if (i < 0) {
-        if (last_label_ + i < first_label_) {
-            isc_throw(Exception, "split(" << i <<") but labelcount: " << getLabelCount());
+        if (-i > getLabelCount()) {
+            isc_throw(OutOfRange, "Label " << i << " out of range (" <<
+                                  getLabelCount() << ")");
         } else {
             last_label_ += i;
-            data_[offsets_[last_label_]] = '\0';
         }
     }
 }

src/lib/dns/labelsequence.h

@@ -26,20 +26,20 @@ public:
     LabelSequence(const Name& name);
     ~LabelSequence();
 
-    const char* getData() const;
     const char* getData(size_t* len) const;
 
     bool equals(const LabelSequence& other, bool case_sensitive = false) const;
 
     void split(int i);
 
-    size_t getLabelCount() { return last_label_ - first_label_ + 1; }
+    size_t getLabelCount() { return last_label_ - first_label_; }
+
+    const Name& getName() const { return name_; }
 
 private:
     const Name& name_;
     size_t first_label_;
     size_t last_label_;
-    char* data_;
     size_t* offsets_;
 };
 

src/lib/dns/name.h

@@ -299,10 +299,30 @@ public:
         return (ndata_[pos]);
     }
 
+    ///
+    /// \brief Provides access to the memory pointer of the wire format at
+    /// the specified position
+    ///
+    /// This method is similar to Name::at(size_t pos), but instead of
+    /// the value at the given position, it returns the memory address
+    /// (as a const char*).
+    /// This is only validly usable as long as the Name object is in scope
+    /// and does not change, so care must be taken when using the value
+    /// returned by this method.
+    /// For example use of this method, see the LabelSequence class. Outside
+    /// of that class, it is advisable not to use raw data as exposed here.
+    ///
+    /// \exception OutOfRange thrown if \c pos is higher than the wire format
+    ///            length of the Name data
+    ///
+    /// \param pos The position in the wire format name %data to be returned.
+    /// \return A char* corresponding to the address of the data at the
+    /// position of \c pos.
     const char* at_p(size_t pos) const
     {
         if (pos >= length_) {
-            isc_throw(OutOfRange, "Out of range access in Name::at()");
+            isc_throw(OutOfRange, "Out of range access in Name::at_p(): " <<
+                                  pos << " > " << length_);
         }
         return (&ndata_[pos]);
     }

src/lib/dns/tests/labelsequence_unittest.cc

@@ -13,6 +13,7 @@
 // PERFORMANCE OF THIS SOFTWARE.
 
 #include <dns/labelsequence.h>
+#include <exceptions/exceptions.h>
 
 #include <gtest/gtest.h>
 
@@ -20,22 +21,18 @@ using namespace isc::dns;
 
 class LabelSequenceTest : public ::testing::Test {
 public:
-    LabelSequenceTest() : ls1(Name("example.org")),
-                          ls2(Name("example.com")),
-                          ls3(Name("example.org")),
-                          ls4(Name("foo.bar")),
-                          ls5(Name("example.ORG")),
-                          ls6(Name("ExAmPlE.org")),
-                          ls7(Name("."))
+    LabelSequenceTest() : n1("example.org"), n2("example.com"),
+                          n3("example.org"), n4("foo.bar.test.example"),
+                          n5("example.ORG"), n6("ExAmPlE.org"),
+                          n7("."), n8("foo.example.org.bar"),
+                          ls1(n1), ls2(n2), ls3(n3), ls4(n4), ls5(n5),
+                          ls6(n6), ls7(n7), ls8(n8)
     {};
+    // Need to keep names in scope for at least the lifetime of
+    // the labelsequences
+    Name n1, n2, n3, n4, n5, n6, n7, n8;
 
-    LabelSequence ls1;
-    LabelSequence ls2;
-    LabelSequence ls3;
-    LabelSequence ls4;
-    LabelSequence ls5;
-    LabelSequence ls6;
-    LabelSequence ls7;
+    LabelSequence ls1, ls2, ls3, ls4, ls5, ls6, ls7, ls8;
 };
 
 // Basic equality tests
@@ -47,6 +44,7 @@ TEST_F(LabelSequenceTest, equals_sensitive) {
     EXPECT_FALSE(ls1.equals(ls5));
     EXPECT_FALSE(ls1.equals(ls6));
     EXPECT_FALSE(ls1.equals(ls7));
+    EXPECT_FALSE(ls1.equals(ls8));
 
     EXPECT_FALSE(ls2.equals(ls1));
     EXPECT_TRUE(ls2.equals(ls2));
@@ -55,14 +53,7 @@ TEST_F(LabelSequenceTest, equals_sensitive) {
     EXPECT_FALSE(ls2.equals(ls5));
     EXPECT_FALSE(ls2.equals(ls6));
     EXPECT_FALSE(ls2.equals(ls7));
-
-    EXPECT_TRUE(ls3.equals(ls1));
-    EXPECT_FALSE(ls3.equals(ls2));
-    EXPECT_TRUE(ls3.equals(ls3));
-    EXPECT_FALSE(ls3.equals(ls4));
-    EXPECT_FALSE(ls3.equals(ls5));
-    EXPECT_FALSE(ls3.equals(ls6));
-    EXPECT_FALSE(ls3.equals(ls7));
+    EXPECT_FALSE(ls2.equals(ls8));
 
     EXPECT_FALSE(ls4.equals(ls1));
     EXPECT_FALSE(ls4.equals(ls2));
@@ -71,6 +62,16 @@ TEST_F(LabelSequenceTest, equals_sensitive) {
     EXPECT_FALSE(ls4.equals(ls5));
     EXPECT_FALSE(ls4.equals(ls6));
     EXPECT_FALSE(ls4.equals(ls7));
+    EXPECT_FALSE(ls4.equals(ls8));
+
+    EXPECT_FALSE(ls5.equals(ls1));
+    EXPECT_FALSE(ls5.equals(ls2));
+    EXPECT_FALSE(ls5.equals(ls3));
+    EXPECT_FALSE(ls5.equals(ls4));
+    EXPECT_TRUE(ls5.equals(ls5));
+    EXPECT_FALSE(ls5.equals(ls6));
+    EXPECT_FALSE(ls5.equals(ls7));
+    EXPECT_FALSE(ls5.equals(ls8));
 }
 
 TEST_F(LabelSequenceTest, equals_insensitive) {
@@ -105,67 +106,112 @@ TEST_F(LabelSequenceTest, equals_insensitive) {
     EXPECT_FALSE(ls4.equals(ls5, true));
     EXPECT_FALSE(ls4.equals(ls6, true));
     EXPECT_FALSE(ls4.equals(ls7, true));
+
+    EXPECT_TRUE(ls5.equals(ls1, true));
+    EXPECT_FALSE(ls5.equals(ls2, true));
+    EXPECT_TRUE(ls5.equals(ls3, true));
+    EXPECT_FALSE(ls5.equals(ls4, true));
+    EXPECT_TRUE(ls5.equals(ls5, true));
+    EXPECT_TRUE(ls5.equals(ls6, true));
+    EXPECT_FALSE(ls5.equals(ls7, true));
 }
 
+void
+getDataCheck(const char* expected_data, size_t expected_len,
+             const LabelSequence& ls)
+{
+    size_t len;
+    const char* data = ls.getData(&len);
+    ASSERT_EQ(expected_len, len) << "Expected data: " << expected_data <<
+                                    " name: " << ls.getName().toText();
+    for (size_t i = 0; i < len; ++i) {
+        EXPECT_EQ(expected_data[i], data[i]) << "Difference at pos " << i <<
+                                                ": Expected data: " <<
+                                                expected_data <<
+                                                " name: " <<
+                                                ls.getName().toText();;
+    }
+}
 
 TEST_F(LabelSequenceTest, getData) {
-    EXPECT_STREQ("\007example\003org\0", ls1.getData());
-    EXPECT_STREQ("\007example\003com\0", ls2.getData());
-    EXPECT_STREQ("\007example\003org\0", ls3.getData());
-    EXPECT_STREQ("\003foo\003bar\0", ls4.getData());
-    EXPECT_STREQ("\007example\003ORG\0", ls5.getData());
-    EXPECT_STREQ("\007ExAmPlE\003org\0", ls6.getData());
-    EXPECT_STREQ("\0", ls7.getData());
+    getDataCheck("\007example\003org", 12, ls1);
+    getDataCheck("\007example\003com", 12, ls2);
+    getDataCheck("\007example\003org", 12, ls3);
+    getDataCheck("\003foo\003bar\004test\007example", 21, ls4);
+    getDataCheck("\007example\003ORG", 12, ls5);
+    getDataCheck("\007ExAmPlE\003org", 12, ls6);
+    getDataCheck("", 0, ls7);
 };
 
 TEST_F(LabelSequenceTest, split_pos) {
     ls1.split(0);
-    EXPECT_STREQ("\007example\003org\0", ls1.getData());
+    getDataCheck("\007example\003org", 12, ls1);
     ls1.split(1);
-    EXPECT_STREQ("\003org\0", ls1.getData());
+    getDataCheck("\003org", 4, ls1);
     ls1.split(1);
-    EXPECT_STREQ("\0", ls1.getData());
+    getDataCheck("", 0, ls1);
     EXPECT_TRUE(ls1.equals(ls7));
 
     ls2.split(2);
-    EXPECT_STREQ("\0", ls2.getData());
+    getDataCheck("", 0, ls2);
     EXPECT_TRUE(ls2.equals(ls7));
 }
 
 TEST_F(LabelSequenceTest, split_neg) {
     ls1.split(0);
-    EXPECT_STREQ("\007example\003org\0", ls1.getData());
+    getDataCheck("\007example\003org", 12, ls1);
     ls1.split(-1);
-    EXPECT_STREQ("\007example\0", ls1.getData());
+    getDataCheck("\007example", 8, ls1);
     ls1.split(-1);
-    EXPECT_STREQ("\0", ls1.getData());
     EXPECT_TRUE(ls1.equals(ls7));
 
     ls2.split(-2);
-    EXPECT_STREQ("\0", ls2.getData());
     EXPECT_TRUE(ls2.equals(ls7));
 }
 
+TEST_F(LabelSequenceTest, split_oor) {
+    EXPECT_THROW(ls1.split(100), isc::OutOfRange);
+    EXPECT_THROW(ls1.split(5), isc::OutOfRange);
+    EXPECT_THROW(ls1.split(4), isc::OutOfRange);
+    EXPECT_THROW(ls1.split(3), isc::OutOfRange);
+    getDataCheck("\007example\003org", 12, ls1);
+
+    EXPECT_THROW(ls1.split(-100), isc::OutOfRange);
+    EXPECT_THROW(ls1.split(-5), isc::OutOfRange);
+    EXPECT_THROW(ls1.split(-4), isc::OutOfRange);
+    EXPECT_THROW(ls1.split(-3), isc::OutOfRange);
+    getDataCheck("\007example\003org", 12, ls1);
+}
+
 TEST_F(LabelSequenceTest, getLabelCount) {
-    EXPECT_EQ(3, ls1.getLabelCount());
+    EXPECT_EQ(2, ls1.getLabelCount());
     ls1.split(0);
-    EXPECT_EQ(3, ls1.getLabelCount());
-    ls1.split(1);
     EXPECT_EQ(2, ls1.getLabelCount());
     ls1.split(1);
     EXPECT_EQ(1, ls1.getLabelCount());
+    ls1.split(1);
+    EXPECT_EQ(0, ls1.getLabelCount());
 
-    EXPECT_EQ(3, ls2.getLabelCount());
+    EXPECT_EQ(2, ls2.getLabelCount());
     ls2.split(2);
-    EXPECT_EQ(1, ls2.getLabelCount());
+    EXPECT_EQ(0, ls2.getLabelCount());
 
-    EXPECT_EQ(3, ls3.getLabelCount());
-    ls3.split(-1);
     EXPECT_EQ(2, ls3.getLabelCount());
     ls3.split(-1);
     EXPECT_EQ(1, ls3.getLabelCount());
+    ls3.split(-1);
+    EXPECT_EQ(0, ls3.getLabelCount());
 
-    EXPECT_EQ(3, ls4.getLabelCount());
-    ls4.split(-2);
+    EXPECT_EQ(4, ls4.getLabelCount());
+    ls4.split(-3);
     EXPECT_EQ(1, ls4.getLabelCount());
 }
+
+TEST_F(LabelSequenceTest, comparePart) {
+    size_t len;
+    const char* data = ls1.getData(&len);
+    ls8.split(1);
+    ls8.split(-1);
+    getDataCheck(data, len, ls8);
+}
+