From: Marcin Siodelski Date: Wed, 10 Sep 2014 12:10:56 +0000 (+0200) Subject: [3357] Clean up in the UnixSocket class and the unit tests. X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=713057737c94ad2d44f1a8d8f57607b418581047;p=thirdparty%2Fkea.git [3357] Clean up in the UnixSocket class and the unit tests. --- diff --git a/src/bin/dhcp4/dhcp4_srv.cc b/src/bin/dhcp4/dhcp4_srv.cc index 8b51fe754d..d9994d5903 100644 --- a/src/bin/dhcp4/dhcp4_srv.cc +++ b/src/bin/dhcp4/dhcp4_srv.cc @@ -2027,7 +2027,7 @@ Dhcpv4Srv::enable4o6() { } if (!ipc_) return; - IfaceMgr::instance().addExternalSocket(ipc_->getSocket(), + IfaceMgr::instance().addExternalSocket(ipc_->get(), boost::bind(&DHCP4o6IPC::recvPkt4o6, ipc_)); } @@ -2035,7 +2035,7 @@ void Dhcpv4Srv::disable4o6() { if (!ipc_) return; - IfaceMgr::instance().deleteExternalSocket(ipc_->getSocket()); + IfaceMgr::instance().deleteExternalSocket(ipc_->get()); ipc_ = boost::shared_ptr(); } diff --git a/src/bin/dhcp6/dhcp6_srv.cc b/src/bin/dhcp6/dhcp6_srv.cc index 47ccb041b5..74c24c726e 100644 --- a/src/bin/dhcp6/dhcp6_srv.cc +++ b/src/bin/dhcp6/dhcp6_srv.cc @@ -2492,7 +2492,7 @@ Dhcpv6Srv::enable4o6() { } if (!ipc_) return; - IfaceMgr::instance().addExternalSocket(ipc_->getSocket(), + IfaceMgr::instance().addExternalSocket(ipc_->get(), boost::bind(&DHCP4o6IPC::recvPkt4o6, ipc_)); } @@ -2500,7 +2500,7 @@ void Dhcpv6Srv::disable4o6() { if (!ipc_) return; - IfaceMgr::instance().deleteExternalSocket(ipc_->getSocket()); + IfaceMgr::instance().deleteExternalSocket(ipc_->get()); ipc_ = boost::shared_ptr(); } diff --git a/src/lib/dhcpsrv/dhcp4o6_ipc.cc b/src/lib/dhcpsrv/dhcp4o6_ipc.cc index 2ae767cd98..e65241a58b 100644 --- a/src/lib/dhcpsrv/dhcp4o6_ipc.cc +++ b/src/lib/dhcpsrv/dhcp4o6_ipc.cc @@ -47,7 +47,8 @@ DHCP4o6IPC::sendPkt4o6(const Pkt4o6Ptr& pkt4o6) { void DHCP4o6IPC::recvPkt4o6() { - isc::util::InputBuffer buf = recv(); + int len = receive(); + isc::util::InputBuffer buf(getReceiveBuffer(), len); size_t len4, len6, len_json; uint8_t buf4[RCVBUFSIZE]; uint8_t buf6[RCVBUFSIZE]; diff --git a/src/lib/util/tests/unix_socket_unittest.cc b/src/lib/util/tests/unix_socket_unittest.cc index f03ffb77f7..5f4b3a4192 100644 --- a/src/lib/util/tests/unix_socket_unittest.cc +++ b/src/lib/util/tests/unix_socket_unittest.cc @@ -19,113 +19,126 @@ using namespace isc::util; - namespace { -/// @brief A test fixture class for UnixSocket. +const char* TEST_FILE1 = "test_unix_socket_1"; +const char* TEST_FILE2 = "test_unix_socket_2"; + +/// @brief A test fixture class for @c UnixSocket. class UnixSocketTest : public ::testing::Test { public: /// @brief Constructor. /// /// It initializes 2 unix sockets. UnixSocketTest() : - sock1_("test_ipc_2to1", "test_ipc_1to2"), - sock2_("test_ipc_1to2", "test_ipc_2to1") + sock1_(absolutePath(TEST_FILE1), absolutePath(TEST_FILE2)), + sock2_(absolutePath(TEST_FILE2), absolutePath(TEST_FILE1)) { } -protected: + + /// @brief Prepends the absolute path to the file specified + /// as an argument. + /// + /// @param filename Name of the file. + /// @return Absolute path to the test file. + std::string absolutePath(const std::string& filename); + /// UnixSocket objects for testing. UnixSocket sock1_, sock2_; }; +std::string +UnixSocketTest::absolutePath(const std::string& filename) { + std::ostringstream s; + s << TEST_DATA_BUILDDIR << "/" << filename; + return (s.str()); +} + // Test UnixSocket constructor TEST_F(UnixSocketTest, constructor) { - EXPECT_EQ(-1, sock1_.getSocket()); -} + EXPECT_EQ(-1, sock1_.get()); + EXPECT_EQ(-1, sock2_.get()); + // The local and remote sockets must not be bound to the same file. + EXPECT_THROW(UnixSocket(absolutePath(TEST_FILE1), absolutePath(TEST_FILE1)), + UnixSocketInvalidName); + // The socket path must not be empty. + EXPECT_THROW(UnixSocket("", absolutePath(TEST_FILE1)), + UnixSocketInvalidName); + EXPECT_THROW(UnixSocket(absolutePath(TEST_FILE1), ""), + UnixSocketInvalidName); + +} -// Test openSocket function -TEST_F(UnixSocketTest, openSocket) { - int fd; - EXPECT_NO_THROW( - fd = sock1_.open(); - ); - - EXPECT_EQ(fd, sock1_.getSocket()); +// Test open() function which opens a unix socket. +TEST_F(UnixSocketTest, open) { + ASSERT_NO_THROW(sock1_.open()); + EXPECT_GE(sock1_.get(), 0); + // Second attempt open socket should fail. + ASSERT_THROW(sock1_.open(), UnixSocketOpenError); + // Close the opened socket and check then we can re-open. + sock1_.closeFd(); + EXPECT_NO_THROW(sock1_.open()); } // Test bidirectional data sending and receiving. TEST_F(UnixSocketTest, bidirectionalTransmission) { - const int LEN1 = 100; - const int LEN2 = 200; - uint8_t data1[LEN2]; - uint8_t data2[LEN2]; - uint8_t data3[LEN2]; - uint8_t data4[LEN2]; - for (int i = 0; i < LEN2; ++i) { - data1[i] = i; - data2[i] = -i; - } - EXPECT_NO_THROW( - sock1_.open(); - ); - EXPECT_NO_THROW( - sock2_.open(); - ); - - OutputBuffer sendbuf1(LEN1), sendbuf2(LEN2); - sendbuf1.writeData((void*)data1, LEN1); - sendbuf2.writeData((void*)data2, LEN2); - EXPECT_NO_THROW( - sock1_.send(sendbuf1); - ); - EXPECT_NO_THROW( - sock2_.send(sendbuf2); - ); - - InputBuffer recvbuf1(0, 0), recvbuf2(0, 0); - EXPECT_NO_THROW( - recvbuf1 = sock1_.recv(); - ); - EXPECT_NO_THROW( - recvbuf2 = sock2_.recv(); - ); - - size_t len1 = recvbuf1.getLength(); - size_t len2 = recvbuf2.getLength(); - recvbuf1.readData((void*)data3, len1); - recvbuf2.readData((void*)data4, len2); - - //check out length. - ASSERT_EQ(LEN2, len1); - ASSERT_EQ(LEN1, len2); - - for (int i = 0; i < len1; i++) { - EXPECT_EQ(data2[i], data3[i]); - } - for (int i = 0; i < len2; i++) { - EXPECT_EQ(data1[i], data4[i]); - } + const int LEN1 = 100; + const int LEN2 = 200; + + // Prepare data to be sent over the unix sockets from both directions. + uint8_t data1[LEN2]; + uint8_t data2[LEN2]; + for (int i = 0; i < LEN2; ++i) { + data1[i] = i; + // Make sure that the data is different for the other side of the + // communication channel so as the socket doesn't receive its own + // data. + data2[i] = i % 2; + } + // Make sure that the sockets can be opened on both ends. + ASSERT_NO_THROW(sock1_.open()); + ASSERT_NO_THROW(sock2_.open()); + + // Prepare send buffers. Differentiate their lengths so we can verify + // that the lengths of the received data are correct. + OutputBuffer sendbuf1(LEN1); + OutputBuffer sendbuf2(LEN2); + sendbuf1.writeData((void*)data1, LEN1); + sendbuf2.writeData((void*)data2, LEN2); + // Send data over the sockets in both directions. + ASSERT_NO_THROW(sock1_.send(sendbuf1)); + ASSERT_NO_THROW(sock2_.send(sendbuf2)); + + // Receive the data from both directions. + int recvlen1, recvlen2; + ASSERT_NO_THROW(recvlen1 = sock1_.receive()); + ASSERT_NO_THROW(recvlen2 = sock2_.receive()); + + // Sanity check the lengths. + ASSERT_EQ(recvlen1, LEN2); + ASSERT_EQ(recvlen2, LEN1); + + // Make sure we can create InputBuffers from the received data. + InputBuffer recvbuf1(sock1_.getReceiveBuffer(), recvlen1); + InputBuffer recvbuf2(sock2_.getReceiveBuffer(), recvlen2); + + // Verify that the received data is correct. + for (int i = 0; i < recvlen1; ++i) { + EXPECT_EQ(recvbuf1.readUint8(), data2[i]); + } + for (int i = 0; i < recvlen2; ++i) { + EXPECT_EQ(recvbuf2.readUint8(), data1[i]); + } } -// Test exceptions +// Test that send and receive throw for close sockets. TEST_F(UnixSocketTest, exceptions) { - EXPECT_THROW( - sock1_.recv(), - UnixSocketRecvError - ); - EXPECT_THROW( - sock1_.send(OutputBuffer(10)), - UnixSocketSendError - ); - EXPECT_NO_THROW( - sock1_.open(); - sock2_.open(); - ); - EXPECT_NO_THROW( - sock1_.send(OutputBuffer(10)) - ); + EXPECT_THROW(sock1_.receive(), UnixSocketRecvError); + EXPECT_THROW(sock1_.send(OutputBuffer(10)), UnixSocketSendError); + ASSERT_NO_THROW(sock1_.open()); + ASSERT_NO_THROW(sock2_.open()); + EXPECT_NO_THROW(sock1_.send(OutputBuffer(10))); } } - diff --git a/src/lib/util/unix_socket.cc b/src/lib/util/unix_socket.cc index e5c1021187..4372283d98 100644 --- a/src/lib/util/unix_socket.cc +++ b/src/lib/util/unix_socket.cc @@ -13,95 +13,111 @@ // PERFORMANCE OF THIS SOFTWARE. #include +#include namespace isc { namespace util { -UnixSocket::UnixSocket(const std::string& local_filename, const std::string& remote_filename) : +UnixSocket::UnixSocket(const std::string& local_filename, + const std::string& remote_filename) : socketfd_(-1), remote_addr_len_(0), local_filename_(local_filename), - remote_filename_(remote_filename) + remote_filename_(remote_filename), + input_buffer_(RCVBUFSIZE) { + // Sanity check that the file names are not empty, nor equal. + if (local_filename_.empty() || remote_filename_.empty()) { + isc_throw(UnixSocketInvalidName, "unix socket file name must not" + " be empty"); + + } else if (local_filename_ == remote_filename_) { + isc_throw(UnixSocketInvalidName, "local and remote file names for the" + " unix socket are equal: '" << local_filename_ << "'"); + } } UnixSocket::~UnixSocket() { - closeIPC(); + closeFd(); + unlink(local_filename_.c_str()); } -int +void UnixSocket::open() { - //create socket - int fd = socket(AF_UNIX, SOCK_DGRAM, 0); - if (fd < 0) { + // Cant't open socket twice. + if (isOpen()) { + isc_throw(UnixSocketOpenError, "unix socket '" << socketfd_ + << "' is already open"); + } + + // Create socket. + socketfd_ = socket(AF_UNIX, SOCK_DGRAM, 0); + if (socketfd_ < 0) { isc_throw(UnixSocketOpenError, "Failed to create a socket"); } - socketfd_ = fd; - bindSocket(); - setRemoteFilename(); - - return socketfd_; + // Convert the filename to the sockaddr structure. + struct sockaddr_un local_addr; + int local_addr_len = 0; + filenameToSockAddr(local_filename_, local_addr, local_addr_len); + filenameToSockAddr(remote_filename_, remote_addr_, remote_addr_len_); + + // Remove existing socket file. If this is not done, the bind call + // may fail with "Address already in use" error. + unlink(local_filename_.c_str()); + if (bind(socketfd_, reinterpret_cast(&local_addr), + local_addr_len) < 0) { + isc_throw(UnixSocketOpenError, + "failed to bind unix socket to '" << local_filename_ + << "': " << strerror(errno)); + } } void -UnixSocket::closeIPC() { - if(socketfd_ >= 0) +UnixSocket::closeFd() { + // Close it only if open. + if (socketfd_ >= 0) close(socketfd_); socketfd_ = -1; } int UnixSocket::send(const isc::util::OutputBuffer &buf) { - if (remote_addr_len_ == 0) { - isc_throw(UnixSocketSendError, "Remote address unset"); - } - int count = sendto(socketfd_, buf.getData(), buf.getLength(), 0, - (struct sockaddr*)&remote_addr_, remote_addr_len_); - if (count < 0) { - isc_throw(UnixSocketSendError, "UnixSocket failed on sendto: " + int len = sendto(socketfd_, buf.getData(), buf.getLength(), 0, + reinterpret_cast(&remote_addr_), + remote_addr_len_); + if (len < 0) { + isc_throw(UnixSocketSendError, "unix socket failed on sendto: " << strerror(errno)); } - return count; + return (len); } -isc::util::InputBuffer -UnixSocket::recv() { - uint8_t buf[RCVBUFSIZE]; - int len = recvfrom(socketfd_, buf, RCVBUFSIZE, 0, NULL, NULL); +int +UnixSocket::receive() { + int len = recvfrom(socketfd_, &input_buffer_[0], input_buffer_.size(), + 0, NULL, NULL); if (len < 0) { - isc_throw(UnixSocketRecvError, "UnixSocket failed on recvfrom: " + isc_throw(UnixSocketRecvError, "unix socket failed on recvfrom: " << strerror(errno)); - } - isc::util::InputBuffer ibuf(buf, len); - return ibuf; -} - -void -UnixSocket::setRemoteFilename() { - memset(&remote_addr_, 0, sizeof(struct sockaddr_un)); - remote_addr_.sun_family = AF_UNIX; - strcpy(&remote_addr_.sun_path[1], remote_filename_.c_str()); - remote_addr_len_ = sizeof(sa_family_t) + remote_filename_.size() + 1; + } + return (len); } void -UnixSocket::bindSocket() { - struct sockaddr_un local_addr_; - int local_addr_len_; - - //init address - memset(&local_addr_, 0, sizeof(struct sockaddr_un)); - local_addr_.sun_family = AF_UNIX; - strcpy(&local_addr_.sun_path[1], local_filename_.c_str()); - local_addr_len_ = sizeof(sa_family_t) + local_filename_.size() + 1; - - //bind to local_address - if (bind(socketfd_, (struct sockaddr *)&local_addr_, local_addr_len_) < 0) { - isc_throw(UnixSocketOpenError, "failed to bind to local address: " + local_filename_); +UnixSocket::filenameToSockAddr(const std::string& filename, + struct sockaddr_un& sockaddr, + int& sockaddr_len) { + if (filename.empty()) { + isc_throw(UnixSocketInvalidName, + "empty file name specified for unix socket"); } -} + memset(&sockaddr, 0, sizeof(struct sockaddr_un)); + sockaddr.sun_family = AF_UNIX; + strncpy(sockaddr.sun_path, filename.c_str(), filename.size()); + sockaddr_len = sizeof(sa_family_t) + filename.size(); +} } } diff --git a/src/lib/util/unix_socket.h b/src/lib/util/unix_socket.h index 75dbc5ab03..4e4f5310ac 100644 --- a/src/lib/util/unix_socket.h +++ b/src/lib/util/unix_socket.h @@ -25,119 +25,161 @@ namespace isc { namespace util { -/// @brief Exception thrown when BaseIPC::open() failed. +/// @brief Exception thrown when invalid file names have been specified for a +/// unix socket. +class UnixSocketInvalidName : public Exception { +public: + UnixSocketInvalidName(const char* file, size_t line, const char* what) : + isc::Exception(file, line, what) { }; +}; + +/// @brief Exception thrown when @c UnixSocket::open fails. class UnixSocketOpenError : public Exception { public: UnixSocketOpenError(const char* file, size_t line, const char* what) : - isc::Exception(file, line, what) { }; + isc::Exception(file, line, what) { }; }; -/// @brief Exception thrown when UnixSocket::recv() failed. +/// @brief Exception thrown when @c UnixSocket::recv fails. class UnixSocketRecvError : public Exception { public: UnixSocketRecvError(const char* file, size_t line, const char* what) : - isc::Exception(file, line, what) { }; + isc::Exception(file, line, what) { }; }; -/// @brief Exception thrown when BaseIPC::send() failed. +/// @brief Exception thrown when @c UnixSocket::send fails. class UnixSocketSendError : public Exception { public: UnixSocketSendError(const char* file, size_t line, const char* what) : - isc::Exception(file, line, what) { }; + isc::Exception(file, line, what) { }; }; -/// @brief An Inter Process Communication(IPC) tool based on UNIX domain socket. +/// @brief Unix socket. /// -/// It is used by 2 processes for data communication. It provides methods for -/// bi-directional binary data transfer. +/// This class provides low level functions to operate on a unix socket. Other +/// classes may use it or derive from it to provide specialized mechanisms for +/// the Inter Process Communication (IPC). /// -/// There should be 2 instances (a sender and a receiver) using this tool -/// at the same time. The filename for the sockets must match (i.e. -/// the remote filename of the sender = the local filename of the receiver). +/// This class requires that the two file names are specified: first file +/// defines a local address that the socket is bound to, the second one +/// specifies a remote address to which the data is sent. If this class is used +/// for IPC, the remote address associated with this socket is a local address +/// of the socket used by the other process. Conversely, when the local addres +/// of this socket is a remote address for the socket used by the other process. /// -/// It should be used as a base class and not directly used for future classes -/// implementing inter process communication. +/// Constructor of this class doesn't open a socket for communication. The +/// socket is opened with the @c UnixSocket::open method. If the socket is +/// opened a second call to @c UnixSocket::open will trigger an exception. +/// +/// @note This class doesn't use abstract sockets, i.e. sockets not associated +/// with the path on the disk, because abstract sockets are the Linux +/// extension and are not portable. class UnixSocket { public: - /// @brief Packet reception buffer size - /// - /// Receive buffer size of UNIX socket + /// @brief Size of the pre-allocated buffer used to receive the data. static const uint32_t RCVBUFSIZE = 4096; - /// @brief BaseIPC constructor. + /// @brief Constructor /// - /// Creates BaseIPC object for UNIX socket communication using the given - /// filenames. It doesn't create the socket immediately. + /// Sets paths to the local and remote file names. The file names are + /// sanity checked by the constructor: + /// - they must not be empty + /// - they must not be equal. /// - /// @param local_filename Filename for receiving socket - /// @param remote_filename Filename for sending socket - UnixSocket(const std::string& local_filename, const std::string& remote_filename); - /// @brief BaseIPC destructor. + /// @param local_filename file name that the socket is bound to. + /// @param remote_filename file name identifying a remote socket. /// - /// It closes the socket explicitly. + /// @throw UnixSocketInvalidName if any of the file names is empty or if + /// file names are equal. + UnixSocket(const std::string& local_filename, + const std::string& remote_filename); + + /// @brief Destructor. + /// + /// Closes open socket and removes socket file. virtual ~UnixSocket(); - - - /// @brief Open UNIX socket + + /// @brief Opens unix socket /// - /// Method will throw if socket creation fails. + /// This method opens a unix socket and binds it to the local file. + /// + /// @throw UnixSocketOpenError if the socket creation, binding fails. + /// @throw UnixSocketOpenError if socket is already open. + void open(); + + /// @brief Convenience function checking if the socket is open. /// - /// @return A int value of the socket descriptor. - int open(); + /// @return true if the unix socket is already open, false otherwise. + bool isOpen() const { + return (socketfd_ >= 0); + } /// @brief Close opened socket. - void closeIPC(); + void closeFd(); - /// @brief Send data. - /// - /// @param buf The data to be sent. + /// @brief Sends data over the unix socket. /// - /// Method will throw if open() has not been called or sendto() failed. - /// open() MUST be called before calling this function. + /// @param buf Reference to a buffer holding data to be sent. + /// + /// @throw UnixSocketSendError if failed to send the data over the socket, + /// e.g. socket hasn't been opened or other error occurred. /// /// @return The number of bytes sent. int send(const isc::util::OutputBuffer &buf); - /// @brief Receive data. + /// @brief Receive data over the socket. /// - /// Method will throw if socket recvfrom() failed. - /// open() MUST be called before calling this function. + /// This method receives the data over the socket and stores it in the + /// pre-allocated buffer. The pointer to this buffer may be obtained + /// by calling @c UnixSocket::getReceiveBuffer. /// - /// @return The number of bytes received. - isc::util::InputBuffer recv(); - - /// @brief Get socket fd. - /// - /// @return The socket fd of the unix socket. - int getSocket() { return socketfd_; } - -protected: + /// @return Length of the received data. + int receive(); - /// @brief Set remote filename + /// @brief Returns a const pointer to the buffer holding received data. /// - /// The remote filename is used for sending data. The filename is given - /// in the constructor. - void setRemoteFilename(); - - /// @brief Bind the UNIX socket to the given filename + /// This method always returns a valid pointer. The pointer holds the data + /// received during the last call to @c UnixSocket::receive. + const uint8_t* getReceiveBuffer() const { + return (&input_buffer_[0]); + } + + /// @brief Get socket descriptor. /// - /// The filename is given in the constructor. + /// @return The socket descriptor. + int get() const { + return (socketfd_); + } + +private: + + /// @brief Converts the file name to the @c sockaddr_un structure. /// - /// Method will throw if socket binding fails. - void bindSocket(); - - /// UNIX socket value. + /// @param filename Path to the socket file. + /// @param [out] sockaddr Structure being initialized. + /// @param [out] sockaddr_len Length of the initialized structure. + void filenameToSockAddr(const std::string& filename, + struct sockaddr_un& sockaddr, + int& sockaddr_len); + + /// @brief Unix socket descriptor. int socketfd_; - - /// Remote UNIX socket address + + /// @brief Remote unix socket address to which this socket is connected. struct sockaddr_un remote_addr_; - - /// Length of remote_addr_ + + /// @brief Length of @c remote_addr_ structure. int remote_addr_len_; - /// Filename for receiving and sending socket - std::string local_filename_, remote_filename_; + /// @brief File name for local socket. + std::string local_filename_; + + /// @brief File name for the remote socket. + std::string remote_filename_; + + /// @brief Buffer holding received data from the socket. + std::vector input_buffer_; }; // UnixSocket class