BIND 10 trac3315, updated. 8d6bd774155061530c7c6cd0029332b0d35fe1ce [3315] Small cppcheck issue fixed
BIND 10 source code commits
bind10-changes at lists.isc.org
Mon Feb 3 15:26:20 UTC 2014
The branch, trac3315 has been updated
via 8d6bd774155061530c7c6cd0029332b0d35fe1ce (commit)
via d17da3a2be2c2649652b4dd0ae5d3d11ba9cc21b (commit)
via 22d124644f6fc0480295c4d02104687965afd24b (commit)
from 3b47e007ac82dcc63c6f99b344211ca4661a8471 (commit)
Those revisions listed above that are new to this repository have
not appeared on any other notification email; so we list those
revisions in full, below.
- Log -----------------------------------------------------------------
commit 8d6bd774155061530c7c6cd0029332b0d35fe1ce
Author: Tomek Mrugalski <tomasz at isc.org>
Date: Mon Feb 3 16:26:07 2014 +0100
[3315] Small cppcheck issue fixed
commit d17da3a2be2c2649652b4dd0ae5d3d11ba9cc21b
Author: Tomek Mrugalski <tomasz at isc.org>
Date: Mon Feb 3 16:25:43 2014 +0100
[3315] IPv6 tests for external sockets implemented.
commit 22d124644f6fc0480295c4d02104687965afd24b
Author: Tomek Mrugalski <tomasz at isc.org>
Date: Mon Feb 3 16:09:14 2014 +0100
[3315] Changes after review:
- SessionCallback renamed to SocketCallback
- Removed references in comments to session
- removed unused names string in receive{4,6}
- extended unit-test to check that unregistered callback is not
triggered
- Implemented similar tests for receive6()
- Fixed a bug that was revealed by those new unit-tests :)
-----------------------------------------------------------------------
Summary of changes:
src/lib/dhcp/iface_mgr.cc | 45 +++----
src/lib/dhcp/iface_mgr.h | 18 +--
src/lib/dhcp/iface_mgr_bsd.cc | 2 +-
src/lib/dhcp/iface_mgr_linux.cc | 2 +-
src/lib/dhcp/iface_mgr_sun.cc | 2 +-
src/lib/dhcp/tests/iface_mgr_unittest.cc | 194 +++++++++++++++++++++++++++++-
6 files changed, 219 insertions(+), 44 deletions(-)
-----------------------------------------------------------------------
diff --git a/src/lib/dhcp/iface_mgr.cc b/src/lib/dhcp/iface_mgr.cc
index cf13028..64892b5 100644
--- a/src/lib/dhcp/iface_mgr.cc
+++ b/src/lib/dhcp/iface_mgr.cc
@@ -227,7 +227,7 @@ IfaceMgr::isDirectResponseSupported() const {
}
void
-IfaceMgr::addExternalSocket(int socketfd, SessionCallback callback) {
+IfaceMgr::addExternalSocket(int socketfd, SocketCallback callback) {
for (SocketCallbackContainer::iterator s = callbacks_.begin();
s != callbacks_.end(); ++s) {
@@ -240,7 +240,7 @@ IfaceMgr::addExternalSocket(int socketfd, SessionCallback callback) {
}
// Add a new entry to the callbacks vector
- SocketCallback x;
+ SocketCallbackInfo x;
x.socket_ = socketfd;
x.callback_ = callback;
callbacks_.push_back(x);
@@ -819,7 +819,6 @@ IfaceMgr::receive4(uint32_t timeout_sec, uint32_t timeout_usec /* = 0 */) {
IfaceCollection::const_iterator iface;
fd_set sockets;
int maxfd = 0;
- stringstream names;
FD_ZERO(&sockets);
@@ -834,7 +833,6 @@ IfaceMgr::receive4(uint32_t timeout_sec, uint32_t timeout_usec /* = 0 */) {
// Only deal with IPv4 addresses.
if (s->addr_.isV4()) {
- names << s->sockfd_ << "(" << iface->getName() << ") ";
// Add this socket to listening set
FD_SET(s->sockfd_, &sockets);
@@ -845,7 +843,7 @@ IfaceMgr::receive4(uint32_t timeout_sec, uint32_t timeout_usec /* = 0 */) {
}
}
- // if there are any session sockets registered...
+ // if there are any callbacks for external sockets registered...
if (!callbacks_.empty()) {
for (SocketCallbackContainer::const_iterator s = callbacks_.begin();
s != callbacks_.end(); ++s) {
@@ -853,7 +851,6 @@ IfaceMgr::receive4(uint32_t timeout_sec, uint32_t timeout_usec /* = 0 */) {
if (maxfd < s->socket_) {
maxfd = s->socket_;
}
- names << s->socket_ << "(session)";
}
}
@@ -877,13 +874,11 @@ IfaceMgr::receive4(uint32_t timeout_sec, uint32_t timeout_usec /* = 0 */) {
continue;
}
- // something received over session socket
+ // something received over external socket
- // in theory we could call io_service.run_one() here, instead of
- // implementing callback mechanism, but that would introduce
- // asiolink dependency to libdhcp++ and that is something we want
- // to avoid (see CPE market and out long term plans for minimalistic
- // implementations.
+ // Calling the external socket's callback provides its service
+ // layer access without integrating any specific features
+ // in IfaceMgr
if (s->callback_) {
s->callback_();
}
@@ -925,7 +920,6 @@ Pkt6Ptr IfaceMgr::receive6(uint32_t timeout_sec, uint32_t timeout_usec /* = 0 */
const SocketInfo* candidate = 0;
fd_set sockets;
int maxfd = 0;
- stringstream names;
FD_ZERO(&sockets);
@@ -940,7 +934,6 @@ Pkt6Ptr IfaceMgr::receive6(uint32_t timeout_sec, uint32_t timeout_usec /* = 0 */
// Only deal with IPv6 addresses.
if (s->addr_.isV6()) {
- names << s->sockfd_ << "(" << iface->getName() << ") ";
// Add this socket to listening set
FD_SET(s->sockfd_, &sockets);
@@ -951,7 +944,7 @@ Pkt6Ptr IfaceMgr::receive6(uint32_t timeout_sec, uint32_t timeout_usec /* = 0 */
}
}
- // if there are any session sockets registered...
+ // if there are any callbacks for external sockets registered...
if (!callbacks_.empty()) {
for (SocketCallbackContainer::const_iterator s = callbacks_.begin();
s != callbacks_.end(); ++s) {
@@ -961,7 +954,6 @@ Pkt6Ptr IfaceMgr::receive6(uint32_t timeout_sec, uint32_t timeout_usec /* = 0 */
if (maxfd < s->socket_) {
maxfd = s->socket_;
}
- names << s->socket_ << "(session)";
}
}
@@ -981,16 +973,17 @@ Pkt6Ptr IfaceMgr::receive6(uint32_t timeout_sec, uint32_t timeout_usec /* = 0 */
// Let's find out which socket has the data
for (SocketCallbackContainer::iterator s = callbacks_.begin();
s != callbacks_.end(); ++s) {
- if (FD_ISSET(s->socket_, &sockets)) {
- // something received over session socket
- if (s->callback_) {
- // in theory we could call io_service.run_one() here, instead of
- // implementing callback mechanism, but that would introduce
- // asiolink dependency to libdhcp++ and that is something we want
- // to avoid (see CPE market and out long term plans for minimalistic
- // implementations.
- s->callback_();
- }
+ if (!FD_ISSET(s->socket_, &sockets)) {
+ continue;
+ }
+
+ // something received over external socket
+
+ // Calling the external socket's callback provides its service
+ // layer access without integrating any specific features
+ // in IfaceMgr
+ if (s->callback_) {
+ s->callback_();
}
return (Pkt6Ptr());
diff --git a/src/lib/dhcp/iface_mgr.h b/src/lib/dhcp/iface_mgr.h
index 75cbc53..268cc9f 100644
--- a/src/lib/dhcp/iface_mgr.h
+++ b/src/lib/dhcp/iface_mgr.h
@@ -394,20 +394,20 @@ boost::function<void(const std::string& errmsg)> IfaceMgrErrorMsgCallback;
///
class IfaceMgr : public boost::noncopyable {
public:
- /// Defines callback used when commands are received over control session.
- typedef void (*SessionCallback) (void);
+ /// Defines callback used when data is received over external sockets.
+ typedef boost::function<void ()> SocketCallback;
/// Keeps callback information for external sockets.
- struct SocketCallback {
+ struct SocketCallbackInfo {
/// Socket descriptor of the external socket.
int socket_;
/// A callback that will be called when data arrives over socket_.
- SessionCallback callback_;
+ SocketCallback callback_;
};
/// Defines storage container for callbacks for external sockets
- typedef std::list<SocketCallback> SocketCallbackContainer;
+ typedef std::list<SocketCallbackInfo> SocketCallbackContainer;
/// @brief Packet reception buffer size
///
@@ -797,14 +797,14 @@ public:
/// @return number of detected interfaces
uint16_t countIfaces() { return ifaces_.size(); }
- /// @brief Sets external socket and a callback
+ /// @brief Adds external socket and a callback
///
- /// Specifies session socket and a callback that will be called
+ /// Specifies external socket and a callback that will be called
/// when data will be received over that socket.
///
/// @param socketfd socket descriptor
/// @param callback callback function
- void addExternalSocket(int socketfd, SessionCallback callback);
+ void addExternalSocket(int socketfd, SocketCallback callback);
/// @brief Deletes external socket
@@ -1026,7 +1026,7 @@ private:
/// error occurs during opening a socket, or NULL if exception should
/// be thrown upon error.
bool openMulticastSocket(Iface& iface,
- const isc::asiolink::IOAddress addr,
+ const isc::asiolink::IOAddress& addr,
const uint16_t port,
IfaceMgrErrorMsgCallback error_handler = NULL);
diff --git a/src/lib/dhcp/iface_mgr_bsd.cc b/src/lib/dhcp/iface_mgr_bsd.cc
index cf6c3e2..7a01228 100644
--- a/src/lib/dhcp/iface_mgr_bsd.cc
+++ b/src/lib/dhcp/iface_mgr_bsd.cc
@@ -152,7 +152,7 @@ IfaceMgr::setMatchingPacketFilter(const bool /* direct_response_desired */) {
bool
IfaceMgr::openMulticastSocket(Iface& iface,
- const isc::asiolink::IOAddress addr,
+ const isc::asiolink::IOAddress& addr,
const uint16_t port,
IfaceMgrErrorMsgCallback error_handler) {
try {
diff --git a/src/lib/dhcp/iface_mgr_linux.cc b/src/lib/dhcp/iface_mgr_linux.cc
index eaa625e..f4b0613 100644
--- a/src/lib/dhcp/iface_mgr_linux.cc
+++ b/src/lib/dhcp/iface_mgr_linux.cc
@@ -536,7 +536,7 @@ bool IfaceMgr::os_receive4(struct msghdr&, Pkt4Ptr&) {
bool
IfaceMgr::openMulticastSocket(Iface& iface,
- const isc::asiolink::IOAddress addr,
+ const isc::asiolink::IOAddress& addr,
const uint16_t port,
IfaceMgrErrorMsgCallback error_handler) {
// This variable will hold a descriptor of the socket bound to
diff --git a/src/lib/dhcp/iface_mgr_sun.cc b/src/lib/dhcp/iface_mgr_sun.cc
index 0a9f9b4..a78de8f 100644
--- a/src/lib/dhcp/iface_mgr_sun.cc
+++ b/src/lib/dhcp/iface_mgr_sun.cc
@@ -156,7 +156,7 @@ IfaceMgr::setMatchingPacketFilter(const bool /* direct_response_desired */) {
bool
IfaceMgr::openMulticastSocket(Iface& iface,
- const isc::asiolink::IOAddress addr,
+ const isc::asiolink::IOAddress& addr,
const uint16_t port,
IfaceMgrErrorMsgCallback error_handler) {
try {
diff --git a/src/lib/dhcp/tests/iface_mgr_unittest.cc b/src/lib/dhcp/tests/iface_mgr_unittest.cc
index a1adf0b..a8cb0cd 100644
--- a/src/lib/dhcp/tests/iface_mgr_unittest.cc
+++ b/src/lib/dhcp/tests/iface_mgr_unittest.cc
@@ -2601,9 +2601,9 @@ void my_callback2(void) {
callback2_ok = true;
}
-// Tests if a signle external socket and its callback can be passed and
+// Tests if a single external socket and its callback can be passed and
// it is supported properly by receive4() method.
-TEST_F(IfaceMgrTest, SingleExternalSession) {
+TEST_F(IfaceMgrTest, SingleExternalSocket4) {
callback_ok = false;
@@ -2642,7 +2642,7 @@ TEST_F(IfaceMgrTest, SingleExternalSession) {
// Tests if multiple external sockets and their callbacks can be passed and
// it is supported properly by receive4() method.
-TEST_F(IfaceMgrTest, MiltipleControlSessions) {
+TEST_F(IfaceMgrTest, MiltipleExternalSockets4) {
callback_ok = false;
callback2_ok = false;
@@ -2683,7 +2683,8 @@ TEST_F(IfaceMgrTest, MiltipleControlSessions) {
EXPECT_FALSE(callback2_ok);
// Read the data sent, because our test callbacks are too dumb to actually
- // do it.
+ // do it. We don't care about the content read, because we're testing
+ // the callbacks, not pipes.
char buf[80];
read(pipefd[0], buf, 80);
@@ -2713,8 +2714,8 @@ TEST_F(IfaceMgrTest, MiltipleControlSessions) {
}
// Tests if existing external socket can be deleted and that such deletion does
-// not affect any other existing sockets.
-TEST_F(IfaceMgrTest, DeleteControlSessions) {
+// not affect any other existing sockets. Tests uses receive4()
+TEST_F(IfaceMgrTest, DeleteExternalSockets4) {
callback_ok = false;
callback2_ok = false;
@@ -2748,6 +2749,187 @@ TEST_F(IfaceMgrTest, DeleteControlSessions) {
EXPECT_FALSE(callback_ok);
EXPECT_TRUE(callback2_ok);
+ // Let's reset the status
+ callback_ok = false;
+ callback2_ok = false;
+
+ // Now let's send something over the first callback that was unregistered.
+ // We should NOT receive any callback.
+ EXPECT_EQ(38, write(pipefd[1], "Hi, this is a message sent over a pipe", 38));
+
+ // Now check that the first callback is NOT called.
+ ASSERT_NO_THROW(pkt4 = ifacemgr->receive4(1));
+ EXPECT_FALSE(callback_ok);
+
+ // close both pipe ends
+ close(pipefd[1]);
+ close(pipefd[0]);
+
+ close(secondpipe[1]);
+ close(secondpipe[0]);
+}
+
+
+// Tests if a single external socket and its callback can be passed and
+// it is supported properly by receive6() method.
+TEST_F(IfaceMgrTest, SingleExternalSocket6) {
+
+ callback_ok = false;
+
+ scoped_ptr<NakedIfaceMgr> ifacemgr(new NakedIfaceMgr());
+
+ // Create pipe and register it as extra socket
+ int pipefd[2];
+ EXPECT_TRUE(pipe(pipefd) == 0);
+ EXPECT_NO_THROW(ifacemgr->addExternalSocket(pipefd[0], my_callback));
+
+ Pkt6Ptr pkt6;
+ ASSERT_NO_THROW(pkt6 = ifacemgr->receive6(1));
+
+ // Our callback should not be called this time (there was no data)
+ EXPECT_FALSE(callback_ok);
+
+ // IfaceMgr should not process control socket data as incoming packets
+ EXPECT_FALSE(pkt6);
+
+ // Now, send some data over pipe (38 bytes)
+ EXPECT_EQ(38, write(pipefd[1], "Hi, this is a message sent over a pipe", 38));
+
+ // ... and repeat
+ ASSERT_NO_THROW(pkt6 = ifacemgr->receive6(1));
+
+ // IfaceMgr should not process control socket data as incoming packets
+ EXPECT_FALSE(pkt6);
+
+ // There was some data, so this time callback should be called
+ EXPECT_TRUE(callback_ok);
+
+ // close both pipe ends
+ close(pipefd[1]);
+ close(pipefd[0]);
+}
+
+// Tests if multiple external sockets and their callbacks can be passed and
+// it is supported properly by receive6() method.
+TEST_F(IfaceMgrTest, MiltipleExternalSockets6) {
+
+ callback_ok = false;
+ callback2_ok = false;
+
+ scoped_ptr<NakedIfaceMgr> ifacemgr(new NakedIfaceMgr());
+
+ // Create first pipe and register it as extra socket
+ int pipefd[2];
+ EXPECT_TRUE(pipe(pipefd) == 0);
+ EXPECT_NO_THROW(ifacemgr->addExternalSocket(pipefd[0], my_callback));
+
+ // 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], my_callback2));
+
+ Pkt6Ptr pkt6;
+ ASSERT_NO_THROW(pkt6 = ifacemgr->receive6(1));
+
+ // Our callbacks should not be called this time (there was no data)
+ EXPECT_FALSE(callback_ok);
+ EXPECT_FALSE(callback2_ok);
+
+ // IfaceMgr should not process control socket data as incoming packets
+ EXPECT_FALSE(pkt6);
+
+ // Now, send some data over the first pipe (38 bytes)
+ EXPECT_EQ(38, write(pipefd[1], "Hi, this is a message sent over a pipe", 38));
+
+ // ... and repeat
+ ASSERT_NO_THROW(pkt6 = ifacemgr->receive6(1));
+
+ // IfaceMgr should not process control socket data as incoming packets
+ EXPECT_FALSE(pkt6);
+
+ // There was some data, so this time callback should be called
+ EXPECT_TRUE(callback_ok);
+ EXPECT_FALSE(callback2_ok);
+
+ // Read the data sent, because our test callbacks are too dumb to actually
+ // do it. We don't care about the content read, because we're testing
+ // the callbacks, not pipes.
+ char buf[80];
+ read(pipefd[0], buf, 80);
+
+ // Clear the status...
+ callback_ok = false;
+ callback2_ok = false;
+
+ // And try again, using the second pipe
+ EXPECT_EQ(38, write(secondpipe[1], "Hi, this is a message sent over a pipe", 38));
+
+ // ... and repeat
+ ASSERT_NO_THROW(pkt6 = ifacemgr->receive6(1));
+
+ // IfaceMgr should not process control socket data as incoming packets
+ EXPECT_FALSE(pkt6);
+
+ // There was some data, so this time callback should be called
+ EXPECT_FALSE(callback_ok);
+ EXPECT_TRUE(callback2_ok);
+
+ // close both pipe ends
+ close(pipefd[1]);
+ close(pipefd[0]);
+
+ close(secondpipe[1]);
+ close(secondpipe[0]);
+}
+
+// Tests if existing external socket can be deleted and that such deletion does
+// not affect any other existing sockets. Tests uses receive6()
+TEST_F(IfaceMgrTest, DeleteExternalSockets6) {
+
+ callback_ok = false;
+ callback2_ok = false;
+
+ scoped_ptr<NakedIfaceMgr> ifacemgr(new NakedIfaceMgr());
+
+ // Create first pipe and register it as extra socket
+ int pipefd[2];
+ EXPECT_TRUE(pipe(pipefd) == 0);
+ EXPECT_NO_THROW(ifacemgr->addExternalSocket(pipefd[0], my_callback));
+
+ // 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], my_callback2));
+
+ // Now delete the first session socket
+ EXPECT_NO_THROW(ifacemgr->deleteExternalSocket(pipefd[0]));
+
+ // Now check whether the second callback is still functional
+ EXPECT_EQ(38, write(secondpipe[1], "Hi, this is a message sent over a pipe", 38));
+
+ // ... and repeat
+ Pkt6Ptr pkt6;
+ ASSERT_NO_THROW(pkt6 = ifacemgr->receive6(1));
+
+ // IfaceMgr should not process control socket data as incoming packets
+ EXPECT_FALSE(pkt6);
+
+ // There was some data, so this time callback should be called
+ EXPECT_FALSE(callback_ok);
+ EXPECT_TRUE(callback2_ok);
+
+ // Let's reset the status
+ callback_ok = false;
+ callback2_ok = false;
+
+ // Now let's send something over the first callback that was unregistered.
+ // We should NOT receive any callback.
+ EXPECT_EQ(38, write(pipefd[1], "Hi, this is a message sent over a pipe", 38));
+
+ // Now check that the first callback is NOT called.
+ ASSERT_NO_THROW(pkt6 = ifacemgr->receive6(1));
+ EXPECT_FALSE(callback_ok);
+
// close both pipe ends
close(pipefd[1]);
close(pipefd[0]);
More information about the bind10-changes
mailing list