added the concatenate() method

fixed signed/unsigned bug
added some more test cases


git-svn-id: svn://bind10.isc.org/svn/bind10/branches/jinmei-dnsmessageapi@361 e5f2f494-b856-4b98-b285-d166d9295462

src/lib/dns/cpp/name.cc

@@ -16,6 +16,7 @@
 
 #include <cctype>
 #include <cassert>
+#include <iterator>
 
 #include "buffer.h"
 #include "name.h"
@@ -76,11 +77,6 @@ typedef enum {
 
 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 @@ Name::Name(const std::string &namestring, bool downcase)
     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) {
+    while (ndata.size() < Name::MAX_WIRE && s != send && !done) {
-        c = *s++;
+        char c = *s++;
 
         switch (state) {
         case ft_init:
@@ -121,31 +124,31 @@ Name::Name(const std::string &namestring, bool downcase)
             }
 
             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;
+                ndata.at(offsets.back()) = count;
-                offsets.push_back(ndata_.size());
+                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 @@ Name::Name(const std::string &namestring, bool downcase)
                 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 @@ Name::Name(const std::string &namestring, bool downcase)
                 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 @@ Name::Name(const std::string &namestring, bool downcase)
                 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 @@ Name::Name(const std::string &namestring, bool downcase)
     }
 
     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 @@ Name::Name(const std::string &namestring, bool downcase)
         }
         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 @@ typedef enum {
 Name::Name(InputBuffer& buffer, bool downcase)
 {
     unsigned int new_current;
-    std::vector<char> offsets;
+    std::vector<unsigned char> offsets;
-    offsets.reserve(128);
+    offsets.reserve(Name::MAX_WIRE / 2);
 
     /*
      * Initialize things to make the compiler happy; they're not required.
@@ -357,10 +361,11 @@ Name::toText(bool omit_final_dot) const
         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 @@ Name::toText(bool omit_final_dot) const
 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 @@ Name::compare(const Name& other) const
         --l;
         --l1;
         --l2;
-        pos1 = offsets_[l1];
+        size_t pos1 = offsets_[l1];
-        pos2 = other.offsets_[l2];
+        size_t pos2 = other.offsets_[l2];
-        count1 = ndata_[pos1++];
+        unsigned int count1 = ndata_[pos1++];
-        count2 = other.ndata_[pos2++];
+        unsigned int count2 = other.ndata_[pos2++];
-        label1 = ndata_[pos1];
-        label2 = 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;
+        int cdiff = (int)count1 - (int)count2;
-        if (cdiff < 0)
+        unsigned int count = (cdiff < 0) ? count1 : count2;
-            count = count1;
-        else
-            count = 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];
+            ++pos1;
-            label2 = other.ndata_[++pos2];
+            ++pos2;
         }
         if (cdiff != 0) {
                 return (NameComparisonResult(cdiff, nlabels,
@@ -493,32 +490,28 @@ Name::compare(const Name& other) const
     return (NameComparisonResult(ldiff, nlabels, NameComparisonResult::EQUAL));
 }
 
-// Are 'this' name and 'other' equal?
 bool
-Name::operator==(const Name& other) const
+Name::equals(const Name& other) const
 {
-    unsigned int l;
-    unsigned char c, count;
-    std::string::const_iterator label1, label2;
-
     if (length_ != other.length_ || labels_ != other.labels_) {
         return (false);
     }
 
-    l = labels_;
+    for (unsigned int l = labels_, pos = 0; l > 0; --l) {
-    label1 = ndata_.begin();
+        unsigned char count = ndata_[pos];
-    label2 = other.ndata_.begin();
+        if (count != other.ndata_[pos]) {
-    while (l > 0) {
-        l--;
-        count = *label1++;
-        if (count != *label2++) {
             return (false);
         }
+        ++pos;
 
         while (count-- > 0) {
-            c = maptolower[(unsigned char)*label1++]; // XXX should avoid cast
+            unsigned char label1 = ndata_[pos];
-            if (c != maptolower[(unsigned char)*label2++])
+            unsigned char label2 = other.ndata_[pos];
+
+            if (maptolower[label1] != maptolower[label2]) {
                 return (false);
+            }
+            ++pos;
         }
     }
 
@@ -531,6 +524,51 @@ Name::isWildcard() const
     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)
 {

src/lib/dns/cpp/name.h

@@ -185,13 +185,14 @@ private:
 /// 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 @@ public:
     /// 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 @@ public:
 
     /// \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;
+    /// \return true if the two names are equal; otherwise false.
-    /// 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.
+    /// This method simply negates the result of \c equal() method, and in that
-    /// \param other the <code>Name</code> object to compare against.
+    /// sense it's redundant.  The separate method is provided just for
-    /// \return true if <code>compare(other).get_order()</code> is non 0;
+    /// convenience.
-    /// otherwise false.
+    bool nequals(const Name& other) const { return !(this->equals(other)); }
-    bool nequals(const Name& other) const;
 
     /// 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 @@ public:
     /// \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 @@ public:
 
     /// \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 @@ public:
 
     /// \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 @@ public:
     /// \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_;
 

src/lib/dns/cpp/name_unittest.cc

@@ -38,6 +38,7 @@ protected:
     NameTest() : example_name("www.example.com") {}
     Name example_name;
 
+    static const size_t MAX_LABELS = Name::MAX_LABELS;
     //
     // helper methods
     //
@@ -128,6 +129,17 @@ TEST_F(NameTest, fromText)
                          "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, compare)
     }
 }
 
+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());
+}
 }