[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 ac67faf74fa9c3f83b00cab6732d99fd8c16e9ec..1d010f326abb8856fe12b6b26ac976038a21dd02 100644 (file)
--- 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 32a2c4369b2b06c4ccf1b427be36e822dbe97f0a..80cd4509c8355ba943761aa3f7cbf138a9641a34 100644 (file)
--- 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 38b12335ca10ba1be3692cfcad858f65877ad5d0..8976dabcd598cdd0c05a6ac0e246a01b420de86c 100644 (file)
--- 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 181da7c43dbb9ed79404938714ffc136985dd0ea..3359cb25d0fa15401be6b038857ba953830afb7e 100644 (file)
--- 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 3cc16233ea5be04009d61cc4c31e6a9587fb22b7..87b56c902be5c3d757b6de2c49f73e15ff3d4b7c 100644 (file)
--- 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 9f0ff8ac19c718991cdf81870c24fc2e78df5e4e..0f8f8f96d2bc78f816a7740db586917a0a9d7f1c 100644 (file)
--- 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 5a85445dd48aca64e0117e901c3d4c45efdb2ae4..4738668c25f0fcf6c2627eda698752d0038381af 100644 (file)
--- 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<uint32_t>& t2,
                const Triplet<uint32_t>& 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 7b9bce7527986522137ca96b7923c4e4908d1076..08b624613d83e9124b0c1f36a70b657fa01d473e 100644 (file)
--- 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 9e8657964648c2f280a8eb7c6eeddf482b795569..0af68cf1ea316ba3d34f0da10704f2940f46d73b 100644 (file)
--- 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) {