[2498] move out some common charstrings functions

And add a number of tests

src/lib/dns/rdata/generic/detail/char_string.cc

@@ -14,9 +14,11 @@
 
 #include <exceptions/exceptions.h>
 
+#include <dns/exceptions.h>
 #include <dns/rdata.h>
 #include <dns/master_lexer.h>
 #include <dns/rdata/generic/detail/char_string.h>
+#include <util/buffer.h>
 
 #include <boost/lexical_cast.hpp>
 
@@ -114,6 +116,50 @@ charStringToString(const CharString& char_string) {
     return (s);
 }
 
+int compareCharStrings(const detail::CharString& self,
+                       const detail::CharString& other) {
+    const size_t self_len = self[0];
+    const size_t other_len = other[0];
+    const size_t cmp_len = std::min(self_len, other_len);
+    const int cmp = memcmp(&self[1], &other[1], cmp_len);
+    if (cmp < 0) {
+        return (-1);
+    } else if (cmp > 0) {
+        return (1);
+    } else if (self_len < other_len) {
+        return (-1);
+    } else if (self_len > other_len) {
+        return (1);
+    } else {
+        return (0);
+    }
+}
+
+size_t
+bufferToCharString(isc::util::InputBuffer& buffer, size_t rdata_len,
+                   CharString& target) {
+    if (rdata_len < 1 || buffer.getLength() < 1) {
+        isc_throw(isc::dns::DNSMessageFORMERR,
+                  "insufficient data to read character-string length");
+    }
+    const uint8_t len = buffer.readUint8();
+    if (rdata_len < len + 1) {
+        isc_throw(isc::dns::DNSMessageFORMERR,
+                  "character string length is too large: " <<
+                  static_cast<int>(len));
+    }
+    if (buffer.getLength() < len) {
+        isc_throw(isc::dns::DNSMessageFORMERR,
+                  "not enough data in buffer to read character-string");
+    }
+
+    target.resize(len + 1);
+    target[0] = len;
+    buffer.readData(&target[0] + 1, len);
+
+    return (len + 1);
+}
+
 } // end of detail
 } // end of generic
 } // end of rdata

src/lib/dns/rdata/generic/detail/char_string.h

@@ -19,6 +19,7 @@
 
 #include <string>
 #include <vector>
+#include <algorithm>
 #include <stdint.h>
 
 namespace isc {
@@ -66,6 +67,38 @@ void stringToCharString(const MasterToken::StringRegion& str_region,
 /// \return A string representation of \c char_string.
 std::string charStringToString(const CharString& char_string);
 
+/// \brief Compare two CharString objects
+///
+/// \param self The CharString field to compare
+/// \param other The CharString field to compare to
+///
+/// \return -1 if \c self would be sorted before \c other
+///          1 if \c self would be sorted after \c other
+///          0 if \c self and \c other are equal
+int compareCharStrings(const CharString& self, const CharString& other);
+
+/// \brief Convert a buffer containing a character-string to CharString
+///
+/// This method reads one character-string from the given buffer (in wire
+/// format) and places the result in the given \c CharString object.
+/// Since this is expected to be used in message parsing, the exception it
+/// raises is of that type.
+///
+/// On success, the buffer position is advanced to the end of the char-string,
+/// and the number of bytes read is returned.
+///
+/// \param buffer The buffer to read from.
+/// \param rdata_len The total size of the rr's rdata currently being read
+/// (used for integrity checks in the wire data)
+/// \param target The \c CharString where the result will be stored. Any
+/// existing data in the target will be overwritten.
+/// \throw DNSMessageFormERR If the available data is not enough to read
+/// the character-string, or if the character-string length is out of bounds
+/// \return The number of bytes read
+size_t bufferToCharString(isc::util::InputBuffer& buffer, size_t rdata_len,
+                          CharString& target);
+
+
 } // namespace detail
 } // namespace generic
 } // namespace rdata

src/lib/dns/rdata/generic/hinfo_13.cc

@@ -35,67 +35,6 @@ using namespace isc::dns;
 // BEGIN_ISC_NAMESPACE
 // BEGIN_RDATA_NAMESPACE
 
