From: Marcin Siodelski Date: Mon, 3 Sep 2012 12:15:22 +0000 (+0200) Subject: [1959] Implemented first set of changes from the code review. X-Git-Tag: trac2351_base~47^2~11^2~16 X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=6ca3dadca8255b8bb2aa58421f3f4289efe4e63a;p=thirdparty%2Fkea.git [1959] Implemented first set of changes from the code review. --- diff --git a/src/lib/dhcp/iface_mgr.cc b/src/lib/dhcp/iface_mgr.cc index 5f78653614..54db434d8f 100644 --- a/src/lib/dhcp/iface_mgr.cc +++ b/src/lib/dhcp/iface_mgr.cc @@ -70,7 +70,6 @@ void IfaceMgr::Iface::closeSockets() { for (SocketCollection::iterator sock = sockets_.begin(); sock != sockets_.end(); ++sock) { - cout << "Closing socket " << sock->sockfd_ << endl; close(sock->sockfd_); } sockets_.clear(); diff --git a/src/lib/dhcp/iface_mgr.h b/src/lib/dhcp/iface_mgr.h index 03ccf2426a..178d24a9ec 100644 --- a/src/lib/dhcp/iface_mgr.h +++ b/src/lib/dhcp/iface_mgr.h @@ -89,7 +89,7 @@ public: /// @param ifindex interface index (unique integer identifier) Iface(const std::string& name, int ifindex); - /// @brief Closes all open sockets on interface + /// @brief Closes all open sockets on interface. void closeSockets(); /// @brief Returns full interface name as "ifname/ifindex" string. @@ -204,6 +204,8 @@ public: /// in this collection. Note that functions like /// @ref IfaceMgr::openSocketFromIface use /// @ref IfaceMgr::openSocket internally. + /// The returned reference is only valid during the lifetime of + /// the IfaceMgr object that returned it. /// /// @return collection of sockets added to interface const SocketCollection& getSockets() const { return sockets_; } diff --git a/src/lib/dhcp/libdhcp++.cc b/src/lib/dhcp/libdhcp++.cc index 4ff07eac6a..22cd47bb6e 100644 --- a/src/lib/dhcp/libdhcp++.cc +++ b/src/lib/dhcp/libdhcp++.cc @@ -56,7 +56,7 @@ LibDHCP::optionFactory(Option::Universe u, isc_throw(BadValue, "invalid universe specified (expected " "Option::V4 or Option::V6"); } - return(it->second(u, type, buf)); + return (it->second(u, type, buf)); } diff --git a/src/lib/dhcp/libdhcp++.h b/src/lib/dhcp/libdhcp++.h index c959609c61..ae907012cc 100644 --- a/src/lib/dhcp/libdhcp++.h +++ b/src/lib/dhcp/libdhcp++.h @@ -45,7 +45,6 @@ public: uint16_t type, const OptionBuffer& buf); - /// Builds collection of options. /// /// Builds raw (on-wire) data for provided collection of options. diff --git a/src/lib/dhcp/option.h b/src/lib/dhcp/option.h index 37069d6df5..9589c37ab9 100644 --- a/src/lib/dhcp/option.h +++ b/src/lib/dhcp/option.h @@ -99,7 +99,6 @@ public: return factory(u, type, OptionBuffer()); } - /// @brief ctor, used for options constructed, usually during transmission /// /// @param u option universe (DHCPv4 or DHCPv6) diff --git a/src/lib/dhcp/tests/iface_mgr_unittest.cc b/src/lib/dhcp/tests/iface_mgr_unittest.cc index 5562551a0e..7f8555c1fa 100644 --- a/src/lib/dhcp/tests/iface_mgr_unittest.cc +++ b/src/lib/dhcp/tests/iface_mgr_unittest.cc @@ -59,6 +59,7 @@ public: ~IfaceMgrTest() { } + }; // We need some known interface to work reliably. Loopback interface @@ -217,6 +218,94 @@ TEST_F(IfaceMgrTest, getIface) { } +TEST_F(IfaceMgrTest, multipleSockets) { + boost::scoped_ptr ifacemgr(new NakedIfaceMgr()); + + // container for initialized socket descriptors + std::list init_sockets; + + // create socket #1 + int socket1 = 0; + ASSERT_NO_THROW( + socket1 = ifacemgr->openSocketFromIface(LOOPBACK, PORT1, AF_INET); + ); + ASSERT_GT(socket1, 0); + init_sockets.push_back(socket1); + + // create socket #2 + IOAddress loAddr("127.0.0.1"); + int socket2 = 0; + ASSERT_NO_THROW( + socket2 = ifacemgr->openSocketFromRemoteAddress(loAddr, PORT2); + ); + ASSERT_GT(socket2, 0); + init_sockets.push_back(socket2); + + // Get loopback interface. If we don't find one we are unable to run + // this test but we don't want to fail. + IfaceMgr::Iface* iface_ptr = ifacemgr->getIface(LOOPBACK); + if (iface_ptr == NULL) { + cout << "Local loopback interface not found. Skipping test. " << endl; + return; + } + // Once sockets have been sucessfully opened, they are supposed to + // be on the list. Here we start to test if all expected sockets + // are on the list and no other (unexpected) socket is there. + IfaceMgr::SocketCollection sockets = iface_ptr->getSockets(); + int matched_sockets = 0; + for (std::list::iterator init_sockets_it = + init_sockets.begin(); + init_sockets_it != init_sockets.end(); ++init_sockets_it) { + // Set socket descriptors non blocking in order to be able + // to call recv() on them without hang. + int flags = fcntl(*init_sockets_it, F_GETFL, 0); + ASSERT_GE(flags, 0); + ASSERT_GE(fcntl(*init_sockets_it, F_SETFL, flags | O_NONBLOCK), 0); + // recv() is expected to result in EWOULDBLOCK error on non-blocking + // socket in case socket is valid but simply no data are coming in. + char buf; + recv(*init_sockets_it, &buf, 1, MSG_PEEK); + EXPECT_EQ(EWOULDBLOCK, errno); + // Apart from the ability to use the socket we want to make + // sure that socket on the list is the one that we created. + for (IfaceMgr::SocketCollection::const_iterator socket_it = + sockets.begin(); socket_it != sockets.end(); ++socket_it) { + if (*init_sockets_it == socket_it->sockfd_) { + // This socket is the one that we created. + ++matched_sockets; + break; + } + } + } + // all created sockets have been matched if this condition works. + EXPECT_EQ(sockets.size(), matched_sockets); + + // closeSockets() is the other function that we want to test. It + // is supposed to close all sockets so as we will not be able to use + // them anymore communication. + ifacemgr->closeSockets(); + + // closed sockets are supposed to be removed from the list + sockets = iface_ptr->getSockets(); + ASSERT_EQ(0, sockets.size()); + + // We are still in posession of socket descriptors that we created + // on the beginning of this test. We can use them to check whether + // closeSockets() only removed them from the list or they have been + // really closed. + for (std::list::const_iterator init_sockets_it = + init_sockets.begin(); + init_sockets_it != init_sockets.end(); ++init_sockets_it) { + // recv() must result in error when using invalid socket. + char buf; + recv(*init_sockets_it, &buf, 1, MSG_PEEK); + // EWOULDBLOCK would mean that socket is valid/open but + // simply no data is received so we have to check for + // other errors. + EXPECT_NE(EWOULDBLOCK, errno); + } +} + TEST_F(IfaceMgrTest, sockets6) { // testing socket operation in a portable way is tricky // without interface detection implemented @@ -317,6 +406,15 @@ TEST_F(IfaceMgrTest, socketsFromRemoteAddress) { ); EXPECT_GT(socket2, 0); close(socket2); + + // Open v4 socket to connect to broadcast address. + int socket3 = 0; + IOAddress bcastAddr("255.255.255.255"); + EXPECT_NO_THROW( + socket3 = ifacemgr->openSocketFromRemoteAddress(bcastAddr, PORT2); + ); + EXPECT_GT(socket3, 0); + close(socket3); } // TODO: disabled due to other naming on various systems diff --git a/src/lib/dhcp/tests/libdhcp++_unittest.cc b/src/lib/dhcp/tests/libdhcp++_unittest.cc index ac758bed9c..d17eb6537f 100644 --- a/src/lib/dhcp/tests/libdhcp++_unittest.cc +++ b/src/lib/dhcp/tests/libdhcp++_unittest.cc @@ -87,21 +87,45 @@ TEST(LibDhcpTest, optionFactory) { DHO_SUBNET_MASK, buf); // Check if non-NULL DHO_SUBNET_MASK option pointer has been returned. - EXPECT_TRUE(opt_subnet_mask); - + ASSERT_TRUE(opt_subnet_mask); + // Validate if type and universe is correct. + EXPECT_EQ(Option::V4, opt_subnet_mask->getUniverse()); + EXPECT_EQ(DHO_SUBNET_MASK, opt_subnet_mask->getType()); + // Expect that option does not have content.. + EXPECT_EQ(0, opt_subnet_mask->len() - opt_subnet_mask->getHeaderLen()); + + // Fill the time offset buffer with 4 bytes of data. Each byte set to 1. + OptionBuffer time_offset_buf(4, 1); OptionPtr opt_time_offset; opt_time_offset = LibDHCP::optionFactory(Option::V4, DHO_TIME_OFFSET, - buf); + time_offset_buf); // Check if non-NULL DHO_TIME_OFFSET option pointer has been returned. - EXPECT_TRUE(opt_time_offset); - + ASSERT_TRUE(opt_time_offset); + // Validate if option length, type and universe is correct. + EXPECT_EQ(Option::V4, opt_time_offset->getUniverse()); + EXPECT_EQ(DHO_TIME_OFFSET, opt_time_offset->getType()); + EXPECT_EQ(time_offset_buf.size(), + opt_time_offset->len() - opt_time_offset->getHeaderLen()); + // Validate data in the option. + EXPECT_TRUE(std::equal(time_offset_buf.begin(), time_offset_buf.end(), + opt_time_offset->getData().begin())); + + // Fill the client id buffer with 20 bytes of data. Each byte set to 2. + OptionBuffer clientid_buf(20, 2); OptionPtr opt_clientid; opt_clientid = LibDHCP::optionFactory(Option::V6, D6O_CLIENTID, - buf); + clientid_buf); // Check if non-NULL D6O_CLIENTID option pointer has been returned. - EXPECT_TRUE(opt_clientid); + ASSERT_TRUE(opt_clientid); + // Validate if option length, type and universe is correct. + EXPECT_EQ(Option::V6, opt_clientid->getUniverse()); + EXPECT_EQ(D6O_CLIENTID, opt_clientid->getType()); + EXPECT_EQ(clientid_buf.size(), opt_clientid->len() - opt_clientid->getHeaderLen()); + // Validate data in the option. + EXPECT_TRUE(std::equal(clientid_buf.begin(), clientid_buf.end(), + opt_clientid->getData().begin())); } TEST(LibDhcpTest, packOptions6) {