[3747] Removed unused "matches" functions from lease.cc and lease.h

src/bin/dhcp6/tests/dhcp6_test_utils.cc

@@ -17,6 +17,7 @@
 #include <dhcp6/tests/dhcp6_test_utils.h>
 #include <dhcp6/json_config_parser.h>
 #include <config/ccsession.h>
+#include <util/pointer_util.h>
 #include <string.h>
 
 using namespace isc::data;
@@ -134,7 +135,9 @@ Dhcpv6SrvTest::checkLease(const isc::dhcp::Lease6& lease) {
         return (Lease6Ptr());
     }
 
-    EXPECT_TRUE(lease_db->matches(lease));
+    EXPECT_TRUE(util::nullOrEqualValues(lease_db->hwaddr_, lease.hwaddr_));
+    EXPECT_TRUE(util::nullOrEqualValues(lease_db->duid_, lease.duid_));
+
     return (lease_db);
 }
 

src/bin/dhcp6/tests/dhcp6_test_utils.h

@@ -386,7 +386,7 @@ public:
     }
 
     // Checks if the lease sent to client is present in the database
-    // and is valid when checked agasint the configured subnet
+    // and is valid when checked against the configured subnet
     isc::dhcp::Lease6Ptr checkLease
         (const isc::dhcp::DuidPtr& duid, const isc::dhcp::OptionPtr& ia_na,
          boost::shared_ptr<isc::dhcp::Option6IAAddr> addr);

src/lib/dhcpsrv/lease.cc

@@ -130,38 +130,6 @@ Lease::getHWAddrVector() const {
 }
 
 bool
