[991] Addressed review comments.

src/bin/dhcp4/dhcp4_srv.cc

@@ -518,7 +518,7 @@ Dhcpv4Srv::assignLease(const Pkt4Ptr& question, Pkt4Ptr& answer) {
         answer->setYiaddr(lease->addr_);
 
         // If remote address is not set, we are dealing with a directly
-        // connected client requesting new lease. We scan end response to
+        // connected client requesting new lease. We can send response to
         // the address assigned in the lease, but first we have to make sure
         // that IfaceMgr supports responding directly to the client when
         // client doesn't have address assigned to its interface yet.
@@ -527,18 +527,14 @@ Dhcpv4Srv::assignLease(const Pkt4Ptr& question, Pkt4Ptr& answer) {
                 answer->setRemoteAddr(lease->addr_);
             } else {
                 // Since IfaceMgr does not support direct responses to
-                // clients not having IP address, we have to send response
-                // to broadcast. Note that we don't check whether the
-                // use_bcast flag was set in the constructor or not.
-                // However, the use_bcast flag is mostly used by unit tests
-                // to prevent opening broadcast sockets in the constructor
-                // of this class because opening broadcast sockets requires
-                // root privileges. For this reason, it is rather unlikely
-                // that use_bcast flag is set to false in the real life
-                // scenario. On the other hand, if we fail to set remote
-                // address to broadcast here, we can't unit-test the case when
-                // server doesn't support direct responses to the client
-                // which doesn't have address yet.
+                // clients not having IP addresses, we have to send response
+                // to broadcast. We don't check whether the use_bcast flag
+                // was set in the constructor, because this flag is only used
+                // by unit tests to prevent opening broadcast sockets, as
+                // it requires root privileges. If this function is invoked by
+                // unit tests, we expect that it sets broadcast address if
+                // direct response is not supported, so as a test can verify
+                // function's behavior, regardless of the use_bcast flag's value.
                 answer->setRemoteAddr(IOAddress("255.255.255.255"));
             }
         }

src/bin/dhcp4/tests/dhcp4_srv_unittest.cc

@@ -180,6 +180,8 @@ public:
         EXPECT_TRUE(a->getOption(DHO_DHCP_SERVER_IDENTIFIER));
         EXPECT_TRUE(a->getOption(DHO_DHCP_LEASE_TIME));
         EXPECT_TRUE(a->getOption(DHO_SUBNET_MASK));
+        EXPECT_TRUE(a->getOption(DHO_DOMAIN_NAME));
+        EXPECT_TRUE(a->getOption(DHO_DOMAIN_NAME_SERVERS));
 
         // Check that something is offered
         EXPECT_TRUE(a->getYiaddr().toText() != "0.0.0.0");
@@ -434,32 +436,39 @@ public:
 
         messageCheck(req, rsp);
 
-        // There are some options that are always present in the
-        // message, even if not requested.
-        EXPECT_TRUE(rsp->getOption(DHO_DOMAIN_NAME));
-        EXPECT_TRUE(rsp->getOption(DHO_DOMAIN_NAME_SERVERS));
-
         // We did not request any options so these should not be present
         // in the RSP.
         EXPECT_FALSE(rsp->getOption(DHO_LOG_SERVERS));
         EXPECT_FALSE(rsp->getOption(DHO_COOKIE_SERVERS));
         EXPECT_FALSE(rsp->getOption(DHO_LPR_SERVERS));
 
+        // Repeat the test but request some options.
         // Add 'Parameter Request List' option.
         addPrlOption(req);
 
-        // Repeat the test but request some options.
-        ASSERT_NO_THROW(
-            rsp = srv->processRequest(req);
-        );
+        if (msg_type == DHCPDISCOVER) {
+            ASSERT_NO_THROW(
+                rsp = srv->processDiscover(req);
+            );
 
-        // Should return something
-        ASSERT_TRUE(rsp);
+            // Should return non-NULL packet.
+            ASSERT_TRUE(rsp);
+            EXPECT_EQ(DHCPOFFER, rsp->getType());
 
-        EXPECT_EQ(DHCPACK, rsp->getType());
+        } else {
+            ASSERT_NO_THROW(
+                rsp = srv->processRequest(req);
+            );
+
+            // Should return non-NULL packet.
+            ASSERT_TRUE(rsp);
+            EXPECT_EQ(DHCPACK, rsp->getType());
+
+        }
 
         // Check that the requested options are returned.
         optionsCheck(rsp);
+
     }
 
     ~Dhcpv4SrvTest() {

src/lib/dhcp/pkt_filter_inet.cc

@@ -68,7 +68,7 @@ int PktFilterInet::openSocket(const Iface&,
                        iface.getName().length() + 1) < 0) {
             close(sock);
             isc_throw(SocketConfigError, "Failed to set SO_BINDTODEVICE option"
-                      << "on socket " << sock);
+                      << " on socket " << sock);
         }
     }
 #endif
@@ -78,8 +78,8 @@ int PktFilterInet::openSocket(const Iface&,
         int flag = 1;
         if (setsockopt(sock, SOL_SOCKET, SO_BROADCAST, &flag, sizeof(flag)) < 0) {
             close(sock);
-            isc_throw(SocketConfigError, "Failed to set SO_BINDTODEVICE option"
-                      << "on socket " << sock);
+            isc_throw(SocketConfigError, "Failed to set SO_BROADCAST option"
+                      << " on socket " << sock);
         }
     }