Browse Source

[4081] Changes after review:

 - Copyright years corrected
 - Additional AM_CXXFLAGS for gcc added
 - LOG4CPLUS libs added to LDFLAGS
 - New unit-test for optionString for IPv6 packets added
 - Several comments in token.h clarified/corrected
 - Exception message extended slightly
Tomek Mrugalski 9 years ago
parent
commit
661e923bbc

+ 7 - 1
src/lib/eval/Makefile.am

@@ -4,6 +4,12 @@ AM_CPPFLAGS = -I$(top_builddir)/src/lib -I$(top_srcdir)/src/lib
 AM_CPPFLAGS += $(BOOST_INCLUDES)
 AM_CPPFLAGS += $(BOOST_INCLUDES)
 AM_CXXFLAGS = $(KEA_CXXFLAGS)
 AM_CXXFLAGS = $(KEA_CXXFLAGS)
 
 
+# Some versions of GCC warn about some versions of Boost regarding
+# missing initializer for members in its posix_time.
+# https://svn.boost.org/trac/boost/ticket/3477
+# But older GCC compilers don't have the flag.
+AM_CXXFLAGS += $(WARNING_NO_MISSING_FIELD_INITIALIZERS_CFLAG)
+
 lib_LTLIBRARIES = libkea-eval.la
 lib_LTLIBRARIES = libkea-eval.la
 libkea_eval_la_SOURCES  =
 libkea_eval_la_SOURCES  =
 libkea_eval_la_SOURCES += token.cc token.h
 libkea_eval_la_SOURCES += token.cc token.h
@@ -13,7 +19,7 @@ libkea_eval_la_CPPFLAGS = $(AM_CPPFLAGS)
 libkea_eval_la_LIBADD   = $(top_builddir)/src/lib/exceptions/libkea-exceptions.la
 libkea_eval_la_LIBADD   = $(top_builddir)/src/lib/exceptions/libkea-exceptions.la
 libkea_eval_la_LIBADD  += $(top_builddir)/src/lib/dhcp/libkea-dhcp++.la
 libkea_eval_la_LIBADD  += $(top_builddir)/src/lib/dhcp/libkea-dhcp++.la
 libkea_eval_la_LDFLAGS  = -no-undefined -version-info 3:0:0
 libkea_eval_la_LDFLAGS  = -no-undefined -version-info 3:0:0
-libkea_eval_la_LDFLAGS += $(CRYPTO_LDFLAGS)
+libkea_eval_la_LDFLAGS += $(LOG4CPLUS_LIBS) $(CRYPTO_LDFLAGS)
 
 
 EXTRA_DIST  = eval.dox
 EXTRA_DIST  = eval.dox
 EXTRA_DIST += eval_messages.mes
 EXTRA_DIST += eval_messages.mes

+ 1 - 1
src/lib/eval/eval.dox

@@ -1,4 +1,4 @@
-// Copyright (C) 2012-2015  Internet Systems Consortium, Inc. ("ISC")
+// Copyright (C) 2015  Internet Systems Consortium, Inc. ("ISC")
 //
 //
 // Permission to use, copy, modify, and/or distribute this software for any
 // Permission to use, copy, modify, and/or distribute this software for any
 // purpose with or without fee is hereby granted, provided that the above
 // purpose with or without fee is hereby granted, provided that the above

+ 1 - 1
src/lib/eval/eval_messages.mes

@@ -1,4 +1,4 @@
-# Copyright (C) 2012-2015  Internet Systems Consortium, Inc. ("ISC")
+# Copyright (C) 2015  Internet Systems Consortium, Inc. ("ISC")
 #
 #
 # Permission to use, copy, modify, and/or distribute this software for any
 # Permission to use, copy, modify, and/or distribute this software for any
 # purpose with or without fee is hereby granted, provided that the above
 # purpose with or without fee is hereby granted, provided that the above

+ 30 - 2
src/lib/eval/tests/token_unittest.cc

@@ -94,8 +94,8 @@ TEST_F(TokenTest, string6) {
 }
 }
 
 
 // This test checks if a token representing an option value is able to extract
 // This test checks if a token representing an option value is able to extract
