BIND 10 trac2375-r1, updated. b3bcbf0497d515f6705c930696a9a7cc7f55fbf9 [2375] Eliminate fake states

BIND 10 source code commits bind10-changes at lists.isc.org
Thu Nov 22 13:05:33 UTC 2012


The branch, trac2375-r1 has been updated
       via  b3bcbf0497d515f6705c930696a9a7cc7f55fbf9 (commit)
       via  5934fc7661d5970427abf52d7f6538ac5b697842 (commit)
       via  644d7aaedf08e25fd7f335956cdb65f0d8d5304a (commit)
       via  5069933d08928eb2933c68ce08397fa2cfdb9e70 (commit)
       via  6b846ec5edfc4833503d266a72b8d7c2ed7b29ce (commit)
       via  e27a2922bca6a9c9f53e867c187a6e589bf3175c (commit)
       via  907061f81ba62cc5f62b55e7ea2b4e28035db6a0 (commit)
       via  0c0888ef6aeb481f70d424766d7500f869899027 (commit)
      from  23b2fe710bb60d7337a67cdaea2fc2d150586f9b (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 b3bcbf0497d515f6705c930696a9a7cc7f55fbf9
Author: Michal 'vorner' Vaner <michal.vaner at nic.cz>
Date:   Thu Nov 22 14:05:15 2012 +0100

    [2375] Eliminate fake states

commit 5934fc7661d5970427abf52d7f6538ac5b697842
Author: Michal 'vorner' Vaner <michal.vaner at nic.cz>
Date:   Thu Nov 22 13:59:21 2012 +0100

    [2375] Cover the getNextToken with tests without fakes

commit 644d7aaedf08e25fd7f335956cdb65f0d8d5304a
Author: Michal 'vorner' Vaner <michal.vaner at nic.cz>
Date:   Thu Nov 22 11:53:29 2012 +0100

    [2375] Simplify the state transition design pattern
    
    We never have a chain of the transitions. We only do start and zero or
    one handle on the returned one. Simplify handle to return void, not
    NULL.
    
    Besides simplification, it allows testing without fake states (TBD) - we
    had no real state to test the multiple transitions.

commit 5069933d08928eb2933c68ce08397fa2cfdb9e70
Author: JINMEI Tatuya <jinmei at isc.org>
Date:   Tue Nov 20 16:47:54 2012 +0900

    [2375] fixed a typo in a comment

commit 6b846ec5edfc4833503d266a72b8d7c2ed7b29ce
Author: JINMEI Tatuya <jinmei at isc.org>
Date:   Thu Nov 15 14:00:40 2012 -0800

    [2375] some style matters: mostly about constness, and folded a long line.
    
    Conflicts (from cherry-pick):
    	src/lib/dns/tests/master_lexer_unittest.cc

commit e27a2922bca6a9c9f53e867c187a6e589bf3175c
Author: Michal 'vorner' Vaner <michal.vaner at nic.cz>
Date:   Thu Nov 22 11:34:48 2012 +0100

    [2375] Return a reference to the token
    
    The copy is shallow anyway, so it wouldn't survive for long.

commit 907061f81ba62cc5f62b55e7ea2b4e28035db6a0
Author: Michal 'vorner' Vaner <michal.vaner at nic.cz>
Date:   Thu Nov 22 11:17:55 2012 +0100

    [2375] Handle EOF after unbalanced parentheses
    
    If there's an error that manifests at the end of a file, like an
    unbalanced parenthesis or a quoted string that does not end, report the
    error. But also, return EOF as the next token, not an exception.

commit 0c0888ef6aeb481f70d424766d7500f869899027
Author: Michal 'vorner' Vaner <michal.vaner at nic.cz>
Date:   Thu Nov 22 11:07:03 2012 +0100

    [2375] Remove exception handling from getNextToken
    
    It was deemed unneeded and is removed therefore, to make the code
    smaller.

-----------------------------------------------------------------------

Summary of changes:
 src/lib/dns/master_lexer.cc                      |  117 ++--------
 src/lib/dns/master_lexer.h                       |   20 +-
 src/lib/dns/master_lexer_state.h                 |   31 +--
 src/lib/dns/tests/master_lexer_state_unittest.cc |   62 +++---
 src/lib/dns/tests/master_lexer_unittest.cc       |  258 +++++-----------------
 5 files changed, 125 insertions(+), 363 deletions(-)

-----------------------------------------------------------------------
diff --git a/src/lib/dns/master_lexer.cc b/src/lib/dns/master_lexer.cc
index eb4c480..e569d03 100644
--- a/src/lib/dns/master_lexer.cc
+++ b/src/lib/dns/master_lexer.cc
@@ -164,40 +164,32 @@ MasterLexer::getSourceLine() const {
     return (impl_->sources_.back()->getCurrentLine());
 }
 
-MasterLexer::Token
+const MasterLexer::Token&
 MasterLexer::getNextToken(Options options) {
     // If the source is not available
-    if (impl_->source_ == NULL || impl_->source_->atEOF()) {
+    if (impl_->source_ == NULL) {
         isc_throw(isc::InvalidOperation, "No source to read tokens from");
     }
-    // Store current state, for the case we need to restore by ungetToken
-    impl_->has_previous_ = false;
-    // If thrown in this section, nothing observable except for not having
-    // previous will happen.
+    // Store the current state so we can restore it in ungetToken
     impl_->previous_paren_count_ = impl_->paren_count_;
     impl_->previous_was_eol_ = impl_->last_was_eol_;
     impl_->source_->mark();
     impl_->has_previous_ = true;
     // Reset the token now. This is to check a token was actually produced.
     // This is debugging aid.
+    impl_->token_ = Token(Token::NO_TOKEN_PRODUCED);
+    // And get the token
 
-    // From now on, if we throw, we can at least restore the old state.
-    try {
-        impl_->token_ = Token(Token::NO_TOKEN_PRODUCED);
-        for (const State *state = start(options); state != NULL;
-             state = state->handle(*this)) {
-            // Do nothing here. All is handled in the for cycle header itself.
-        }
-        // Make sure a token was produced. Since this Can Not Happen, we assert
-        // here instead of throwing.
-        assert(impl_->token_.getType() != Token::ERROR ||
-               impl_->token_.getErrorCode() != Token::NO_TOKEN_PRODUCED);
-        return (impl_->token_);
-    } catch (...) {
-        // Restore the old state and let the exception continue.
-        ungetToken();
-        throw;
+    // This actually handles EOF internally too.
+    const State* state = State::start(*this, options);
+    if (state != NULL) {
+        state->handle(*this);
     }
+    // Make sure a token was produced. Since this Can Not Happen, we assert
+    // here instead of throwing.
+    assert(impl_->token_.getType() != Token::ERROR ||
+           impl_->token_.getErrorCode() != Token::NO_TOKEN_PRODUCED);
+    return (impl_->token_);
 }
 
 void
@@ -212,11 +204,6 @@ MasterLexer::ungetToken() {
     }
 }
 
-const State*
-MasterLexer::start(Options options) {
-    return (State::start(*this, options));
-}
-
 namespace {
 const char* const error_text[] = {
     "lexer not started",        // NOT_STARTED
@@ -267,7 +254,7 @@ class CRLF : public State {
 public:
     CRLF() {}
     virtual ~CRLF() {}          // see the base class for the destructor
-    virtual const State* handle(MasterLexer& lexer) const {
+    virtual void handle(MasterLexer& lexer) const {
         // We've just seen '\r'.  If this is part of a sequence of '\r\n',
         // we combine them as a single END-OF-LINE.  Otherwise we treat the
         // single '\r' as an EOL and continue tokeniziation from the character
@@ -284,7 +271,6 @@ public:
         }
         getLexerImpl(lexer)->token_ = Token(Token::END_OF_LINE);
         getLexerImpl(lexer)->last_was_eol_ = true;
-        return (NULL);
     }
 };
 
@@ -292,14 +278,14 @@ class String : public State {
 public:
     String() {}
     virtual ~String() {}      // see the base class for the destructor
-    virtual const State* handle(MasterLexer& lexer) const;
+    virtual void handle(MasterLexer& lexer) const;
 };
 
 class QString : public State {
 public:
     QString() {}
     virtual ~QString() {}      // see the base class for the destructor
-    virtual const State* handle(MasterLexer& lexer) const;
+    virtual void handle(MasterLexer& lexer) const;
 };
 
 // We use a common instance of a each state in a singleton-like way to save
@@ -391,7 +377,7 @@ State::start(MasterLexer& lexer, MasterLexer::Options options) {
     }
 }
 
-const State*
+void
 String::handle(MasterLexer& lexer) const {
     std::vector<char>& data = getLexerImpl(lexer)->data_;
     data.clear();
@@ -405,14 +391,14 @@ String::handle(MasterLexer& lexer) const {
             getLexerImpl(lexer)->source_->ungetChar();
             getLexerImpl(lexer)->token_ =
                 MasterLexer::Token(&data.at(0), data.size());
-            return (NULL);
+            return;
         }
         escaped = (c == '\\' && !escaped);
         data.push_back(c);
     }
 }
 
-const State*
+void
 QString::handle(MasterLexer& lexer) const {
     MasterLexer::Token& token = getLexerImpl(lexer)->token_;
     std::vector<char>& data = getLexerImpl(lexer)->data_;
@@ -423,7 +409,7 @@ QString::handle(MasterLexer& lexer) const {
         const int c = getLexerImpl(lexer)->source_->getChar();
         if (c == InputSource::END_OF_STREAM) {
             token = Token(Token::UNEXPECTED_END);
-            return (NULL);
+            return;
         } else if (c == '"') {
             if (escaped) {
                 // found escaped '"'. overwrite the preceding backslash.
@@ -432,12 +418,12 @@ QString::handle(MasterLexer& lexer) const {
                 data.back() = '"';
             } else {
                 token = MasterLexer::Token(&data.at(0), data.size(), true);
-                return (NULL);
+                return;
             }
         } else if (c == '\n' && !escaped) {
             getLexerImpl(lexer)->source_->ungetChar();
             token = Token(Token::UNBALANCED_QUOTES);
-            return (NULL);
+            return;
         } else {
             escaped = (c == '\\' && !escaped);
             data.push_back(c);
@@ -445,63 +431,6 @@ QString::handle(MasterLexer& lexer) const {
     }
 }
 
-namespace {
-
-// A fake state that just eats something from the input, pushes
-// a given token and calls a callback if it is set. It refers to
-// another state to return.
-class FakeState : public State {
-public:
-    FakeState(const State* next, size_t eat_chars,
-              MasterLexer::Token* token,
-              int paren_change, const bool* set_eol,
-              const boost::function<void (const std::string&)>& callback) :
-        next_(next),
-        eat_chars_(eat_chars),
-        token_(token),
-        paren_change_(paren_change),
-        set_eol_(set_eol),
-        callback_(callback)
-    {}
-    virtual const State* handle(MasterLexer& lexer) const {
-        std::string input;
-        for (size_t i = 0; i < eat_chars_; ++i) {
-            input += getLexerImpl(lexer)->source_->getChar();
-        }
-        if (!callback_.empty()) {
-            callback_(input);
-        }
-        if (token_ != NULL) {
-            getLexerImpl(lexer)->token_ = *token_;
-        }
-        getLexerImpl(lexer)->paren_count_ += paren_change_;
-        if (set_eol_ != NULL) {
-            getLexerImpl(lexer)->last_was_eol_ = *set_eol_;
-        }
-        return (next_);
-    }
-private:
-    const State* const next_;
-    size_t eat_chars_;
-    MasterLexer::Token* const token_;
-    const int paren_change_;
-    const bool* const set_eol_;
-    const boost::function<void (const std::string&)> callback_;
-};
-
-}
-
-State*
-State::getFakeState(const State* next, size_t eat_chars,
-                    MasterLexer::Token* token,
-                    int paren_change, const bool* set_eol,
-                    const boost::function<void (const std::string&)>& callback)
-{
-    // Just allocate new FakeState with the parameters.
-    return (new FakeState(next, eat_chars, token, paren_change, set_eol,
-                          callback));
-}
-
 } // namespace master_lexer_internal
 
 } // end of namespace dns
diff --git a/src/lib/dns/master_lexer.h b/src/lib/dns/master_lexer.h
index ce16bf4..431e0ce 100644
--- a/src/lib/dns/master_lexer.h
+++ b/src/lib/dns/master_lexer.h
@@ -191,11 +191,11 @@ public:
     /// It reads a bit of the last opened source and produces another token
     /// found in it.
     ///
-    /// This method provides the weak exception guarantee. It'll return the
-    /// class to the state before the call on exception, except that it is
-    /// not possible to call ungetToken() after the failed call. This is
-    /// considered OK, since the caller was expecting the call to succeed
-    /// in which case the previous token would not be ungettable as well.
+    /// This method does not provide the strong exception guarantee. Generally,
+    /// if it throws, the object should not be used any more and should be
+    /// discarded. It was decided all the exceptions thrown from here are
+    /// serious enough that aborting the loading process is the only reasonable
+    /// recovery anyway, so the strong exception guarantee is not needed.
     ///
     /// \param options The options can be used to modify the tokenization.
     ///     The method can be made reporting things which are usually ignored
@@ -213,7 +213,7 @@ public:
     ///     source (eg. I/O error in the file on the disk).
     /// \throw std::bad_alloc in case allocation of some internal resources
     ///     or the token fail.
-    Token getNextToken(Options options = NONE);
+    const Token& getNextToken(Options options = NONE);
 
     /// \brief Return the last token back to the lexer.
     ///
@@ -232,14 +232,6 @@ public:
     ///     getNextToken() was not called since the last change of the source.
     void ungetToken();
 
-protected:
-    /// \brief Call the State::start()
-    ///
-    /// This calls the State::start() method and returns the result. It is
-    /// a virtual method so tests can override it to mock some different
-    /// behaviour.
-    virtual const master_lexer_internal::State* start(Options options);
-
 private:
     struct MasterLexerImpl;
     MasterLexerImpl* impl_;
diff --git a/src/lib/dns/master_lexer_state.h b/src/lib/dns/master_lexer_state.h
index fd041b1..af9c1e9 100644
--- a/src/lib/dns/master_lexer_state.h
+++ b/src/lib/dns/master_lexer_state.h
@@ -82,16 +82,16 @@ public:
     /// \brief Handle the process of one specific state.
     ///
     /// This method is expected to be called on the object returned by
-    /// start(), and keep called on the returned object until NULL is
-    /// returned.  The call chain will form the complete state transition.
+    /// start(). In the usual state transition design pattern, it would
+    /// return the next state. But as we noticed, we never have another
+    /// state, so we simplify it by not returning anything instead of
+    /// returning NULL every time.
     ///
     /// \throw MasterLexer::ReadError Unexpected I/O error
     /// \throw std::bad_alloc Internal resource allocation failure
     ///
     /// \param lexer The lexer object that holds the main context.
-    /// \return A pointer to the next state object or NULL if the transition
-    /// is completed.
-    virtual const State* handle(MasterLexer& lexer) const = 0;
+    virtual void handle(MasterLexer& lexer) const = 0;
 
     /// \brief Types of states.
     ///
@@ -111,27 +111,6 @@ public:
     /// need this method.
     static const State& getInstance(ID state_id);
 
-    /// \brief Returns a fake State instance.
-    ///
-    /// The returned State will eat eat_chars from the input source,
-    /// it'll set the given token if not NULL, call the given callback
-    /// and return the next state when its handle() is called. Also, the
-    /// parentheses count is changed accordingly to paren_change (positive
-    /// to increase, negative to decrease) and the last_was_eof condition
-    /// is set if set_eol is non-NULL.
-    ///
-    /// This is provided only for testing purposes. MasterLexer shouldn't
-    /// need this method.
-    ///
-    /// The caller is responsible for deleting the State.
-    static State* getFakeState(const State* next, size_t eat_chars,
-                               MasterLexer::Token* token = NULL,
-                               int paren_change = 0,
-                               const bool* set_eol = NULL,
-                               const boost::function<void
-                                   (const std::string&)>& callback =
-                               boost::function<void (const std::string&)>());
-
     /// \name Read-only accessors for testing purposes.
     ///
     /// These allow tests to inspect some selected portion of the internal
diff --git a/src/lib/dns/tests/master_lexer_state_unittest.cc b/src/lib/dns/tests/master_lexer_state_unittest.cc
index 615e49e..d2208fd 100644
--- a/src/lib/dns/tests/master_lexer_state_unittest.cc
+++ b/src/lib/dns/tests/master_lexer_state_unittest.cc
@@ -227,7 +227,7 @@ TEST_F(MasterLexerStateTest, crlf) {
 
     // 1. A sequence of \r, \n is recognized as a single 'end-of-line'
     EXPECT_EQ(&s_crlf, State::start(lexer, common_options)); // recognize '\r'
-    EXPECT_EQ(s_null, s_crlf.handle(lexer));   // recognize '\n'
+    s_crlf.handle(lexer);   // recognize '\n'
     EXPECT_EQ(Token::END_OF_LINE, s_crlf.getToken(lexer).getType());
     EXPECT_TRUE(s_crlf.wasLastEOL(lexer));
 
@@ -235,22 +235,22 @@ TEST_F(MasterLexerStateTest, crlf) {
     // 'end-of-line'.  then there will be "initial WS"
     EXPECT_EQ(&s_crlf, State::start(lexer, common_options)); // recognize '\r'
     // see ' ', "unget" it
-    EXPECT_EQ(s_null, s_crlf.handle(lexer));
+    s_crlf.handle(lexer);
     EXPECT_EQ(s_null, State::start(lexer, common_options)); // recognize ' '
     EXPECT_EQ(Token::INITIAL_WS, s_crlf.getToken(lexer).getType());
 
     // 3. comment between \r and \n
     EXPECT_EQ(&s_crlf, State::start(lexer, common_options)); // recognize '\r'
     // skip comments, recognize '\n'
-    EXPECT_EQ(s_null, s_crlf.handle(lexer));
+    s_crlf.handle(lexer);
     EXPECT_EQ(Token::END_OF_LINE, s_crlf.getToken(lexer).getType());
     EXPECT_EQ(&s_string, State::start(lexer, common_options));
-    EXPECT_EQ(s_null, s_string.handle(lexer)); // skip 'a'
+    s_string.handle(lexer); // skip 'a'
 
     // 4. \r then EOF
     EXPECT_EQ(&s_crlf, State::start(lexer, common_options)); // recognize '\r'
     // see EOF, then "unget" it
-    EXPECT_EQ(s_null, s_crlf.handle(lexer));
+    s_crlf.handle(lexer);
     EXPECT_EQ(s_null, State::start(lexer, common_options));  // recognize EOF
     EXPECT_EQ(Token::END_OF_FILE, s_crlf.getToken(lexer).getType());
 }
@@ -281,41 +281,41 @@ TEST_F(MasterLexerStateTest, string) {
     lexer.pushSource(ss);
 
     EXPECT_EQ(&s_string, State::start(lexer, common_options));
-    EXPECT_EQ(s_null, s_string.handle(lexer)); // recognize str, see \n
+    s_string.handle(lexer); // recognize str, see \n
     EXPECT_FALSE(s_string.wasLastEOL(lexer));
     stringTokenCheck("followed-by-EOL", s_string.getToken(lexer));
     EXPECT_EQ(s_null, State::start(lexer, common_options)); // skip \n
 
     EXPECT_EQ(&s_string, State::start(lexer, common_options));
-    EXPECT_EQ(s_null, s_string.handle(lexer)); // recognize str, see \r
+    s_string.handle(lexer); // recognize str, see \r
     stringTokenCheck("followed-by-CR", s_string.getToken(lexer));
     EXPECT_EQ(&s_crlf, State::start(lexer, common_options)); // handle \r...
-    EXPECT_EQ(s_null, s_crlf.handle(lexer)); // ...and skip it
+    s_crlf.handle(lexer); // ...and skip it
 
     EXPECT_EQ(&s_string, State::start(lexer, common_options));
-    EXPECT_EQ(s_null, s_string.handle(lexer)); // recognize str, see ' '
+    s_string.handle(lexer); // recognize str, see ' '
     stringTokenCheck("followed-by-space", s_string.getToken(lexer));
 
     // skip ' ', then recognize the next string
     EXPECT_EQ(&s_string, State::start(lexer, common_options));
-    EXPECT_EQ(s_null, s_string.handle(lexer)); // recognize str, see \t
+    s_string.handle(lexer); // recognize str, see \t
     stringTokenCheck("followed-by-tab", s_string.getToken(lexer));
 
     // skip \t, then recognize the next string
     EXPECT_EQ(&s_string, State::start(lexer, common_options));
-    EXPECT_EQ(s_null, s_string.handle(lexer)); // recognize str, see comment
+    s_string.handle(lexer); // recognize str, see comment
     stringTokenCheck("followed-by-comment", s_string.getToken(lexer));
     EXPECT_EQ(s_null, State::start(lexer, common_options)); // skip \n after it
 
     EXPECT_EQ(&s_string, State::start(lexer, common_options));
-    EXPECT_EQ(s_null, s_string.handle(lexer)); // recognize str, see '('
+    s_string.handle(lexer); // recognize str, see '('
     stringTokenCheck("followed-by-paren", s_string.getToken(lexer));
     EXPECT_EQ(&s_string, State::start(lexer, common_options)); // str in ()
-    EXPECT_EQ(s_null, s_string.handle(lexer)); // recognize the str, see ')'
+    s_string.handle(lexer); // recognize the str, see ')'
     stringTokenCheck("closing", s_string.getToken(lexer));
 
     EXPECT_EQ(&s_string, State::start(lexer, common_options));
-    EXPECT_EQ(s_null, s_string.handle(lexer)); // recognize str, see EOF
+    s_string.handle(lexer); // recognize str, see EOF
     stringTokenCheck("followed-by-EOF", s_string.getToken(lexer));
 }
 
@@ -331,32 +331,32 @@ TEST_F(MasterLexerStateTest, stringEscape) {
     lexer.pushSource(ss);
 
     EXPECT_EQ(&s_string, State::start(lexer, common_options));
-    EXPECT_EQ(s_null, s_string.handle(lexer)); // recognize str, see ' ' at end
+    s_string.handle(lexer); // recognize str, see ' ' at end
     stringTokenCheck("escaped\\ space", s_string.getToken(lexer));
 
     EXPECT_EQ(&s_string, State::start(lexer, common_options));
-    EXPECT_EQ(s_null, s_string.handle(lexer)); // recognize str, see ' ' at end
+    s_string.handle(lexer); // recognize str, see ' ' at end
     stringTokenCheck("escaped\\\ttab", s_string.getToken(lexer));
 
     EXPECT_EQ(&s_string, State::start(lexer, common_options));
-    EXPECT_EQ(s_null, s_string.handle(lexer)); // recognize str, see ' ' at end
+    s_string.handle(lexer); // recognize str, see ' ' at end
     stringTokenCheck("escaped\\(paren", s_string.getToken(lexer));
 
     EXPECT_EQ(&s_string, State::start(lexer, common_options));
-    EXPECT_EQ(s_null, s_string.handle(lexer)); // recognize str, see ' ' at end
+    s_string.handle(lexer); // recognize str, see ' ' at end
     stringTokenCheck("escaped\\)close", s_string.getToken(lexer));
 
     EXPECT_EQ(&s_string, State::start(lexer, common_options));
-    EXPECT_EQ(s_null, s_string.handle(lexer)); // recognize str, see ' ' at end
+    s_string.handle(lexer); // recognize str, see ' ' at end
     stringTokenCheck("escaped\\;comment", s_string.getToken(lexer));
 
     EXPECT_EQ(&s_string, State::start(lexer, common_options));
-    EXPECT_EQ(s_null, s_string.handle(lexer)); // recognize str, see ' ' in mid
+    s_string.handle(lexer); // recognize str, see ' ' in mid
     stringTokenCheck("escaped\\\\", s_string.getToken(lexer));
 
     // Confirm the word that follows the escaped '\' is correctly recognized.
     EXPECT_EQ(&s_string, State::start(lexer, common_options));
-    EXPECT_EQ(s_null, s_string.handle(lexer)); // recognize str, see ' ' at end
+    s_string.handle(lexer); // recognize str, see ' ' at end
     stringTokenCheck("backslash", s_string.getToken(lexer));
 }
 
@@ -376,7 +376,7 @@ TEST_F(MasterLexerStateTest, quotedString) {
 
     // by default, '"' doesn't have any special meaning and part of string
     EXPECT_EQ(&s_string, State::start(lexer, common_options));
-    EXPECT_EQ(s_null, s_string.handle(lexer)); // recognize str, see \n
+    s_string.handle(lexer); // recognize str, see \n
     stringTokenCheck("\"ignore-quotes\"", s_string.getToken(lexer));
     EXPECT_EQ(s_null, State::start(lexer, common_options)); // skip \n after it
     EXPECT_TRUE(s_string.wasLastEOL(lexer));
@@ -386,35 +386,35 @@ TEST_F(MasterLexerStateTest, quotedString) {
     const MasterLexer::Options options = common_options | MasterLexer::QSTRING;
     EXPECT_EQ(&s_qstring, State::start(lexer, options));
     EXPECT_FALSE(s_string.wasLastEOL(lexer)); // EOL is canceled due to '"'
-    EXPECT_EQ(s_null, s_qstring.handle(lexer));
+    s_qstring.handle(lexer);
     stringTokenCheck("quoted string", s_string.getToken(lexer), true);
 
     // Also checks other separator characters within a qstring
     EXPECT_EQ(&s_qstring, State::start(lexer, options));
-    EXPECT_EQ(s_null, s_qstring.handle(lexer));
+    s_qstring.handle(lexer);
     stringTokenCheck("quoted()\t\rstring", s_string.getToken(lexer), true);
 
     // escape character mostly doesn't have any effect in the qstring
     // processing
     EXPECT_EQ(&s_qstring, State::start(lexer, options));
-    EXPECT_EQ(s_null, s_qstring.handle(lexer));
+    s_qstring.handle(lexer);
     stringTokenCheck("escape\\ in quote", s_string.getToken(lexer), true);
 
     // The only exception is the quotation mark itself.  Note that the escape
     // only works on the quotation mark immediately after it.
     EXPECT_EQ(&s_qstring, State::start(lexer, options));
-    EXPECT_EQ(s_null, s_qstring.handle(lexer));
+    s_qstring.handle(lexer);
     stringTokenCheck("escaped\"", s_string.getToken(lexer), true);
 
     // quoted '\' then '"'.  Unlike the previous case '"' shouldn't be
     // escaped.
     EXPECT_EQ(&s_qstring, State::start(lexer, options));
-    EXPECT_EQ(s_null, s_qstring.handle(lexer));
+    s_qstring.handle(lexer);
     stringTokenCheck("escaped backslash\\\\", s_string.getToken(lexer), true);
 
     // ';' has no meaning in a quoted string (not indicating a comment)
     EXPECT_EQ(&s_qstring, State::start(lexer, options));
-    EXPECT_EQ(s_null, s_qstring.handle(lexer));
+    s_qstring.handle(lexer);
     stringTokenCheck("no;comment", s_string.getToken(lexer), true);
 }
 
@@ -427,7 +427,7 @@ TEST_F(MasterLexerStateTest, brokenQuotedString) {
     // EOL is encountered without closing the quote
     const MasterLexer::Options options = common_options | MasterLexer::QSTRING;
     EXPECT_EQ(&s_qstring, State::start(lexer, options));
-    EXPECT_EQ(s_null, s_qstring.handle(lexer));
+    s_qstring.handle(lexer);
     ASSERT_EQ(Token::ERROR, s_qstring.getToken(lexer).getType());
     EXPECT_EQ(Token::UNBALANCED_QUOTES,
               s_qstring.getToken(lexer).getErrorCode());
@@ -437,12 +437,12 @@ TEST_F(MasterLexerStateTest, brokenQuotedString) {
 
     // \n is okay in a quoted string if escaped
     EXPECT_EQ(&s_qstring, State::start(lexer, options));
-    EXPECT_EQ(s_null, s_qstring.handle(lexer));
+    s_qstring.handle(lexer);
     stringTokenCheck("quoted\\\n", s_string.getToken(lexer), true);
 
     // EOF is encountered without closing the quote
     EXPECT_EQ(&s_qstring, State::start(lexer, options));
-    EXPECT_EQ(s_null, s_qstring.handle(lexer));
+    s_qstring.handle(lexer);
     ASSERT_EQ(Token::ERROR, s_qstring.getToken(lexer).getType());
     EXPECT_EQ(Token::UNEXPECTED_END, s_qstring.getToken(lexer).getErrorCode());
     // If we continue we'll simply see the EOF
diff --git a/src/lib/dns/tests/master_lexer_unittest.cc b/src/lib/dns/tests/master_lexer_unittest.cc
index 78263c4..bb2def6 100644
--- a/src/lib/dns/tests/master_lexer_unittest.cc
+++ b/src/lib/dns/tests/master_lexer_unittest.cc
@@ -36,43 +36,13 @@ using master_lexer_internal::State;
 
 namespace {
 
-// This acts like the normal MasterLexer. It, however, allows to mock the start()
-// method to return some given state instead of the auto-detected ones.
-class TestedMasterLexer : public MasterLexer {
-public:
-    TestedMasterLexer() :
-        fake_start_(NULL)
-    {}
-    // During the next call to start(), return the given state instead of the
-    // auto-detected one.
-    void pushFakeStart(const State* state) {
-        fake_start_ = state;
-    }
-protected:
-    virtual const State* start(Options options) {
-        if (fake_start_ != NULL) {
-            // There's a fake start, so remove it (not to be used next time)
-            // and return it.
-            const State* result = fake_start_;
-            fake_start_ = NULL;
-            return (result);
-        } else {
-            // No fake start ready. So we act the usual way, by delegating it to
-            // the parent class.
-            return (MasterLexer::start(options));
-        }
-    }
-private:
-    const State* fake_start_;
-};
-
 class MasterLexerTest : public ::testing::Test {
 protected:
     MasterLexerTest() :
         expected_stream_name("stream-" + lexical_cast<string>(&ss))
     {}
 
-    TestedMasterLexer lexer;
+    MasterLexer lexer;
     stringstream ss;
     const string expected_stream_name;
 };
@@ -165,90 +135,9 @@ TEST_F(MasterLexerTest, noSource) {
     EXPECT_THROW(lexer.getNextToken(), isc::InvalidOperation);
 }
 
-// Getting a token directly from the start() method.
-TEST_F(MasterLexerTest, tokenFromStart) {
-    // A class that sets the token directly in start() and returns no
-    // state. This is equivalent to the State::start() doing so.
-    class StartLexer : public MasterLexer {
-    public:
-        StartLexer() :
-            token_(MasterLexer::Token::END_OF_LINE)
-        {}
-        virtual const State* start(Options) {
-            // We don't have access directly inside the implementation.
-            // We get the fake state, run it to install the token.
-            // Then we just delete it ourself and return NULL.
-            scoped_ptr<const State> state(State::getFakeState(NULL, 0,
-                                                              &token_));
-            state->handle(*this);
-            return (NULL);
-        }
-    private:
-        MasterLexer::Token token_;
-    } lexer;
-    lexer.pushSource(ss);
-
-    // The token gets out.
-    MasterLexer::Token generated(lexer.getNextToken());
-    EXPECT_EQ(MasterLexer::Token::END_OF_LINE, generated.getType());
-}
-
-// Getting a token with a single iteration through the states.
-TEST_F(MasterLexerTest, simpleGetToken) {
-    // Prepare the fake state.
-    MasterLexer::Token token(MasterLexer::Token::END_OF_LINE);
-    scoped_ptr<State> state(State::getFakeState(NULL, 3, &token));
-    lexer.pushFakeStart(state.get());
-    // Push some source inside.
-    ss << "12345";
-    lexer.pushSource(ss);
-
-    // Get the token.
-    MasterLexer::Token generated(lexer.getNextToken());
-    // It is the same token (well, on a different address)
-    // We can't compare directly, so compare types.
-    EXPECT_EQ(token.getType(), generated.getType());
-    // 3 characters were read from the source.
-    // We test by extracting the rest and comparing.
-    int rest;
-    ss >> rest;
-    EXPECT_EQ(45, rest);
-}
-
-// A token that takes multiple states.
-//
-// The first state sets the token as well as the second. The second one should
-// survive and be returned.
-TEST_F(MasterLexerTest, chainGetToken) {
-    // Build the states
-    MasterLexer::Token t1(MasterLexer::Token::END_OF_LINE);
-    MasterLexer::Token t2(MasterLexer::Token::INITIAL_WS);
-    scoped_ptr<State> s2(State::getFakeState(NULL, 1, &t2));
-    scoped_ptr<State> s1(State::getFakeState(s2.get(), 2, &t1));
-    lexer.pushFakeStart(s1.get());
-    // Put something into the source
-    ss << "12345";
-    lexer.pushSource(ss);
-
-    // Get the token.
-    MasterLexer::Token generated(lexer.getNextToken());
-    // It is the same token as the second one (well, on a different address)
-    // We can't compare directly, so compare types.
-    EXPECT_EQ(t2.getType(), generated.getType());
-    // 3 characters were read from the source.
-    // We test by extracting the rest and comparing.
-    int rest;
-    ss >> rest;
-    EXPECT_EQ(45, rest);
-}
-
-// Test getting a token without overriding the start() method (well, it
-// is overriden, but no fake state is set, so it refers to the real one).
-//
-// This also tests the real start() passes the options, otherwise we wouldn't
-// get the initial whitespace.
-TEST_F(MasterLexerTest, realStart) {
-    ss << "\n   \n";
+// Test getting some tokens
+TEST_F(MasterLexerTest, getNextToken) {
+    ss << "\n   \n\"STRING\"\n";
     lexer.pushSource(ss);
 
     // First, the newline should get out.
@@ -256,26 +145,34 @@ TEST_F(MasterLexerTest, realStart) {
     // Then the whitespace, if we specify the option.
     EXPECT_EQ(MasterLexer::Token::INITIAL_WS,
               lexer.getNextToken(MasterLexer::INITIAL_WS).getType());
+    // The newline
+    EXPECT_EQ(MasterLexer::Token::END_OF_LINE, lexer.getNextToken().getType());
+    // The (quoted) string
+    EXPECT_EQ(MasterLexer::Token::QSTRING,
+              lexer.getNextToken(MasterLexer::QSTRING).getType());
+
+    // And the end of line and file
+    EXPECT_EQ(MasterLexer::Token::END_OF_LINE, lexer.getNextToken().getType());
+    EXPECT_EQ(MasterLexer::Token::END_OF_FILE, lexer.getNextToken().getType());
 }
 
-// Test we correctly find end of file. Then, upon more attempts to produce
-// tokens past the end, it throws.
+// Test we correctly find end of file.
 TEST_F(MasterLexerTest, eof) {
     // Let the ss empty.
     lexer.pushSource(ss);
 
     // The first one is found to be EOF
     EXPECT_EQ(MasterLexer::Token::END_OF_FILE, lexer.getNextToken().getType());
-    // And it is not allowed to use this one any more.
-    EXPECT_THROW(lexer.getNextToken(), isc::InvalidOperation);
-    // But if we unget a step back, we should get the EOF again
+    // And it stays on EOF for any following attempts
+    EXPECT_EQ(MasterLexer::Token::END_OF_FILE, lexer.getNextToken().getType());
+    // And we can step back one token, but that is the EOF too.
     lexer.ungetToken();
     EXPECT_EQ(MasterLexer::Token::END_OF_FILE, lexer.getNextToken().getType());
 }
 
 // Check we properly return error when there's an opened parentheses and no
 // closing one
-TEST_F(MasterLexerTest, getUnbalanced) {
+TEST_F(MasterLexerTest, getUnbalancedParen) {
     ss << "(\"string\"";
     lexer.pushSource(ss);
 
@@ -284,58 +181,57 @@ TEST_F(MasterLexerTest, getUnbalanced) {
     // Then an unbalanced parenthesis
     EXPECT_EQ(MasterLexer::Token::UNBALANCED_PAREN,
               lexer.getNextToken().getErrorCode());
+    // And then EOF
+    EXPECT_EQ(MasterLexer::Token::END_OF_FILE, lexer.getNextToken().getType());
 }
 
-void
-checkInput(const std::string& expected, const std::string& received) {
-    EXPECT_EQ(expected, received);
+// Check we properly return error when there's an opened parentheses and no
+// closing one
+TEST_F(MasterLexerTest, getUnbalancedString) {
+    ss << "\"string";
+    lexer.pushSource(ss);
+
+    // Then an unbalanced parenthesis
+    EXPECT_EQ(MasterLexer::Token::UNEXPECTED_END,
+              lexer.getNextToken(MasterLexer::QSTRING).getErrorCode());
+    // And then EOF
+    EXPECT_EQ(MasterLexer::Token::END_OF_FILE, lexer.getNextToken().getType());
 }
 
-// Check ungetting a token, which should get to the previous state. We do
-// so with changing the state a little bit.
-TEST_F(MasterLexerTest, ungetSimple) {
-    ss << "12345";
+// Test ungetting tokens works
+TEST_F(MasterLexerTest, ungetToken) {
+    ss << "\n (\"string\"\n) more";
     lexer.pushSource(ss);
 
-    const bool true_value = true, false_value = false;
-    // Make sure we change the state to non-default, so we return to previous
-    // not default state.
-    MasterLexer::Token t0(MasterLexer::Token::INITIAL_WS);
-    scoped_ptr<State> s0(State::getFakeState(NULL, 1, &t0, 1, &true_value));
-    lexer.pushFakeStart(s0.get());
-    EXPECT_EQ(MasterLexer::Token::INITIAL_WS, lexer.getNextToken().getType());
-
-    // Prepare the token to get and return
-    const std::string expected = "234";
-    MasterLexer::Token token(MasterLexer::Token::END_OF_LINE);
-    // Change the internal state with it too. So we can check it is retured.
-    scoped_ptr<State> state(State::getFakeState(NULL, 3, &token, 1,
-                                                &false_value,
-                                                boost::bind(&checkInput,
-                                                            expected, _1)));
-    lexer.pushFakeStart(state.get());
-
-    // Check the internal state before getting the token
-    // We access the lexer through any state, so use the one we have.
-    EXPECT_EQ(1, state->getParenCount(lexer));
-    EXPECT_TRUE(state->wasLastEOL(lexer));
-
-    // Now get the token and check the state changed
+    // Try getting the newline
     EXPECT_EQ(MasterLexer::Token::END_OF_LINE, lexer.getNextToken().getType());
-    EXPECT_EQ(2, state->getParenCount(lexer));
-    EXPECT_FALSE(state->wasLastEOL(lexer));
-
-    // Return the token back. Check the state is as it was before.
+    // Return it and get again
     lexer.ungetToken();
-    EXPECT_EQ(1, state->getParenCount(lexer));
-    EXPECT_TRUE(state->wasLastEOL(lexer));
-    // By calling getToken again, we verify even the source got back to
-    // original (as the second fake state checks it gets "234"). We must
-    // push it as a fake start again so it is picked.
-    lexer.pushFakeStart(state.get());
     EXPECT_EQ(MasterLexer::Token::END_OF_LINE, lexer.getNextToken().getType());
-    EXPECT_EQ(2, state->getParenCount(lexer));
-    EXPECT_FALSE(state->wasLastEOL(lexer));
+    // Get the string and return it back
+    EXPECT_EQ(MasterLexer::Token::QSTRING,
+              lexer.getNextToken(MasterLexer::QSTRING).getType());
+    lexer.ungetToken();
+    // But if we change the options, it honors them
+    EXPECT_EQ(MasterLexer::Token::INITIAL_WS,
+              lexer.getNextToken(MasterLexer::QSTRING |
+                                 MasterLexer::INITIAL_WS).getType());
+    // Get to the "more" string
+    EXPECT_EQ(MasterLexer::Token::QSTRING,
+              lexer.getNextToken(MasterLexer::QSTRING).getType());
+    EXPECT_EQ(MasterLexer::Token::STRING,
+              lexer.getNextToken(MasterLexer::QSTRING).getType());
+    // Return it back. It should get inside the parentheses.
+    // Upon next attempt to get it again, the newline inside the parentheses
+    // should be still ignored.
+    lexer.ungetToken();
+    EXPECT_EQ(MasterLexer::Token::STRING,
+              lexer.getNextToken(MasterLexer::QSTRING).getType());
+}
+
+void
+checkInput(const std::string& expected, const std::string& received) {
+    EXPECT_EQ(expected, received);
 }
 
 // Check ungetting token without overriding the start method. We also
@@ -393,38 +289,4 @@ TEST_F(MasterLexerTest, ungetAfterSwitch) {
     EXPECT_THROW(lexer.ungetToken(), isc::InvalidOperation);
 }
 
-class TestException {};
-
-void
-doThrow(const std::string&) {
-    throw TestException();
-}
-
-// Check the getNextToken provides at least the weak exception guarantee.
-TEST_F(MasterLexerTest, getTokenExceptions) {
-    ss << "\n12345";
-    lexer.pushSource(ss);
-
-    // Prepare a chain that changes the internal state, reads something.
-    // The next item in the chain will throw an exception (we explicitly
-    // throw something not known to it, so we know it can handle anything).
-    // Then the thing should get to the previous state and getting the
-    // token the usual way without mock should work.
-    const bool true_value = true;
-    boost::scoped_ptr<State> s2(State::getFakeState(NULL, 3, NULL, 0, NULL,
-                                                    &doThrow));
-    boost::scoped_ptr<State> s1(State::getFakeState(s2.get(), 3, NULL, 1,
-                                                    &true_value));
-    lexer.pushFakeStart(s1.get());
-
-    // Getting the token with the fake start should throw. But then, the
-    // current state should be untouched.
-    EXPECT_THROW(lexer.getNextToken(), TestException);
-    EXPECT_EQ(0, s1->getParenCount(lexer));
-    EXPECT_FALSE(s1->wasLastEOL(lexer));
-
-    // It gets back to the original state, so getting the newline works.
-    EXPECT_EQ(MasterLexer::Token::END_OF_LINE, lexer.getNextToken().getType());
-}
-
 }



More information about the bind10-changes mailing list