Browse Source

[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
Tomek Mrugalski 11 years ago
parent
commit
0c74cff26a

+ 1 - 1
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);

+ 1 - 1
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

+ 1 - 1
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);

+ 1 - 1
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

+ 1 - 1
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) {

+ 1 - 1
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

+ 3 - 3
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;
 }
 

+ 22 - 7
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:

+ 7 - 7
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) {