[#3074] code cleaning

diff --git a/src/lib/dhcp/option_classless_static_route.cc b/src/lib/dhcp/option_classless_static_route.cc
index ec1952cb9ad971b28f1ad294f2181e8fff162672..fa15b3670aa0cbb4c8bc17cc4b373dc174139495 100644 (file)
--- a/src/lib/dhcp/option_classless_static_route.cc
+++ b/src/lib/dhcp/option_classless_static_route.cc
@@ -87,17 +87,11 @@ OptionClasslessStaticRoute::toText(int indent) const {
 
 uint16_t
 OptionClasslessStaticRoute::len() const {
-    uint16_t len = OPTION4_HDR_LEN;
+    uint16_t len = getHeaderLen();
     len += data_len_;
     return (len);
 }
 
-void
-OptionClasslessStaticRoute::addRoute(const StaticRouteTuple& route) {
-    static_routes_.push_back(route);
-    calcDataLen();
-}
-
 std::vector<uint8_t>
 OptionClasslessStaticRoute::encodeDestinationDescriptor(const StaticRouteTuple& route) {
     // Encoding as per RFC3442

diff --git a/src/lib/dhcp/option_classless_static_route.h b/src/lib/dhcp/option_classless_static_route.h
index f4b5d2674f3c3c3a75655829a3671dcbd7956a96..cd4a977e0185feb92a90006efbcd67f16c83fe33 100644 (file)
--- a/src/lib/dhcp/option_classless_static_route.h
+++ b/src/lib/dhcp/option_classless_static_route.h
@@ -20,15 +20,6 @@ typedef std::tuple<asiolink::IOAddress, uint8_t, asiolink::IOAddress> StaticRout
 /// @brief Represents DHCPv4 Classless Static Route %Option (code 121).
 class OptionClasslessStaticRoute : public Option {
 public:
-    /// @brief Constructor of the %Option with no static routes.
-    ///
-    /// This constructor creates an instance of the option with no static routes.
-    /// Any static routes must be added after. Intended use of this ctor are unit tests.
-    OptionClasslessStaticRoute()
-        : Option(V4, DHO_CLASSLESS_STATIC_ROUTE), static_routes_(), data_len_(0),
-          convenient_notation_(false) {
-    }
-
     /// @brief Constructor of the %Option from data in the buffer.
     ///
     /// This constructor creates an instance of the option using a buffer with
@@ -84,11 +75,6 @@ public:
     /// @return length of the option
     uint16_t len() const override;
 
-    /// @brief Adds static route to collection of all static routes.
-    ///
-    /// @param route A tuple defining new static route
-    void addRoute(const StaticRouteTuple& route);
-
 private:
     /// @brief Container holding all static routes.
     std::vector<StaticRouteTuple> static_routes_;
@@ -147,10 +133,6 @@ private:
     /// e.g.:
     /// 10.0.0.0/8 - 10.2.3.1, 10.229.0.128/25 - 10.1.0.3
     ///
-    /// This notation may be used in the server config, thanks to the possibility of specifying
-    /// data for binary option as a single-quoted text string within double quotes
-    /// (@c csv-format flag must be set to @c false).
-    ///
     /// @param config_txt convenient notation of the option data received as string
     ///
     /// @throw BadValue Thrown in case parser found wrong format of received string.

diff --git a/src/lib/dhcp/tests/option_classless_static_route_unittest.cc b/src/lib/dhcp/tests/option_classless_static_route_unittest.cc
index cabb1ca803365d04c1d97c64798b86eeb3043eeb..329952dc900e396005ca3f86bbd55db6803d9e1b 100644 (file)
--- a/src/lib/dhcp/tests/option_classless_static_route_unittest.cc
+++ b/src/lib/dhcp/tests/option_classless_static_route_unittest.cc
@@ -16,65 +16,6 @@ using namespace isc::asiolink;
 
 namespace {
 
-// This test verifies constructor of the empty OptionClasslessStaticRoute class.
-TEST(OptionClasslessStaticRouteTest, emptyCtor) {
-    // Create option instance. Check that constructor doesn't throw.
-    OptionClasslessStaticRoutePtr option;
-    EXPECT_NO_THROW(option.reset(new OptionClasslessStaticRoute()));
-    ASSERT_TRUE(option);
-
-    // Check if member variables were correctly set by ctor.
-    EXPECT_EQ(Option::V4, option->getUniverse());
-    EXPECT_EQ(DHO_CLASSLESS_STATIC_ROUTE, option->getType());
-}
-
-// This test verifies adding one route to OptionClasslessStaticRoute class.
-TEST(OptionClasslessStaticRouteTest, emptyCtorAddOneRoute) {
-    // Create option instance. Check that constructor doesn't throw.
-    OptionClasslessStaticRoutePtr option;
-    EXPECT_NO_THROW(option.reset(new OptionClasslessStaticRoute()));
-    ASSERT_TRUE(option);
-
-    // Add one static route.
-    StaticRouteTuple route = std::make_tuple(IOAddress("0.0.0.0"), 0, IOAddress("10.198.122.1"));
-    option->addRoute(route);
-
-    // Expected len: 2 (option code + option len headers) + 5 (1 dest descriptor + 4 router addr).
-    EXPECT_EQ(7, option->len());
-
-    // Verify toText() is working fine.
-    EXPECT_EQ("type=121(CLASSLESS_STATIC_ROUTE), len=5, Route 1 (subnet 0.0.0.0/0,"
-              " router IP 10.198.122.1)",
-              option->toText());
-}
-
-// This test verifies adding more routes to OptionClasslessStaticRoute class.
-TEST(OptionClasslessStaticRouteTest, emptyCtorAddMoreRoutes) {
-    // Create option instance. Check that constructor doesn't throw.
-    OptionClasslessStaticRoutePtr option;
-    EXPECT_NO_THROW(option.reset(new OptionClasslessStaticRoute()));
-    ASSERT_TRUE(option);
-
-    // Add 3 static routes.
-    StaticRouteTuple route1 = std::make_tuple(IOAddress("0.0.0.0"), 0, IOAddress("10.198.122.1"));
-    option->addRoute(route1);
-    StaticRouteTuple route2 = std::make_tuple(IOAddress("1.2.3.4"), 32, IOAddress("10.20.30.40"));
-    option->addRoute(route2);
-    StaticRouteTuple route3 = std::make_tuple(IOAddress("5.6.0.0"), 16, IOAddress("50.60.70.80"));
-    option->addRoute(route3);
-
-    // Expected len: 2 (option code + option len headers) + 5 (1 dest descriptor + 4 router addr)
-    // + 9 (5 d.d. + 4 r.a.) + 7 (3 d.d. + 4 r.a.).
-    EXPECT_EQ(23, option->len());
-
-    // Verify toText() is working fine.
-    EXPECT_EQ("type=121(CLASSLESS_STATIC_ROUTE), len=21, "
-              "Route 1 (subnet 0.0.0.0/0, router IP 10.198.122.1), "
-              "Route 2 (subnet 1.2.3.4/32, router IP 10.20.30.40), "
-              "Route 3 (subnet 5.6.0.0/16, router IP 50.60.70.80)",
-              option->toText());
-}
-
 // This test verifies constructor of the OptionClasslessStaticRoute class from config data.
 // Only one static route is defined.
 TEST(OptionClasslessStaticRouteTest, bufferFromStrCtorWithOneRoute) {
@@ -88,6 +29,10 @@ TEST(OptionClasslessStaticRouteTest, bufferFromStrCtorWithOneRoute) {
     EXPECT_NO_THROW(option.reset(new OptionClasslessStaticRoute(buf.begin(), buf.end(), true)));
     ASSERT_TRUE(option);
 
+    // Check if member variables were correctly set by ctor.
+    EXPECT_EQ(Option::V4, option->getUniverse());
+    EXPECT_EQ(DHO_CLASSLESS_STATIC_ROUTE, option->getType());
+
     // Expected len: 2 (option code + option len headers) + 6 (2 dest descriptor + 4 router addr).
     EXPECT_EQ(8, option->len());
 
@@ -288,9 +233,9 @@ TEST(OptionClasslessStaticRouteTest, bufferFromStrCtorDataTruncated) {
 TEST(OptionClasslessStaticRouteTest, wireDatabufferCtorWithOneRoute) {
     // Prepare data to decode - one route with mask width = 8.
     const OptionBuffer buf = {
-        8,               // mask width
-        10,              // significant subnet octet for 10.0.0.0/8
-        10, 45, 122, 1   // router IP address
+        8,              // mask width
+        10,             // significant subnet octet for 10.0.0.0/8
+        10, 45, 122, 1  // router IP address
     };
 
     // Create option instance. Check that constructor doesn't throw. Unpack is also tested here.
@@ -298,6 +243,10 @@ TEST(OptionClasslessStaticRouteTest, wireDatabufferCtorWithOneRoute) {
     EXPECT_NO_THROW(option.reset(new OptionClasslessStaticRoute(buf.begin(), buf.end())));
     ASSERT_TRUE(option);
 
+    // Check if member variables were correctly set by ctor.
+    EXPECT_EQ(Option::V4, option->getUniverse());
+    EXPECT_EQ(DHO_CLASSLESS_STATIC_ROUTE, option->getType());
+
     // Expected len: 2 (option code + option len headers) + 6 (2 dest descriptor + 4 router addr).
     EXPECT_EQ(8, option->len());
 
@@ -465,20 +414,24 @@ TEST(OptionClasslessStaticRouteTest, pack) {
 // This test verifies that pack() method throws an exception when option len to be packed is
 // bigger than 255 octets.
 TEST(OptionClasslessStaticRouteTest, packThrows) {
-    // Create option instance. Check that constructor doesn't throw.
-    OptionClasslessStaticRoutePtr option;
-    EXPECT_NO_THROW(option.reset(new OptionClasslessStaticRoute()));
-    ASSERT_TRUE(option);
-
     // Create 9 octets long static route.
-    StaticRouteTuple route = std::make_tuple(IOAddress("1.2.3.4"), 32, IOAddress("10.198.122.1"));
-    // Add 50 routes
-    uint8_t i = 0;
-    while (i < 50) {
-        option->addRoute(route);
-        ++i;
+    std::ostringstream stream;
+    stream << "1.2.3.4/32 - 10.198.122.1";
+
+    // Add 49 more such routes.
+    for (uint8_t i = 0; i < 49; ++i) {
+        stream << ", 1.2.3.4/32 - 10.198.122.1";
     }
 
+    const std::string config = stream.str();
+    OptionBuffer buf;
+    buf.assign(config.begin(), config.end());
+
+    // Create option instance from very long config. Check that constructor doesn't throw.
+    OptionClasslessStaticRoutePtr option;
+    EXPECT_NO_THROW(option.reset(new OptionClasslessStaticRoute(buf.begin(), buf.end(), true)));
+    ASSERT_TRUE(option);
+
     // Expected len: 2 (headers) + 50x9 = 452
     EXPECT_EQ(452, option->len());