[4302] Address further review comments.

- cppcheck issues
- Unique index for prefix and prefix length.

diff --git a/src/bin/admin/scripts/mysql/dhcpdb_create.mysql b/src/bin/admin/scripts/mysql/dhcpdb_create.mysql
index bd159d547c78966bf75a0e42847f9d3b957732ac..85b768b0baebb5e91208418cba1fee07c5226236 100755 (executable)
--- a/src/bin/admin/scripts/mysql/dhcpdb_create.mysql
+++ b/src/bin/admin/scripts/mysql/dhcpdb_create.mysql
@@ -413,7 +413,7 @@ CREATE UNIQUE INDEX key_dhcp4_ipv4_address_subnet_id ON hosts (ipv4_address ASC
 
 # Create index to search for reservations using address/prefix and prefix
 # length.
-CREATE INDEX key_dhcp6_address_prefix_len ON ipv6_reservations (address ASC , prefix_len ASC);
+CREATE UNIQUE INDEX key_dhcp6_address_prefix_len ON ipv6_reservations (address ASC , prefix_len ASC);
 
 # Create a table mapping host identifiers to their names. Values in this
 # table are used as a foreign key in hosts table to guarantee that only

diff --git a/src/lib/dhcpsrv/mysql_host_data_source.cc b/src/lib/dhcpsrv/mysql_host_data_source.cc
index 1c2300b8b66242ba7cdc75e6c9ad88b5a4cdda32..d364d7283f3f8caa68dad4f3c09ec20f28814261 100644 (file)
--- a/src/lib/dhcpsrv/mysql_host_data_source.cc
+++ b/src/lib/dhcpsrv/mysql_host_data_source.cc
@@ -718,6 +718,8 @@ public:
         : MySqlHostExchange(), reserv_type_(0), reserv_type_null_(MLM_FALSE),
           ipv6_address_buffer_len_(0), prefix_len_(0), iaid_(0) {
 
+        memset(ipv6_address_buffer_, 0, sizeof(ipv6_address_buffer_));
+
         // Append additional columns returned by the queries.
         columns_.push_back("address");
         columns_.push_back("prefix_len");
@@ -922,7 +924,7 @@ public:
     ///
     /// Initialize class members representing a single IPv6 reservation.
     MySqlIPv6ReservationExchange()
-        : host_id_(0), address_("::"), prefix_len_(0), type_(0),
+        : host_id_(0), address_("::"), address_len_(0), prefix_len_(0), type_(0),
           iaid_(0), resv_(IPv6Resrv::TYPE_NA, asiolink::IOAddress("::"), 128) {
 
         // Reset error table.

diff --git a/src/lib/dhcpsrv/tests/generic_host_data_source_unittest.cc b/src/lib/dhcpsrv/tests/generic_host_data_source_unittest.cc
index 4818d03d9b431becaf3f9202d8e04066ed542214..90b70a6e3083ba4e02c9754cd3122c2d8eada497 100644 (file)
--- a/src/lib/dhcpsrv/tests/generic_host_data_source_unittest.cc
+++ b/src/lib/dhcpsrv/tests/generic_host_data_source_unittest.cc
@@ -27,7 +27,7 @@ GenericHostDataSourceTest::~GenericHostDataSourceTest() {
 }
 
 std::string
-GenericHostDataSourceTest::generateHWAddr() {
+GenericHostDataSourceTest::generateHWAddr(const bool new_identifier) {
     /// @todo: Consider moving this somewhere to lib/testutils.
 
     // Let's use something that is easily printable. That's convenient
@@ -43,18 +43,20 @@ GenericHostDataSourceTest::generateHWAddr() {
             << static_cast<unsigned int>(hwaddr[i]);
     }
 
-    // Increase the address for the next time we use it.
-    // This is primitive, but will work for 65k unique
-    // addresses.
-    hwaddr[sizeof(hwaddr) - 1]++;
-    if (hwaddr[sizeof(hwaddr) - 1] == 0) {
-        hwaddr[sizeof(hwaddr) - 2]++;
+    if (new_identifier) {
+        // Increase the address for the next time we use it.
+        // This is primitive, but will work for 65k unique
+        // addresses.
+        hwaddr[sizeof(hwaddr) - 1]++;
+        if (hwaddr[sizeof(hwaddr) - 1] == 0) {
+            hwaddr[sizeof(hwaddr) - 2]++;
+        }
     }
     return (tmp.str());
 }
 
 std::string
-GenericHostDataSourceTest::generateDuid() {
+GenericHostDataSourceTest::generateDuid(const bool new_identifier) {
     /// @todo: Consider moving this somewhere to lib/testutils.
 
     // Let's use something that is easily printable. That's convenient
@@ -70,9 +72,11 @@ GenericHostDataSourceTest::generateDuid() {
     // Increase the DUID for the next time we use it.
     // This is primitive, but will work for 65k unique
     // DUIDs.
-    duid[sizeof(duid) - 1]++;
-    if (duid[sizeof(duid) - 1] == 0) {
-        duid[sizeof(duid) - 2]++;
+    if (new_identifier) {
+        duid[sizeof(duid) - 1]++;
+        if (duid[sizeof(duid) - 1] == 0) {
+            duid[sizeof(duid) - 2]++;
+        }
     }
     return (tmp.str());
 }
@@ -106,17 +110,18 @@ HostPtr GenericHostDataSourceTest::initializeHost4(std::string address,
 
 HostPtr GenericHostDataSourceTest::initializeHost6(std::string address,
                                                    Host::IdentifierType identifier,
-                                                   bool prefix) {
+                                                   bool prefix,
+                                                   bool new_identifier) {
     string ident;
     string ident_type;
 
     switch (identifier) {
     case Host::IDENT_HWADDR:
-        ident = generateHWAddr();
+        ident = generateHWAddr(new_identifier);
         ident_type = "hw-address";
         break;
     case Host::IDENT_DUID:
-        ident = generateDuid();
+        ident = generateDuid(new_identifier);
         ident_type = "duid";
         break;
     default:
@@ -724,24 +729,32 @@ GenericHostDataSourceTest::testSubnetId6(int subnets, Host::IdentifierType id) {
     // Make sure we have a pointer to the host data source.
     ASSERT_TRUE(hdsptr_);
 
-    HostPtr host = initializeHost6("2001:db8::0", id, true);
-
+    HostPtr host;
+    IOAddress current_address("2001:db8::0");
     for (int i = 0; i < subnets; ++i) {
+        // Last boolean value set to false indicates that the same identifier
+        // must be used for each generated host.
+        host = initializeHost6(current_address.toText(), id, true, false);
+
         host->setIPv4SubnetID(i + 1000);
         host->setIPv6SubnetID(i + 1000);
 
         // Check that the same host can have reservations in multiple subnets.
         EXPECT_NO_THROW(hdsptr_->add(host));
+
+        // Increase address to make sure we don't assign the same address
+        // in different subnets.
+        current_address = IOAddress::increase(current_address);
     }
 
     // Check that the reservations can be retrieved from each subnet separately.
     for (int i = 0; i < subnets; ++i) {
 
         // Try to retrieve the host
-        ConstHostPtr from_hds = hdsptr_->get6(i + 1000, host->getDuid(),
-                                              host->getHWAddress());
+        ConstHostPtr from_hds = hdsptr_->get6(i + 1000, id, &host->getIdentifier()[0],
+                                              host->getIdentifier().size());
 
-        ASSERT_TRUE(from_hds);
+        ASSERT_TRUE(from_hds) << "failed for i=" << i;
         EXPECT_EQ(i + 1000, from_hds->getIPv6SubnetID());
     }
 

diff --git a/src/lib/dhcpsrv/tests/generic_host_data_source_unittest.h b/src/lib/dhcpsrv/tests/generic_host_data_source_unittest.h
index 1e9259bfa5e591f76c090ba37a20993a9695d15a..e2b468dbc8d313dff7af6b9a5ce40b19d74923c6 100644 (file)
--- a/src/lib/dhcpsrv/tests/generic_host_data_source_unittest.h
+++ b/src/lib/dhcpsrv/tests/generic_host_data_source_unittest.h
@@ -49,19 +49,26 @@ public:
     /// @param address IPv6 address to be reserved
     /// @param id type of identifier (IDENT_DUID or IDENT_HWADDR are supported)
     /// @param prefix reservation type (true = prefix, false = address)
+    /// @param new_identifier Boolean value indicating if new host
+    /// identifier should be generated or the same as previously.
     ///
     /// @return generated Host object
     HostPtr initializeHost6(std::string address, Host::IdentifierType id,
-                            bool prefix);
+                            bool prefix, bool new_identifier = true);
 
     /// @brief Generates a hardware address in text version.
     ///
+    /// @param increase A boolean value indicating if new address (increased)
+    /// must be generated or the same address as previously.
     /// @return HW address in textual form acceptable by Host constructor
-    std::string generateHWAddr();
+    std::string generateHWAddr(const bool new_identifier = true);
 
     /// @brief Generates a hardware address in text version.
+    ///
+    /// @param increase A boolean value indicating if new DUID (increased)
+    /// must be generated or the same DUID as previously.
     /// @return DUID in textual form acceptable by Host constructor
-    std::string generateDuid();
+    std::string generateDuid(const bool new_identifier = true);
 
     /// @brief Compares hardware addresses of the two hosts.
     ///

diff --git a/src/lib/dhcpsrv/tests/generic_lease_mgr_unittest.cc b/src/lib/dhcpsrv/tests/generic_lease_mgr_unittest.cc
index b393847be2776f6f9e62867644881a53ca764e31..baa33c2167df8c7697437118679808f3bc9f4524 100644 (file)
--- a/src/lib/dhcpsrv/tests/generic_lease_mgr_unittest.cc
+++ b/src/lib/dhcpsrv/tests/generic_lease_mgr_unittest.cc
@@ -2213,9 +2213,6 @@ GenericLeaseMgrTest::testGetDeclinedLeases4() {
     EXPECT_NE(0, declined_state);
     EXPECT_NE(0, default_state);
 
-    // Remember expired leases returned.
-    std::vector<Lease4Ptr> saved_expired_leases = expired_leases;
-
     // Remove expired leases again.
     expired_leases.clear();
 
@@ -2366,9 +2363,6 @@ GenericLeaseMgrTest::testGetDeclinedLeases6() {
     EXPECT_NE(0, declined_state);
     EXPECT_NE(0, default_state);
 
-    // Remember expired leases returned.
-    std::vector<Lease6Ptr> saved_expired_leases = expired_leases;
-
     // Remove expired leases again.
     expired_leases.clear();
 

diff --git a/src/lib/dhcpsrv/testutils/schema_mysql_copy.h b/src/lib/dhcpsrv/testutils/schema_mysql_copy.h
index 8656d6d496be4dba7c45a73f9bf68961a2653e0f..05db8a1dd7a54f431082191ab1670465491bcc80 100755 (executable)
--- a/src/lib/dhcpsrv/testutils/schema_mysql_copy.h
+++ b/src/lib/dhcpsrv/testutils/schema_mysql_copy.h
@@ -270,7 +270,7 @@ const char* create_statement[] = {
       "ON hosts "
         "(ipv4_address ASC, dhcp4_subnet_id ASC)",
 
-    "CREATE INDEX key_dhcp6_address_prefix_len "
+    "CREATE UNIQUE INDEX key_dhcp6_address_prefix_len "
       "ON ipv6_reservations (address ASC , prefix_len ASC)",
 
     "CREATE TABLE IF NOT EXISTS host_identifier_type ("