[2375] Don't restore internal token on ungetToken

It was not safe, because it may refer to some data elsewhere. Luckily,
it seems it is not needed anyway, since the only way that a token can
get out of the lexer is through the getNextToken, which would get a new
one, and through a testing method not meant for public use.

diff --git a/src/lib/dns/master_lexer.cc b/src/lib/dns/master_lexer.cc
index 62550827a919fa2362b6ed3435452fc154de14a2..eb4c480b4860897375b1373cf93d360c061411ae 100644 (file)
--- a/src/lib/dns/master_lexer.cc
+++ b/src/lib/dns/master_lexer.cc
@@ -38,7 +38,6 @@ struct MasterLexer::MasterLexerImpl {
     MasterLexerImpl() : source_(NULL), token_(Token::NOT_STARTED),
                         paren_count_(0), last_was_eol_(false),
                         has_previous_(false),
-                        previous_token_(Token::NOT_STARTED),
                         previous_paren_count_(0),
                         previous_was_eol_(false)
     {
@@ -99,7 +98,6 @@ struct MasterLexer::MasterLexerImpl {
 
     // These are to allow restoring state before previous token.
     bool has_previous_;
-    Token previous_token_;
     size_t previous_paren_count_;
     bool previous_was_eol_;
 };
@@ -176,7 +174,6 @@ MasterLexer::getNextToken(Options options) {
     impl_->has_previous_ = false;
     // If thrown in this section, nothing observable except for not having
     // previous will happen.
-    impl_->previous_token_ = impl_->token_;
     impl_->previous_paren_count_ = impl_->paren_count_;
     impl_->previous_was_eol_ = impl_->last_was_eol_;
     impl_->source_->mark();
@@ -206,9 +203,6 @@ MasterLexer::getNextToken(Options options) {
 void
 MasterLexer::ungetToken() {
     if (impl_->has_previous_) {
-        // The one that could fail due to bad_alloc first
-        impl_->token_ = impl_->previous_token_;
-        // The rest should not throw.
         impl_->has_previous_ = false;
         impl_->source_->ungetAll();
         impl_->last_was_eol_ = impl_->previous_was_eol_;

diff --git a/src/lib/dns/master_lexer.h b/src/lib/dns/master_lexer.h
index a1db405f31d92a0130a68fc1de8ec42307295d2a..8c3701fb6a38a21c9117645b9a3928a1b4a8da6c 100644 (file)
--- a/src/lib/dns/master_lexer.h
+++ b/src/lib/dns/master_lexer.h
@@ -202,7 +202,10 @@ public:
     ///     by this parameter. Multiple options can be passed at once by
     ///     bitwise or (eg. option1 | option 2). See description of available
     ///     options.
-    /// \return Next token found in the input.
+    /// \return Next token found in the input. Note that the token refers to
+    ///     some internal data in in the lexer. It is valid only until
+    ///     getNextToken or ungetToken is called. Also, the token becomes
+    ///     invalid when the lexer is destroyed.
     /// \throw isc::InvalidOperation in case the source is not available. This
     ///     may mean the pushSource() has not been called yet, or that the
     ///     current source has been read past the end.

diff --git a/src/lib/dns/tests/master_lexer_unittest.cc b/src/lib/dns/tests/master_lexer_unittest.cc
index b02ae70cd28c1d48f7728afad3b041d8d24867a2..b076bfd044d8040831af4cc89a6f831cece64afd 100644 (file)
--- a/src/lib/dns/tests/master_lexer_unittest.cc
+++ b/src/lib/dns/tests/master_lexer_unittest.cc
@@ -303,30 +303,22 @@ TEST_F(MasterLexerTest, ungetSimple) {
     // We access the lexer through any state, so use the one we have.
     EXPECT_EQ(1, state->getParenCount(lexer));
     EXPECT_TRUE(state->wasLastEOL(lexer));
-    EXPECT_EQ(MasterLexer::Token::INITIAL_WS,
-              state->getToken(lexer).getType());
 
     // Now get the token and check the state changed
     EXPECT_EQ(MasterLexer::Token::END_OF_LINE, lexer.getNextToken().getType());
     EXPECT_EQ(2, state->getParenCount(lexer));
     EXPECT_FALSE(state->wasLastEOL(lexer));
-    EXPECT_EQ(MasterLexer::Token::END_OF_LINE,
-              state->getToken(lexer).getType());
 
     // Return the token back. Check the state is as it was before.
     lexer.ungetToken();
     EXPECT_EQ(1, state->getParenCount(lexer));
     EXPECT_TRUE(state->wasLastEOL(lexer));
-    EXPECT_EQ(MasterLexer::Token::INITIAL_WS,
-              state->getToken(lexer).getType());
     // By calling getToken again, we verify even the source got back to
     // original. 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));
-    EXPECT_EQ(MasterLexer::Token::END_OF_LINE,
-              state->getToken(lexer).getType());
 }
 
 // Check ungetting token without overriding the start method. We also
@@ -413,8 +405,6 @@ TEST_F(MasterLexerTest, getTokenExceptions) {
     EXPECT_THROW(lexer.getNextToken(), TestException);
     EXPECT_EQ(0, s1->getParenCount(lexer));
     EXPECT_FALSE(s1->wasLastEOL(lexer));
-    EXPECT_EQ(MasterLexer::Token::NOT_STARTED,
-              s1->getToken(lexer).getErrorCode());
 
     // It gets back to the original state, so getting the newline works.
     EXPECT_EQ(MasterLexer::Token::END_OF_LINE, lexer.getNextToken().getType());