From: Piotrek Zadroga Date: Wed, 10 Jan 2024 11:44:44 +0000 (+0100) Subject: [#3074] code cleaning X-Git-Tag: Kea-2.5.5~45 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=21e80710fea79cf69b9fa7697b31eadd532e0de4;p=thirdparty%2Fkea.git [#3074] code cleaning --- diff --git a/src/lib/dhcp/option_classless_static_route.cc b/src/lib/dhcp/option_classless_static_route.cc index ec1952cb9a..fa15b3670a 100644 --- 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 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 f4b5d2674f..cd4a977e01 100644 --- 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 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 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 cabb1ca803..329952dc90 100644 --- 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());