From: Shawn Routhier Date: Mon, 2 Nov 2015 19:22:23 +0000 (-0800) Subject: [trac4090] Update per review comments. X-Git-Tag: trac4116_base^2~3 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=b36a613740284c5197e99b8fdd3a78bcad3202b3;p=thirdparty%2Fkea.git [trac4090] Update per review comments. Update per the review comments except for the logging stuff which will be done next. --- diff --git a/ChangeLog b/ChangeLog index cd2ad8527b..8058ef04bf 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,7 @@ +XXXX. [func] sar + Added client classification substring token. + (Trac #4090, git + 1041. [func] tomek A new library, libkea-eval has been edded. It is not functional yet, but its purpose is to provide a generic expression diff --git a/src/lib/eval/eval_messages.mes b/src/lib/eval/eval_messages.mes index 1ce1ca3d12..3d86f758b4 100644 --- a/src/lib/eval/eval_messages.mes +++ b/src/lib/eval/eval_messages.mes @@ -21,4 +21,5 @@ client classification expressions. % EVAL_SUBSTRING_BAD_PARAM_CONVERSION starting %1, length %2 This debug message indicates that the parameter for the starting postion -or length of the substring couldn't be converted to an integer. +or length of the substring couldn't be converted to an integer. In this +case the substring routine returns an empty string. diff --git a/src/lib/eval/tests/token_unittest.cc b/src/lib/eval/tests/token_unittest.cc index d3bb997bf3..273bb37fef 100644 --- a/src/lib/eval/tests/token_unittest.cc +++ b/src/lib/eval/tests/token_unittest.cc @@ -233,7 +233,7 @@ TEST_F(TokenTest, optionEqualTrue) { }; // This test checks if an a token representing a substring request -// throws an excpetion if there aren't enough values on the stack. +// throws an exception if there aren't enough values on the stack. // The stack from the top is: length, start, string. // The actual packet is not used. TEST_F(TokenTest, substringNotEnoughValues) { @@ -340,8 +340,9 @@ TEST_F(TokenTest, substringBadParams) { verifySubstringEval("foobar", "0", "allaboard", ""); } -// lastly check that we don't get anything if the string is empty -TEST_F(TokenTest, substringStartingEmpty) { +// lastly check that we don't get anything if the string is empty or +// we don't ask for any characters from it. +TEST_F(TokenTest, substringReturnEmpty) { verifySubstringEval("", "0", "all", ""); verifySubstringEval("foobar", "0", "0", ""); } @@ -352,7 +353,7 @@ TEST_F(TokenTest, substringStartingEmpty) { // result on the bottom with the substring result on next. // Evaulating the equals should produce true for the first // and false for the second. -// throws an excpetion if there aren't enough values on the stack. +// throws an exception if there aren't enough values on the stack. // The stack from the top is: length, start, string. // The actual packet is not used. TEST_F(TokenTest, substringEquals) { diff --git a/src/lib/eval/token.cc b/src/lib/eval/token.cc index 3d3266a7dd..4701976865 100644 --- a/src/lib/eval/token.cc +++ b/src/lib/eval/token.cc @@ -76,14 +76,15 @@ TokenSubstring::evaluate(const Pkt& /*pkt*/, ValueStack& values) { return; } - // Convert the starting position and legnth from strings to numbers + // Convert the starting position and length from strings to numbers // the length may also be "all" in which case simply make it the - // legnth of the string. + // length of the string. // If we have a problem push an empty string and leave - int start_pos, length; + int start_pos; + int length; try { start_pos = boost::lexical_cast(start_str); - if ("all" == len_str) { + if (len_str == "all") { length = string_str.length(); } else { length = boost::lexical_cast(len_str); @@ -100,19 +101,19 @@ TokenSubstring::evaluate(const Pkt& /*pkt*/, ValueStack& values) { return; } + const int string_length = string_str.length(); // If the starting postion is outside of the string push an // empty string and leave - if (((start_pos >= 0) && (start_pos >= string_str.length())) | - ((start_pos < 0) && (-start_pos > string_str.length()))) { + if ((start_pos < -string_length) || (start_pos >= string_length)) { values.push(""); return; } // Adjust the values to be something for substr. We first figure out - // the staring postion, then update it and the length to get the - // characters before or after it depnding on the sign of length + // the starting postion, then update it and the length to get the + // characters before or after it depending on the sign of length if (start_pos < 0) { - start_pos = string_str.length() + start_pos; + start_pos = string_length + start_pos; } if (length < 0) { diff --git a/src/lib/eval/token.h b/src/lib/eval/token.h index dee4c46212..1e34051225 100644 --- a/src/lib/eval/token.h +++ b/src/lib/eval/token.h @@ -150,10 +150,10 @@ public: /// either "true" or "false". It requires at least two parameters to be /// present on stack. /// - /// @throw EvalBadStack if there's less than 2 values on stack + /// @throw EvalBadStack if there are less than 2 values on stack /// - /// @brief pkt (unused) - /// @brief values - stack of values (2 arguments will be poped, 1 result + /// @param pkt (unused) + /// @param values - stack of values (2 arguments will be popped, 1 result /// will be pushed) void evaluate(const Pkt& pkt, ValueStack& values); }; @@ -170,36 +170,45 @@ public: /// @brief Extract a substring from a string /// - /// Evaluation does not use packet information, but rather consumes the last - /// three parameters and requires at least three parameters to be present on - /// stack. From the top it expects the values on the stack as: - /// len - /// start - /// str + /// Evaluation does not use packet information. It requires at least + /// three values to be present on the stack. It will consume the top + /// three values on the stack as parameters and push the resulting substring + /// onto the stack. From the top it expects the values on the stack as: + /// - len + /// - start + /// - str /// - /// str is the string to extract a substring from. If it is empty an empty + /// str is the string to extract a substring from. If it is empty, an empty /// string is pushed onto the value stack. /// /// start is the postion from which the code starts extracting the substring. - /// 0 is before the first character and a negative number starts from the end, - /// with -1 being before the last character. If the starting point is - /// outside of the original string an empty string is pushed onto the value - /// stack. + /// 0 is the first character and a negative number starts from the end, with + /// -1 being the last character. If the starting point is outside of the + /// original string an empty string is pushed onto the value stack. /// /// length is the number of characters from the string to extract. /// "all" means all remaining characters from start to the end of string. /// A negative number means to go from start towards the beginning of - /// string, but doesn't include start. + /// the string, but doesn't include start. /// If length is longer than the remaining portion of string /// then the entire remaining portion is placed on the value stack. - /// Note: a negative value of length only indicates which characters to - /// extract, it does NOT indicate any attempt to reverse the string. - /// For example substring("foobar", 4, -2) will result in "ob". - /// - /// @throw EvalBadStack if there's less than 3 values on stack /// - /// @brief pkt (unused) - /// @brief values - stack of values (3 arguments will be poped, 1 result + /// The following examples all use the base string "foobar", the first number + /// is the starting position and the second is the length. Note that + /// a negative length only selects which characters to extract it does not + /// indicate an attempt to reverse the string. + /// - 0, all => "foobar" + /// - 0, 6 => "foobar" + /// - 0, 4 => "foob" + /// - 2, all => "obar" + /// - 2, 6 => "obar" + /// - -1, all => "r" + /// - -1, -4 => "ooba" + /// + /// @throw EvalBadStack if there are less than 3 values on stack + /// + /// @param pkt (unused) + /// @param values - stack of values (3 arguments will be popped, 1 result /// will be pushed) void evaluate(const Pkt& pkt, ValueStack& values); };