[5443] Moved to extra argument vs deconst

diff --git a/src/bin/dhcp4/dhcp4_srv.cc b/src/bin/dhcp4/dhcp4_srv.cc
index 9a12a33b6ab1ccb86a1b8ead1906dd88061e6d93..84f1db40bf1b417c014d9aaba26a367168f3f757 100644 (file)
--- a/src/bin/dhcp4/dhcp4_srv.cc
+++ b/src/bin/dhcp4/dhcp4_srv.cc
@@ -483,11 +483,11 @@ Dhcpv4Srv::shutdown() {
 }
 
 isc::dhcp::Subnet4Ptr
-Dhcpv4Srv::selectSubnet(Pkt4Ptr& query) {
+Dhcpv4Srv::selectSubnet(const Pkt4Ptr& query, bool& drop) const {
 
     // DHCPv4-over-DHCPv6 is a special (and complex) case
     if (query->isDhcp4o6()) {
-        return (selectSubnet4o6(query));
+        return (selectSubnet4o6(query, drop));
     }
 
     Subnet4Ptr subnet;
@@ -570,13 +570,12 @@ Dhcpv4Srv::selectSubnet(Pkt4Ptr& query) {
         }
 
         // Callouts decided to drop the packet. It is a superset of the
-        // skip case so no subnet will be selected. For the caller to
-        // know it has to drop the packet the query pointer is cleared.
+        // skip case so no subnet will be selected.
         if (callout_handle->getStatus() == CalloutHandle::NEXT_STEP_DROP) {
             LOG_DEBUG(hooks_logger, DBG_DHCP4_HOOKS,
                       DHCP4_HOOK_SUBNET4_SELECT_DROP)
                 .arg(query->getLabel());
-            query.reset();
+            drop = true;
             return (Subnet4Ptr());
         }
 
@@ -605,7 +604,7 @@ Dhcpv4Srv::selectSubnet(Pkt4Ptr& query) {
 }
 
 isc::dhcp::Subnet4Ptr
-Dhcpv4Srv::selectSubnet4o6(Pkt4Ptr& query) {
+Dhcpv4Srv::selectSubnet4o6(const Pkt4Ptr& query, bool& drop) const {
 
     Subnet4Ptr subnet;
 
@@ -683,13 +682,12 @@ Dhcpv4Srv::selectSubnet4o6(Pkt4Ptr& query) {
         }
 
         // Callouts decided to drop the packet. It is a superset of the
-        // skip case so no subnet will be selected. For the caller to
-        // know it has to drop the packet the query pointer is cleared.
+        // skip case so no subnet will be selected.
         if (callout_handle->getStatus() == CalloutHandle::NEXT_STEP_DROP) {
             LOG_DEBUG(hooks_logger, DBG_DHCP4_HOOKS,
                       DHCP4_HOOK_SUBNET4_SELECT_DROP)
                 .arg(query->getLabel());
-            query.reset();
+            drop = true;
             return (Subnet4Ptr());
         }
 
@@ -2341,10 +2339,11 @@ Pkt4Ptr
 Dhcpv4Srv::processDiscover(Pkt4Ptr& discover) {
     sanityCheck(discover, FORBIDDEN);
 
-    Dhcpv4Exchange ex(alloc_engine_, discover, selectSubnet(discover));
+    bool drop = false;
+    Dhcpv4Exchange ex(alloc_engine_, discover, selectSubnet(discover, drop));
 
     // Stop here if selectSubnet decided to drop the packet
-    if (!discover) {
+    if (drop) {
         return (Pkt4Ptr());
     }
 
@@ -2398,10 +2397,11 @@ Dhcpv4Srv::processRequest(Pkt4Ptr& request) {
     /// @todo Uncomment this (see ticket #3116)
     /// sanityCheck(request, MANDATORY);
 
-    Dhcpv4Exchange ex(alloc_engine_, request, selectSubnet(request));
+    bool drop = false;
+    Dhcpv4Exchange ex(alloc_engine_, request, selectSubnet(request, drop));
 
     // Stop here if selectSubnet decided to drop the packet
-    if (!request) {
+    if (drop) {
         return (Pkt4Ptr());
     }
 
@@ -2704,10 +2704,11 @@ Dhcpv4Srv::processInform(Pkt4Ptr& inform) {
     // DHCPINFORM MUST not include server identifier.
     sanityCheck(inform, FORBIDDEN);
 
-    Dhcpv4Exchange ex(alloc_engine_, inform, selectSubnet(inform));
+    bool drop = false;
+    Dhcpv4Exchange ex(alloc_engine_, inform, selectSubnet(inform, drop));
 
     // Stop here if selectSubnet decided to drop the packet
-    if (!inform) {
+    if (drop) {
         return (Pkt4Ptr());
     }
 
@@ -2747,7 +2748,7 @@ Dhcpv4Srv::processInform(Pkt4Ptr& inform) {
 }
 
 bool
-Dhcpv4Srv::accept(Pkt4Ptr& query) {
+Dhcpv4Srv::accept(const Pkt4Ptr& query) const {
     // Check that the message type is accepted by the server. We rely on the
     // function called to log a message if needed.
     if (!acceptMessageType(query)) {
@@ -2775,7 +2776,7 @@ Dhcpv4Srv::accept(Pkt4Ptr& query) {
 }
 
 bool
-Dhcpv4Srv::acceptDirectRequest(Pkt4Ptr& pkt) {
+Dhcpv4Srv::acceptDirectRequest(const Pkt4Ptr& pkt) const {
     // Accept all relayed messages.
     if (pkt->isRelayed()) {
         return (true);
@@ -2802,8 +2803,9 @@ Dhcpv4Srv::acceptDirectRequest(Pkt4Ptr& pkt) {
         // we validate the message type prior to calling this function.
         return (false);
     }
-    bool result = (!pkt->getLocalAddr().isV4Bcast() || selectSubnet(pkt));
-    if (!pkt) {
+    bool drop = false;
+    bool result = (!pkt->getLocalAddr().isV4Bcast() || selectSubnet(pkt, drop));
+    if (drop) {
         // The packet must be dropped.
         return (false);
     }

diff --git a/src/bin/dhcp4/dhcp4_srv.h b/src/bin/dhcp4/dhcp4_srv.h
index d601ddddbe08110c26b4d3e5fd02e34f62c8e52b..f6b673cf0fb128b41b98ca88500cc79f5159e466 100644 (file)
--- a/src/bin/dhcp4/dhcp4_srv.h
+++ b/src/bin/dhcp4/dhcp4_srv.h
@@ -348,7 +348,7 @@ protected:
     ///
     /// @return true if the message should be further processed, or false if
     /// the message should be discarded.
-    bool accept(Pkt4Ptr& query);
+    bool accept(const Pkt4Ptr& query) const;
 
     /// @brief Check if a message sent by directly connected client should be
     /// accepted or discarded.
@@ -377,7 +377,7 @@ protected:
     ///
     /// @return true if message is accepted for further processing, false
     /// otherwise.
-    bool acceptDirectRequest(Pkt4Ptr& query);
+    bool acceptDirectRequest(const Pkt4Ptr& query) const;
 
     /// @brief Check if received message type is valid for the server to
     /// process.
@@ -766,19 +766,19 @@ protected:
 
     /// @brief Selects a subnet for a given client's packet.
     ///
-    /// When the packet has to be dropped the query pointer is cleared
-    ///
     /// @param query client's message
+    /// @param drop if it is true the packet will be dropped
     /// @return selected subnet (or NULL if no suitable subnet was found)
-    isc::dhcp::Subnet4Ptr selectSubnet(Pkt4Ptr& query);
+    isc::dhcp::Subnet4Ptr selectSubnet(const Pkt4Ptr& query,
+                                       bool& drop) const;
 
     /// @brief Selects a subnet for a given client's DHCP4o6 packet.
     ///
-    /// When the packet has to be dropped the query pointer is cleared
-    ///
     /// @param query client's message
+    /// @param drop if it is true the packet will be dropped
     /// @return selected subnet (or NULL if no suitable subnet was found)
-    isc::dhcp::Subnet4Ptr selectSubnet4o6(Pkt4Ptr& query);
+    isc::dhcp::Subnet4Ptr selectSubnet4o6(const Pkt4Ptr& query,
+                                          bool& drop) const;
 
     /// indicates if shutdown is in progress. Setting it to true will
     /// initiate server shutdown procedure.

diff --git a/src/bin/dhcp4/tests/dhcp4_srv_unittest.cc b/src/bin/dhcp4/tests/dhcp4_srv_unittest.cc
index df1ae36fb93cbb02abc2a8f95c4e051673f81ee5..bd9238f7fc319b7e45aebe710ad3d971b21a997a 100644 (file)
--- a/src/bin/dhcp4/tests/dhcp4_srv_unittest.cc
+++ b/src/bin/dhcp4/tests/dhcp4_srv_unittest.cc
@@ -2303,19 +2303,23 @@ TEST_F(Dhcpv4SrvTest, clientClassify) {
 
     // This discover does not belong to foo class, so it will not
     // be serviced
-    EXPECT_FALSE(srv_.selectSubnet(dis));
+    bool drop = false;
+    EXPECT_FALSE(srv_.selectSubnet(dis, drop));
+    EXPECT_FALSE(drop);
 
     // Let's add the packet to bar class and try again.
     dis->addClass("bar");
 
     // Still not supported, because it belongs to wrong class.
-    EXPECT_FALSE(srv_.selectSubnet(dis));
+    EXPECT_FALSE(srv_.selectSubnet(dis, drop));
+    EXPECT_FALSE(drop);
 
     // Let's add it to matching class.
     dis->addClass("foo");
 
     // This time it should work
-    EXPECT_TRUE(srv_.selectSubnet(dis));
+    EXPECT_TRUE(srv_.selectSubnet(dis, drop));
+    EXPECT_FALSE(drop);
 }
 
 // Verifies last resort option 43 is backward compatible
@@ -3493,31 +3497,38 @@ TEST_F(Dhcpv4SrvTest, relayOverride) {
     // This is just a sanity check, we're using regular method: ciaddr 192.0.2.1
     // belongs to the first subnet, so it is selected
     dis->setGiaddr(IOAddress("192.0.2.1"));
-    EXPECT_TRUE(subnet1 == srv_.selectSubnet(dis));
+    bool drop = false;
+    EXPECT_TRUE(subnet1 == srv_.selectSubnet(dis, drop));
+    EXPECT_FALSE(drop);
 
     // Relay belongs to the second subnet, so it  should be selected.
     dis->setGiaddr(IOAddress("192.0.3.1"));
-    EXPECT_TRUE(subnet2 == srv_.selectSubnet(dis));
+    EXPECT_TRUE(subnet2 == srv_.selectSubnet(dis, drop));
+    EXPECT_FALSE(drop);
 
     // Now let's check if the relay override for the first subnets works
     dis->setGiaddr(IOAddress("192.0.5.1"));
-    EXPECT_TRUE(subnet1 == srv_.selectSubnet(dis));
+    EXPECT_TRUE(subnet1 == srv_.selectSubnet(dis, drop));
+    EXPECT_FALSE(drop);
 
     // The same check for the second subnet...
     dis->setGiaddr(IOAddress("192.0.5.2"));
-    EXPECT_TRUE(subnet2 == srv_.selectSubnet(dis));
+    EXPECT_TRUE(subnet2 == srv_.selectSubnet(dis, drop));
+    EXPECT_FALSE(drop);
 
     // And finally, let's check if mis-matched relay address will end up
     // in not selecting a subnet at all
     dis->setGiaddr(IOAddress("192.0.5.3"));
-    EXPECT_FALSE(srv_.selectSubnet(dis));
+    EXPECT_FALSE(srv_.selectSubnet(dis, drop));
+    EXPECT_FALSE(drop);
 
     // Finally, check that the relay override works only with relay address
     // (GIADDR) and does not affect client address (CIADDR)
     dis->setGiaddr(IOAddress("0.0.0.0"));
     dis->setHops(0);
     dis->setCiaddr(IOAddress("192.0.5.1"));
-    EXPECT_FALSE(srv_.selectSubnet(dis));
+    EXPECT_FALSE(srv_.selectSubnet(dis, drop));
+    EXPECT_FALSE(drop);
 }
 
 // Checks if relay IP address specified in the relay-info structure can be
@@ -3573,12 +3584,15 @@ TEST_F(Dhcpv4SrvTest, relayOverrideAndClientClass) {
     // subnet[0], even though the relay-ip matches. It should be accepted in
     // subnet[1], because the subnet matches and there are no class
     // requirements.
-    EXPECT_TRUE(subnet2 == srv_.selectSubnet(dis));
+    bool drop = false;
+    EXPECT_TRUE(subnet2 == srv_.selectSubnet(dis, drop));
+    EXPECT_FALSE(drop);
 
     // Now let's add this packet to class foo and recheck. This time it should
     // be accepted in the first subnet, because both class and relay-ip match.
     dis->addClass("foo");
-    EXPECT_TRUE(subnet1 == srv_.selectSubnet(dis));
+    EXPECT_TRUE(subnet1 == srv_.selectSubnet(dis, drop));
+    EXPECT_FALSE(drop);
 }
 
 // Checks if a RAI link selection sub-option works as expected
@@ -3644,16 +3658,20 @@ TEST_F(Dhcpv4SrvTest, relayLinkSelect) {
     // This is just a sanity check, we're using regular method: ciaddr 192.0.3.1
     // belongs to the second subnet, so it is selected
     dis->setGiaddr(IOAddress("192.0.3.1"));
-    EXPECT_TRUE(subnet2 == srv_.selectSubnet(dis));
+    bool drop = false;
+    EXPECT_TRUE(subnet2 == srv_.selectSubnet(dis, drop));
+    EXPECT_FALSE(drop);
 
     // Setup a relay override for the first subnet as it has a high precedence
     dis->setGiaddr(IOAddress("192.0.5.1"));
-    EXPECT_TRUE(subnet1 == srv_.selectSubnet(dis));
+    EXPECT_TRUE(subnet1 == srv_.selectSubnet(dis, drop));
+    EXPECT_FALSE(drop);
 
     // Put a RAI to select back the second subnet as it has
     // the highest precedence
     dis->addOption(rai);
-    EXPECT_TRUE(subnet2 == srv_.selectSubnet(dis));
+    EXPECT_TRUE(subnet2 == srv_.selectSubnet(dis, drop));
+    EXPECT_FALSE(drop);
 
     // Subnet select option has a lower precedence
     OptionDefinitionPtr sbnsel_def = LibDHCP::getOptionDef(DHCP4_OPTION_SPACE,
@@ -3663,7 +3681,8 @@ TEST_F(Dhcpv4SrvTest, relayLinkSelect) {
     ASSERT_TRUE(sbnsel);
     sbnsel->writeAddress(IOAddress("192.0.2.3"));
     dis->addOption(sbnsel);
-    EXPECT_TRUE(subnet2 == srv_.selectSubnet(dis));
+    EXPECT_TRUE(subnet2 == srv_.selectSubnet(dis, drop));
+    EXPECT_FALSE(drop);
     dis->delOption(DHO_SUBNET_SELECTION);
 
     // Check client-classification still applies
@@ -3675,10 +3694,12 @@ TEST_F(Dhcpv4SrvTest, relayLinkSelect) {
     rai->addOption(ols);
     dis->addOption(rai);
     // Note it shall fail (vs. try the next criterion).
-    EXPECT_FALSE(srv_.selectSubnet(dis));
+    EXPECT_FALSE(srv_.selectSubnet(dis, drop));
+    EXPECT_FALSE(drop);
     // Add the packet to the class and check again: now it shall succeed
     dis->addClass("foo");
-    EXPECT_TRUE(subnet3 == srv_.selectSubnet(dis));
+    EXPECT_TRUE(subnet3 == srv_.selectSubnet(dis, drop));
+    EXPECT_FALSE(drop);
 
     // Check it fails with a bad address in the sub-option
     IOAddress addr_bad("10.0.0.1");
@@ -3688,7 +3709,8 @@ TEST_F(Dhcpv4SrvTest, relayLinkSelect) {
     dis->delOption(DHO_DHCP_AGENT_OPTIONS);
     rai->addOption(ols);
     dis->addOption(rai);
-    EXPECT_FALSE(srv_.selectSubnet(dis));
+    EXPECT_FALSE(srv_.selectSubnet(dis, drop));
+    EXPECT_FALSE(drop);
 }
 
 // Checks if a subnet selection option works as expected
@@ -3749,28 +3771,35 @@ TEST_F(Dhcpv4SrvTest, subnetSelect) {
     // This is just a sanity check, we're using regular method: ciaddr 192.0.3.1
     // belongs to the second subnet, so it is selected
     dis->setGiaddr(IOAddress("192.0.3.1"));
-    EXPECT_TRUE(subnet2 == srv_.selectSubnet(dis));
+    bool drop = false;
+    EXPECT_TRUE(subnet2 == srv_.selectSubnet(dis, drop));
+    EXPECT_FALSE(drop);
 
     // Setup a relay override for the first subnet as it has a high precedence
     dis->setGiaddr(IOAddress("192.0.5.1"));
-    EXPECT_TRUE(subnet1 == srv_.selectSubnet(dis));
+    EXPECT_TRUE(subnet1 == srv_.selectSubnet(dis, drop));
+    EXPECT_FALSE(drop);
 
     // Put a subnet select option to select back the second subnet as
     // it has the second highest precedence
     dis->addOption(sbnsel);
-    EXPECT_TRUE(subnet2 == srv_.selectSubnet(dis));
+    EXPECT_TRUE(subnet2 == srv_.selectSubnet(dis, drop));
+    EXPECT_FALSE(drop);
 
     // Check client-classification still applies
     sbnsel->writeAddress(IOAddress("192.0.4.2"));
     // Note it shall fail (vs. try the next criterion).
-    EXPECT_FALSE(srv_.selectSubnet(dis));
+    EXPECT_FALSE(srv_.selectSubnet(dis, drop));
+    EXPECT_FALSE(drop);
     // Add the packet to the class and check again: now it shall succeed
     dis->addClass("foo");
-    EXPECT_TRUE(subnet3 == srv_.selectSubnet(dis));
+    EXPECT_TRUE(subnet3 == srv_.selectSubnet(dis, drop));
+    EXPECT_FALSE(drop);
 
     // Check it fails with a bad address in the sub-option
     sbnsel->writeAddress(IOAddress("10.0.0.1"));
-    EXPECT_FALSE(srv_.selectSubnet(dis));
+    EXPECT_FALSE(srv_.selectSubnet(dis, drop));
+    EXPECT_FALSE(drop);
 }
 
 // This test verifies that the direct message is dropped when it has been

diff --git a/src/bin/dhcp4/tests/dhcp4_test_utils.cc b/src/bin/dhcp4/tests/dhcp4_test_utils.cc
index bd02d5f6fd42c95895582a7ab6eb93573cd6ff41..75c31f3f676b9cc46ed23f7b983b4aebe0e31ac4 100644 (file)
--- a/src/bin/dhcp4/tests/dhcp4_test_utils.cc
+++ b/src/bin/dhcp4/tests/dhcp4_test_utils.cc
@@ -631,8 +631,12 @@ Dhcpv4SrvTest::configure(const std::string& config, NakedDhcpv4Srv& srv,
  }
 
 Dhcpv4Exchange
-Dhcpv4SrvTest::createExchange(Pkt4Ptr& query) {
-    return (Dhcpv4Exchange(srv_.alloc_engine_, query, srv_.selectSubnet(query)));
+Dhcpv4SrvTest::createExchange(const Pkt4Ptr& query) {
+    bool drop = false;
+    Dhcpv4Exchange ex(srv_.alloc_engine_, query,
+                      srv_.selectSubnet(query, drop));
+    EXPECT_FALSE(drop);
+    return (ex);
 }
 
 void

diff --git a/src/bin/dhcp4/tests/dhcp4_test_utils.h b/src/bin/dhcp4/tests/dhcp4_test_utils.h
index dd4c1a795b4824cb63d6a352d02c2c119ce3ba26..48e571a447503fc041633e5904ad159f399b11ce 100644 (file)
--- a/src/bin/dhcp4/tests/dhcp4_test_utils.h
+++ b/src/bin/dhcp4/tests/dhcp4_test_utils.h
@@ -434,7 +434,7 @@ public:
                              uint8_t pkt_type, const std::string& stat_name);
 
     /// @brief Create @c Dhcpv4Exchange from client's query.
-    Dhcpv4Exchange createExchange(Pkt4Ptr& query);
+    Dhcpv4Exchange createExchange(const Pkt4Ptr& query);
 
     /// @brief Performs 4-way exchange to obtain new lease.
     ///

diff --git a/src/bin/dhcp4/tests/fqdn_unittest.cc b/src/bin/dhcp4/tests/fqdn_unittest.cc
index 2afa40955da1a870d66ad3aabec60283610ddb94..264ec433b7fb46432de7b7a06911c5a303ba3bf9 100644 (file)
--- a/src/bin/dhcp4/tests/fqdn_unittest.cc
+++ b/src/bin/dhcp4/tests/fqdn_unittest.cc
@@ -396,7 +396,7 @@ public:
 
     // Test that server generates the appropriate FQDN option in response to
     // client's FQDN option.
-    void testProcessFqdn(Pkt4Ptr& query, const uint8_t exp_flags,
+    void testProcessFqdn(const Pkt4Ptr& query, const uint8_t exp_flags,
                          const std::string& exp_domain_name,
                          Option4ClientFqdn::DomainNameType
                          exp_domain_type = Option4ClientFqdn::FULL) {
@@ -524,7 +524,7 @@ public:
     /// packet must contain the hostname option
     ///
     /// @return a pointer to the hostname option constructed by the server
-    OptionStringPtr processHostname(Pkt4Ptr& query,
+    OptionStringPtr processHostname(const Pkt4Ptr& query,
                                     bool must_have_host = true) {
         if (!getHostnameOption(query) && must_have_host) {
             ADD_FAILURE() << "Hostname option not carried in the query";