From 949e3476070fec6be8d90fa2213a8a8ed1755bbe Mon Sep 17 00:00:00 2001 From: Razvan Becheriu Date: Fri, 23 Apr 2021 15:14:40 +0300 Subject: [PATCH] [#1680] addressed comments --- doc/sphinx/arm/classify.rst | 12 ++++----- src/lib/eval/eval.dox | 14 +++++----- src/lib/eval/eval_context.cc | 8 +++--- src/lib/eval/eval_messages.mes | 12 ++++----- src/lib/eval/tests/context_unittest.cc | 26 +++++++++---------- src/lib/eval/token.cc | 35 +++---------------------- src/lib/eval/token.h | 36 +++++++++++++------------- 7 files changed, 58 insertions(+), 85 deletions(-) diff --git a/doc/sphinx/arm/classify.rst b/doc/sphinx/arm/classify.rst index 7dbf896759..40b97296dc 100644 --- a/doc/sphinx/arm/classify.rst +++ b/doc/sphinx/arm/classify.rst @@ -476,27 +476,27 @@ Notes: | | | IPv6 address in human | | | | readable format | +-----------------------+---------------------------+------------------------+ - | Int8ToText | int8totext (-1) | Represents the 8 bits | + | Int8ToText | int8totext (-1) | Represents the 8 bit | | | | signed integer in text | | | | format | +-----------------------+---------------------------+------------------------+ - | Int16ToText | int16totext (-1) | Represents the 16 bits | + | Int16ToText | int16totext (-1) | Represents the 16 bit | | | | signed integer in text | | | | format | +-----------------------+---------------------------+------------------------+ - | Int32ToText | int32totext (-1) | Represents the 32 bits | + | Int32ToText | int32totext (-1) | Represents the 32 bit | | | | signed integer in text | | | | format | +-----------------------+---------------------------+------------------------+ - | UInt8ToText | uint8totext (255) | Represents the 8 bits | + | UInt8ToText | uint8totext (255) | Represents the 8 bit | | | | unsigned integer in | | | | text format | +-----------------------+---------------------------+------------------------+ - | UInt16ToText | uint16totext (65535) | Represents the 16 bits | + | UInt16ToText | uint16totext (65535) | Represents the 16 bit | | | | unsigned integer in | | | | text format | +-----------------------+---------------------------+------------------------+ - | UInt32ToText | uint32totext (4294967295) | Represents the 32 bits | + | UInt32ToText | uint32totext (4294967295) | Represents the 32 bit | | | | unsigned integer in | | | | text format | +-----------------------+---------------------------+------------------------+ diff --git a/src/lib/eval/eval.dox b/src/lib/eval/eval.dox index bea1c87ab7..a057e0f371 100644 --- a/src/lib/eval/eval.dox +++ b/src/lib/eval/eval.dox @@ -163,18 +163,18 @@ instantiated with the appropriate value and put onto the expression vector. concatenate two other tokens. - isc::dhcp::TokenIfElse -- represents the ifelse(cond, iftrue, ifelse) operator. - isc::dhcp::TokenToHexString -- represents the hexstring operator which - converts a binary value to its hexadecimal string representation. - - isc::dhcp::TokenInt8ToText -- represents the signed 8 bits integer in string + converts a binary value to its hexadecimal string representation. + - isc::dhcp::TokenInt8ToText -- represents the signed 8 bit integer in string representation. - - isc::dhcp::TokenInt16ToText -- represents the signed 16 bits integer in string + - isc::dhcp::TokenInt16ToText -- represents the signed 16 bit integer in string representation. - - isc::dhcp::TokenInt32ToText -- represents the signed 32 bits integer in string + - isc::dhcp::TokenInt32ToText -- represents the signed 32 bit integer in string representation. - - isc::dhcp::TokenUInt8ToText -- represents the unsigned 8 bits integer in string + - isc::dhcp::TokenUInt8ToText -- represents the unsigned 8 bit integer in string representation. - - isc::dhcp::TokenUInt16ToText -- represents the unsigned 16 bits integer in string + - isc::dhcp::TokenUInt16ToText -- represents the unsigned 16 bit integer in string representation. - - isc::dhcp::TokenUInt32ToText -- represents the unsigned 32 bits integer in string + - isc::dhcp::TokenUInt32ToText -- represents the unsigned 32 bit integer in string representation. - isc::dhcp::TokenNot -- the logical not operator. - isc::dhcp::TokenAnd -- the logical and (strict) operator. diff --git a/src/lib/eval/eval_context.cc b/src/lib/eval/eval_context.cc index 5830007e2a..df12de567a 100644 --- a/src/lib/eval/eval_context.cc +++ b/src/lib/eval/eval_context.cc @@ -127,9 +127,9 @@ EvalContext::convertNestLevelNumber(const std::string& nest_level, uint8_t EvalContext::convertUint8(const std::string& number, const isc::eval::location& loc) { - int32_t n = 0; + int64_t n = 0; try { - n = boost::lexical_cast(number); + n = boost::lexical_cast(number); } catch (const boost::bad_lexical_cast &) { error(loc, "Invalid integer value in " + number); } @@ -144,9 +144,9 @@ EvalContext::convertUint8(const std::string& number, int8_t EvalContext::convertInt8(const std::string& number, const isc::eval::location& loc) { - int32_t n = 0; + int64_t n = 0; try { - n = boost::lexical_cast(number); + n = boost::lexical_cast(number); } catch (const boost::bad_lexical_cast &) { error(loc, "Invalid integer value in " + number); } diff --git a/src/lib/eval/eval_messages.mes b/src/lib/eval/eval_messages.mes index f65f933c2a..4d58251f7f 100644 --- a/src/lib/eval/eval_messages.mes +++ b/src/lib/eval/eval_messages.mes @@ -64,37 +64,37 @@ address. % EVAL_DEBUG_INT8TOTEXT Pushing Int8 %1 This debug message indicates that the given address string representation is -being pushed onto the value stack. This represents an 8 bits integer. +being pushed onto the value stack. This represents an 8 bit integer. # For use with TokenInt16ToText % EVAL_DEBUG_INT16TOTEXT Pushing Int16 %1 This debug message indicates that the given address string representation is -being pushed onto the value stack. This represents a 16 bits integer. +being pushed onto the value stack. This represents a 16 bit integer. # For use with TokenInt32ToText % EVAL_DEBUG_INT32TOTEXT Pushing Int32 %1 This debug message indicates that the given address string representation is -being pushed onto the value stack. This represents a 32 bits integer. +being pushed onto the value stack. This represents a 32 bit integer. # For use with TokenUInt8ToText % EVAL_DEBUG_UINT8TOTEXT Pushing UInt8 %1 This debug message indicates that the given address string representation is -being pushed onto the value stack. This represents an 8 bits unsigned integer. +being pushed onto the value stack. This represents an 8 bit unsigned integer. # For use with TokenUInt16ToText % EVAL_DEBUG_UINT16TOTEXT Pushing UInt16 %1 This debug message indicates that the given address string representation is -being pushed onto the value stack. This represents a 16 bits unsigned integer. +being pushed onto the value stack. This represents a 16 bit unsigned integer. # For use with TokenUInt32ToText % EVAL_DEBUG_UINT32TOTEXT Pushing UInt32 %1 This debug message indicates that the given address string representation is -being pushed onto the value stack. This represents a 32 bits unsigned integer. +being pushed onto the value stack. This represents a 32 bit unsigned integer. # For use with TokenMember diff --git a/src/lib/eval/tests/context_unittest.cc b/src/lib/eval/tests/context_unittest.cc index 2c03c39d5b..13eb9b8a77 100644 --- a/src/lib/eval/tests/context_unittest.cc +++ b/src/lib/eval/tests/context_unittest.cc @@ -493,7 +493,7 @@ public: } /// @brief checks if the given token is a inttotext operator - template + template void checkTokenIntToText(const TokenPtr& token, const std::string& expected) { ASSERT_TRUE(token); @@ -504,13 +504,13 @@ public: Pkt4Ptr pkt4(new Pkt4(DHCPDISCOVER, 12345)); ValueStack values; - Integer n; + IntegerType n; try { - if (is_signed()) { - n = static_cast(boost::lexical_cast(expected)); + if (is_signed()) { + n = static_cast(boost::lexical_cast(expected)); } else { - n = static_cast(boost::lexical_cast(expected)); + n = static_cast(boost::lexical_cast(expected)); } } catch (const boost::bad_lexical_cast& e) { FAIL() << "invalid value " << expected << " while expecting " @@ -518,7 +518,7 @@ public: << e.what(); } - values.push(std::string(const_cast(reinterpret_cast(&n)), sizeof(Integer))); + values.push(std::string(const_cast(reinterpret_cast(&n)), sizeof(IntegerType))); EXPECT_NO_THROW(token->evaluate(*pkt4, values)); @@ -1512,7 +1512,7 @@ TEST_F(EvalContextTest, toHexString) { checkTokenToHexString(tmp3); } -// Test the parsing of a addrtotext expression +// Test the parsing of an addrtotext expression TEST_F(EvalContextTest, addressToText) { { EvalContext eval(Option::V4); @@ -1943,27 +1943,27 @@ TEST_F(EvalContextTest, typeErrors) { checkError("addrtotext('192.100.1.1')", ":1.26: syntax error, unexpected end of file, expecting =="); - // Int8totext requires string storing the binary representation of the 8 bits integer. + // Int8totext requires string storing the binary representation of the 8 bit integer. checkError("int8totext('0123')", ":1.19: syntax error, unexpected end of file, expecting =="); - // Int16totext requires string storing the binary representation of the 16 bits integer. + // Int16totext requires string storing the binary representation of the 16 bit integer. checkError("int16totext('01')", ":1.18: syntax error, unexpected end of file, expecting =="); - // Int32totext requires string storing the binary representation of the 32 bits integer. + // Int32totext requires string storing the binary representation of the 32 bit integer. checkError("int32totext('01')", ":1.18: syntax error, unexpected end of file, expecting =="); - // Uint8totext requires string storing the binary representation of the 8 bits unsigned integer. + // Uint8totext requires string storing the binary representation of the 8 bit unsigned integer. checkError("uint8totext('0123')", ":1.20: syntax error, unexpected end of file, expecting =="); - // Uint16totext requires string storing the binary representation of the 16 bits unsigned integer. + // Uint16totext requires string storing the binary representation of the 16 bit unsigned integer. checkError("uint16totext('01')", ":1.19: syntax error, unexpected end of file, expecting =="); - // Uint32totext requires string storing the binary representation of the 32 bits unsigned integer. + // Uint32totext requires string storing the binary representation of the 32 bit unsigned integer. checkError("uint32totext('01')", ":1.19: syntax error, unexpected end of file, expecting =="); } diff --git a/src/lib/eval/token.cc b/src/lib/eval/token.cc index f27eb69f7d..f42123c7c0 100644 --- a/src/lib/eval/token.cc +++ b/src/lib/eval/token.cc @@ -32,7 +32,6 @@ using isc::util::encode::toHex; void TokenString::evaluate(Pkt& /*pkt*/, ValueStack& values) { - // Literals only push, nothing to pop values.push(value_); @@ -71,7 +70,6 @@ TokenHexString::TokenHexString(const string& str) : value_("") { void TokenHexString::evaluate(Pkt& /*pkt*/, ValueStack& values) { - // Literals only push, nothing to pop values.push(value_); @@ -97,7 +95,6 @@ TokenIpAddress::TokenIpAddress(const string& addr) : value_("") { void TokenIpAddress::evaluate(Pkt& /*pkt*/, ValueStack& values) { - // Literals only push, nothing to pop values.push(value_); @@ -108,26 +105,25 @@ TokenIpAddress::evaluate(Pkt& /*pkt*/, ValueStack& values) { void TokenIpAddressToText::evaluate(Pkt& /*pkt*/, ValueStack& values) { - if (values.size() == 0) { isc_throw(EvalBadStack, "Incorrect empty stack."); } - size_t size; string op = values.top(); + size_t size = op.size(); - if (!(size = op.size())) { + if (!size) { return; } values.pop(); - if ((size != sizeof(uint32_t)) && (size != INET_ADDRSTRLEN)) { + if ((size != V4ADDRESS_LEN) && (size != V6ADDRESS_LEN)) { isc_throw(EvalTypeError, "Can not convert to valid address."); } std::vector binary(op.begin(), op.end()); - if (size == sizeof(uint32_t)) { + if (size == V4ADDRESS_LEN) { op = asiolink::IOAddress::fromBytes(AF_INET, binary.data()).toText(); } else { op = asiolink::IOAddress::fromBytes(AF_INET6, binary.data()).toText(); @@ -142,7 +138,6 @@ TokenIpAddressToText::evaluate(Pkt& /*pkt*/, ValueStack& values) { void TokenInt8ToText::evaluate(Pkt& /*pkt*/, ValueStack& values) { - if (values.size() == 0) { isc_throw(EvalBadStack, "Incorrect empty stack."); } @@ -171,7 +166,6 @@ TokenInt8ToText::evaluate(Pkt& /*pkt*/, ValueStack& values) { void TokenInt16ToText::evaluate(Pkt& /*pkt*/, ValueStack& values) { - if (values.size() == 0) { isc_throw(EvalBadStack, "Incorrect empty stack."); } @@ -202,7 +196,6 @@ TokenInt16ToText::evaluate(Pkt& /*pkt*/, ValueStack& values) { void TokenInt32ToText::evaluate(Pkt& /*pkt*/, ValueStack& values) { - if (values.size() == 0) { isc_throw(EvalBadStack, "Incorrect empty stack."); } @@ -233,7 +226,6 @@ TokenInt32ToText::evaluate(Pkt& /*pkt*/, ValueStack& values) { void TokenUInt8ToText::evaluate(Pkt& /*pkt*/, ValueStack& values) { - if (values.size() == 0) { isc_throw(EvalBadStack, "Incorrect empty stack."); } @@ -262,7 +254,6 @@ TokenUInt8ToText::evaluate(Pkt& /*pkt*/, ValueStack& values) { void TokenUInt16ToText::evaluate(Pkt& /*pkt*/, ValueStack& values) { - if (values.size() == 0) { isc_throw(EvalBadStack, "Incorrect empty stack."); } @@ -293,7 +284,6 @@ TokenUInt16ToText::evaluate(Pkt& /*pkt*/, ValueStack& values) { void TokenUInt32ToText::evaluate(Pkt& /*pkt*/, ValueStack& values) { - if (values.size() == 0) { isc_throw(EvalBadStack, "Incorrect empty stack."); } @@ -329,7 +319,6 @@ TokenOption::getOption(Pkt& pkt) { void TokenOption::evaluate(Pkt& pkt, ValueStack& values) { - OptionPtr opt = getOption(pkt); std::string opt_str; if (opt) { @@ -429,7 +418,6 @@ OptionPtr TokenRelay6Option::getOption(Pkt& pkt) { void TokenPkt::evaluate(Pkt& pkt, ValueStack& values) { - string value; vector binary; string type_str; @@ -481,7 +469,6 @@ TokenPkt::evaluate(Pkt& pkt, ValueStack& values) { void TokenPkt4::evaluate(Pkt& pkt, ValueStack& values) { - vector binary; string value; string type_str; @@ -561,7 +548,6 @@ TokenPkt4::evaluate(Pkt& pkt, ValueStack& values) { void TokenPkt6::evaluate(Pkt& pkt, ValueStack& values) { - string value; string type_str; try { @@ -602,7 +588,6 @@ TokenPkt6::evaluate(Pkt& pkt, ValueStack& values) { void TokenRelay6Field::evaluate(Pkt& pkt, ValueStack& values) { - vector binary; string type_str; try { @@ -665,7 +650,6 @@ TokenRelay6Field::evaluate(Pkt& pkt, ValueStack& values) { void TokenEqual::evaluate(Pkt& /*pkt*/, ValueStack& values) { - if (values.size() < 2) { isc_throw(EvalBadStack, "Incorrect stack order. Expected at least " "2 values for == operator, got " << values.size()); @@ -690,7 +674,6 @@ TokenEqual::evaluate(Pkt& /*pkt*/, ValueStack& values) { void TokenSubstring::evaluate(Pkt& /*pkt*/, ValueStack& values) { - if (values.size() < 3) { isc_throw(EvalBadStack, "Incorrect stack order. Expected at least " "3 values for substring operator, got " << values.size()); @@ -786,7 +769,6 @@ TokenSubstring::evaluate(Pkt& /*pkt*/, ValueStack& values) { void TokenConcat::evaluate(Pkt& /*pkt*/, ValueStack& values) { - if (values.size() < 2) { isc_throw(EvalBadStack, "Incorrect stack order. Expected at least " "2 values for concat, got " << values.size()); @@ -809,7 +791,6 @@ TokenConcat::evaluate(Pkt& /*pkt*/, ValueStack& values) { void TokenIfElse::evaluate(Pkt& /*pkt*/, ValueStack& values) { - if (values.size() < 3) { isc_throw(EvalBadStack, "Incorrect stack order. Expected at least " "3 values for ifelse, got " << values.size()); @@ -845,7 +826,6 @@ TokenIfElse::evaluate(Pkt& /*pkt*/, ValueStack& values) { void TokenToHexString::evaluate(Pkt& /*pkt*/, ValueStack& values) { - if (values.size() < 2) { isc_throw(EvalBadStack, "Incorrect stack order. Expected at least " "2 values for hexstring, got " << values.size()); @@ -879,7 +859,6 @@ TokenToHexString::evaluate(Pkt& /*pkt*/, ValueStack& values) { void TokenNot::evaluate(Pkt& /*pkt*/, ValueStack& values) { - if (values.size() == 0) { isc_throw(EvalBadStack, "Incorrect empty stack."); } @@ -902,7 +881,6 @@ TokenNot::evaluate(Pkt& /*pkt*/, ValueStack& values) { void TokenAnd::evaluate(Pkt& /*pkt*/, ValueStack& values) { - if (values.size() < 2) { isc_throw(EvalBadStack, "Incorrect stack order. Expected at least " "2 values for and operator, got " << values.size()); @@ -930,7 +908,6 @@ TokenAnd::evaluate(Pkt& /*pkt*/, ValueStack& values) { void TokenOr::evaluate(Pkt& /*pkt*/, ValueStack& values) { - if (values.size() < 2) { isc_throw(EvalBadStack, "Incorrect stack order. Expected at least " "2 values for or operator, got " << values.size()); @@ -958,7 +935,6 @@ TokenOr::evaluate(Pkt& /*pkt*/, ValueStack& values) { void TokenMember::evaluate(Pkt& pkt, ValueStack& values) { - if (pkt.inClass(client_class_)) { values.push("true"); } else { @@ -994,7 +970,6 @@ TokenVendor::FieldType TokenVendor::getField() const { } void TokenVendor::evaluate(Pkt& pkt, ValueStack& values) { - // Get the option first. uint16_t code = 0; switch (universe_) { @@ -1100,7 +1075,6 @@ uint16_t TokenVendorClass::getDataIndex() const { } void TokenVendorClass::evaluate(Pkt& pkt, ValueStack& values) { - // Get the option first. uint16_t code = 0; switch (universe_) { @@ -1206,7 +1180,6 @@ TokenSubOption::getSubOption(const OptionPtr& parent) { void TokenSubOption::evaluate(Pkt& pkt, ValueStack& values) { - OptionPtr parent = getOption(pkt); std::string txt; isc::log::MessageID msgid = EVAL_DEBUG_SUB_OPTION; diff --git a/src/lib/eval/token.h b/src/lib/eval/token.h index c42acbe759..3e4e3be474 100644 --- a/src/lib/eval/token.h +++ b/src/lib/eval/token.h @@ -224,9 +224,9 @@ public: void evaluate(Pkt& pkt, ValueStack& values); }; -/// @brief Token representing an 8 bits integer as a string +/// @brief Token representing an 8 bit integer as a string /// -/// This token holds the value of an 8 bits integer as a string, for instance +/// This token holds the value of an 8 bit integer as a string, for instance /// 0xff is '-1' class TokenInt8ToText : public Token { public: @@ -237,14 +237,14 @@ public: /// decoding) /// /// @param pkt (ignored) - /// @param values (represented 8 bits integer as a string will be pushed + /// @param values (represented 8 bit integer as a string will be pushed /// here) void evaluate(Pkt& pkt, ValueStack& values); }; -/// @brief Token representing a 16 bits integer as a string +/// @brief Token representing a 16 bit integer as a string /// -/// This token holds the value of a 16 bits integer as a string, for instance +/// This token holds the value of a 16 bit integer as a string, for instance /// 0xffff is '-1' class TokenInt16ToText : public Token { public: @@ -255,14 +255,14 @@ public: /// decoding) /// /// @param pkt (ignored) - /// @param values (represented 16 bits integer as a string will be pushed + /// @param values (represented 16 bit integer as a string will be pushed /// here) void evaluate(Pkt& pkt, ValueStack& values); }; -/// @brief Token representing a 32 bits integer as a string +/// @brief Token representing a 32 bit integer as a string /// -/// This token holds the value of a 32 bits integer as a string, for instance +/// This token holds the value of a 32 bit integer as a string, for instance /// 0xffffffff is '-1' class TokenInt32ToText : public Token { public: @@ -273,14 +273,14 @@ public: /// decoding) /// /// @param pkt (ignored) - /// @param values (represented 32 bits integer as a string will be pushed + /// @param values (represented 32 bit integer as a string will be pushed /// here) void evaluate(Pkt& pkt, ValueStack& values); }; -/// @brief Token representing an 8 bits unsigned integer as a string +/// @brief Token representing an 8 bit unsigned integer as a string /// -/// This token holds the value of an 8 bits unsigned integer as a string, for +/// This token holds the value of an 8 bit unsigned integer as a string, for /// instance 0xff is '255' class TokenUInt8ToText : public Token { public: @@ -291,14 +291,14 @@ public: /// decoding) /// /// @param pkt (ignored) - /// @param values (represented 8 bits unsigned integer as a string will be + /// @param values (represented 8 bit unsigned integer as a string will be /// pushed here) void evaluate(Pkt& pkt, ValueStack& values); }; -/// @brief Token representing a 16 bits unsigned integer as a string +/// @brief Token representing a 16 bit unsigned integer as a string /// -/// This token holds the value of a 16 bits unsigned integer as a string, for +/// This token holds the value of a 16 bit unsigned integer as a string, for /// instance 0xffff is '65535' class TokenUInt16ToText : public Token { public: @@ -309,14 +309,14 @@ public: /// decoding) /// /// @param pkt (ignored) - /// @param values (represented 16 bits unsigned integer as a string will be + /// @param values (represented 16 bit unsigned integer as a string will be /// pushed here) void evaluate(Pkt& pkt, ValueStack& values); }; -/// @brief Token representing a 32 bits unsigned integer as a string +/// @brief Token representing a 32 bit unsigned integer as a string /// -/// This token holds the value of a 32 bits unsigned integer as a string, for +/// This token holds the value of a 32 bit unsigned integer as a string, for /// instance 0xffffffff is '4294967295' class TokenUInt32ToText : public Token { public: @@ -327,7 +327,7 @@ public: /// decoding) /// /// @param pkt (ignored) - /// @param values (represented 32 bits unsigned integer as a string will be + /// @param values (represented 32 bit unsigned integer as a string will be /// pushed here) void evaluate(Pkt& pkt, ValueStack& values); };