-// the option from a packet and properly store the option's value.
-TEST_F(TokenTest, optionString) {
+// the option from an IPv4 packet and properly store the option's value.
+TEST_F(TokenTest, optionString4) {
     TokenPtr found;
     TokenPtr found;
     TokenPtr not_found;
     TokenPtr not_found;
 
 
@@ -121,6 +121,34 @@ TEST_F(TokenTest, optionString) {
     EXPECT_EQ("hundred4", values_.top());
     EXPECT_EQ("hundred4", values_.top());
 }
 }
 
 
+// This test checks if a token representing an option value is able to extract
+// the option from an IPv6 packet and properly store the option's value.
+TEST_F(TokenTest, optionString6) {
+    TokenPtr found;
+    TokenPtr not_found;
+
+    // The packets we use have option 100 with a string in them.
+    ASSERT_NO_THROW(found.reset(new TokenOption(100)));
+    ASSERT_NO_THROW(not_found.reset(new TokenOption(101)));
+
+    // This should evaluate to the content of the option 100 (i.e. "hundred6")
+    ASSERT_NO_THROW(found->evaluate(*pkt6_, values_));
+
+    // This should evaluate to "" as there is no option 101.
+    ASSERT_NO_THROW(not_found->evaluate(*pkt6_, values_));
+
+    // There should be 2 values evaluated.
+    ASSERT_EQ(2, values_.size());
+
+    // This is a stack, so the pop order is inversed. We should get the empty
+    // string first.
+    EXPECT_EQ("", values_.top());
+    values_.pop();
+
+    // Then the content of the option 100.
+    EXPECT_EQ("hundred6", values_.top());
+}
+
 // This test checks if a token representing an == operator is able to
 // This test checks if a token representing an == operator is able to
 // compare two values (with incorrectly built stack).
 // compare two values (with incorrectly built stack).
 TEST_F(TokenTest, optionEqualInvalid) {
 TEST_F(TokenTest, optionEqualInvalid) {

+ 1 - 1
src/lib/eval/token.cc

@@ -40,7 +40,7 @@ TokenEqual::evaluate(const Pkt& /*pkt*/, ValueStack& values) {
 
 
     if (values.size() < 2) {
     if (values.size() < 2) {
         isc_throw(EvalBadStack, "Incorrect stack order. Expected at least "
         isc_throw(EvalBadStack, "Incorrect stack order. Expected at least "
-                  "2 values, got " << values.size());
+                  "2 values for == operator, got " << values.size());
     }
     }
 
 
     string op1 = values.top();
     string op1 = values.top();

+ 3 - 3
src/lib/eval/token.h

@@ -64,7 +64,7 @@ public:
     ///
     ///
     /// We need to pass the packet being evaluated and possibly previously
     /// We need to pass the packet being evaluated and possibly previously
     /// evaluated values. Specific implementations may ignore the packet altogether
     /// evaluated values. Specific implementations may ignore the packet altogether
-    /// and just put theirr own value on the stack (constant tokens), look at the
+    /// and just put their own value on the stack (constant tokens), look at the
     /// packet and put some data extracted from it on the stack (option tokens),
     /// packet and put some data extracted from it on the stack (option tokens),
     /// or pop arguments from the stack and put back the result (operators).
     /// or pop arguments from the stack and put back the result (operators).
     ///
     ///
@@ -87,7 +87,7 @@ public:
     /// Value is set during token construction.
     /// Value is set during token construction.
     ///
     ///
     /// @param str constant string to be represented.
     /// @param str constant string to be represented.
-    TokenString(std::string str)
+    TokenString(const std::string& str)
         :value_(str){
         :value_(str){
     }
     }
 
 
@@ -126,7 +126,7 @@ public:
     /// to extract the option from the packet and put its value on the stack.
     /// to extract the option from the packet and put its value on the stack.
     /// If the option is not there, an empty string ("") is put on the stack.
     /// If the option is not there, an empty string ("") is put on the stack.
     ///
     ///
-    /// @param pkt not used
+    /// @param pkt specified option will be extracted from this packet (if present)
     /// @param values value of the option will be pushed here (or "")
     /// @param values value of the option will be pushed here (or "")
     void evaluate(const Pkt& pkt, ValueStack& values);
     void evaluate(const Pkt& pkt, ValueStack& values);