From: Francis Dupont <fdupont@isc.org>
Date: Fri, 5 Jul 2019 11:09:51 +0000 (+0200)
Subject: [710-cb_cmds-add-support-for-server-tags-in-global-options] Addressed some comments
X-Git-Tag: Kea-1.6.0-beta2~94
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=22aa7d842223314aaf816f7de58fafaca54de427;p=thirdparty%2Fkea.git

[710-cb_cmds-add-support-for-server-tags-in-global-options] Addressed some comments
---

diff --git a/src/lib/dhcpsrv/testutils/test_config_backend.h b/src/lib/dhcpsrv/testutils/test_config_backend.h
index e29d2cce4c..85bcbd9bf3 100644
--- a/src/lib/dhcpsrv/testutils/test_config_backend.h
+++ b/src/lib/dhcpsrv/testutils/test_config_backend.h
@@ -99,10 +99,9 @@ public:
         for (auto tag : tags) {
             // Until the stamped element tags are in a set (vs a vector)
             // we have to avoid duplicates.
-            if (dst->hasServerTag(tag)) {
-                continue;
+            if (!dst->hasServerTag(tag)) {
+                dst->setServerTag(tag.get());
             }
-            dst->setServerTag(tag.get());
         }
     }
 
@@ -116,10 +115,9 @@ public:
         for (auto tag : tags) {
             // Until the stamped element tags are in a set (vs a vector)
             // we have to avoid duplicates.
-            if (elem->hasServerTag(tag)) {
-                continue;
+            if (!elem->hasServerTag(tag)) {
+                elem->setServerTag(tag.get());
             }
-            elem->setServerTag(tag.get());
         }
     }
 
diff --git a/src/lib/dhcpsrv/testutils/test_config_backend_dhcp4.cc b/src/lib/dhcpsrv/testutils/test_config_backend_dhcp4.cc
index 5c161fb3f6..f91796ea86 100644
--- a/src/lib/dhcpsrv/testutils/test_config_backend_dhcp4.cc
+++ b/src/lib/dhcpsrv/testutils/test_config_backend_dhcp4.cc
@@ -562,29 +562,36 @@ TestConfigBackendDHCPv4::createUpdateOption4(const db::ServerSelector& server_se
     auto tags = server_selector.getTags();
     auto not_in_tags = false;
     for (auto subnet : subnets_) {
+	// Get the pool: if it is not here we can directly go to the next subnet.
         auto pool = subnet->getPool(Lease::TYPE_V4, pool_start_address);
         if (!pool) {
             continue;
         }
+
+	// Verify the subnet is in all or one of the given servers.
         if (!subnet->hasAllServerTag()) {
+	    auto in_tags = false;
             for (auto tag : tags) {
                 if (subnet->hasServerTag(ServerTag(tag))) {
-                    not_in_tags = true;
+		    in_tags = true;
                     break;
                 }
             }
-            if (not_in_tags) {
+            if (!in_tags) {
+		// Records the fact a subnet was found but not in a server.
+		not_in_tags = true;
                 continue;
             }
         }
 
+	// Update the option.
         pool->getCfgOption()->del(option->space_name_, option->option_->getType());
         pool->getCfgOption()->add(*option, option->space_name_);
 
         return;
     }
 
-    if (!not_in_tags) {
+    if (not_in_tags) {
         isc_throw(BadValue, "attempted to create or update option in "
                   "a non existing pool " << pool_start_address
                   << " - " << pool_end_address);
diff --git a/src/lib/dhcpsrv/testutils/test_config_backend_dhcp6.cc b/src/lib/dhcpsrv/testutils/test_config_backend_dhcp6.cc
index f4e5561587..e557f94b1c 100644
--- a/src/lib/dhcpsrv/testutils/test_config_backend_dhcp6.cc
+++ b/src/lib/dhcpsrv/testutils/test_config_backend_dhcp6.cc
@@ -561,29 +561,36 @@ TestConfigBackendDHCPv6::createUpdateOption6(const db::ServerSelector& server_se
     auto tags = server_selector.getTags();
     auto not_in_tags = false;
     for (auto subnet : subnets_) {
+	// Get the pool: if it is not here we can directly go to the next subnet.
         auto pool = subnet->getPool(Lease::TYPE_NA, pool_start_address);
         if (!pool) {
             continue;
         }
+
+	// Verify the subnet is in all or one of the given servers.
         if (!subnet->hasAllServerTag()) {
+	    auto in_tags = false;
             for (auto tag : tags) {
                 if (subnet->hasServerTag(ServerTag(tag))) {
-                    not_in_tags = true;
+		    in_tags = true;
                     break;
                 }
             }
-            if (not_in_tags) {
+            if (!in_tags) {
+		// Records the fact a subnet was found but not in a server.
+		not_in_tags = true;
                 continue;
             }
         }
 
+	// Update the option.
         pool->getCfgOption()->del(option->space_name_, option->option_->getType());
         pool->getCfgOption()->add(*option, option->space_name_);
 
         return;
     }
 
-    if (!not_in_tags) {
+    if (not_in_tags) {
         isc_throw(BadValue, "attempted to create or update option in "
                   "a non existing pool " << pool_start_address
                   << " - " << pool_end_address);
@@ -603,29 +610,36 @@ TestConfigBackendDHCPv6::createUpdateOption6(const db::ServerSelector& server_se
     auto tags = server_selector.getTags();
     auto not_in_tags = false;
     for (auto subnet : subnets_) {
+	// Get the pd pool: if it is not here we can directly go to the next subnet.
         auto pdpool = subnet->getPool(Lease::TYPE_PD, pd_pool_prefix);
         if (!pdpool) {
             continue;
         }
+
+	// Verify the subnet is in all or one of the given servers.
         if (!subnet->hasAllServerTag()) {
+	    auto in_tags = false;
             for (auto tag : tags) {
                 if (subnet->hasServerTag(ServerTag(tag))) {
-                    not_in_tags = true;
+                    in_tags = true;
                     break;
                 }
             }
-            if (not_in_tags) {
+            if (!in_tags) {
+		// Records the fact a subnet was found but not in a server.
+		not_in_tags = true;
                 continue;
             }
         }
 
+	// Update the option.
         pdpool->getCfgOption()->del(option->space_name_, option->option_->getType());
         pdpool->getCfgOption()->add(*option, option->space_name_);
 
         return;
     }
 
-    if (!not_in_tags) {
+    if (not_in_tags) {
         isc_throw(BadValue, "attempted to create or update option in "
                   "a non existing prefix pool " << pd_pool_prefix
                   << "/" << static_cast<unsigned>(pd_pool_prefix_length));