-Lease4::matches(const Lease4& other) const {
-    if ((client_id_ && !other.client_id_) ||
-        (!client_id_ && other.client_id_)) {
-        // One lease has client-id, but the other doesn't
-        return false;
-    }
-
-    if (client_id_ && other.client_id_ &&
-        *client_id_ != *other.client_id_) {
-        // Different client-ids
-        return false;
-    }
-
-    // Note that hwaddr_ is now a poiner to the HWAddr structure.
-    // We can't simply compare smart pointers, we need to compare the
-    // actual objects they point to. If one of the leases had a hardware address
-    // and the other doesn't, they clearly don't match.
-    if ( (!hwaddr_ && other.hwaddr_) ||
-         (hwaddr_ && !other.hwaddr_) ) {
-        return (false);
-    }
-
-    // The lease is equal if addresses and extensions match and there is
-    // either the same hardware address on both or neither have hardware address
-    // specified.
-    return (addr_ == other.addr_ &&
-            ext_ == other.ext_ &&
-            (!hwaddr_ ||
-             *hwaddr_ == *other.hwaddr_) );
-}
-
-bool
 Lease4::belongsToClient(const HWAddrPtr& hw_address,
                         const ClientIdPtr& client_id) const {
     // If client id matches, lease matches.
@@ -295,19 +263,10 @@ Lease4::toText() const {
 
 bool
 Lease4::operator==(const Lease4& other) const {
-    if ( (client_id_ && !other.client_id_) ||
-         (!client_id_ && other.client_id_) ) {
-        // One lease has client-id, but the other doesn't
-        return false;
-    }
-
-    if (client_id_ && other.client_id_ &&
-        *client_id_ != *other.client_id_) {
-        // Different client-ids
-        return false;
-    }
-
-    return (matches(other) &&
+    return (nullOrEqualValues(hwaddr_, other.hwaddr_) &&
+            nullOrEqualValues(client_id_, other.client_id_) &&
+            addr_ == other.addr_ &&
+            ext_ == other.ext_ &&
             subnet_id_ == other.subnet_id_ &&
             t1_ == other.t1_ &&
             t2_ == other.t2_ &&
@@ -321,29 +280,13 @@ Lease4::operator==(const Lease4& other) const {
 }
 
 bool
-Lease6::matches(const Lease6& other) const {
-
-    // One lease has a hardware address, the other doesn't.
-    if ( (!hwaddr_ && other.hwaddr_) ||
-         (hwaddr_ && !other.hwaddr_) ) {
-        return (false);
-    }
-
-    // Both leases have hardware address, but they are not equal.
-    if (hwaddr_ && (*hwaddr_ != *other.hwaddr_)) {
-        return (false);
-    }
-
-    return (addr_ == other.addr_ &&
+Lease6::operator==(const Lease6& other) const {
+    return (nullOrEqualValues(duid_, other.duid_) &&
+            nullOrEqualValues(hwaddr_, other.hwaddr_) &&
+            addr_ == other.addr_ &&
             type_ == other.type_ &&
             prefixlen_ == other.prefixlen_ &&
             iaid_ == other.iaid_ &&
-            *duid_ == *other.duid_);
-}
-
-bool
-Lease6::operator==(const Lease6& other) const {
-    return (matches(other) &&
             preferred_lft_ == other.preferred_lft_ &&
             valid_lft_ == other.valid_lft_ &&
             t1_ == other.t1_ &&

src/lib/dhcpsrv/lease.h

@@ -265,20 +265,6 @@ struct Lease4 : public Lease {
     /// or an empty vector if client identifier is NULL.
     const std::vector<uint8_t>& getClientIdVector() const;
 
-    /// @brief Check if two objects encapsulate the lease for the same
-    /// client.
-    ///
-    /// Checks if two @c Lease4 objects have the same address, client id,
-    /// HW address and ext_ value.  If these parameters match it is an
-    /// indication that both objects describe the lease for the same
-    /// client but apparently one is a result of renewal of the other. The
-    /// special case of the matching lease is the one that is equal to another.
-    ///
-    /// @param other A lease to compare with.
-    ///
-    /// @return true if the selected parameters of the two leases match.
-    bool matches(const Lease4& other) const;
-
     /// @brief Check if the lease belongs to the client with the given
     /// identifiers.
     ///
@@ -462,20 +448,6 @@ struct Lease6 : public Lease {
     /// @return A reference to a vector holding a DUID.
     const std::vector<uint8_t>& getDuidVector() const;
 
-    /// @brief Checks if two lease objects encapsulate the lease for the same
-    /// client.
-    ///
-    /// This function compares address, type, prefix length, IAID and DUID
-    /// parameters between two @c Lease6 objects. If these parameters match
-    /// it is an indication that both objects describe the lease for the same
-    /// client but apparently one is a result of renewal of the other. The
-    /// special case of the matching lease is the one that is equal to another.
-    ///
-    /// @param other A lease to compare to.
-    ///
-    /// @return true if selected parameters of the two leases match.
-    bool matches(const Lease6& other) const;
-
     /// @brief Compare two leases for equality
     ///
     /// @param other lease6 object with which to compare

src/lib/dhcpsrv/tests/lease_unittest.cc

@@ -184,57 +184,6 @@ TEST_F(Lease4Test, operatorAssign) {
     EXPECT_TRUE(lease == copied_lease);
 }
 
-// This test verifies that the matches() returns true if two leases differ
-// by values other than address, HW address, Client ID and ext_.
-TEST_F(Lease4Test, matches) {
-    // Create two leases which share the same address, HW address, client id
-    // and ext_ value.
-    const time_t current_time = time(NULL);
-    Lease4 lease1(IOAddress("192.0.2.3"), hwaddr_, clientid_, VALID_LIFETIME,
-                  current_time, 0, 0, SUBNET_ID);
-    lease1.hostname_ = "lease1.example.com.";
-    lease1.fqdn_fwd_ = true;
-    lease1.fqdn_rev_ = true;
-
-    // We need to make an explicit copy. Otherwise the second lease will just
-    // store a pointer and we'll have two leases pointing to a single HWAddr.
-    // That would make modifications to only one impossible.
-    HWAddrPtr hwcopy(new HWAddr(*hwaddr_));
-
-    Lease4 lease2(IOAddress("192.0.2.3"), hwcopy, CLIENTID,
-                  sizeof(CLIENTID), VALID_LIFETIME + 10, current_time - 10,
-                  100, 200, SUBNET_ID);
-    lease2.hostname_ = "lease2.example.com.";
-    lease2.fqdn_fwd_ = false;
-    lease2.fqdn_rev_ = true;
-
-    // Leases should match.
-    EXPECT_TRUE(lease1.matches(lease2));
-    EXPECT_TRUE(lease2.matches(lease1));
-
-    // Change address, leases should not match anymore.
-    lease1.addr_ = IOAddress("192.0.2.4");
-    EXPECT_FALSE(lease1.matches(lease2));
-    lease1.addr_ = lease2.addr_;
-
-    // Change HW address, leases should not match.
-    lease1.hwaddr_->hwaddr_[1] += 1;
-    EXPECT_FALSE(lease1.matches(lease2));
-    lease1.hwaddr_ = lease2.hwaddr_;
-
-    // Chanage client id, leases should not match.
-    std::vector<uint8_t> client_id = lease1.client_id_->getClientId();
-    client_id[1] += 1;
-    lease1.client_id_.reset(new ClientId(client_id));
-    EXPECT_FALSE(lease1.matches(lease2));
-    lease1.client_id_ = lease2.client_id_;
-
-    // Change ext_, leases should not match.
-    lease1.ext_ += 1;
-    EXPECT_FALSE(lease1.matches(lease2));
-    lease1.ext_ = lease2.ext_;
-}
-
 // This test verifies that it is correctly determined when the lease
 // belongs to the particular client identified by the client identifier
 // and hw address.
@@ -637,88 +586,6 @@ TEST(Lease6, Lease6ConstructorWithFQDN) {
                                          subnet_id)), InvalidOperation);
 }
 
-// This test verifies that the matches() function returns true if two leases
-// differ by values other than address, type, prefix length, IAID and DUID.
-TEST(Lease6, matches) {
-
-    // Create two matching leases.
-    uint8_t llt[] = {0, 1, 2, 3, 4, 5, 6, 0xa, 0xb, 0xc, 0xd, 0xe, 0xf};
-    DuidPtr duid(new DUID(llt, sizeof(llt)));
-
-    Lease6 lease1(Lease6::TYPE_NA, IOAddress("2001:db8:1::1"), duid,
-                  IAID, 100, 200, 50, 80,
-                  SUBNET_ID);
-    lease1.hostname_ = "lease1.example.com.";
-    lease1.fqdn_fwd_ = true;
-    lease1.fqdn_rev_ = true;
-    Lease6 lease2(Lease6::TYPE_NA, IOAddress("2001:db8:1::1"), duid,
-                  IAID, 200, 300, 90, 70,
-                  SUBNET_ID);
-    lease2.hostname_ = "lease1.example.com.";
-    lease2.fqdn_fwd_ = false;
-    lease2.fqdn_rev_ = true;
-
-    EXPECT_TRUE(lease1.matches(lease2));
-
-    // Modify each value used to match both leases, and make sure that
-    // leases don't match.
-
-    // Modify address.
-    lease1.addr_ = IOAddress("2001:db8:1::2");
-    EXPECT_FALSE(lease1.matches(lease2));
-    lease1.addr_ = lease2.addr_;
-
-    // Modify lease type.
-    lease1.type_ = Lease6::TYPE_TA;
-    EXPECT_FALSE(lease1.matches(lease2));
-    lease1.type_ = lease2.type_;
-
-    // Modify prefix length.
-    lease1.prefixlen_ += 1;
-    EXPECT_FALSE(lease1.matches(lease2));
-    lease1.prefixlen_ = lease2.prefixlen_;
-
-    // Modify IAID.
-    lease1.iaid_ += 1;
-    EXPECT_FALSE(lease1.matches(lease2));
-    lease1.iaid_ = lease2.iaid_;
-
-    // Modify DUID.
-    llt[1] += 1;
-    duid.reset(new DUID(llt, sizeof(llt)));
-    lease1.duid_ = duid;
-    EXPECT_FALSE(lease1.matches(lease2));
-    lease1.duid_ = lease2.duid_;
-
-    // Hardware address checks
-    EXPECT_TRUE(lease1.matches(lease2)); // Neither lease have hardware address.
-
-    // Let's add a hardware lease to the first one.
-    HWAddrPtr hwaddr(new HWAddr(HWADDR, sizeof(HWADDR), HTYPE_ETHER));
-    lease1.hwaddr_ = hwaddr;
-
-    // Only the first one has a hardware address, so not equal.
-    EXPECT_FALSE(lease1.matches(lease2));
-
-    // Only the second one has it, so still not equal.
-    lease1.hwaddr_.reset();
-    lease2.hwaddr_ = hwaddr;
-    EXPECT_FALSE(lease1.matches(lease2));
-
-    // Ok, now both have it - they should be equal.
-    lease1.hwaddr_ = hwaddr;
-    EXPECT_TRUE(lease1.matches(lease2));
-
-    // Let's create a second instance that have the same values.
-    HWAddrPtr hwaddr2(new HWAddr(HWADDR, sizeof(HWADDR), HTYPE_ETHER));
-    lease2.hwaddr_ = hwaddr2;
-    EXPECT_TRUE(lease1.matches(lease2));
-
-    // Let's modify the second address and check that they won't be equal anymore.
-    hwaddr2->hwaddr_[0]++;
-    EXPECT_FALSE(lease1.matches(lease2));
-}
-
 /// @brief Lease6 Equality Test
 ///
 /// Checks that the operator==() correctly compares two leases for equality.

src/lib/util/pointer_util.h

@@ -24,6 +24,9 @@ namespace util {
 /// This function performs the typical comparison of values encapsulated by
 /// the smart pointers, with checking if the pointers are non-null.
 ///
+/// @param ptr1 First pointer.
+/// @param ptr2 Second pointer.
+///
 /// @tparam T Pointer type, e.g. boost::shared_ptr, or boost::scoped_ptr.
 ///
 /// @return true only if both pointers are non-null and the values which they
@@ -33,6 +36,21 @@ bool equalValues(const T& ptr1, const T& ptr2) {
     return (ptr1 && ptr2 && (*ptr1 == *ptr2));
 }
 
+/// @brief This function checks if two pointers are both null or both
+/// are non-null and they point to equal values.
+///
+/// @param ptr1 First pointer.
+/// @param ptr2 Second pointer.
+///
+/// @tparam T Pointer type, e.g. boost::shared_ptr, or boost::scoped_ptr.
+///
+/// @return true if both pointers are null or if they are both non-null
+/// and the values pointed to are equal.
+template<typename T>
+bool nullOrEqualValues(const T& ptr1, const T& ptr2) {
+    return ((!ptr1 && !ptr2) || equalValues(ptr1, ptr2));
+}
+
 } // end of namespace isc::util
 } // end of namespace isc