[#1584] Addressed minor review comments

modified:
    src/bin/dhcp4/dhcp4_srv.cc
    src/bin/dhcp4/tests/dora_unittest.cc
    src/bin/dhcp4/tests/hooks_unittest.cc
    src/lib/dhcpsrv/alloc_engine.cc

diff --git a/src/bin/dhcp4/dhcp4_srv.cc b/src/bin/dhcp4/dhcp4_srv.cc
index e3ffbae559182f3b5885c8f9dbc5237f7546390a..ed3b56baa47cf8d690d9cb464599da164ae19292 100644 (file)
--- a/src/bin/dhcp4/dhcp4_srv.cc
+++ b/src/bin/dhcp4/dhcp4_srv.cc
@@ -2578,7 +2578,7 @@ Dhcpv4Srv::assignLease(Dhcpv4Exchange& ex) {
             // Address might have been rejected via class guard (i.e. not
             // allowed for this client). We need to determine if we truly
             // do not know about the address or whether this client just
-            // isn't allowed to have that address. We should only NAK
+            // isn't allowed to have that address. We should only DHCPNAK
             // For the latter.
             while (s) {
                 if (s->inPool(Lease::TYPE_V4, hint)) {
@@ -2589,7 +2589,7 @@ Dhcpv4Srv::assignLease(Dhcpv4Exchange& ex) {
             }
 
             // If we didn't find a subnet, it's not an address we know about
-            // so we drop the NAK.
+            // so we drop the DHCPNAK.
             if (!s) {
                 LOG_DEBUG(bad_packet4_logger, DBG_DHCP4_DETAIL,
                           DHCP4_UNKNOWN_ADDRESS_REQUESTED)
@@ -2886,9 +2886,9 @@ Dhcpv4Srv::adjustRemoteAddr(Dhcpv4Exchange& ex) {
     } else if (!query->getCiaddr().isV4Zero()) {
         response->setRemoteAddr(query->getCiaddr());
 
-    // We can't unicast the response to the client when sending NAK,
+    // We can't unicast the response to the client when sending DHCPNAK,
     // because we haven't allocated address for him. Therefore,
-    // NAK is broadcast.
+    // DHCPNAK is broadcast.
     } else if (response->getType() == DHCPNAK) {
         response->setRemoteAddr(IOAddress::IPV4_BCAST_ADDRESS());
 

diff --git a/src/bin/dhcp4/tests/dora_unittest.cc b/src/bin/dhcp4/tests/dora_unittest.cc
index 78df3176e91a20ab64a5bc4408fb0bf67f04c8cb..6fa77c5e052de8da7a1a4f6dd524f0f1ee134243 100644 (file)
--- a/src/bin/dhcp4/tests/dora_unittest.cc
+++ b/src/bin/dhcp4/tests/dora_unittest.cc
@@ -2001,10 +2001,10 @@ DORATest::reservationsWithConflicts() {
     ASSERT_NO_THROW(client.doRequest());
 
     // The reservation has been removed. Since address that the client is
-    // using doesn't belong a dynamic pool and the server is not
+    // using doesn't belong to a dynamic pool and the server is not
     // authoritative it should not send a DHCPNAK.
     resp = client.getContext().response_;
-    ASSERT_FALSE(client.getContext().response_);
+    ASSERT_FALSE(resp);
 
     // A conforming client would go back to the server discovery.
     client.setState(Dhcp4Client::SELECTING);

diff --git a/src/bin/dhcp4/tests/hooks_unittest.cc b/src/bin/dhcp4/tests/hooks_unittest.cc
index c30b709323413a8a86f646d1dd011450bc1f02f0..6733e87c504c4fed11873bc79fb139b45820e788 100644 (file)
--- a/src/bin/dhcp4/tests/hooks_unittest.cc
+++ b/src/bin/dhcp4/tests/hooks_unittest.cc
@@ -2040,14 +2040,13 @@ TEST_F(HooksDhcpv4SrvTest, leases4CommittedRequest) {
 
     resetCalloutBuffers();
 
-    // Now request an address that can't be allocated. The client should receive
-    // the DHCPNAK.
+    // Now request an address that can't be allocated.
     client.ciaddr_ = IOAddress("10.0.0.1");
 
     ASSERT_NO_THROW(client.doRequest());
 
-    // Make sure that we did not receive a response. If we're
-    // not authoritative, there should not be a NAK.
+    // Make sure that we did not receive a response. Since we're
+    // not authoritative, there should not be a DHCPNAK.
     ASSERT_FALSE(client.getContext().response_);
 
     // Check that no callback was not called.

diff --git a/src/lib/dhcpsrv/alloc_engine.cc b/src/lib/dhcpsrv/alloc_engine.cc
index 0829c96ca55ff7760f57d4ea0b2ce3bd6b9f029b..3c0e870b1f1a21e88e66409b8b6be7b646e68a00 100644 (file)
--- a/src/lib/dhcpsrv/alloc_engine.cc
+++ b/src/lib/dhcpsrv/alloc_engine.cc
@@ -3600,7 +3600,7 @@ AllocEngine::findGlobalReservation(ClientContext4& ctx) {
 
 Lease4Ptr
 AllocEngine::discoverLease4(AllocEngine::ClientContext4& ctx) {
-    // Find an existing lease for this client. This function will return true
+    // Find an existing lease for this client. This function will return null
     // if there is a conflict with existing lease and the allocation should
     // not be continued.
     Lease4Ptr client_lease;
@@ -3711,7 +3711,7 @@ AllocEngine::discoverLease4(AllocEngine::ClientContext4& ctx) {
 
 Lease4Ptr
 AllocEngine::requestLease4(AllocEngine::ClientContext4& ctx) {
-    // Find an existing lease for this client. This function will return true
+    // Find an existing lease for this client. This function will return null
     // if there is a conflict with existing lease and the allocation should
     // not be continued.
     Lease4Ptr client_lease;