From: Tomek Mrugalski Date: Thu, 29 Oct 2015 16:19:33 +0000 (+0900) Subject: [4081] Changes after review: X-Git-Tag: trac4106_base~1^2~1 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=661e923bbc245014bfcd3e3567b63fbccb8ad306;p=thirdparty%2Fkea.git [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 --- diff --git a/src/lib/eval/Makefile.am b/src/lib/eval/Makefile.am index 2627175768..1fd66b96f8 100644 --- a/src/lib/eval/Makefile.am +++ b/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_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 libkea_eval_la_SOURCES = 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/dhcp/libkea-dhcp++.la 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_messages.mes diff --git a/src/lib/eval/eval.dox b/src/lib/eval/eval.dox index 314834d287..bde1f918cc 100644 --- a/src/lib/eval/eval.dox +++ b/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 // purpose with or without fee is hereby granted, provided that the above diff --git a/src/lib/eval/eval_messages.mes b/src/lib/eval/eval_messages.mes index 0ed3fa0e20..cca2b0e19e 100644 --- a/src/lib/eval/eval_messages.mes +++ b/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 # purpose with or without fee is hereby granted, provided that the above diff --git a/src/lib/eval/tests/token_unittest.cc b/src/lib/eval/tests/token_unittest.cc index 670d637a6e..6b29763c62 100644 --- a/src/lib/eval/tests/token_unittest.cc +++ b/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 -// 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 not_found; @@ -121,6 +121,34 @@ TEST_F(TokenTest, optionString) { 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 // compare two values (with incorrectly built stack). TEST_F(TokenTest, optionEqualInvalid) { diff --git a/src/lib/eval/token.cc b/src/lib/eval/token.cc index ed0bc33552..66ebf1c5b1 100644 --- a/src/lib/eval/token.cc +++ b/src/lib/eval/token.cc @@ -40,7 +40,7 @@ TokenEqual::evaluate(const Pkt& /*pkt*/, ValueStack& values) { if (values.size() < 2) { 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(); diff --git a/src/lib/eval/token.h b/src/lib/eval/token.h index 4138061f99..930c707fc5 100644 --- a/src/lib/eval/token.h +++ b/src/lib/eval/token.h @@ -64,7 +64,7 @@ public: /// /// We need to pass the packet being evaluated and possibly previously /// 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), /// or pop arguments from the stack and put back the result (operators). /// @@ -87,7 +87,7 @@ public: /// Value is set during token construction. /// /// @param str constant string to be represented. - TokenString(std::string str) + TokenString(const std::string& str) :value_(str){ } @@ -126,7 +126,7 @@ public: /// 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. /// - /// @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 "") void evaluate(const Pkt& pkt, ValueStack& values);