BIND 10 trac1593, updated. e40cec7b4d5a0fc473bb5d4abfb6a93a712e0cb4 [1593] Minor tidy up
BIND 10 source code commits
bind10-changes at lists.isc.org
Wed Feb 15 16:27:01 UTC 2012
The branch, trac1593 has been updated
via e40cec7b4d5a0fc473bb5d4abfb6a93a712e0cb4 (commit)
via 61dd18377938c6f2e5d026d432062db6773abf65 (commit)
via aa39c499c1f3b58685987252e7e5dc1e41a4b915 (commit)
from 0a3535e441c8b183c0d20f1cc8236ad191e6a4ca (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 e40cec7b4d5a0fc473bb5d4abfb6a93a712e0cb4
Author: Stephen Morris <stephen at isc.org>
Date: Wed Feb 15 16:20:21 2012 +0000
[1593] Minor tidy up
commit 61dd18377938c6f2e5d026d432062db6773abf65
Author: Stephen Morris <stephen at isc.org>
Date: Wed Feb 15 14:54:43 2012 +0000
[1593] Update comments in tests (+ minor style changes)
commit aa39c499c1f3b58685987252e7e5dc1e41a4b915
Author: Stephen Morris <stephen at isc.org>
Date: Wed Feb 15 12:30:26 2012 +0000
[1593] Replace macros in test code with temmplates/overloaded functions
-----------------------------------------------------------------------
Summary of changes:
src/bin/sockcreator/sockcreator.cc | 9 +-
src/bin/sockcreator/tests/sockcreator_tests.cc | 417 ++++++++++++++----------
2 files changed, 246 insertions(+), 180 deletions(-)
-----------------------------------------------------------------------
diff --git a/src/bin/sockcreator/sockcreator.cc b/src/bin/sockcreator/sockcreator.cc
index 55b7786..639ae75 100644
--- a/src/bin/sockcreator/sockcreator.cc
+++ b/src/bin/sockcreator/sockcreator.cc
@@ -98,8 +98,7 @@ handle_request(const int input_fd, const int output_fd,
switch (type[1]) { // The address family
// The casting to apparently incompatible types by reinterpret_cast
- // is required by the C low-level interface. Unions are not used
- // because of the possibility of alignment issues.
+ // is required by the C low-level interface.
case '4':
addr = reinterpret_cast<sockaddr*>(&addr_in);
@@ -162,7 +161,7 @@ handle_request(const int input_fd, const int output_fd,
// ...and append the reason code to the error message
int error = errno;
- write_message(output_fd, static_cast<void *>(&error), sizeof error);
+ write_message(output_fd, static_cast<void*>(&error), sizeof error);
}
}
@@ -175,11 +174,11 @@ namespace socket_creator {
int
get_sock(const int type, struct sockaddr *bind_addr, const socklen_t addr_len)
{
- int sock(socket(bind_addr->sa_family, type, 0));
+ int sock = socket(bind_addr->sa_family, type, 0);
if (sock == -1) {
return -1;
}
- const int on(1);
+ const int on = 1;
if (setsockopt(sock, SOL_SOCKET, SO_REUSEADDR, &on, sizeof(on)) == -1) {
return -2; // This is part of the binding process, so it's a bind error
}
diff --git a/src/bin/sockcreator/tests/sockcreator_tests.cc b/src/bin/sockcreator/tests/sockcreator_tests.cc
index 22151d3..5bafbb8 100644
--- a/src/bin/sockcreator/tests/sockcreator_tests.cc
+++ b/src/bin/sockcreator/tests/sockcreator_tests.cc
@@ -1,4 +1,4 @@
-// Copyright (C) 2011 Internet Systems Consortium, Inc. ("ISC")
+// Copyright (C) 2011-2012 Internet Systems Consortium, Inc. ("ISC")
//
// Permission to use, copy, modify, and/or distribute this software for any
// purpose with or without fee is hereby granted, provided that the above
@@ -12,11 +12,14 @@
// OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR
// PERFORMANCE OF THIS SOFTWARE.
+#include <iostream>
+
#include "../sockcreator.h"
#include <util/unittests/fork.h>
#include <util/io/fd.h>
+#include <boost/lexical_cast.hpp>
#include <gtest/gtest.h>
#include <sys/types.h>
#include <sys/socket.h>
@@ -29,174 +32,227 @@ using namespace isc::socket_creator;
using namespace isc::util::unittests;
using namespace isc::util::io;
+// The tests check both TCP and UDP sockets on IPv4 and IPv6.
+//
+// Essentially we need to check all four combinations of TCP/UDP and IPv4/IPv6.
+// The different address families (IPv4/IPv6) require different structures to
+// hold the address information, and so some common code is in the form of
+// templates (or overloads), parameterised on the structure type.
+//
+// The protocol is determined by an integer (SOCK_STREAM or SOCK_DGRAM) so
+// cannot be templated in the same way. Relevant check functions are
+// selected manually.
+
namespace {
-/*
- * Generic version of the creation of socket test. It just tries to
- * create the socket and checks the result is not negative (eg.
- * it is valid descriptor) and that it can listen.
- *
- * This is a macro so ASSERT_* does abort the TEST, not just the
- * function inside.
- */
-#define TEST_ANY_CREATE(SOCK_TYPE, ADDR_TYPE, ADDR_FAMILY, FAMILY_FIELD, \
- ADDR_SET, CHECK_SOCK) \
- do { \
- /*
- * This should create an address that binds on all interfaces
- * and lets the OS choose a free port.
- */ \
- struct ADDR_TYPE addr; \
- memset(&addr, 0, sizeof addr); \
- ADDR_SET(addr); \
- addr.FAMILY_FIELD = ADDR_FAMILY; \
- struct sockaddr *addr_ptr = static_cast<struct sockaddr *>( \
- static_cast<void *>(&addr)); \
- \
- int socket = get_sock(SOCK_TYPE, addr_ptr, sizeof addr); \
- /* Provide even nice error message. */ \
- ASSERT_GE(socket, 0) << "Couldn't create a socket of type " \
- #SOCK_TYPE " and family " #ADDR_FAMILY ", failed with " \
- << socket << " and error " << strerror(errno); \
- CHECK_SOCK(ADDR_TYPE, socket); \
- int on; \
- socklen_t len(sizeof(on)); \
- EXPECT_EQ(0, getsockopt(socket, SOL_SOCKET, SO_REUSEADDR, &on, &len));\
- EXPECT_NE(0, on); \
- if (ADDR_FAMILY == AF_INET6) { \
- EXPECT_EQ(0, getsockopt(socket, IPPROTO_IPV6, IPV6_V6ONLY, &on, \
- &len)); \
- EXPECT_NE(0, on); \
- } \
- EXPECT_EQ(0, close(socket)); \
- } while (0)
-
-// Just helper macros
-#define INADDR_SET(WHAT) do { WHAT.sin_addr.s_addr = INADDR_ANY; } while (0)
-#define IN6ADDR_SET(WHAT) do { WHAT.sin6_addr = in6addr_loopback; } while (0)
-// If the get_sock returned something useful, listen must work
-#define TCP_CHECK(UNUSED, SOCKET) do { \
- EXPECT_EQ(0, listen(SOCKET, 1)); \
- } while (0)
-// More complicated with UDP, so we send a packet to ourselfs and se if it
-// arrives
-#define UDP_CHECK(ADDR_TYPE, SOCKET) do { \
- struct ADDR_TYPE addr; \
- memset(&addr, 0, sizeof addr); \
- struct sockaddr *addr_ptr = static_cast<struct sockaddr *>( \
- static_cast<void *>(&addr)); \
- \
- socklen_t len = sizeof addr; \
- ASSERT_EQ(0, getsockname(SOCKET, addr_ptr, &len)); \
- ASSERT_EQ(5, sendto(SOCKET, "test", 5, 0, addr_ptr, sizeof addr)) << \
- "Send failed with error " << strerror(errno) << " on socket " << \
- SOCKET; \
- char buffer[5]; \
- ASSERT_EQ(5, recv(SOCKET, buffer, 5, 0)) << \
- "Recv failed with error " << strerror(errno) << " on socket " << \
- SOCKET; \
- EXPECT_STREQ("test", buffer); \
- } while (0)
-
-/*
- * Several tests to ensure we can create the sockets.
- */
+// Set IP-version-specific fields.
+
+void
+setAddressFamilyFields(sockaddr_in* address) {
+ address->sin_family = AF_INET;
+ address->sin_addr.s_addr = INADDR_ANY;
+}
+
+void
+setAddressFamilyFields(sockaddr_in6* address) {
+ address->sin6_family = AF_INET6;
+ address->sin6_addr = in6addr_loopback;
+}
+
+// Socket has been opened, peform a check on it. The sole argument is the
+// socket descriptor. The TCP check is the same regardless of the address
+// family. The UDP check requires that the socket address be obtained so
+// is parameterised on the type of structure required to hold the address.
+
+void
+tcpCheck(const int socknum) {
+ // Sufficient to be able to listen on the socket.
+ EXPECT_EQ(0, listen(socknum, 1));
+}
+
+template <typename ADDRTYPE>
+void
+udpCheck(const int socknum) {
+ // UDP testing is more complicated than TCP: send a packet to ourselves and
+ // see if it arrives.
+
+ // Get details of the socket so that we can use it as the target of the
+ // sendto().
+ ADDRTYPE addr;
+ memset(&addr, 0, sizeof(addr));
+ sockaddr* addr_ptr = reinterpret_cast<sockaddr*>(&addr);
+ socklen_t len = sizeof(addr);
+ ASSERT_EQ(0, getsockname(socknum, addr_ptr, &len));
+
+ // Send the packet to ourselves and check we receive it.
+ ASSERT_EQ(5, sendto(socknum, "test", 5, 0, addr_ptr, sizeof(addr))) <<
+ "Send failed with error " << strerror(errno) << " on socket " <<
+ socknum;
+ char buffer[5];
+ ASSERT_EQ(5, recv(socknum, buffer, 5, 0)) <<
+ "Recv failed with error " << strerror(errno) << " on socket " <<
+ socknum;
+ EXPECT_STREQ("test", buffer);
+}
+
+// The check function (tcpCheck/udpCheck) is passed as a parameter to the test
+// code, so provide a conveniet typedef.
+typedef void (*socket_check_t)(const int);
+
+// Address-family-specific scoket checks.
+//
+// The first argument is used to select the overloaded check function.
+// The other argument is the socket descriptor number.
+
+// IPv4 check
+void addressFamilySpecificCheck(const sockaddr_in*, const int) {
+};
+
+// IPv6 check
+void addressFamilySpecificCheck(const sockaddr_in6*, const int socknum) {
+ int options;
+ socklen_t len = sizeof(options);
+ EXPECT_EQ(0, getsockopt(socknum, IPPROTO_IPV6, IPV6_V6ONLY, &options, &len));
+ EXPECT_NE(0, options);
+};
+
+
+// Generic version of the socket test. It creates the socket and checks that
+// it is a valid descriptor. The family-specific check functions are called
+// to check that the socket is valid. The function is parameterised according
+// to the structure used to hold the address.
+//
+// Arguments:
+// socket_type Type of socket to create (SOCK_DGRAM or SOCK_STREAM)
+// socket_check Associated check function - udpCheck() or tcpCheck()
+template <typename ADDRTYPE>
+void testAnyCreate(int socket_type, socket_check_t socket_check) {
+
+ // Create the socket.
+ ADDRTYPE addr;
+ memset(&addr, 0, sizeof(addr));
+ setAddressFamilyFields(&addr);
+ sockaddr* addr_ptr = reinterpret_cast<sockaddr*>(&addr);
+ int socket = get_sock(socket_type, addr_ptr, sizeof(addr));
+ ASSERT_GE(socket, 0) << "Couldn't create socket: failed with " <<
+ "return code " << socket << " and error " << strerror(errno);
+
+ // Perform socket-type-specific testing.
+ socket_check(socket);
+
+ // Do address-family-independent
+ int options;
+ socklen_t len = sizeof(options);
+ EXPECT_EQ(0, getsockopt(socket, SOL_SOCKET, SO_REUSEADDR, &options, &len));
+ EXPECT_NE(0, options);
+
+ // ...and the address-family specific tests.
+ addressFamilySpecificCheck(&addr, socket);
+
+ // Tidy up and exit.
+ EXPECT_EQ(0, close(socket));
+}
+
+
+// Several tests to ensure we can create the sockets.
TEST(get_sock, udp4_create) {
- TEST_ANY_CREATE(SOCK_DGRAM, sockaddr_in, AF_INET, sin_family, INADDR_SET,
- UDP_CHECK);
+ testAnyCreate<sockaddr_in>(SOCK_DGRAM, udpCheck<sockaddr_in>);
}
TEST(get_sock, tcp4_create) {
- TEST_ANY_CREATE(SOCK_STREAM, sockaddr_in, AF_INET, sin_family, INADDR_SET,
- TCP_CHECK);
+ testAnyCreate<sockaddr_in>(SOCK_STREAM, tcpCheck);
}
TEST(get_sock, udp6_create) {
- TEST_ANY_CREATE(SOCK_DGRAM, sockaddr_in6, AF_INET6, sin6_family,
- IN6ADDR_SET, UDP_CHECK);
+ testAnyCreate<sockaddr_in6>(SOCK_DGRAM, udpCheck<sockaddr_in6>);
}
TEST(get_sock, tcp6_create) {
- TEST_ANY_CREATE(SOCK_STREAM, sockaddr_in6, AF_INET6, sin6_family,
- IN6ADDR_SET, TCP_CHECK);
+ testAnyCreate<sockaddr_in6>(SOCK_STREAM, tcpCheck);
}
-/*
- * Try to ask the get_sock function some nonsense and test if it
- * is able to report error.
- */
+// Ask the get_sock function for some nonsense and test if it is able to report
+// an error.
TEST(get_sock, fail_with_nonsense) {
- struct sockaddr addr;
- memset(&addr, 0, sizeof addr);
+ sockaddr addr;
+ memset(&addr, 0, sizeof(addr));
ASSERT_LT(get_sock(0, &addr, sizeof addr), 0);
}
-/*
- * Helper functions to pass to run during testing.
- */
+// The main run() function in the socket creator takes three functions to
+// get the socket, send information to it, and close it. These allow for
+// alternatives to the system functions to be used for testing.
+
+// Replacement get_sock() function.
+// The return value indicates the result of checks and is encoded. Using LSB
+// bit numbering (least-significant bit is bit 0) then:
+//
+// bit 0: 1 if "type" is known, 0 otherwise
+// bit 1: 1 for UDP, 0 for TCP
+// bit 2: 1 if address family is known, 0 otherwise
+// bit 3: 1 for IPv6, 0 for IPv4
+// bit 4: 1 if port passed was valid
+//
+// Other possible return values are:
+//
+// -1: The simulated bind() call has failed
+// -2: The simulated socket() call has failed
int
get_sock_dummy(const int type, struct sockaddr *addr, const socklen_t)
{
- int result(0);
- int port(0);
- /*
- * We encode the type and address family into the int and return it.
- * Lets ignore the port and address for now
- * First bit is 1 if it is known type. Second tells if TCP or UDP.
- * The familly is similar - third bit is known address family,
- * the fourth is the family.
- */
+ int result = 0;
+ int port = 0;
+
+ // Validate type field
switch (type) {
case SOCK_STREAM:
- result += 1;
+ result |= 0x01;
break;
+
case SOCK_DGRAM:
- result += 3;
+ result |= 0x03;
break;
}
+
+ // Validate address family
switch (addr->sa_family) {
case AF_INET:
- result += 4;
- port = static_cast<struct sockaddr_in *>(
- static_cast<void *>(addr))->sin_port;
+ result |= 0x04;
+ port = reinterpret_cast<sockaddr_in*>(addr)->sin_port;
break;
+
case AF_INET6:
- result += 12;
- port = static_cast<struct sockaddr_in6 *>(
- static_cast<void *>(addr))->sin6_port;
+ result |= 0x0C;
+ port = reinterpret_cast<sockaddr_in6*>(addr)->sin6_port;
break;
}
- /*
- * The port should be 0xffff. If it's not, we change the result.
- * The port of 0xbbbb means bind should fail and 0xcccc means
- * socket should fail.
- */
+
+ // The port should be 0xffff. If it's not, we change the result.
+ // The port of 0xbbbb means bind should fail and 0xcccc means
+ // socket should fail.
if (port != 0xffff) {
errno = 0;
if (port == 0xbbbb) {
- return -2;
+ return (-2);
} else if (port == 0xcccc) {
- return -1;
+ return (-1);
} else {
- result += 16;
+ result |= 0x10;
}
}
- return result;
+ return (result);
}
+// Dummy send function - return data (the result of get_sock()) to the destination.
int
send_fd_dummy(const int destination, const int what)
{
- /*
- * Make sure it is 1 byte so we know the length. We do not use more during
- * the test anyway.
- */
- char fd_data(what);
- if (!write_data(destination, &fd_data, 1)) {
- return -1;
- } else {
- return 0;
- }
+ // Make sure it is 1 byte so we know the length. We do not use more during
+ // the test anyway. And even with the LS bute, we can distinguish between
+ // the different results.
+ const char fd_data = what & 0xff;
+ const bool status = write_data(destination, &fd_data, sizeof(fd_data));
+ return (status ? 0 : -1);
}
// Just ignore the fd and pretend success. We close invalid fds in the tests.
@@ -205,23 +261,27 @@ closeIgnore(int) {
return (0);
}
-/*
- * Generic test that it works, with various inputs and outputs.
- * It uses different functions to create the socket and send it and pass
- * data to it and check it returns correct data back, to see if the run()
- * parses the commands correctly.
- */
-void run_test(const char *input_data, const size_t input_size,
- const char *output_data, const size_t output_size,
- bool should_succeed = true, const close_t test_close = closeIgnore,
- const send_fd_t send_fd = send_fd_dummy)
+// Generic test that it works, with various inputs and outputs.
+// It uses different functions to create the socket and send it and pass
+// data to it and check it returns correct data back, to see if the run()
+// parses the commands correctly.
+void run_test(const char* input_data, const size_t input_size,
+ const char* output_data, const size_t output_size,
+ bool should_succeed = true,
+ const close_t test_close = closeIgnore,
+ const send_fd_t send_fd = send_fd_dummy)
{
- // Prepare the input feeder and output checker processes
- int input_fd(0), output_fd(0);
- pid_t input(provide_input(&input_fd, input_data, input_size)),
- output(check_output(&output_fd, output_data, output_size));
+ // Prepare the input feeder and output checker processes. The feeder
+ // process sends data from the client to run() and the checker process
+ // reads the response and checks it against the expected response.
+ int input_fd = 0;
+ pid_t input = provide_input(&input_fd, input_data, input_size);
ASSERT_NE(-1, input) << "Couldn't start input feeder";
+
+ int output_fd = 0;
+ pid_t output = check_output(&output_fd, output_data, output_size);
ASSERT_NE(-1, output) << "Couldn't start output checker";
+
// Run the body
if (should_succeed) {
EXPECT_NO_THROW(run(input_fd, output_fd, get_sock_dummy, send_fd,
@@ -234,79 +294,86 @@ void run_test(const char *input_data, const size_t input_size,
// Close the pipes
close(input_fd);
close(output_fd);
- // Did it run well?
+
// Check the subprocesses say everything is OK too
EXPECT_TRUE(process_ok(input));
EXPECT_TRUE(process_ok(output));
}
-/*
- * Check it terminates successfully when asked to.
- */
+
+// Check it terminates successfully when asked to.
TEST(run, terminate) {
run_test("T", 1, NULL, 0);
}
-/*
- * Check it rejects incorrect input.
- */
+// Check it rejects incorrect input.
TEST(run, bad_input) {
run_test("XXX", 3, "FI", 2, false);
}
-/*
- * Check it correctly parses queries to create sockets.
- */
+// Check it correctly parses query stream to create sockets.
TEST(run, sockets) {
run_test(
- "SU4\xff\xff\0\0\0\0" // This has 9 bytes
- "ST4\xff\xff\0\0\0\0" // This has 9 bytes
- "ST6\xff\xff\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0" // This has 21 bytes
- "SU6\xff\xff\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0" // This has 21 bytes
- "T", 61,
- "S\x07S\x05S\x0dS\x0f", 8);
+ // Commands:
+ "SU4\xff\xff\0\0\0\0" // IPv4 UDP socket, port 0xffffff, address 0.0.0.0
+ "ST4\xff\xff\0\0\0\0" // IPv4 TCP socket, port 0xffffff, address 0.0.0.0
+ "ST6\xff\xff\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0"
+ // IPv6 UDP socket, port 0xffffff, address ::
+ "SU6\xff\xff\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0"
+ // IPv6 TCP socket, port 0xffffff, address ::
+ "T", // ... and terminate
+ 9 + 9 + 21 + 21 + 1, // Length of command string
+ "S\x07S\x05S\x0dS\x0f", // Response ("S" + LS byte of get_sock() return)
+ 8); // Length of response
}
-/*
- * Check if failures of get_socket are handled correctly.
- */
+
+// Check if failures of get_socket are handled correctly.
TEST(run, bad_sockets) {
- // We need to construct the answer, but it depends on int length.
- size_t int_len(sizeof(int));
- size_t result_len(4 + 2 * int_len);
- char result[4 + sizeof(int) * 2];
- // Both errno parts should be 0
- memset(result, 0, result_len);
- // Fill the 2 control parts
+ // We need to construct the answer, but it depends on int length. We expect
+ // two failure answers in this test, each answer comprising two characters
+ // followed by the (int) errno value.
+ char result[2 * (2 + sizeof(int))];
+
+ // We expect the errno parts to be zero but the characters to depend on the
+ // exact failure.
+ memset(result, 0, sizeof(result));
strcpy(result, "EB");
- strcpy(result + 2 + int_len, "ES");
+ strcpy(result + 2 + sizeof(int), "ES");
+
// Run the test
run_test(
- "SU4\xbb\xbb\0\0\0\0"
- "SU4\xcc\xcc\0\0\0\0"
- "T", 19,
- result, result_len);
+ "SU4\xbb\xbb\0\0\0\0" // Port number will trigger simulated bind() fail
+ "SU4\xcc\xcc\0\0\0\0" // Port number will trigger simulated socket() fail
+ "T", // Terminate
+ 19, // Length of command string
+ result, sizeof(result));
}
-// A close that fails
+// A close that fails. (This causes an abort.)
int
closeFail(int) {
return (-1);
}
TEST(run, cant_close) {
- run_test("SU4\xff\xff\0\0\0\0", // This has 9 bytes
- 9, "S\x07", 2, false, closeFail);
+ run_test("SU4\xff\xff\0\0\0\0", 9,
+ "S\x07", 2,
+ false, closeFail);
}
+// A send of the file descriptor that fails. In this case we expect the client
+// to receive the "S" indicating that the descriptor is being sent and nothing
+// else. This causes an abort.
int
sendFDFail(const int, const int) {
return (FD_SYSTEM_ERROR);
}
TEST(run, cant_send_fd) {
- run_test("SU4\xff\xff\0\0\0\0", // This has 9 bytes
- 9, "S", 1, false, closeIgnore, sendFDFail);
+ run_test("SU4\xff\xff\0\0\0\0", 9,
+ "S", 1,
+ false, closeIgnore, sendFDFail);
}
-}
+} // Anonymous namespace
More information about the bind10-changes
mailing list