From 92e9995c0edcb4bb8b0035de7f7bcb59c66ccc6c Mon Sep 17 00:00:00 2001 From: Francis Dupont Date: Fri, 6 Nov 2015 09:29:44 +0100 Subject: [PATCH] [4088fd] Fixed error handling --- src/lib/eval/lexer.cc | 4 +- src/lib/eval/lexer.ll | 4 +- src/lib/eval/parser.cc | 45 ++++++++----------- src/lib/eval/parser.yy | 5 --- src/lib/eval/tests/context_unittest.cc | 61 +++++++++++++++++++++++++- 5 files changed, 83 insertions(+), 36 deletions(-) diff --git a/src/lib/eval/lexer.cc b/src/lib/eval/lexer.cc index faf24515a9..fdc3e69cdd 100644 --- a/src/lib/eval/lexer.cc +++ b/src/lib/eval/lexer.cc @@ -2234,6 +2234,7 @@ using namespace isc::eval; void EvalContext::scanFileBegin() { + loc.initialize(&file_); yy_flex_debug = trace_scanning_; if (file_.empty () || file_ == "-") { yyin = stdin; @@ -2253,8 +2254,9 @@ EvalContext::scanFileEnd() void EvalContext::scanStringBegin() { - YY_BUFFER_STATE buffer; + loc.initialize(&file_); yy_flex_debug = trace_scanning_; + YY_BUFFER_STATE buffer; buffer = yy_scan_bytes(string_.c_str(),string_.size()); if (!buffer) { error("cannot scan string"); diff --git a/src/lib/eval/lexer.ll b/src/lib/eval/lexer.ll index 6b04514ba7..7903b3bbf5 100644 --- a/src/lib/eval/lexer.ll +++ b/src/lib/eval/lexer.ll @@ -147,6 +147,7 @@ using namespace isc::eval; void EvalContext::scanFileBegin() { + loc.initialize(&file_); yy_flex_debug = trace_scanning_; if (file_.empty () || file_ == "-") { yyin = stdin; @@ -166,8 +167,9 @@ EvalContext::scanFileEnd() void EvalContext::scanStringBegin() { - YY_BUFFER_STATE buffer; + loc.initialize(&file_); yy_flex_debug = trace_scanning_; + YY_BUFFER_STATE buffer; buffer = yy_scan_bytes(string_.c_str(), string_.size()); if (!buffer) { error("cannot scan string"); diff --git a/src/lib/eval/parser.cc b/src/lib/eval/parser.cc index 7b5c7ef5f6..c971e90812 100644 --- a/src/lib/eval/parser.cc +++ b/src/lib/eval/parser.cc @@ -49,7 +49,7 @@ #line 51 "parser.cc" // lalr1.cc:412 // Unqualified %code blocks. -#line 43 "parser.yy" // lalr1.cc:413 +#line 38 "parser.yy" // lalr1.cc:413 # include "eval_context.h" @@ -322,21 +322,21 @@ namespace isc { namespace eval { { case 11: // "constant string" -#line 61 "parser.yy" // lalr1.cc:636 +#line 56 "parser.yy" // lalr1.cc:636 { yyoutput << yysym.value.template as< std::string > (); } #line 328 "parser.cc" // lalr1.cc:636 break; case 12: // "constant hexstring" -#line 61 "parser.yy" // lalr1.cc:636 +#line 56 "parser.yy" // lalr1.cc:636 { yyoutput << yysym.value.template as< std::string > (); } #line 335 "parser.cc" // lalr1.cc:636 break; case 13: // "option code" -#line 61 "parser.yy" // lalr1.cc:636 +#line 56 "parser.yy" // lalr1.cc:636 { yyoutput << yysym.value.template as< uint16_t > (); } #line 342 "parser.cc" // lalr1.cc:636 break; @@ -450,15 +450,6 @@ namespace isc { namespace eval { YYCDEBUG << "Starting parse" << std::endl; - // User initialization code. - #line 36 "parser.yy" // lalr1.cc:745 -{ - // Initialize the initial location. - yyla.location.begin.filename = yyla.location.end.filename = &ctx.file_; -} - -#line 461 "parser.cc" // lalr1.cc:745 - /* Initialize the stack. The initial state will be set in yynewstate, since the latter expects the semantical and the location values to have been already stored, initialize these @@ -574,52 +565,52 @@ namespace isc { namespace eval { switch (yyn) { case 2: -#line 70 "parser.yy" // lalr1.cc:859 +#line 65 "parser.yy" // lalr1.cc:859 { TokenPtr eq(new TokenEqual()); ctx.expression.push_back(eq); } -#line 583 "parser.cc" // lalr1.cc:859 +#line 574 "parser.cc" // lalr1.cc:859 break; case 4: -#line 78 "parser.yy" // lalr1.cc:859 +#line 73 "parser.yy" // lalr1.cc:859 { TokenPtr str(new TokenString(yystack_[0].value.as< std::string > ())); ctx.expression.push_back(str); } -#line 592 "parser.cc" // lalr1.cc:859 +#line 583 "parser.cc" // lalr1.cc:859 break; case 5: -#line 82 "parser.yy" // lalr1.cc:859 +#line 77 "parser.yy" // lalr1.cc:859 { TokenPtr hex(new TokenHexString(yystack_[0].value.as< std::string > ())); ctx.expression.push_back(hex); } -#line 601 "parser.cc" // lalr1.cc:859 +#line 592 "parser.cc" // lalr1.cc:859 break; case 6: -#line 86 "parser.yy" // lalr1.cc:859 +#line 81 "parser.yy" // lalr1.cc:859 { TokenPtr opt(new TokenOption(yystack_[1].value.as< uint16_t > ())); ctx.expression.push_back(opt); } -#line 610 "parser.cc" // lalr1.cc:859 +#line 601 "parser.cc" // lalr1.cc:859 break; case 7: -#line 90 "parser.yy" // lalr1.cc:859 +#line 85 "parser.yy" // lalr1.cc:859 { TokenPtr sub(new TokenSubstring()); ctx.expression.push_back(sub); } -#line 619 "parser.cc" // lalr1.cc:859 +#line 610 "parser.cc" // lalr1.cc:859 break; -#line 623 "parser.cc" // lalr1.cc:859 +#line 614 "parser.cc" // lalr1.cc:859 default: break; } @@ -954,7 +945,7 @@ namespace isc { namespace eval { const unsigned char EvalParser::yyrline_[] = { - 0, 70, 70, 74, 78, 82, 86, 90 + 0, 65, 65, 69, 73, 77, 81, 85 }; // Print the state stack on the debug stream. @@ -989,8 +980,8 @@ namespace isc { namespace eval { #line 21 "parser.yy" // lalr1.cc:1167 } } // isc::eval -#line 993 "parser.cc" // lalr1.cc:1167 -#line 96 "parser.yy" // lalr1.cc:1168 +#line 984 "parser.cc" // lalr1.cc:1167 +#line 91 "parser.yy" // lalr1.cc:1168 void isc::eval::EvalParser::error(const location_type& loc, diff --git a/src/lib/eval/parser.yy b/src/lib/eval/parser.yy index b27d6533e0..1a388c4930 100644 --- a/src/lib/eval/parser.yy +++ b/src/lib/eval/parser.yy @@ -32,11 +32,6 @@ using namespace isc::eval; // The parsing context. %param { EvalContext& ctx } %locations -%initial-action -{ - // Initialize the initial location. - @$.begin.filename = @$.end.filename = &ctx.file_; -}; %define parse.trace %define parse.error verbose %code diff --git a/src/lib/eval/tests/context_unittest.cc b/src/lib/eval/tests/context_unittest.cc index 6f8c1e9f85..df6e2bb84e 100644 --- a/src/lib/eval/tests/context_unittest.cc +++ b/src/lib/eval/tests/context_unittest.cc @@ -27,8 +27,10 @@ using namespace isc::dhcp; namespace { +/// @brief Test class for testing EvalContext aka class test parsing class EvalContextTest : public ::testing::Test { public: + /// @brief checks if the given token is a string with the expected value void checkTokenString(const TokenPtr& token, const std::string& expected) { ASSERT_TRUE(token); boost::shared_ptr str = @@ -45,6 +47,7 @@ public: EXPECT_EQ(expected, values.top()); } + /// @brief checks if the given token is a hex string with the expected value void checkTokenHexString(const TokenPtr& token, const std::string& expected) { ASSERT_TRUE(token); @@ -62,6 +65,7 @@ public: EXPECT_EQ(expected, values.top()); } + /// @brief checks if the given token is an equal operator void checkTokenEq(const TokenPtr& token) { ASSERT_TRUE(token); boost::shared_ptr eq = @@ -69,15 +73,17 @@ public: EXPECT_TRUE(eq); } - void checkTokenOption(const TokenPtr& token, uint16_t expected_option) { + /// @brief checks if the given token is an option with the expected code + void checkTokenOption(const TokenPtr& token, uint16_t expected_code) { ASSERT_TRUE(token); boost::shared_ptr opt = boost::dynamic_pointer_cast(token); ASSERT_TRUE(opt); - EXPECT_EQ(expected_option, opt->getCode()); + EXPECT_EQ(expected_code, opt->getCode()); } + /// @brief checks if the given token is a substring operator void checkTokenSubstring(const TokenPtr& token) { ASSERT_TRUE(token); boost::shared_ptr sub = @@ -85,9 +91,28 @@ public: EXPECT_TRUE(sub); } + /// @brief checks if the given expression raises the expected message + /// when it is parsed. + void checkError(const string& expr, const string& msg) { + EvalContext eval; + parsed_ = false; + try { + parsed_ = eval.parseString(expr); + FAIL() << "Expected EvalParseError but nothing was raised"; + } + catch (const EvalParseError& ex) { + EXPECT_EQ(msg, ex.what()); + EXPECT_FALSE(parsed_); + } + catch (...) { + FAIL() << "Expected EvalParseError but something else was raised"; + } + } + bool parsed_; ///< Parsing status }; +// Test the parsing of a basic expression TEST_F(EvalContextTest, basic) { EvalContext tmp; @@ -96,6 +121,7 @@ TEST_F(EvalContextTest, basic) { EXPECT_TRUE(parsed_); } +// Test the parsing of a string terminal TEST_F(EvalContextTest, string) { EvalContext eval; @@ -109,6 +135,7 @@ TEST_F(EvalContextTest, string) { checkTokenString(tmp, "foo"); } +// Test the parsing of a hexstring terminal TEST_F(EvalContextTest, hexstring) { EvalContext eval; @@ -122,6 +149,7 @@ TEST_F(EvalContextTest, hexstring) { checkTokenHexString(tmp, "foo"); } +// Test the parsing of an equal expression TEST_F(EvalContextTest, equal) { EvalContext eval; @@ -139,6 +167,7 @@ TEST_F(EvalContextTest, equal) { checkTokenEq(tmp3); } +// Test the parsing of an option terminal TEST_F(EvalContextTest, option) { EvalContext eval; @@ -148,6 +177,7 @@ TEST_F(EvalContextTest, option) { checkTokenOption(eval.expression.at(0), 123); } +// Test the parsing of a substring expression TEST_F(EvalContextTest, substring) { EvalContext eval; @@ -167,4 +197,31 @@ TEST_F(EvalContextTest, substring) { checkTokenSubstring(tmp4); } +// Test some scanner error cases +TEST_F(EvalContextTest, scanErrors) { + checkError("'", ":1.1: Invalid character: '"); + checkError("'\''", ":1.3: Invalid character: '"); + checkError("'\n'", ":1.1: Invalid character: '"); + checkError("0x123h", ":1.6: Invalid character: h"); + checkError("=", ":1.1: Invalid character: ="); + checkError("option[65536]", ":1.8-12: Option code has invalid " + "value in 65536. Allowed range: 0..65535"); + checkError("subtring", ":1.1: Invalid character: s"); + checkError("foo", ":1.1: Invalid character: f"); + checkError(" bar", ":1.2: Invalid character: b"); +} + +// Tests some scanner/parser error cases +TEST_F(EvalContextTest, scanParseErrors) { + checkError("", ":1.1: syntax error, unexpected end of file, " + "expecting option or substring or constant string or " + "constant hexstring"); + checkError("0x", ":1.1: syntax error, unexpected option code, " + "expecting option or substring or constant string or " + "constant hexstring"); + checkError("===", ":1.1-2: syntax error, unexpected ==, " + "expecting option or substring or constant string " + "or constant hexstring"); +} + }; -- 2.47.3