[svn] commit: r361 - in /branches/jinmei-dnsmessageapi/src/lib/dns/cpp: name.cc name.h name_unittest.cc
BIND 10 source code commits
bind10-changes at lists.isc.org
Sat Dec 12 02:44:57 UTC 2009
Author: jinmei
Date: Sat Dec 12 02:44:57 2009
New Revision: 361
Log:
added the concatenate() method
fixed signed/unsigned bug
added some more test cases
Modified:
branches/jinmei-dnsmessageapi/src/lib/dns/cpp/name.cc
branches/jinmei-dnsmessageapi/src/lib/dns/cpp/name.h
branches/jinmei-dnsmessageapi/src/lib/dns/cpp/name_unittest.cc
Modified: branches/jinmei-dnsmessageapi/src/lib/dns/cpp/name.cc
==============================================================================
--- branches/jinmei-dnsmessageapi/src/lib/dns/cpp/name.cc (original)
+++ branches/jinmei-dnsmessageapi/src/lib/dns/cpp/name.cc Sat Dec 12 02:44:57 2009
@@ -16,6 +16,7 @@
#include <cctype>
#include <cassert>
+#include <iterator>
#include "buffer.h"
#include "name.h"
@@ -76,11 +77,6 @@
Name::Name(const std::string &namestring, bool downcase)
{
- char c;
- std::vector<char> offsets;
- offsets.reserve(128);
- offsets.push_back(0);
-
//
// Initialize things to make the compiler happy; they're not required.
//
@@ -97,13 +93,20 @@
bool is_root = false;
ft_state state = ft_init;
+ std::vector<unsigned char> offsets;
+ offsets.reserve(128);
+ offsets.push_back(0);
+
+ std::string ndata;
+ ndata.reserve(Name::MAX_WIRE);
+
// should we refactor this code using, e.g, the state pattern? Probably
// not at this point, as this is based on proved code (derived from BIND9)
// and it's less likely that we'll have more variations in the domain name
// syntax. If this ever happens next time, we should consider refactor
// the code, rather than adding more states and cases below.
- while (ndata_.size() < Name::MAX_WIRE && s != send && !done) {
- c = *s++;
+ while (ndata.size() < Name::MAX_WIRE && s != send && !done) {
+ char c = *s++;
switch (state) {
case ft_init:
@@ -121,31 +124,31 @@
}
if (is_root) {
- ndata_.push_back(0);
+ ndata.push_back(0);
done = true;
break;
}
// FALLTHROUGH
case ft_start: // begin of a label
- ndata_.push_back(0); // placeholder for the label length field
+ ndata.push_back(0); // placeholder for the label length field
count = 0;
if (c == '\\') {
state = ft_initialescape;
break;
}
state = ft_ordinary;
- assert(ndata_.size() < Name::MAX_WIRE);
+ assert(ndata.size() < Name::MAX_WIRE);
// FALLTHROUGH
case ft_ordinary: // parsing a normal label
if (c == '.') {
if (count == 0) {
dns_throw(EmptyLabel, "duplicate period");
}
- ndata_[offsets.back()] = count;
- offsets.push_back(ndata_.size());
+ ndata.at(offsets.back()) = count;
+ offsets.push_back(ndata.size());
if (s == send) {
- ndata_.push_back(0);
+ ndata.push_back(0);
done = true;
}
state = ft_start;
@@ -155,7 +158,7 @@
if (++count > Name::MAX_LABELLEN) {
dns_throw(TooLongLabel, "label is too long");
}
- ndata_.push_back(downcase ? maptolower[c] : c);
+ ndata.push_back(downcase ? maptolower[c] : c);
}
break;
case ft_initialescape: // just found '\'
@@ -171,7 +174,7 @@
if (++count > Name::MAX_LABELLEN) {
dns_throw(TooLongLabel, "label is too long");
}
- ndata_.push_back(downcase ? maptolower[c] : c);
+ ndata.push_back(downcase ? maptolower[c] : c);
state = ft_ordinary;
break;
}
@@ -193,7 +196,7 @@
if (++count > Name::MAX_LABELLEN) {
dns_throw(TooLongLabel, "label is too long");
}
- ndata_.push_back(downcase ? maptolower[value] : value);
+ ndata.push_back(downcase ? maptolower[value] : value);
state = ft_ordinary;
}
break;
@@ -204,7 +207,7 @@
}
if (!done) { // no trailing '.' was found.
- if (ndata_.size() == Name::MAX_WIRE) {
+ if (ndata.size() == Name::MAX_WIRE) {
dns_throw(TooLongName, "name is too long for termination");
}
assert(s == send);
@@ -213,16 +216,17 @@
}
if (state == ft_ordinary) {
assert(count != 0);
- ndata_[offsets.back()] = count;
+ ndata.at(offsets.back()) = count;
- offsets.push_back(ndata_.size());
+ offsets.push_back(ndata.size());
// add a trailing \0
- ndata_.push_back('\0');
+ ndata.push_back('\0');
}
}
labels_ = offsets.size();
- assert(labels_ <= 127);
+ assert(labels_ > 0 && labels_ <= Name::MAX_LABELS);
+ ndata_.assign(ndata.data(), ndata.size());
length_ = ndata_.size();
offsets_.assign(offsets.begin(), offsets.end());
}
@@ -236,8 +240,8 @@
Name::Name(InputBuffer& buffer, bool downcase)
{
unsigned int new_current;
- std::vector<char> offsets;
- offsets.reserve(128);
+ std::vector<unsigned char> offsets;
+ offsets.reserve(Name::MAX_WIRE / 2);
/*
* Initialize things to make the compiler happy; they're not required.
@@ -357,10 +361,11 @@
return (".");
}
- unsigned int count;
std::string::const_iterator np = ndata_.begin();
std::string::const_iterator np_end = ndata_.end();
unsigned int labels = labels_; // use for integrity check
+ // init with an impossible value to catch error cases in the end:
+ unsigned int count = Name::MAX_LABELLEN + 1;
// result string: it will roughly have the same length as the wire format
// name data. reserve that length to minimize reallocation.
@@ -428,12 +433,6 @@
NameComparisonResult
Name::compare(const Name& other) const
{
- unsigned int count1, count2, count;
- int cdiff, chdiff;
- unsigned char label1, label2;
- size_t pos1, pos2;
- NameComparisonResult::NameRelation namereln;
-
// Determine the relative ordering under the DNSSEC order relation of
// 'this' and 'other', and also determine the hierarchical relationship
// of the names.
@@ -448,32 +447,30 @@
--l;
--l1;
--l2;
- pos1 = offsets_[l1];
- pos2 = other.offsets_[l2];
- count1 = ndata_[pos1++];
- count2 = other.ndata_[pos2++];
- label1 = ndata_[pos1];
- label2 = other.ndata_[pos2];
+ size_t pos1 = offsets_[l1];
+ size_t pos2 = other.offsets_[l2];
+ 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 <= Name::MAX_LABELLEN && count2 <= Name::MAX_LABELLEN);
- cdiff = (int)count1 - (int)count2;
- if (cdiff < 0)
- count = count1;
- else
- count = count2;
+ int cdiff = (int)count1 - (int)count2;
+ unsigned int count = (cdiff < 0) ? count1 : count2;
while (count > 0) {
- chdiff = (int)maptolower[label1] - (int)maptolower[label2];
+ unsigned char label1 = ndata_[pos1];
+ unsigned char label2 = other.ndata_[pos2];
+
+ int chdiff = (int)maptolower[label1] - (int)maptolower[label2];
if (chdiff != 0) {
return (NameComparisonResult(chdiff, nlabels,
NameComparisonResult::COMMONANCESTOR));
}
--count;
- label1 = ndata_[++pos1];
- label2 = other.ndata_[++pos2];
+ ++pos1;
+ ++pos2;
}
if (cdiff != 0) {
return (NameComparisonResult(cdiff, nlabels,
@@ -493,32 +490,28 @@
return (NameComparisonResult(ldiff, nlabels, NameComparisonResult::EQUAL));
}
-// Are 'this' name and 'other' equal?
bool
-Name::operator==(const Name& other) const
-{
- unsigned int l;
- unsigned char c, count;
- std::string::const_iterator label1, label2;
-
+Name::equals(const Name& other) const
+{
if (length_ != other.length_ || labels_ != other.labels_) {
return (false);
}
- l = labels_;
- label1 = ndata_.begin();
- label2 = other.ndata_.begin();
- while (l > 0) {
- l--;
- count = *label1++;
- if (count != *label2++) {
+ for (unsigned int l = labels_, pos = 0; l > 0; --l) {
+ unsigned char count = ndata_[pos];
+ if (count != other.ndata_[pos]) {
return (false);
}
+ ++pos;
while (count-- > 0) {
- c = maptolower[(unsigned char)*label1++]; // XXX should avoid cast
- if (c != maptolower[(unsigned char)*label2++])
+ unsigned char label1 = ndata_[pos];
+ unsigned char label2 = other.ndata_[pos];
+
+ if (maptolower[label1] != maptolower[label2]) {
return (false);
+ }
+ ++pos;
}
}
@@ -531,6 +524,51 @@
return (length_ >= 2 && ndata_[0] == 1 && ndata_[1] == '*');
}
+namespace { // hide the local class
+struct OffsetAdjuster : public std::binary_function<unsigned char,
+ unsigned int,
+ unsigned char> {
+ unsigned char operator()(unsigned char ch, unsigned int offset) const
+ {
+ return (ch + offset);
+ }
+};
+}
+
+Name
+Name::concatenate(const Name& suffix) const
+{
+ assert(this->length_ > 0 && suffix.length_ > 0);
+ assert(this->labels_ > 0 && suffix.labels_ > 0);
+
+ unsigned int length = this->length_ + suffix.length_ - 1;
+ if (length > Name::MAX_WIRE) {
+ dns_throw(TooLongName, "names are too long to concatenate");
+ }
+
+ Name retname;
+ retname.ndata_.reserve(length);
+ retname.ndata_.assign(this->ndata_, 0, this->length_ - 1);
+ retname.ndata_.insert(retname.ndata_.end(),
+ suffix.ndata_.begin(), suffix.ndata_.end());
+ assert(retname.ndata_.size() == length);
+ retname.length_ = length;
+
+ unsigned int labels = this->labels_ + suffix.labels_ - 1;
+ assert(labels <= Name::MAX_LABELS);
+ retname.offsets_.reserve(labels);
+ retname.offsets_.assign(&this->offsets_[0],
+ &this->offsets_[0] + this->labels_ - 1);
+ transform(suffix.offsets_.begin(), suffix.offsets_.end(),
+ back_inserter(retname.offsets_),
+ bind2nd(OffsetAdjuster(), this->length_ - 1));
+ assert(retname.offsets_.back() == retname.length_ - 1);
+ assert(retname.offsets_.size() == labels);
+ retname.labels_ = labels;
+
+ return (retname);
+}
+
std::ostream&
operator<<(std::ostream& os, const Name& name)
{
Modified: branches/jinmei-dnsmessageapi/src/lib/dns/cpp/name.h
==============================================================================
--- branches/jinmei-dnsmessageapi/src/lib/dns/cpp/name.h (original)
+++ branches/jinmei-dnsmessageapi/src/lib/dns/cpp/name.h Sat Dec 12 02:44:57 2009
@@ -185,13 +185,14 @@
/// names as a special case.
///
class Name {
-public:
///
/// \name Constructors and Destructor
///
//@{
+private:
/// The default constructor
Name() : length_(0), labels_(0) {}
+public:
/// Constructor from a string
///
/// \param namestr A string representation of the name to be constructed.
@@ -275,7 +276,7 @@
/// returns the result in the form of a <code>NameComparisonResult</code>
/// object.
///
- /// Note that this is a case-insensitive comparison.
+ /// Note that this is case-insensitive comparison.
///
/// \param other the right-hand operand to compare against.
/// \return a <code>NameComparisonResult</code> object representing the
@@ -284,29 +285,33 @@
/// \brief Return true iff two names are equal.
///
- /// The comparison is based on the result of the compare() method.
+ /// Semantically this could be implemented based on the result of the
+ /// \c compare() method, but the actual implementation uses different code
+ /// that simply performs character-by-character comparison (case
+ /// insensitive for the name label parts) on the two names. This is because
+ /// it would be much faster and the simple equality check would be pretty
+ /// common.
+ ///
/// \param other the <code>Name</code> object to compare against.
- /// \return true if <code>compare(other).get_order()</code> is 0;
- /// otherwise false.
+ /// \return true if the two names are equal; otherwise false.
bool equals(const Name& other) const;
/// Same as equals()
- bool operator==(const Name& other) const;
+ bool operator==(const Name& other) const { return (this->equals(other)); }
/// \brief Return true iff two names are not equal.
///
- /// The comparison is based on the result of the compare() method.
- /// \param other the <code>Name</code> object to compare against.
- /// \return true if <code>compare(other).get_order()</code> is non 0;
- /// otherwise false.
- bool nequals(const Name& other) const;
+ /// This method simply negates the result of \c equal() method, and in that
+ /// sense it's redundant. The separate method is provided just for
+ /// convenience.
+ bool nequals(const Name& other) const { return !(this->equals(other)); }
/// Same as nequals()
- bool operator!=(const Name& other) const { return (!(*this == other)); }
+ bool operator!=(const Name& other) const { return (this->nequals(other)); }
/// \brief Less-than or equal comparison for Name against <code>other</code>
///
- /// The comparison is based on the result of the compare() method.
+ /// The comparison is based on the result of the \c compare() method.
/// \param other the <code>Name</code> object to compare against.
/// \return true if <code>compare(other).get_order() <= 0</code>;
/// otherwise false.
@@ -318,7 +323,7 @@
/// \brief Greater-than or equal comparison for Name against
/// <code>other</code>
///
- /// The comparison is based on the result of the compare() method.
+ /// The comparison is based on the result of the \c compare() method.
/// \param other the <code>Name</code> object to compare against.
/// \return true if <code>compare(other).get_order() >= 0</code>;
/// otherwise false.
@@ -329,7 +334,7 @@
/// \brief Less-than comparison for Name against <code>other</code>
///
- /// The comparison is based on the result of the compare() method.
+ /// The comparison is based on the result of the \c compare() method.
/// \param other the <code>Name</code> object to compare against.
/// \return true if <code>compare(other).get_order() < 0</code>;
/// otherwise false.
@@ -340,7 +345,7 @@
/// \brief Greater-than comparison for Name against <code>other</code>
///
- /// The comparison is based on the result of the compare() method.
+ /// The comparison is based on the result of the \c compare() method.
/// \param other the <code>Name</code> object to compare against.
/// \return true if <code>compare(other).get_order() > 0</code>;
/// otherwise false.
@@ -393,13 +398,19 @@
/// \brief Max allowable length of domain names.
static const size_t MAX_WIRE = 255;
+ /// \brief Max allowable labels of domain names.
+ ///
+ /// This is <code>ceil(MAX_WIRE / 2)</code>, and is equal to the number of
+ /// labels of name "a.a.a.a....a." (127 "a"'s and trailing dot).
+ static const size_t MAX_LABELS = 128;
+
/// \brief Max allowable length of labels of a domain name.
static const size_t MAX_LABELLEN = 63;
//@}
private:
std::string ndata_;
- std::vector<char> offsets_;
+ std::vector<unsigned char> offsets_;
unsigned int length_;
unsigned int labels_;
Modified: branches/jinmei-dnsmessageapi/src/lib/dns/cpp/name_unittest.cc
==============================================================================
--- branches/jinmei-dnsmessageapi/src/lib/dns/cpp/name_unittest.cc (original)
+++ branches/jinmei-dnsmessageapi/src/lib/dns/cpp/name_unittest.cc Sat Dec 12 02:44:57 2009
@@ -38,6 +38,7 @@
NameTest() : example_name("www.example.com") {}
Name example_name;
+ static const size_t MAX_LABELS = Name::MAX_LABELS;
//
// helper methods
//
@@ -128,6 +129,17 @@
"123"));
// \DDD must consist of 3 digits.
EXPECT_THROW(Name("\\12"), isc::dns::BadLabelType);
+
+ // a name with the max number of labels. should be constructed without
+ // an error, and its length should be the max value.
+ Name maxlabels = Name("0.1.2.3.4.5.6.7.8.9.0.1.2.3.4.5.6.7.8.9." // 40
+ "0.1.2.3.4.5.6.7.8.9.0.1.2.3.4.5.6.7.8.9." // 80
+ "0.1.2.3.4.5.6.7.8.9.0.1.2.3.4.5.6.7.8.9." // 120
+ "0.1.2.3.4.5.6.7.8.9.0.1.2.3.4.5.6.7.8.9." // 160
+ "0.1.2.3.4.5.6.7.8.9.0.1.2.3.4.5.6.7.8.9." // 200
+ "0.1.2.3.4.5.6.7.8.9.0.1.2.3.4.5.6.7.8.9." // 240
+ "0.1.2.3.4.5.6.");
+ EXPECT_EQ(MAX_LABELS, maxlabels.getLabels());
}
TEST_F(NameTest, fromWire)
@@ -250,10 +262,31 @@
}
}
+TEST_F(NameTest, equal)
+{
+ EXPECT_TRUE(example_name == Name("WWW.EXAMPLE.COM."));
+ EXPECT_TRUE(example_name.equals(Name("WWW.EXAMPLE.COM.")));
+ EXPECT_TRUE(example_name != Name("www.example.org."));
+ EXPECT_TRUE(example_name.nequals(Name("www.example.org.")));
+}
+
TEST_F(NameTest, isWildcard)
{
EXPECT_EQ(false, example_name.isWildcard());
EXPECT_EQ(true, Name("*.a.example.com").isWildcard());
EXPECT_EQ(false, Name("a.*.example.com").isWildcard());
}
-}
+
+TEST_F(NameTest, concatenate)
+{
+ NameComparisonResult result =
+ Name("aaa.www.example.com.").compare(Name("aaa").concatenate(example_name));
+ EXPECT_EQ(NameComparisonResult::EQUAL, result.getRelation());
+
+ result = example_name.compare(Name(".").concatenate(example_name));
+ EXPECT_EQ(NameComparisonResult::EQUAL, result.getRelation());
+
+ result = example_name.compare(example_name.concatenate(Name(".")));
+ EXPECT_EQ(NameComparisonResult::EQUAL, result.getRelation());
+}
+}
More information about the bind10-changes
mailing list