BIND 10 trac1602, updated. c271fcc053b0ff9b2e7273e992591549258c18b7 [1602] Address review comments
BIND 10 source code commits
bind10-changes at lists.isc.org
Thu Mar 1 11:01:09 UTC 2012
The branch, trac1602 has been updated
via c271fcc053b0ff9b2e7273e992591549258c18b7 (commit)
from f6bd9f9c413e93070c2eddc6d84d6e77dd789454 (commit)
Those revisions listed above that are new to this repository have
not appeared on any other notification email; so we list those
revisions in full, below.
- Log -----------------------------------------------------------------
commit c271fcc053b0ff9b2e7273e992591549258c18b7
Author: Jelte Jansen <jelte at isc.org>
Date: Thu Mar 1 11:46:46 2012 +0100
[1602] Address review comments
- split up split() into leftSplit() and rightSplit()
- made LabelSequence a Friend class of Name
- no need for separate offsets vector
- no need for Name::at_p()
- fixed copyright year :)
-----------------------------------------------------------------------
Summary of changes:
src/lib/dns/labelsequence.cc | 39 +++++++---------
src/lib/dns/labelsequence.h | 33 +++++++++-----
src/lib/dns/name.h | 33 ++-----------
src/lib/dns/tests/labelsequence_unittest.cc | 66 +++++++++++++-------------
4 files changed, 75 insertions(+), 96 deletions(-)
-----------------------------------------------------------------------
diff --git a/src/lib/dns/labelsequence.cc b/src/lib/dns/labelsequence.cc
index c23a3a1..693702d 100644
--- a/src/lib/dns/labelsequence.cc
+++ b/src/lib/dns/labelsequence.cc
@@ -1,4 +1,4 @@
-// Copyright (C) 2009 Internet Systems Consortium, Inc. ("ISC")
+// Copyright (C) 2012 Internet Systems Consortium, Inc. ("ISC")
//
// Permission to use, copy, modify, and/or distribute this software for any
// purpose with or without fee is hereby granted, provided that the above
@@ -19,18 +19,6 @@
namespace isc {
namespace dns {
-LabelSequence::LabelSequence(const Name& name) : name_(name),
- first_label_(0), offsets_(name.getLabelCount()) {
- offsets_[0] = 0;
- last_label_ = name.getLabelCount();
- // Walk through the wire format data and store all offsets
- for (size_t i = 1; i < last_label_; ++i) {
- // Each offset is the previous offset plus the length of the
- // label plus 1 (for the label length octet)
- offsets_[i] = offsets_[i - 1] + name.at(offsets_[i - 1]) + 1;
- }
-}
-
const char*
LabelSequence::getData(size_t *len) const {
// If the labelsequence is absolute, the current last_label_ falls
@@ -39,11 +27,11 @@ LabelSequence::getData(size_t *len) const {
// the length for the 'previous' label (the root label) plus
// one (for the root label zero octet)
if (isAbsolute()) {
- *len = offsets_[last_label_ - 1] - offsets_[first_label_] + 1;
+ *len = name_.offsets_[last_label_ - 1] - name_.offsets_[first_label_] + 1;
} else {
- *len = offsets_[last_label_] - offsets_[first_label_];
+ *len = name_.offsets_[last_label_] - name_.offsets_[first_label_];
}
- return (name_.at_p(offsets_[first_label_]));
+ return (&name_.ndata_[name_.offsets_[first_label_]]);
}
bool
@@ -63,21 +51,26 @@ LabelSequence::equals(const LabelSequence& other, bool case_sensitive) const {
}
void
-LabelSequence::split(int i) {
- if (abs(i) >= getLabelCount()) {
+LabelSequence::leftSplit(size_t i) {
+ if (i >= getLabelCount()) {
isc_throw(OutOfRange, "Cannot split to zero or less labels; " << i <<
" (labelcount: " << getLabelCount() << ")");
}
- if (i > 0) {
- first_label_ += i;
- } else if (i < 0) {
- last_label_ += i;
+ first_label_ += i;
+}
+
+void
+LabelSequence::rightSplit(size_t i) {
+ if (i >= getLabelCount()) {
+ isc_throw(OutOfRange, "Cannot split to zero or less labels; " << i <<
+ " (labelcount: " << getLabelCount() << ")");
}
+ last_label_ -= i;
}
bool
LabelSequence::isAbsolute() const {
- return (last_label_ == offsets_.size());
+ return (last_label_ == name_.offsets_.size());
}
} // end namespace dns
diff --git a/src/lib/dns/labelsequence.h b/src/lib/dns/labelsequence.h
index c999c2d..bae6298 100644
--- a/src/lib/dns/labelsequence.h
+++ b/src/lib/dns/labelsequence.h
@@ -1,4 +1,4 @@
-// Copyright (C) 2009 Internet Systems Consortium, Inc. ("ISC")
+// Copyright (C) 2012 Internet Systems Consortium, Inc. ("ISC")
//
// Permission to use, copy, modify, and/or distribute this software for any
// purpose with or without fee is hereby granted, provided that the above
@@ -49,7 +49,10 @@ public:
/// to the labels in the Name object).
///
/// \param name The Name to construct a LabelSequence for
- LabelSequence(const Name& name);
+ LabelSequence(const Name& name): name_(name),
+ first_label_(0),
+ last_label_(name.getLabelCount())
+ {}
/// \brief Return the wire-format data for this LabelSequence
///
@@ -76,20 +79,27 @@ public:
/// and contain the same data.
bool equals(const LabelSequence& other, bool case_sensitive = false) const;
- /// \brief Remove one or more labels from this LabelSequence
+ /// \brief Remove labels from the front of this LabelSequence
///
- /// Removes labels from either the front or the back of the LabelSequence
+ /// \note No actual memory is changed, this operation merely updates the
+ /// internal pointers based on the offsets in the Name object.
+ ///
+ /// \exeption OutOfRange if i is greater than or equal to the number
+ /// of labels currently pointed to by this LabelSequence
+ ///
+ /// \param i The number of labels to remove.
+ void leftSplit(size_t i);
+
+ /// \brief Remove labels from the end of this LabelSequence
///
/// \note No actual memory is changed, this operation merely updates the
- /// internal pointers based on the offsets at creation time.
+ /// internal pointers based on the offsets in the Name object.
///
- /// \exeption OutOfRange if abs(i) is greater than or equal to the
- /// number of labels currently pointed to by this LabelSequence
+ /// \exeption OutOfRange if i is greater than or equal to the number
+ /// of labels currently pointed to by this LabelSequence
///
- /// \param i When positive, removes i labels from the front of the
- /// LabelSequence. When negative, removes i labels from the
- /// end of it. When zero, this is a no-op.
- void split(int i);
+ /// \param i The number of labels to remove.
+ void rightSplit(size_t i);
/// \brief Returns the current number of labels for this LabelSequence
///
@@ -116,7 +126,6 @@ private:
const Name& name_;
size_t first_label_;
size_t last_label_;
- std::vector<size_t> offsets_;
};
diff --git a/src/lib/dns/name.h b/src/lib/dns/name.h
index aeaf77d..ca64d69 100644
--- a/src/lib/dns/name.h
+++ b/src/lib/dns/name.h
@@ -210,6 +210,11 @@ private:
/// names as a special case.
///
class Name {
+ // LabelSequences use knowledge about the internal data structure
+ // of this class for efficiency (they use the offsets_ vector and
+ // the ndata_ string)
+ friend class LabelSequence;
+
///
/// \name Constructors and Destructor
///
@@ -299,34 +304,6 @@ 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_p(): " <<
- pos << " > " << length_);
- }
- return (&ndata_[pos]);
- }
-
/// \brief Gets the length of the <code>Name</code> in its wire format.
///
/// This method never throws an exception.
diff --git a/src/lib/dns/tests/labelsequence_unittest.cc b/src/lib/dns/tests/labelsequence_unittest.cc
index 62c73ee..f974219 100644
--- a/src/lib/dns/tests/labelsequence_unittest.cc
+++ b/src/lib/dns/tests/labelsequence_unittest.cc
@@ -1,4 +1,4 @@
-// Copyright (C) 2009 Internet Systems Consortium, Inc. ("ISC")
+// Copyright (C) 2012 Internet Systems Consortium, Inc. ("ISC")
//
// Permission to use, copy, modify, and/or distribute this software for any
// purpose with or without fee is hereby granted, provided that the above
@@ -143,77 +143,77 @@ TEST_F(LabelSequenceTest, getData) {
getDataCheck("\000", 1, ls7);
};
-TEST_F(LabelSequenceTest, split_pos) {
+TEST_F(LabelSequenceTest, leftSplit) {
EXPECT_TRUE(ls1.equals(ls3));
- ls1.split(0);
+ ls1.leftSplit(0);
getDataCheck("\007example\003org\000", 13, ls1);
EXPECT_TRUE(ls1.equals(ls3));
- ls1.split(1);
+ ls1.leftSplit(1);
getDataCheck("\003org\000", 5, ls1);
EXPECT_FALSE(ls1.equals(ls3));
- ls1.split(1);
+ ls1.leftSplit(1);
getDataCheck("\000", 1, ls1);
EXPECT_TRUE(ls1.equals(ls7));
- ls2.split(2);
+ ls2.leftSplit(2);
getDataCheck("\000", 1, ls2);
EXPECT_TRUE(ls2.equals(ls7));
}
-TEST_F(LabelSequenceTest, split_neg) {
+TEST_F(LabelSequenceTest, rightSplit) {
EXPECT_TRUE(ls1.equals(ls3));
- ls1.split(-1);
+ ls1.rightSplit(1);
getDataCheck("\007example\003org", 12, ls1);
EXPECT_FALSE(ls1.equals(ls3));
- ls1.split(-1);
+ ls1.rightSplit(1);
getDataCheck("\007example", 8, ls1);
EXPECT_FALSE(ls1.equals(ls3));
ASSERT_FALSE(ls1.equals(ls2));
- ls2.split(-2);
+ ls2.rightSplit(2);
getDataCheck("\007example", 8, ls2);
EXPECT_TRUE(ls1.equals(ls2));
}
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);
+ EXPECT_THROW(ls1.leftSplit(100), isc::OutOfRange);
+ EXPECT_THROW(ls1.leftSplit(5), isc::OutOfRange);
+ EXPECT_THROW(ls1.leftSplit(4), isc::OutOfRange);
+ EXPECT_THROW(ls1.leftSplit(3), isc::OutOfRange);
getDataCheck("\007example\003org\000", 13, 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);
+ EXPECT_THROW(ls1.rightSplit(100), isc::OutOfRange);
+ EXPECT_THROW(ls1.rightSplit(5), isc::OutOfRange);
+ EXPECT_THROW(ls1.rightSplit(4), isc::OutOfRange);
+ EXPECT_THROW(ls1.rightSplit(3), isc::OutOfRange);
getDataCheck("\007example\003org\000", 13, ls1);
}
TEST_F(LabelSequenceTest, getLabelCount) {
EXPECT_EQ(3, ls1.getLabelCount());
- ls1.split(0);
+ ls1.leftSplit(0);
EXPECT_EQ(3, ls1.getLabelCount());
- ls1.split(1);
+ ls1.leftSplit(1);
EXPECT_EQ(2, ls1.getLabelCount());
- ls1.split(1);
+ ls1.leftSplit(1);
EXPECT_EQ(1, ls1.getLabelCount());
EXPECT_EQ(3, ls2.getLabelCount());
- ls2.split(-1);
+ ls2.rightSplit(1);
EXPECT_EQ(2, ls2.getLabelCount());
- ls2.split(-1);
+ ls2.rightSplit(1);
EXPECT_EQ(1, ls2.getLabelCount());
EXPECT_EQ(3, ls3.getLabelCount());
- ls3.split(-2);
+ ls3.rightSplit(2);
EXPECT_EQ(1, ls3.getLabelCount());
EXPECT_EQ(5, ls4.getLabelCount());
- ls4.split(-3);
+ ls4.rightSplit(3);
EXPECT_EQ(2, ls4.getLabelCount());
EXPECT_EQ(3, ls5.getLabelCount());
- ls5.split(2);
+ ls5.leftSplit(2);
EXPECT_EQ(1, ls5.getLabelCount());
}
@@ -221,11 +221,11 @@ TEST_F(LabelSequenceTest, comparePart) {
EXPECT_FALSE(ls1.equals(ls8));
// Split root label from example.org.
- ls1.split(-1);
+ ls1.rightSplit(1);
// Split foo from foo.example.org.bar.
- ls8.split(1);
+ ls8.leftSplit(1);
// Split bar. (i.e. bar and root) too
- ls8.split(-2);
+ ls8.rightSplit(2);
EXPECT_TRUE(ls1.equals(ls8));
@@ -238,16 +238,16 @@ TEST_F(LabelSequenceTest, comparePart) {
TEST_F(LabelSequenceTest, isAbsolute) {
ASSERT_TRUE(ls1.isAbsolute());
- ls1.split(1);
+ ls1.leftSplit(1);
ASSERT_TRUE(ls1.isAbsolute());
- ls1.split(-1);
+ ls1.rightSplit(1);
ASSERT_FALSE(ls1.isAbsolute());
ASSERT_TRUE(ls2.isAbsolute());
- ls2.split(-1);
+ ls2.rightSplit(1);
ASSERT_FALSE(ls2.isAbsolute());
ASSERT_TRUE(ls3.isAbsolute());
- ls3.split(2);
+ ls3.leftSplit(2);
ASSERT_TRUE(ls3.isAbsolute());
}
More information about the bind10-changes
mailing list