-// helper(s)
-namespace {
-
-/// \brief Reads one text field from the given buffer into the given CharString
-///
-/// \return The number of bytes read
-size_t
-readTextField(detail::CharString& target, InputBuffer& buffer,
-              size_t rdata_len) {
-    if (rdata_len < 1 || buffer.getLength() < 1) {
-        isc_throw(isc::dns::DNSMessageFORMERR, "Error in parsing "
-                  "HINFO RDATA: insufficient data");
-    }
-    const uint8_t len = buffer.readUint8();
-    if (rdata_len < len + 1) {
-        isc_throw(isc::dns::DNSMessageFORMERR, "Error in parsing "
-                  "HINFO RDATA: character string length is too large: " <<
-                  static_cast<int>(len));
-    }
-    if (buffer.getLength() < len) {
-        isc_throw(isc::dns::DNSMessageFORMERR, "Error in parsing "
-                  "HINFO RDATA: not enough data in buffer to read: " <<
-                  static_cast<int>(len) << " bytes");
-    }
-
-    target.resize(len + 1);
-    target[0] = len;
-    buffer.readData(&target[0] + 1, len);
-
-    return (len + 1);
-}
-
-/// \brief Compare one CharString field to another
-///
-/// \param self The CharString field to compare
-/// \param other The CharString field to compare to
-///
-/// \return -1 if \c self would be sorted before \c other
-///          1 if \c self would be sorted after \c other
-///          0 if \c self and \c other are equal
-int compareField(const detail::CharString& self,
-                 const detail::CharString& other) {
-    const size_t self_len = self[0];
-    const size_t other_len = other[0];
-    const size_t cmp_len = min(self_len, other_len);
-    const int cmp = memcmp(&self[1], &other[1], cmp_len);
-    if (cmp < 0) {
-        return (-1);
-    } else if (cmp > 0) {
-        return (1);
-    } else if (self_len < other_len) {
-        return (-1);
-    } else if (self_len > other_len) {
-        return (1);
-    } else {
-        return (0);
-    }
-}
-
-} // end unnamed namespace
-
 class HINFOImpl {
 public:
     HINFOImpl(const std::string& hinfo_str) {
@@ -118,8 +57,8 @@ public:
     }
 
     HINFOImpl(InputBuffer& buffer, size_t rdata_len) {
-        rdata_len -= readTextField(cpu, buffer, rdata_len);
-        rdata_len -= readTextField(os, buffer, rdata_len);
+        rdata_len -= detail::bufferToCharString(buffer, rdata_len, cpu);
+        rdata_len -= detail::bufferToCharString(buffer, rdata_len, os);
         if (rdata_len != 0) {
             isc_throw(isc::dns::DNSMessageFORMERR, "Error in parsing " <<
                       "HINFO RDATA: bytes left at end: " <<
@@ -193,11 +132,11 @@ int
 HINFO::compare(const Rdata& other) const {
     const HINFO& other_hinfo = dynamic_cast<const HINFO&>(other);
 
-    const int cmp = compareField(impl_->cpu, other_hinfo.impl_->cpu);
+    const int cmp = compareCharStrings(impl_->cpu, other_hinfo.impl_->cpu);
     if (cmp != 0) {
         return (cmp);
     }
-    return (compareField(impl_->os, other_hinfo.impl_->os));
+    return (compareCharStrings(impl_->os, other_hinfo.impl_->os));
 }
 
 const std::string

src/lib/dns/rdata/generic/naptr_35.cc

@@ -45,9 +45,9 @@ public:
         preference = buffer.readUint16();
         rdata_len -= 4;
 
-        rdata_len -= readTextField(flags, buffer, rdata_len);
-        rdata_len -= readTextField(services, buffer, rdata_len);
-        rdata_len -= readTextField(regexp, buffer, rdata_len);
+        rdata_len -= detail::bufferToCharString(buffer, rdata_len, flags);
+        rdata_len -= detail::bufferToCharString(buffer, rdata_len, services);
+        rdata_len -= detail::bufferToCharString(buffer, rdata_len, regexp);
         replacement = Name(buffer);
         if (rdata_len < 1) {
             isc_throw(isc::dns::DNSMessageFORMERR, "Error in parsing "
@@ -192,22 +192,22 @@ NAPTR::compare(const Rdata& other) const {
         return (1);
     }
 
-    if (impl_->flags < other_naptr.impl_->flags) {
-        return (-1);
-    } else if (impl_->flags > other_naptr.impl_->flags) {
-        return (1);
+    const int fcmp = detail::compareCharStrings(impl_->flags,
+                                                other_naptr.impl_->flags);
+    if (fcmp != 0) {
+        return (fcmp);
     }
 
-    if (impl_->services < other_naptr.impl_->services) {
-        return (-1);
-    } else if (impl_->services > other_naptr.impl_->services) {
-        return (1);
+    const int scmp = detail::compareCharStrings(impl_->services,
+                                                other_naptr.impl_->services);
+    if (scmp != 0) {
+        return (scmp);
     }
 
-    if (impl_->regexp < other_naptr.impl_->regexp) {
-        return (-1);
-    } else if (impl_->regexp > other_naptr.impl_->regexp) {
-        return (1);
+    const int rcmp = detail::compareCharStrings(impl_->regexp,
+                                                other_naptr.impl_->regexp);
+    if (rcmp != 0) {
+        return (rcmp);
     }
 
     return (compareNames(impl_->replacement, other_naptr.impl_->replacement));

src/lib/dns/tests/rdata_naptr_unittest.cc

@@ -199,16 +199,27 @@ TEST_F(Rdata_NAPTR_Test, compare) {
     NAPTR naptr_large5(naptr_str_large5);
 
     EXPECT_EQ(0, naptr.compare(NAPTR(naptr_str)));
-    EXPECT_EQ(1, naptr.compare(NAPTR(naptr_str_small1)));
-    EXPECT_EQ(1, naptr.compare(NAPTR(naptr_str_small2)));
-    EXPECT_EQ(1, naptr.compare(NAPTR(naptr_str_small3)));
-    EXPECT_EQ(1, naptr.compare(NAPTR(naptr_str_small4)));
-    EXPECT_EQ(1, naptr.compare(NAPTR(naptr_str_small5)));
-    EXPECT_EQ(-1, naptr.compare(NAPTR(naptr_str_large1)));
-    EXPECT_EQ(-1, naptr.compare(NAPTR(naptr_str_large2)));
-    EXPECT_EQ(-1, naptr.compare(NAPTR(naptr_str_large3)));
-    EXPECT_EQ(-1, naptr.compare(NAPTR(naptr_str_large4)));
-    EXPECT_EQ(-1, naptr.compare(NAPTR(naptr_str_large5)));
+    EXPECT_EQ(1, naptr.compare(naptr_small1));
+    EXPECT_EQ(1, naptr.compare(naptr_small2));
+    EXPECT_EQ(1, naptr.compare(naptr_small3));
+    EXPECT_EQ(1, naptr.compare(naptr_small4));
+    EXPECT_EQ(1, naptr.compare(naptr_small5));
+    EXPECT_EQ(-1, naptr.compare(naptr_large1));
+    EXPECT_EQ(-1, naptr.compare(naptr_large2));
+    EXPECT_EQ(-1, naptr.compare(naptr_large3));
+    EXPECT_EQ(-1, naptr.compare(naptr_large4));
+    EXPECT_EQ(-1, naptr.compare(naptr_large5));
+    EXPECT_EQ(-1, naptr_small1.compare(naptr));
+    EXPECT_EQ(-1, naptr_small2.compare(naptr));
+    EXPECT_EQ(-1, naptr_small3.compare(naptr));
+    EXPECT_EQ(-1, naptr_small4.compare(naptr));
+    EXPECT_EQ(-1, naptr_small5.compare(naptr));
+    EXPECT_EQ(1, naptr_large1.compare(naptr));
+    EXPECT_EQ(1, naptr_large2.compare(naptr));
+    EXPECT_EQ(1, naptr_large3.compare(naptr));
+    EXPECT_EQ(1, naptr_large4.compare(naptr));
+    EXPECT_EQ(1, naptr_large5.compare(naptr));
+
 }
 
 }