From: JINMEI Tatuya Date: Tue, 6 Nov 2012 03:48:47 +0000 (-0800) Subject: [2372] even further simplification of option handling X-Git-Tag: trac2487_base~1^2~21^2~10 X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=3ce2efa154f8eff8fd7029925ffd29116e2be8fa;p=thirdparty%2Fkea.git [2372] even further simplification of option handling --- diff --git a/src/lib/dns/master_lexer.cc b/src/lib/dns/master_lexer.cc index 3bcf00ee5d..88b77486ab 100644 --- a/src/lib/dns/master_lexer.cc +++ b/src/lib/dns/master_lexer.cc @@ -53,7 +53,6 @@ struct MasterLexer::MasterLexerImpl { std::vector sources_; InputSource* source_; // current source size_t paren_count_; - Options options_; bool last_was_eol_; Token token_; }; @@ -160,13 +159,15 @@ State::getParenCount(const MasterLexer& lexer) const { class Start : public State { public: Start() {} - virtual const State* handle(MasterLexer& lexer) const; + virtual const State* handle(MasterLexer& lexer, + MasterLexer::Options options) const; }; class CRLF : public State { public: CRLF() {} - virtual const State* handle(MasterLexer& lexer) const { + virtual const State* handle(MasterLexer& lexer, MasterLexer::Options) 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 @@ -191,7 +192,9 @@ public: class String : public State { public: String() {} - virtual const State* handle(MasterLexer& /*lexer*/) const { + virtual const State* handle(MasterLexer& /*lexer*/, + MasterLexer::Options /*options*/) const + { return (NULL); } }; @@ -215,9 +218,8 @@ State::getInstance(ID state_id) { } const State* -Start::handle(MasterLexer& lexer) const { - MasterLexer::Options options = getLexerImpl(lexer)->options_; - size_t& paren_count = getLexerImpl(lexer)->paren_count_; +Start::handle(MasterLexer& lexer, MasterLexer::Options options) const { + size_t& paren_count = getLexerImpl(lexer)->paren_count_; // shortcut while (true) { const int c = getLexerImpl(lexer)->skipComment( @@ -260,9 +262,7 @@ Start::handle(MasterLexer& lexer) const { getLexerImpl(lexer)->token_ = Token(Token::UNBALANCED_PAREN); return (NULL); } - if (--paren_count == 0) { - options = getLexerImpl(lexer)->options_; - } + --paren_count; continue; } else { // Note: in #2373 we should probably ungetChar(). @@ -272,13 +272,6 @@ Start::handle(MasterLexer& lexer) const { } } -const State* -State::getStartInstance(MasterLexer& lexer, MasterLexer::Options options) -{ - lexer.impl_->options_ = options; - return (&START_STATE); -} - } // namespace master_lexer_internal } // end of namespace dns diff --git a/src/lib/dns/master_lexer_state.h b/src/lib/dns/master_lexer_state.h index 302ed50b84..b6782fd41a 100644 --- a/src/lib/dns/master_lexer_state.h +++ b/src/lib/dns/master_lexer_state.h @@ -25,11 +25,8 @@ class InputSource; class State { public: - virtual const State* handle(MasterLexer& lexer) const = 0; - - // Note: Pass options mainly for the convenience of tests. - static const State* getStartInstance(MasterLexer& lexer, - MasterLexer::Options options); + virtual const State* handle(MasterLexer& lexer, + MasterLexer::Options options) const = 0; /// Specific states are basically hidden within the implementation, /// but we'd like to allow tests to examine them, so we provide diff --git a/src/lib/dns/tests/master_lexer_state_unittest.cc b/src/lib/dns/tests/master_lexer_state_unittest.cc index 4904f0dfb3..2d0a9d770f 100644 --- a/src/lib/dns/tests/master_lexer_state_unittest.cc +++ b/src/lib/dns/tests/master_lexer_state_unittest.cc @@ -30,8 +30,7 @@ class MasterLexerStateTest : public ::testing::Test { protected: MasterLexerStateTest() : common_options(MasterLexer::INITIAL_WS), s_null(NULL), - s_start(*State::getStartInstance( - lexer, common_options)), + s_start(State::getInstance(State::Start)), s_crlf(State::getInstance(State::CRLF)), s_string(State::getInstance(State::String)), options(MasterLexer::NONE), @@ -65,18 +64,18 @@ eofCheck(const State& state, MasterLexer& lexer) { TEST_F(MasterLexerStateTest, startAndEnd) { // A simple case: the input is empty, so we begin with start and // are immediately done. - EXPECT_EQ(s_null, s_start.handle(lexer)); + EXPECT_EQ(s_null, s_start.handle(lexer, common_options)); eofCheck(s_start, lexer); } TEST_F(MasterLexerStateTest, startToEOL) { ss << "\n"; - EXPECT_EQ(s_null, s_start.handle(lexer)); + EXPECT_EQ(s_null, s_start.handle(lexer, common_options)); EXPECT_TRUE(s_start.wasLastEOL(lexer)); EXPECT_EQ(Token::END_OF_LINE, s_start.getToken(lexer).getType()); // The next lexer session will reach EOF. Same eof check should pass. - EXPECT_EQ(s_null, s_start.handle(lexer)); + EXPECT_EQ(s_null, s_start.handle(lexer, common_options)); eofCheck(s_start, lexer); } @@ -85,20 +84,17 @@ TEST_F(MasterLexerStateTest, space) { // twice; at the second iteration, it's a white space at the beginning // of line, but since we don't specify INITIAL_WS option, it's treated as // normal space and ignored. - State::getStartInstance(lexer, MasterLexer::NONE); for (size_t i = 0; i < 2; ++i) { ss << " \t\n"; - EXPECT_EQ(s_null, s_start.handle(lexer)); + EXPECT_EQ(s_null, s_start.handle(lexer, MasterLexer::NONE)); EXPECT_TRUE(s_start.wasLastEOL(lexer)); EXPECT_EQ(Token::END_OF_LINE, s_start.getToken(lexer).getType()); } // Now we specify the INITIAL_WS option. It will be recognized and the // corresponding token will be returned. - State::getStartInstance(lexer, common_options); ss << " "; - options = MasterLexer::INITIAL_WS; - EXPECT_EQ(s_null, s_start.handle(lexer)); + EXPECT_EQ(s_null, s_start.handle(lexer, MasterLexer::INITIAL_WS)); EXPECT_FALSE(s_start.wasLastEOL(lexer)); EXPECT_EQ(Token::INITIAL_WS, s_start.getToken(lexer).getType()); } @@ -106,55 +102,55 @@ TEST_F(MasterLexerStateTest, space) { TEST_F(MasterLexerStateTest, parentheses) { ss << "\n(\na\n )\n "; // 1st \n is to check if 'was EOL' is set to false - EXPECT_EQ(s_null, s_start.handle(lexer)); // handle \n + EXPECT_EQ(s_null, s_start.handle(lexer, common_options)); // handle \n // Now handle '('. It skips \n and recognize 'a' as string EXPECT_EQ(0, s_start.getParenCount(lexer)); // check pre condition - EXPECT_EQ(&s_string, s_start.handle(lexer)); + EXPECT_EQ(&s_string, s_start.handle(lexer, common_options)); EXPECT_EQ(1, s_start.getParenCount(lexer)); // check post condition EXPECT_FALSE(s_start.wasLastEOL(lexer)); // skip 'a' (note: until #2373 it's actually skipped as part of the '(' // handling) - s_string.handle(lexer); + s_string.handle(lexer, common_options); // Then handle ')'. '\n' before ')' isn't recognized because // it's canceled due to the '('. Likewise, the space after the '\n' // shouldn't be recognized but should be just ignored. - EXPECT_EQ(s_null, s_start.handle(lexer)); + EXPECT_EQ(s_null, s_start.handle(lexer, common_options)); EXPECT_EQ(0, s_start.getParenCount(lexer)); // Now, temporarily disabled options are restored: Both EOL and the // initial WS are recognized EXPECT_EQ(Token::END_OF_LINE, s_start.getToken(lexer).getType()); - EXPECT_EQ(s_null, s_start.handle(lexer)); + EXPECT_EQ(s_null, s_start.handle(lexer, common_options)); EXPECT_EQ(Token::INITIAL_WS, s_start.getToken(lexer).getType()); } TEST_F(MasterLexerStateTest, nestedParentheses) { // This is an unusual, but allowed (in this implementation) case. ss << "(a(b)\n c)\n "; - EXPECT_EQ(&s_string, s_start.handle(lexer)); // consume '(' - s_string.handle(lexer); // consume 'a' - EXPECT_EQ(&s_string, s_start.handle(lexer)); // consume '(' - s_string.handle(lexer); // consume 'b' + EXPECT_EQ(&s_string, s_start.handle(lexer, common_options)); // consume '(' + s_string.handle(lexer, common_options); // consume 'a' + EXPECT_EQ(&s_string, s_start.handle(lexer, common_options)); // consume '(' + s_string.handle(lexer, common_options); // consume 'b' EXPECT_EQ(2, s_start.getParenCount(lexer)); // now the count is 2 // Close the inner most parentheses. count will be decreased, but option // shouldn't be restored yet, so the intermediate EOL or initial WS won't // be recognized. - EXPECT_EQ(&s_string, s_start.handle(lexer)); // consume ')' - s_string.handle(lexer); // consume 'c' + EXPECT_EQ(&s_string, s_start.handle(lexer, common_options)); // consume ')' + s_string.handle(lexer, common_options); // consume 'c' EXPECT_EQ(1, s_start.getParenCount(lexer)); // Close the outermost parentheses. count will be reset to 0, and original // options are restored. - EXPECT_EQ(s_null, s_start.handle(lexer)); + EXPECT_EQ(s_null, s_start.handle(lexer, common_options)); // Now, temporarily disabled options are restored: Both EOL and the // initial WS are recognized EXPECT_EQ(Token::END_OF_LINE, s_start.getToken(lexer).getType()); - EXPECT_EQ(s_null, s_start.handle(lexer)); + EXPECT_EQ(s_null, s_start.handle(lexer, common_options)); EXPECT_EQ(Token::INITIAL_WS, s_start.getToken(lexer).getType()); } @@ -163,13 +159,13 @@ TEST_F(MasterLexerStateTest, unbalancedParentheses) { // correctly canceled after detecting the error. ss << "\n)"; - EXPECT_EQ(s_null, s_start.handle(lexer)); // consume '\n' + EXPECT_EQ(s_null, s_start.handle(lexer, common_options)); // consume '\n' EXPECT_TRUE(s_start.wasLastEOL(lexer)); // this \n was remembered // Now checking ')'. The result should be error, count shouldn't be // changed. "last EOL" should be canceled. EXPECT_EQ(0, s_start.getParenCount(lexer)); - EXPECT_EQ(s_null, s_start.handle(lexer)); + EXPECT_EQ(s_null, s_start.handle(lexer, common_options)); EXPECT_EQ(0, s_start.getParenCount(lexer)); ASSERT_EQ(Token::ERROR, s_start.getToken(lexer).getType()); EXPECT_EQ(Token::UNBALANCED_PAREN, s_start.getToken(lexer).getErrorCode()); @@ -177,10 +173,10 @@ TEST_F(MasterLexerStateTest, unbalancedParentheses) { // Reach EOF with a dangling open parenthesis. ss << "(a"; - EXPECT_EQ(&s_string, s_start.handle(lexer)); // consume '(' - s_string.handle(lexer); // consume 'a' + EXPECT_EQ(&s_string, s_start.handle(lexer, common_options)); // consume '(' + s_string.handle(lexer, common_options); // consume 'a' EXPECT_EQ(1, s_start.getParenCount(lexer)); - EXPECT_EQ(s_null, s_start.handle(lexer)); // reach EOF + EXPECT_EQ(s_null, s_start.handle(lexer, common_options)); // reach EOF ASSERT_EQ(Token::ERROR, s_start.getToken(lexer).getType()); EXPECT_EQ(Token::UNBALANCED_PAREN, s_start.getToken(lexer).getErrorCode()); EXPECT_EQ(0, s_start.getParenCount(lexer)); // should be reset to 0 @@ -191,12 +187,12 @@ TEST_F(MasterLexerStateTest, startToComment) { // the rest of the line, and recognize the new line. Note that the // second ';' is simply ignored. ss << " ;a;\n"; - EXPECT_EQ(s_null, s_start.handle(lexer)); + EXPECT_EQ(s_null, s_start.handle(lexer, common_options)); EXPECT_EQ(Token::END_OF_LINE, s_start.getToken(lexer).getType()); // Likewise, but the comment ends with EOF. ss << ";a;"; - EXPECT_EQ(s_null, s_start.handle(lexer)); + EXPECT_EQ(s_null, s_start.handle(lexer, common_options)); EXPECT_EQ(Token::END_OF_FILE, s_start.getToken(lexer).getType()); } @@ -205,39 +201,42 @@ TEST_F(MasterLexerStateTest, commentAfterParen) { // other tests should also ensure that it works correctly, but we // check it explicitly. ss << "( ;this is a comment\na)\n"; - EXPECT_EQ(&s_string, s_start.handle(lexer)); // consume '(', skip comments - s_string.handle(lexer); // consume 'a' - EXPECT_EQ(s_null, s_start.handle(lexer)); // consume ')', see '\n' + // consume '(', skip comments, consume 'a', then consume ')' + EXPECT_EQ(&s_string, s_start.handle(lexer, common_options)); + s_string.handle(lexer, common_options); + EXPECT_EQ(s_null, s_start.handle(lexer, common_options)); EXPECT_EQ(Token::END_OF_LINE, s_start.getToken(lexer).getType()); } TEST_F(MasterLexerStateTest, crlf) { // A sequence of \r, \n is recognized as a single 'end-of-line' ss << "\r\n"; - EXPECT_EQ(&s_crlf, s_start.handle(lexer)); // recognize '\r' - EXPECT_EQ(s_null, s_crlf.handle(lexer)); // recognize '\n' + EXPECT_EQ(&s_crlf, s_start.handle(lexer, common_options)); // recognize '\r' + EXPECT_EQ(s_null, s_crlf.handle(lexer, common_options)); // recognize '\n' EXPECT_EQ(Token::END_OF_LINE, s_start.getToken(lexer).getType()); EXPECT_TRUE(s_start.wasLastEOL(lexer)); // Single '\r' (not followed by \n) is recognized as a single 'end-of-line' ss << "\r "; // then there will be "initial WS" - State::getStartInstance(lexer, common_options); // specify usual options - EXPECT_EQ(&s_crlf, s_start.handle(lexer)); // recognize '\r' - EXPECT_EQ(s_null, s_crlf.handle(lexer)); // recognize ' ', "unget" it - EXPECT_EQ(s_null, s_start.handle(lexer)); // re-recognize ' ' + EXPECT_EQ(&s_crlf, s_start.handle(lexer, common_options)); // recognize '\r' + // see ' ', "unget" it + EXPECT_EQ(s_null, s_crlf.handle(lexer, common_options)); + EXPECT_EQ(s_null, s_start.handle(lexer, common_options)); // recognize ' ' EXPECT_EQ(Token::INITIAL_WS, s_start.getToken(lexer).getType()); ss << "\r;comment\na"; - EXPECT_EQ(&s_crlf, s_start.handle(lexer)); // recognize '\r' - EXPECT_EQ(s_null, s_crlf.handle(lexer)); // skip comments, recognize '\n' + EXPECT_EQ(&s_crlf, s_start.handle(lexer, common_options)); // recognize '\r' + // skip comments, recognize '\n' + EXPECT_EQ(s_null, s_crlf.handle(lexer, common_options)); EXPECT_EQ(Token::END_OF_LINE, s_start.getToken(lexer).getType()); - EXPECT_EQ(&s_string, s_start.handle(lexer)); + EXPECT_EQ(&s_string, s_start.handle(lexer, common_options)); // \r then EOF ss << "\r"; - EXPECT_EQ(&s_crlf, s_start.handle(lexer)); // recognize '\r' - EXPECT_EQ(s_null, s_crlf.handle(lexer)); // see EOF, then "unget" it - EXPECT_EQ(s_null, s_start.handle(lexer)); // re-recognize EOF + EXPECT_EQ(&s_crlf, s_start.handle(lexer, common_options)); // recognize '\r' + // see EOF, then "unget" it + EXPECT_EQ(s_null, s_crlf.handle(lexer, common_options)); + EXPECT_EQ(s_null, s_start.handle(lexer, common_options)); // recognize EOF EXPECT_EQ(Token::END_OF_FILE, s_start.getToken(lexer).getType()); }