[#691,!395] Review comments 3

    IfaceMgr and unit tests clean up.

diff --git a/src/lib/dhcp/iface_mgr.cc b/src/lib/dhcp/iface_mgr.cc
index d66bf0fcbf1ba2870eb06ed52c64f34599e5451c..c7d4bdd2278de08b4b03abd34ae2a0ddecd5ad3b 100644 (file)
--- a/src/lib/dhcp/iface_mgr.cc
+++ b/src/lib/dhcp/iface_mgr.cc
@@ -349,7 +349,7 @@ IfaceMgr::deleteExternalSocket(int socketfd) {
 int
 IfaceMgr::purgeBadSockets() {
     std::vector<int> bad_fds;
-    BOOST_FOREACH(SocketCallbackInfo s, callbacks_) {
+    for (SocketCallbackInfo s : callbacks_) {
         errno = 0;
         if (fcntl(s.socket_, F_GETFD) < 0 && (errno == EBADF)) {
             bad_fds.push_back(s.socket_);

diff --git a/src/lib/dhcp/iface_mgr.h b/src/lib/dhcp/iface_mgr.h
index fc308526ade2c94b88536ffcf84f8ded718d0fc4..9f36a4e50a3b837245668e0b7c0fe68a3b3ea6b7 100644 (file)
--- a/src/lib/dhcp/iface_mgr.h
+++ b/src/lib/dhcp/iface_mgr.h
@@ -928,10 +928,11 @@ public:
 
     /// @brief Scans registered socket set and removes any that are invalid.
     ///
-    /// Walks the list of regiseterd external sockets and test each for
-    /// validity.  If any are found to be invalid they are removed.  This
+    /// Walks the list of registered external sockets and testing each for
+    /// validity.  If any are found to be invalid they are removed. This is
     /// primarily a self-defense mechanism against hook libs or other users
-    /// of external sockets, leaving a closed socket registered by mistake.
+    /// of external sockets that may leave a closed socket registered by
+    /// mistake.
     ///
     /// @return A count of the sockets purged.
     int purgeBadSockets();

diff --git a/src/lib/dhcp/tests/iface_mgr_unittest.cc b/src/lib/dhcp/tests/iface_mgr_unittest.cc
index 56e71001d4bf55c2e92d3c6441cad46000cec0f0..2af93ddf83775cf7efdd6d2f849bed81232e45b7 100644 (file)
--- a/src/lib/dhcp/tests/iface_mgr_unittest.cc
+++ b/src/lib/dhcp/tests/iface_mgr_unittest.cc
@@ -52,6 +52,9 @@ const uint16_t PORT2 = 10548;   // V4 socket
 // tolerance to 0.01s.
 const uint32_t TIMEOUT_TOLERANCE = 10000;
 
+// Macro for making select wait time arguments for receive functions
+#define RECEIVE_WAIT_MS(m) 0,(m*1000)
+
 /// This test verifies that the socket read buffer can be used to
 /// receive the data and that the data can be read from it.
 TEST(IfaceTest, readBuffer) {
@@ -692,18 +695,18 @@ public:
         int pipefd[2];
         EXPECT_TRUE(pipe(pipefd) == 0);
         EXPECT_NO_THROW(ifacemgr->addExternalSocket(pipefd[0],
-                        [this, &callback_ok](){ callback_ok = true; }));
+                        [&callback_ok](){ callback_ok = true; }));
 
 
         // Let's create a second pipe and register it as well
         int secondpipe[2];
         EXPECT_TRUE(pipe(secondpipe) == 0);
         EXPECT_NO_THROW(ifacemgr->addExternalSocket(secondpipe[0],
-                        [this, &callback2_ok](){ callback2_ok = true; }));
+                        [&callback2_ok](){ callback2_ok = true; }));
 
         // Verify a call with no data and normal external sockets works ok.
         Pkt4Ptr pkt4;
-        ASSERT_NO_THROW(pkt4 = ifacemgr->receive4(1));
+        ASSERT_NO_THROW(pkt4 = ifacemgr->receive4(RECEIVE_WAIT_MS(10)));
 
         // No callback invocations and no DHCPv4 pkt.
         EXPECT_FALSE(callback_ok);
@@ -716,7 +719,7 @@ public:
 
         // We call receive4() which should detect and remove the invalid socket.
         try {
-            pkt4 = ifacemgr->receive4(1);
+            pkt4 = ifacemgr->receive4(RECEIVE_WAIT_MS(10));
             ADD_FAILURE() << "receive4 should have failed";
         } catch (const SocketReadError& ex) {
             EXPECT_EQ(std::string("SELECT interrupted by one invalid sockets,"
@@ -735,7 +738,7 @@ public:
         EXPECT_EQ(38, write(secondpipe[1], "Hi, this is a message sent over a pipe", 38));
 
         // Call recevie4 again, this should work.
-        ASSERT_NO_THROW(pkt4 = ifacemgr->receive4(1));
+        ASSERT_NO_THROW(pkt4 = ifacemgr->receive4(RECEIVE_WAIT_MS(10)));
 
         // Should have callback2 data only.
         EXPECT_FALSE(callback_ok);
@@ -775,18 +778,18 @@ public:
         int pipefd[2];
         EXPECT_TRUE(pipe(pipefd) == 0);
         EXPECT_NO_THROW(ifacemgr->addExternalSocket(pipefd[0],
-                        [this, &callback_ok](){ callback_ok = true; }));
+                        [&callback_ok](){ callback_ok = true; }));
 
 
         // Let's create a second pipe and register it as well
         int secondpipe[2];
         EXPECT_TRUE(pipe(secondpipe) == 0);
         EXPECT_NO_THROW(ifacemgr->addExternalSocket(secondpipe[0],
-                        [this, &callback2_ok](){ callback2_ok = true; }));
+                        [&callback2_ok](){ callback2_ok = true; }));
 
         // Verify a call with no data and normal external sockets works ok.
         Pkt6Ptr pkt6;
-        ASSERT_NO_THROW(pkt6 = ifacemgr->receive6(1));
+        ASSERT_NO_THROW(pkt6 = ifacemgr->receive6(RECEIVE_WAIT_MS(10)));
 
         // No callback invocations and no DHCPv6 pkt.
         EXPECT_FALSE(callback_ok);
@@ -799,7 +802,7 @@ public:
 
         // We call receive6() which should detect and remove the invalid socket.
         try {
-            pkt6 = ifacemgr->receive6(1);
+            pkt6 = ifacemgr->receive6(RECEIVE_WAIT_MS(10));
             ADD_FAILURE() << "receive6 should have failed";
         } catch (const SocketReadError& ex) {
             EXPECT_EQ(std::string("SELECT interrupted by one invalid sockets,"
@@ -818,7 +821,7 @@ public:
         EXPECT_EQ(38, write(secondpipe[1], "Hi, this is a message sent over a pipe", 38));
 
         // Call recevie6 again, this should work.
-        ASSERT_NO_THROW(pkt6 = ifacemgr->receive6(1));
+        ASSERT_NO_THROW(pkt6 = ifacemgr->receive6(RECEIVE_WAIT_MS(10)));
 
         // Should have callback2 data only.
         EXPECT_FALSE(callback_ok);
@@ -2928,16 +2931,18 @@ TEST_F(IfaceMgrTest, DeleteExternalSockets4) {
 
 // Tests that an existing external socket that becomes invalid
 // is detected and purged, without affecting other sockets.
-// Tests uses receive4().
-TEST_F(IfaceMgrTest, purgeExternalSockets4) {
-    // Run purge test without packet queuing.
+// Tests uses receive4() without queuing..
+TEST_F(IfaceMgrTest, purgeExternalSockets4Direct) {
     purgeExternalSockets4Test();
-
-    // Run purge test with packet queuing.
-    purgeExternalSockets4Test(true);
 }
 
 
+// Tests that an existing external socket that becomes invalid
+// is detected and purged, without affecting other sockets.
+// Tests uses receive4() with queuing..
+TEST_F(IfaceMgrTest, purgeExternalSockets4Indirect) {
+    purgeExternalSockets4Test(true);
+}
 
 // Tests if a single external socket and its callback can be passed and
 // it is supported properly by receive6() method.
@@ -3107,13 +3112,18 @@ TEST_F(IfaceMgrTest, DeleteExternalSockets6) {
     close(secondpipe[0]);
 }
 
-// Tests if existing external socket become invalid and be purged and not
-// not affect any other existing sockets. Tests uses receive6()
-TEST_F(IfaceMgrTest, purgeExternalSockets6) {
-    // Run purge test without packet queuing.
+// Tests that an existing external socket that becomes invalid
+// is detected and purged, without affecting other sockets.
+// Tests uses receive6() without queuing..
+TEST_F(IfaceMgrTest, purgeExternalSockets6Direct) {
     purgeExternalSockets6Test();
+}
 
-    // Run purge test with packet queuing.
+
+// Tests that an existing external socket that becomes invalid
+// is detected and purged, without affecting other sockets.
+// Tests uses receive6() with queuing..
+TEST_F(IfaceMgrTest, purgeExternalSockets6Indirect) {
     purgeExternalSockets6Test(true);
 }