[4091] Addressed comments (0x prefix, deciding in constructor)

src/lib/eval/tests/token_unittest.cc

@@ -135,33 +135,41 @@ TEST_F(TokenTest, string6) {
 TEST_F(TokenTest, hexstring4) {
     TokenPtr empty;
     TokenPtr bad;
+    TokenPtr nodigit;
+    TokenPtr baddigit;
     TokenPtr bell;
     TokenPtr foo;
     TokenPtr cookie;
 
     // Store constant empty hexstring "" ("") in the TokenHexString object.
     ASSERT_NO_THROW(empty.reset(new TokenHexString("")));
-    // Store bad encoded hexstring "xabc" ("") in the TokenHexString object.
-    ASSERT_NO_THROW(bad.reset(new TokenHexString("xabc")));
-    // Store hexstring with an odd number of hexdigits "7" ("\a")
-    ASSERT_NO_THROW(bell.reset(new TokenHexString("7")));
-    // Store constant hexstring "666f6f" ("foo") in the TokenHexString object.
-    ASSERT_NO_THROW(foo.reset(new TokenHexString("666f6f")));
-    // Store constant hexstring "63825363" (DHCP_OPTIONS_COOKIE)
-    ASSERT_NO_THROW(cookie.reset(new TokenHexString("63825363")));
+    // Store bad encoded hexstring "0abc" ("").
+    ASSERT_NO_THROW(bad.reset(new TokenHexString("0abc")));
+    // Store hexstring with no digits "0x" ("").
+    ASSERT_NO_THROW(nodigit.reset(new TokenHexString("0x")));
+    // Store hexstring with a bad hexdigit "0xxabc" ("").
+    ASSERT_NO_THROW(baddigit.reset(new TokenHexString("0xxabc")));
+    // Store hexstring with an odd number of hexdigits "0x7" ("\a").
+    ASSERT_NO_THROW(bell.reset(new TokenHexString("0x7")));
+    // Store constant hexstring "0x666f6f" ("foo").
+    ASSERT_NO_THROW(foo.reset(new TokenHexString("0x666f6f")));
+    // Store constant hexstring "0x63825363" (DHCP_OPTIONS_COOKIE).
+    ASSERT_NO_THROW(cookie.reset(new TokenHexString("0x63825363")));
 
     // Make sure that tokens can be evaluated without exceptions.
     ASSERT_NO_THROW(empty->evaluate(*pkt4_, values_));
     ASSERT_NO_THROW(bad->evaluate(*pkt4_, values_));
+    ASSERT_NO_THROW(nodigit->evaluate(*pkt4_, values_));
+    ASSERT_NO_THROW(baddigit->evaluate(*pkt4_, values_));
     ASSERT_NO_THROW(bell->evaluate(*pkt4_, values_));
     ASSERT_NO_THROW(foo->evaluate(*pkt4_, values_));
     ASSERT_NO_THROW(cookie->evaluate(*pkt4_, values_));
 
     // Check that the evaluation put its value on the values stack.
-    ASSERT_EQ(5, values_.size());
+    ASSERT_EQ(7, values_.size());
     uint32_t expected = htonl(DHCP_OPTIONS_COOKIE);
     EXPECT_EQ(4, values_.top().size());
-    EXPECT_EQ(0, std::memcmp(&expected, &values_.top()[0], 4));
+    EXPECT_EQ(0, memcmp(&expected, &values_.top()[0], 4));
     values_.pop();
     EXPECT_EQ("foo", values_.top());
     values_.pop();
@@ -170,6 +178,10 @@ TEST_F(TokenTest, hexstring4) {
     EXPECT_EQ("", values_.top());
     values_.pop();
     EXPECT_EQ("", values_.top());
+    values_.pop();
+    EXPECT_EQ("", values_.top());
+    values_.pop();
+    EXPECT_EQ("", values_.top());
 }
 
 // This simple test checks that a TokenHexString, representing a constant
@@ -178,33 +190,41 @@ TEST_F(TokenTest, hexstring4) {
 TEST_F(TokenTest, hexstring6) {
     TokenPtr empty;
     TokenPtr bad;
+    TokenPtr nodigit;
+    TokenPtr baddigit;
     TokenPtr bell;
     TokenPtr foo;
     TokenPtr cookie;
 
     // Store constant empty hexstring "" ("") in the TokenHexString object.
     ASSERT_NO_THROW(empty.reset(new TokenHexString("")));
-    // Store bad encoded hexstring "xabc" ("") in the TokenHexString object.
-    ASSERT_NO_THROW(bad.reset(new TokenHexString("xabc")));
-    // Store hexstring with an odd number of hexdigits "7" ("\a")
-    ASSERT_NO_THROW(bell.reset(new TokenHexString("7")));
-    // Store constant hexstring "666f6f" ("foo") in the TokenHexString object.
-    ASSERT_NO_THROW(foo.reset(new TokenHexString("666f6f")));
-    // Store constant hexstring "63825363" (DHCP_OPTIONS_COOKIE)
-    ASSERT_NO_THROW(cookie.reset(new TokenHexString("63825363")));
+    // Store bad encoded hexstring "0abc" ("").
+    ASSERT_NO_THROW(bad.reset(new TokenHexString("0abc")));
+    // Store hexstring with no digits "0x" ("").
+    ASSERT_NO_THROW(nodigit.reset(new TokenHexString("0x")));
+    // Store hexstring with a bad hexdigit "0xxabc" ("").
+    ASSERT_NO_THROW(baddigit.reset(new TokenHexString("0xxabc")));
+    // Store hexstring with an odd number of hexdigits "0x7" ("\a").
+    ASSERT_NO_THROW(bell.reset(new TokenHexString("0x7")));
+    // Store constant hexstring "0x666f6f" ("foo").
+    ASSERT_NO_THROW(foo.reset(new TokenHexString("0x666f6f")));
+    // Store constant hexstring "0x63825363" (DHCP_OPTIONS_COOKIE).
+    ASSERT_NO_THROW(cookie.reset(new TokenHexString("0x63825363")));
 
     // Make sure that tokens can be evaluated without exceptions.
     ASSERT_NO_THROW(empty->evaluate(*pkt6_, values_));
     ASSERT_NO_THROW(bad->evaluate(*pkt6_, values_));
+    ASSERT_NO_THROW(nodigit->evaluate(*pkt6_, values_));
+    ASSERT_NO_THROW(baddigit->evaluate(*pkt6_, values_));
     ASSERT_NO_THROW(bell->evaluate(*pkt6_, values_));
     ASSERT_NO_THROW(foo->evaluate(*pkt6_, values_));
     ASSERT_NO_THROW(cookie->evaluate(*pkt6_, values_));
 
     // Check that the evaluation put its value on the values stack.
-    ASSERT_EQ(5, values_.size());
+    ASSERT_EQ(7, values_.size());
     uint32_t expected = htonl(DHCP_OPTIONS_COOKIE);
     EXPECT_EQ(4, values_.top().size());
-    EXPECT_EQ(0, std::memcmp(&expected, &values_.top()[0], 4));
+    EXPECT_EQ(0, memcmp(&expected, &values_.top()[0], 4));
     values_.pop();
     EXPECT_EQ("foo", values_.top());
     values_.pop();
@@ -213,6 +233,10 @@ TEST_F(TokenTest, hexstring6) {
     EXPECT_EQ("", values_.top());
     values_.pop();
     EXPECT_EQ("", values_.top());
+    values_.pop();
+    EXPECT_EQ("", values_.top());
+    values_.pop();
+    EXPECT_EQ("", values_.top());
 }
 
 // This test checks if a token representing an option value is able to extract

src/lib/eval/token.cc

@@ -28,33 +28,43 @@ TokenString::evaluate(const Pkt& /*pkt*/, ValueStack& values) {
     values.push(value_);
 }
 
-void
-TokenHexString::evaluate(const Pkt& /*pkt*/, ValueStack& values) {
-    // Eliminate the empty string case first
-    if (repr_.empty()) {
-        values.push("");
+TokenHexString::TokenHexString(const string& str) : value_("") {
+    // Check "0x" or "0x" in front
+    if ((str.size() < 2) ||
+        (str[0] != '0') ||
+        ((str[1] != 'x') && (str[1] != 'X'))) {
         return;
     }
+    string digits = str.substr(2);
+
+    // Eliminate the no digit case first
+    if (digits.empty()) {
+        return;
+    }
+
     // Transform string of hexadecimal digits into binary format
-    std::vector<uint8_t> binary;
+    vector<uint8_t> binary;
     try {
         // The decodeHex function expects that the string contains an
         // even number of digits. If we don't meet this requirement,
         // we have to insert a leading 0.
-        if ((repr_.length() % 2) != 0) {
-            repr_ = repr_.insert(0, "0");
+        if ((digits.length() % 2) != 0) {
+            digits = digits.insert(0, "0");
         }
-        util::encode::decodeHex(repr_, binary);
+        util::encode::decodeHex(digits, binary);
     } catch (...) {
-        values.push("");
         return;
     }
-    // Convert to a string
-    std::string chars(binary.size(), '\0');
-    // Note that binary.size() cannot be 0
-    std::memmove(&chars[0], &binary[0], binary.size());
+
+    // Convert to a string (note that binary.size() cannot be 0)
+    value_.resize(binary.size());
+    memmove(&value_[0], &binary[0], binary.size());
+}
+
+void
+TokenHexString::evaluate(const Pkt& /*pkt*/, ValueStack& values) {
     // Literals only push, nothing to pop
-    values.push(chars);
+    values.push(value_);
 }
 
 void

src/lib/eval/token.h

@@ -110,10 +110,9 @@ public:
     /// Value is set during token construction.
     ///
     /// @param str constant string to be represented
-    /// (must be a string of hexadecimal digits or decoding will fail)
-    TokenHexString(const std::string& str)
-        :repr_(str){
-    }
+    /// (must be "0x" or "0X" followed by a string of hexadecimal digits
+    /// or decoding will fail)
+    TokenHexString(const std::string& str);
 
     /// @brief Token evaluation (puts value of the constant string on
     /// the stack after decoding or an empty string if decoding fails
@@ -124,7 +123,7 @@ public:
     void evaluate(const Pkt& pkt, ValueStack& values);
 
 protected:
-    std::string repr_; ///< Constant value
+    std::string value_; ///< Constant value
 };
 
 /// @brief Token that represents a value of an option