Parcourir la source

[2383] Extract name parser to separate function

This way it can be reused by the about to be new constructor.
Michal 'vorner' Vaner il y a 12 ans
Parent
commit
67500d3bd8
2 fichiers modifiés avec 35 ajouts et 10 suppressions
  1. 28 10
      src/lib/dns/name.cc
  2. 7 0
      src/lib/dns/tests/name_unittest.cc

+ 28 - 10
src/lib/dns/name.cc

@@ -129,9 +129,15 @@ typedef enum {
     // parser where @ is used to mean an origin name.
     ft_at
 } ft_state;
-}
 
-Name::Name(const std::string &namestring, bool downcase) {
+// The parser of name from a string. It is a template, because
+// some parameters are used with two different types, while others
+// are private type aliases.
+template<class String, class Iterator, class Offsets, class Data>
+void
+stringParse(const String& namestring, Iterator s, Iterator send,
+            bool downcase, Offsets& offsets, Data& ndata)
+{
     //
     // Initialize things to make the compiler happy; they're not required.
     //
@@ -142,17 +148,13 @@ Name::Name(const std::string &namestring, bool downcase) {
     //
     // Set up the state machine.
     //
-    std::string::const_iterator s = namestring.begin();
-    std::string::const_iterator send = namestring.end();
     bool done = false;
     bool is_root = false;
     ft_state state = ft_init;
 
-    NameOffsets offsets;
+    // Prepare the output buffers.
     offsets.reserve(Name::MAX_LABELS);
     offsets.push_back(0);
-
-    NameString ndata;
     ndata.reserve(Name::MAX_WIRE);
 
     // should we refactor this code using, e.g, the state pattern?  Probably
@@ -212,7 +214,7 @@ Name::Name(const std::string &namestring, bool downcase) {
             } else if (c == '\\') {
                 state = ft_escape;
             } else {
-                if (++count > MAX_LABELLEN) {
+                if (++count > Name::MAX_LABELLEN) {
                     isc_throw(TooLongLabel,
                               "label is too long in " << namestring);
                 }
@@ -230,7 +232,7 @@ Name::Name(const std::string &namestring, bool downcase) {
             // FALLTHROUGH
         case ft_escape:
             if (!isdigit(c & 0xff)) {
-                if (++count > MAX_LABELLEN) {
+                if (++count > Name::MAX_LABELLEN) {
                     isc_throw(TooLongLabel,
                               "label is too long in " << namestring);
                 }
@@ -257,7 +259,7 @@ Name::Name(const std::string &namestring, bool downcase) {
                               "escaped decimal is too large in "
                               << namestring);
                 }
-                if (++count > MAX_LABELLEN) {
+                if (++count > Name::MAX_LABELLEN) {
                     isc_throw(TooLongLabel,
                               "label is too long in " << namestring);
                 }
@@ -291,7 +293,23 @@ Name::Name(const std::string &namestring, bool downcase) {
             ndata.push_back('\0');
         }
     }
+}
+
+}
+
+Name::Name(const std::string &namestring, bool downcase) {
+    // Prepare inputs for the parser
+    std::string::const_iterator s = namestring.begin();
+    std::string::const_iterator send = namestring.end();
+
+    // Prepare outputs
+    NameOffsets offsets;
+    NameString ndata;
+
+    // To the parsing
+    stringParse(namestring, s, send, downcase, offsets, ndata);
 
+    // And get the output
     labelcount_ = offsets.size();
     assert(labelcount_ > 0 && labelcount_ <= Name::MAX_LABELS);
     ndata_.assign(ndata.data(), ndata.size());

+ 7 - 0
src/lib/dns/tests/name_unittest.cc

@@ -157,11 +157,14 @@ checkBadTextName(const string& txt) {
     // NameParserException.
     EXPECT_THROW(Name(txt, false), ExceptionType);
     EXPECT_THROW(Name(txt, false), NameParserException);
+#if 0
+    // TODO: Enable once the new constructor exists.
     // The same is thrown when constructing by the master-file constructor
     EXPECT_THROW(Name(txt.c_str(), txt.length(), &Name::ROOT_NAME()),
                  ExceptionType);
     EXPECT_THROW(Name(txt.c_str(), txt.length(), &Name::ROOT_NAME()),
                  NameParserException);
+#endif
 }
 
 TEST_F(NameTest, fromText) {
@@ -236,6 +239,9 @@ TEST_F(NameTest, fromText) {
     EXPECT_EQ(Name::MAX_LABELS, maxlabels.getLabelCount());
 }
 
+#if 0
+// TODO: Enable once the constructor exists, there's a need to test the changes
+// on the rest while we prepare it.
 // Check the @ syntax is accepted and it just copies the origin.
 TEST_F(NameTest, copyOrigin) {
     EXPECT_EQ(origin_name, Name("@", 1, &origin_name));
@@ -293,6 +299,7 @@ TEST_F(NameTest, combinedTooLong) {
     EXPECT_NO_THROW(Name(max_labels_str, strlen(max_labels_str),
                          &Name::ROOT_NAME()));
 }
+#endif
 
 TEST_F(NameTest, fromWire) {
     //