[2091a] refactor LabelSequence: ensure last_label_ is always valid index.

This simplifies some internal part of the code, and eliminates the need for
offsets_size_ (so it's removed).  This change is a preparation for making
the "from raw data" construction more generic to support non absolute labels.
This is a purely internal refactoring, and public interfaces aren't changed.

src/lib/dns/labelsequence.cc

@@ -25,14 +25,15 @@ namespace dns {
 
 LabelSequence::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_)
+                             size_t offsets_size) :
+    data_(data),
+    offsets_(offsets),
+    first_label_(0),
+    last_label_(offsets_size - 1)
 {
     if (data == NULL || offsets == NULL) {
-        isc_throw(BadValue, "Null pointer passed to LabelSequence constructor");
+        isc_throw(BadValue,
+                  "Null pointer passed to LabelSequence constructor");
     }
     if (offsets_size == 0) {
         isc_throw(BadValue, "Zero offsets to LabelSequence constructor");
@@ -59,26 +60,18 @@ LabelSequence::getData(size_t *len) const {
 
 void
 LabelSequence::getOffsetData(size_t* len,
-                             uint8_t placeholder[Name::MAX_LABELS]) const {
-  *len = last_label_ - first_label_;
-  for (size_t i = 0; i < *len; i++) {
-      placeholder[i] = offsets_[first_label_ + i] - offsets_[first_label_];
-  }
+                             uint8_t placeholder[Name::MAX_LABELS]) const
+{
+    *len = getLabelCount();
+    for (size_t i = 0; i < *len; ++i) {
+        placeholder[i] = offsets_[first_label_ + i] - offsets_[first_label_];
+    }
 }
 
 size_t
 LabelSequence::getDataLength() const {
-    // If the labelsequence is absolute, the current last_label_ falls
-    // out of the vector (since it points to the 'label' after the
-    // root label, which doesn't exist; in that case, return
-    // the length for the 'previous' label (the root label) plus
-    // one (for the root label zero octet)
-    if (isAbsolute()) {
-        return (offsets_[last_label_ - 1] -
-                offsets_[first_label_] + 1);
-    } else {
-        return (offsets_[last_label_] - offsets_[first_label_]);
-    }
+    const size_t last_label_len = data_[offsets_[last_label_]] + 1;
+    return (offsets_[last_label_] - offsets_[first_label_] + last_label_len);
 }
 
 bool
@@ -117,8 +110,8 @@ LabelSequence::compare(const LabelSequence& other,
     // of the labels.
 
     unsigned int nlabels = 0;
-    int l1 = last_label_ - first_label_;
-    int l2 = other.last_label_ - other.first_label_;
+    int l1 = getLabelCount();
+    int l2 = other.getLabelCount();
     const int ldiff = static_cast<int>(l1) - static_cast<int>(l2);
     unsigned int l = (ldiff < 0) ? l1 : l2;
 
@@ -202,7 +195,7 @@ LabelSequence::stripRight(size_t i) {
 
 bool
 LabelSequence::isAbsolute() const {
-    return (last_label_ == offsets_size_);
+    return (data_[offsets_[last_label_]] == 0);
 }
 
 size_t
@@ -229,7 +222,7 @@ LabelSequence::toText(bool omit_final_dot) const {
     const uint8_t* np_end = np + getDataLength();
 
     // use for integrity check
-    unsigned int labels = last_label_ - first_label_;
+    unsigned int labels = getLabelCount();
     // init with an impossible value to catch error cases in the end:
     unsigned int count = Name::MAX_LABELLEN + 1;
 

src/lib/dns/labelsequence.h

@@ -21,7 +21,7 @@
 namespace isc {
 namespace dns {
 
-/// \brief Light-weight Accessor to data of Name object
+/// \brief Light-weight Accessor to Name data.
 ///
 /// The purpose of this class is to easily match Names and parts of Names,
 /// without needing to copy the underlying data on each label strip.
@@ -54,11 +54,10 @@ public:
     ///
     /// \param name The Name to construct a LabelSequence for
     explicit LabelSequence(const Name& name):
-                                     data_(&name.ndata_[0]),
-                                     offsets_(&name.offsets_[0]),
-                                     offsets_size_(name.offsets_.size()),
-                                     first_label_(0),
-                                     last_label_(name.getLabelCount())
+        data_(&name.ndata_[0]),
+        offsets_(&name.offsets_[0]),
+        first_label_(0),
+        last_label_(name.getLabelCount() - 1)
     {}
 
     /// \brief Constructs a LabelSequence for the given data
@@ -90,11 +89,10 @@ public:
     ///
     /// \param ls The LabelSequence to construct a LabelSequence from
     LabelSequence(const LabelSequence& ls):
-                                     data_(ls.data_),
-                                     offsets_(ls.offsets_),
-                                     offsets_size_(ls.offsets_size_),
-                                     first_label_(ls.first_label_),
-                                     last_label_(ls.last_label_)
+        data_(ls.data_),
+        offsets_(ls.offsets_),
+        first_label_(ls.first_label_),
+        last_label_(ls.last_label_)
     {}
 
     /// \brief Return the wire-format data for this LabelSequence
@@ -186,7 +184,9 @@ public:
     /// \brief Returns the current number of labels for this LabelSequence
     ///
     /// \return The number of labels
-    size_t getLabelCount() const { return (last_label_ - first_label_); }
+    size_t getLabelCount() const {
+        return (last_label_ - first_label_ + 1);
+    }
 
     /// \brief Convert the LabelSequence to a string.
     ///
@@ -251,11 +251,12 @@ public:
     bool isAbsolute() const;
 
 private:
-    const uint8_t* data_;
-    const uint8_t* offsets_;
-    size_t offsets_size_;
-    size_t first_label_;
-    size_t last_label_;
+    const uint8_t* data_;       // wire-format name data
+    const uint8_t* offsets_;    // an array of offsets in data_ for the labels
+    size_t first_label_;        // index of offsets_ for the first label
+    size_t last_label_;         // index of offsets_ for the last label.
+                                // can be equal to first_label_, but must not
+                                // be smaller (the class ensures that)
 };