From: Tomek Mrugalski Date: Mon, 10 Feb 2014 15:07:02 +0000 (+0100) Subject: [3274] Changes after second round of reviews: X-Git-Tag: bind10-1.2.0beta1-release~57^2~1^2~1 X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=0c74cff26aa147bd1f8b807b441d86a064a90f60;p=thirdparty%2Fkea.git [3274] Changes after second round of reviews: - getRelayInfo() implemented, relay_ is now protected - setRelay => setRelayInfo - Option::Universe is passed as const reference - Further clarified white_list_ field in Subnet.h --- diff --git a/src/bin/dhcp4/config_parser.cc b/src/bin/dhcp4/config_parser.cc index ac67faf74f..1d010f326a 100644 --- a/src/bin/dhcp4/config_parser.cc +++ b/src/bin/dhcp4/config_parser.cc @@ -180,7 +180,7 @@ public: // Set relay information if it was parsed if (relay_info_) { - sub4ptr->setRelay(*relay_info_); + sub4ptr->setRelayInfo(*relay_info_); } isc::dhcp::CfgMgr::instance().addSubnet4(sub4ptr); diff --git a/src/bin/dhcp4/tests/config_parser_unittest.cc b/src/bin/dhcp4/tests/config_parser_unittest.cc index 32a2c4369b..80cd4509c8 100644 --- a/src/bin/dhcp4/tests/config_parser_unittest.cc +++ b/src/bin/dhcp4/tests/config_parser_unittest.cc @@ -2850,7 +2850,7 @@ TEST_F(Dhcp4ParserTest, subnetRelayInfo) { Subnet4Ptr subnet = CfgMgr::instance().getSubnet4(IOAddress("192.0.2.200"), classify_); ASSERT_TRUE(subnet); - EXPECT_EQ("192.0.2.123", subnet->relay_.addr_.toText()); + EXPECT_EQ("192.0.2.123", subnet->getRelayInfo().addr_.toText()); } // Goal of this test is to verify that multiple subnets can be configured diff --git a/src/bin/dhcp6/config_parser.cc b/src/bin/dhcp6/config_parser.cc index 38b12335ca..8976dabcd5 100644 --- a/src/bin/dhcp6/config_parser.cc +++ b/src/bin/dhcp6/config_parser.cc @@ -384,7 +384,7 @@ public: // Set relay infomation if it was provided if (relay_info_) { - sub6ptr->setRelay(*relay_info_); + sub6ptr->setRelayInfo(*relay_info_); } isc::dhcp::CfgMgr::instance().addSubnet6(sub6ptr); diff --git a/src/bin/dhcp6/tests/config_parser_unittest.cc b/src/bin/dhcp6/tests/config_parser_unittest.cc index 181da7c43d..3359cb25d0 100644 --- a/src/bin/dhcp6/tests/config_parser_unittest.cc +++ b/src/bin/dhcp6/tests/config_parser_unittest.cc @@ -2968,7 +2968,7 @@ TEST_F(Dhcp6ParserTest, subnetRelayInfo) { Subnet6Ptr subnet = CfgMgr::instance().getSubnet6(IOAddress("2001:db8:1::1"), classify_); ASSERT_TRUE(subnet); - EXPECT_EQ("2001:db8:1::abcd", subnet->relay_.addr_.toText()); + EXPECT_EQ("2001:db8:1::abcd", subnet->getRelayInfo().addr_.toText()); } // Goal of this test is to verify that multiple subnets can be configured diff --git a/src/lib/dhcpsrv/dhcp_parsers.cc b/src/lib/dhcpsrv/dhcp_parsers.cc index 3cc16233ea..87b56c902b 100644 --- a/src/lib/dhcpsrv/dhcp_parsers.cc +++ b/src/lib/dhcpsrv/dhcp_parsers.cc @@ -814,7 +814,7 @@ OptionDefListParser::commit() { //****************************** RelayInfoParser ******************************** RelayInfoParser::RelayInfoParser(const std::string&, const isc::dhcp::Subnet::RelayInfoPtr& relay_info, - const Option::Universe family) + const Option::Universe& family) :storage_(relay_info), local_(isc::asiolink::IOAddress( family == Option::V4 ? "0.0.0.0" : "::")), string_values_(new StringStorage()), family_(family) { diff --git a/src/lib/dhcpsrv/dhcp_parsers.h b/src/lib/dhcpsrv/dhcp_parsers.h index 9f0ff8ac19..0f8f8f96d2 100644 --- a/src/lib/dhcpsrv/dhcp_parsers.h +++ b/src/lib/dhcpsrv/dhcp_parsers.h @@ -771,7 +771,7 @@ public: /// @param family specifies protocol family (IPv4 or IPv6) RelayInfoParser(const std::string& unused, const isc::dhcp::Subnet::RelayInfoPtr& relay_info, - const isc::dhcp::Option::Universe family); + const isc::dhcp::Option::Universe& family); /// @brief parses the actual relay parameters /// @param relay_info JSON structure holding relay parameters to parse diff --git a/src/lib/dhcpsrv/subnet.cc b/src/lib/dhcpsrv/subnet.cc index 5a85445dd4..4738668c25 100644 --- a/src/lib/dhcpsrv/subnet.cc +++ b/src/lib/dhcpsrv/subnet.cc @@ -32,11 +32,11 @@ Subnet::Subnet(const isc::asiolink::IOAddress& prefix, uint8_t len, const Triplet& t2, const Triplet& valid_lifetime, const isc::dhcp::Subnet::RelayInfo& relay) - :relay_(relay), id_(generateNextID()), prefix_(prefix), prefix_len_(len), + :id_(generateNextID()), prefix_(prefix), prefix_len_(len), t1_(t1), t2_(t2), valid_(valid_lifetime), last_allocated_ia_(lastAddrInPrefix(prefix, len)), last_allocated_ta_(lastAddrInPrefix(prefix, len)), - last_allocated_pd_(lastAddrInPrefix(prefix, len)) + last_allocated_pd_(lastAddrInPrefix(prefix, len)), relay_(relay) { if ((prefix.isV6() && len > 128) || (prefix.isV4() && len > 32)) { @@ -72,7 +72,7 @@ Subnet::addOption(const OptionPtr& option, bool persistent, } void -Subnet::setRelay(const isc::dhcp::Subnet::RelayInfo& relay) { +Subnet::setRelayInfo(const isc::dhcp::Subnet::RelayInfo& relay) { relay_ = relay; } diff --git a/src/lib/dhcpsrv/subnet.h b/src/lib/dhcpsrv/subnet.h index 7b9bce7527..08b624613d 100644 --- a/src/lib/dhcpsrv/subnet.h +++ b/src/lib/dhcpsrv/subnet.h @@ -415,15 +415,18 @@ public: /// be extended in the future. /// /// @param relay structure that contains relay information - void setRelay(const isc::dhcp::Subnet::RelayInfo& relay); + void setRelayInfo(const isc::dhcp::Subnet::RelayInfo& relay); - /// @brief Relay information + + /// @brief Returns const reference to relay information /// - /// See @ref RelayInfo for detailed description. This structure is public, - /// so its fields are easily accessible. Making it protected would bring in - /// the issue of returning references that may become stale after its parent - /// subnet object disappears. - RelayInfo relay_; + /// @note The returned reference is only valid as long as the object + /// returned it is valid. + /// + /// @return const reference to the relay information + const isc::dhcp::Subnet::RelayInfo& getRelayInfo() { + return (relay_); + } /// @brief checks whether this subnet supports client that belongs to /// specified classes. @@ -574,6 +577,14 @@ protected: /// @brief Name of the network interface (if connected directly) std::string iface_; + /// @brief Relay information + /// + /// See @ref RelayInfo for detailed description. This structure is public, + /// so its fields are easily accessible. Making it protected would bring in + /// the issue of returning references that may become stale after its parent + /// subnet object disappears. + RelayInfo relay_; + /// @brief optional definition of a client class /// /// If defined, only clients belonging to that class will be allowed to use @@ -585,6 +596,10 @@ protected: /// rejected) and we'll also have black-list (only classes on the list are /// rejected, the rest are allowed). Implementing this will require /// fancy parser logic, so it may be a while until we support this. + /// This will lead to changing type of white_list_ to ClientClasses. + /// Note that the interface will remain the same (allowClientClass() + /// would still take a single ClientClass. It will just be possible + /// to call it multiple times to allow multiple classes). ClientClass white_list_; private: diff --git a/src/lib/dhcpsrv/tests/subnet_unittest.cc b/src/lib/dhcpsrv/tests/subnet_unittest.cc index 9e86579646..0af68cf1ea 100644 --- a/src/lib/dhcpsrv/tests/subnet_unittest.cc +++ b/src/lib/dhcpsrv/tests/subnet_unittest.cc @@ -49,7 +49,7 @@ TEST(Subnet4Test, in_range) { EXPECT_EQ(2000, subnet.getT2()); EXPECT_EQ(3000, subnet.getValid()); - EXPECT_EQ("0.0.0.0", subnet.relay_.addr_.toText()); + EXPECT_EQ("0.0.0.0", subnet.getRelayInfo().addr_.toText()); EXPECT_FALSE(subnet.inRange(IOAddress("192.0.0.0"))); EXPECT_TRUE(subnet.inRange(IOAddress("192.0.2.0"))); @@ -65,10 +65,10 @@ TEST(Subnet4Test, in_range) { TEST(Subnet4Test, relay) { Subnet4 subnet(IOAddress("192.0.2.1"), 24, 1000, 2000, 3000); - EXPECT_EQ("0.0.0.0", subnet.relay_.addr_.toText()); + EXPECT_EQ("0.0.0.0", subnet.getRelayInfo().addr_.toText()); - subnet.setRelay(IOAddress("192.0.123.45")); - EXPECT_EQ("192.0.123.45", subnet.relay_.addr_.toText()); + subnet.setRelayInfo(IOAddress("192.0.123.45")); + EXPECT_EQ("192.0.123.45", subnet.getRelayInfo().addr_.toText()); } // Checks whether siaddr field can be set and retrieved correctly. @@ -347,11 +347,11 @@ TEST(Subnet6Test, in_range) { TEST(Subnet6Test, relay) { Subnet6 subnet(IOAddress("2001:db8:1::"), 64, 1000, 2000, 3000, 4000); - EXPECT_EQ("::", subnet.relay_.addr_.toText()); + EXPECT_EQ("::", subnet.getRelayInfo().addr_.toText()); - subnet.setRelay(IOAddress("2001:ffff::1")); + subnet.setRelayInfo(IOAddress("2001:ffff::1")); - EXPECT_EQ("2001:ffff::1", subnet.relay_.addr_.toText()); + EXPECT_EQ("2001:ffff::1", subnet.getRelayInfo().addr_.toText()); } TEST(Subnet6Test, Pool6InSubnet6) {