BIND 10 trac1593, updated. 0a3535e441c8b183c0d20f1cc8236ad191e6a4ca [1593] Miscellaneous cleanup of comments etc
BIND 10 source code commits
bind10-changes at lists.isc.org
Tue Feb 14 18:15:43 UTC 2012
The branch, trac1593 has been updated
via 0a3535e441c8b183c0d20f1cc8236ad191e6a4ca (commit)
via c5a39e131286add03835505d9cf7d82cd9a23efc (commit)
via 1aa4924e5a9560ceca73c0e777156309e125f5bb (commit)
via 1cde47ac78e581f185afe2b7e8523c34bcec9a38 (commit)
from 6e12d95c6a1fac2d4a182e58072934d2ab4b237e (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 0a3535e441c8b183c0d20f1cc8236ad191e6a4ca
Author: Stephen Morris <stephen at isc.org>
Date: Tue Feb 14 18:03:22 2012 +0000
[1593] Miscellaneous cleanup of comments etc
commit c5a39e131286add03835505d9cf7d82cd9a23efc
Author: Stephen Morris <stephen at isc.org>
Date: Tue Feb 14 16:18:30 2012 +0000
[1593] Restructure code into a more logical order
commit 1aa4924e5a9560ceca73c0e777156309e125f5bb
Author: Stephen Morris <stephen at isc.org>
Date: Mon Feb 13 18:19:59 2012 +0000
[1593] Separate socket creation from run loop
commit 1cde47ac78e581f185afe2b7e8523c34bcec9a38
Author: Stephen Morris <stephen at isc.org>
Date: Mon Feb 13 17:33:02 2012 +0000
[1593] Remove macros
Remove the READ/WRITE and DEFAULT macros by replacing them with
functions that thrown an exception on failure. Other modifications
also made to make the code consistent with this model.
-----------------------------------------------------------------------
Summary of changes:
src/bin/sockcreator/main.cc | 8 +-
src/bin/sockcreator/sockcreator.cc | 264 ++++++++++++++----------
src/bin/sockcreator/sockcreator.h | 178 ++++++++++-------
src/bin/sockcreator/tests/sockcreator_tests.cc | 14 +-
4 files changed, 280 insertions(+), 184 deletions(-)
-----------------------------------------------------------------------
diff --git a/src/bin/sockcreator/main.cc b/src/bin/sockcreator/main.cc
index 37da303..c97e0ff 100644
--- a/src/bin/sockcreator/main.cc
+++ b/src/bin/sockcreator/main.cc
@@ -22,5 +22,11 @@ main() {
* TODO Maybe use some OS-specific caps interface and drop everything
* but ability to bind ports? It would be nice.
*/
- return run(0, 1); // Read commands from stdin, output to stdout
+ int status = 0;
+ try {
+ run(0, 1); // Read commands from stdin, output to stdout
+ } catch (const SocketCreatorError& ec) {
+ status = ec.getExitCode();
+ }
+ return (status);
}
diff --git a/src/bin/sockcreator/sockcreator.cc b/src/bin/sockcreator/sockcreator.cc
index 827969c..55b7786 100644
--- a/src/bin/sockcreator/sockcreator.cc
+++ b/src/bin/sockcreator/sockcreator.cc
@@ -16,18 +16,162 @@
#include <util/io/fd.h>
-#include <unistd.h>
#include <cerrno>
-#include <string.h>
+#include <cstring>
+
+#include <unistd.h>
#include <sys/types.h>
#include <sys/socket.h>
#include <netinet/in.h>
using namespace isc::util::io;
+using namespace isc::socket_creator;
+
+namespace {
+
+// Simple wrappers for read_data/write_data that throw an exception on error.
+void
+read_message(const int fd, void* where, const size_t length) {
+ if (read_data(fd, where, length) < length) {
+ isc_throw(ReadError, "Error reading from socket creator client");
+ }
+}
+
+void
+write_message(const int fd, const void* what, const size_t length) {
+ if (!write_data(fd, what, length)) {
+ isc_throw(WriteError, "Error writing to socket creator client");
+ }
+}
+
+// Exit on a protocol error after informing the client of the problem.
+void
+protocol_error(const int fd, const char reason = 'I') {
+
+ // Tell client we have a problem
+ char message[2];
+ message[0] = 'F';
+ message[1] = reason;
+ write_message(fd, message, sizeof(message));
+
+ // ... and exit
+ isc_throw(ProtocolError, "Fatal error, reason: " << reason);
+}
+
+// Handle the request from the client.
+//
+// Reads the type and family of socket required, creates the socket and returns
+// it to the client.
+//
+// The arguments passed (and the exceptions thrown) are the same as those for
+// run().
+void
+handle_request(const int input_fd, const int output_fd,
+ const get_sock_t get_sock, const send_fd_t send_fd_fun,
+ const close_t close_fun)
+{
+ // Read the message from the client
+ char type[2];
+ read_message(input_fd, type, sizeof(type));
+
+ // Decide what type of socket is being asked for
+ int sock_type = 0;
+ switch (type[0]) {
+ case 'T':
+ sock_type = SOCK_STREAM;
+ break;
+
+ case 'U':
+ sock_type = SOCK_DGRAM;
+ break;
+
+ default:
+ protocol_error(output_fd);
+ }
+
+ // Read the address they ask for depending on what address family was
+ // specified.
+ sockaddr* addr = NULL;
+ size_t addr_len = 0;
+ sockaddr_in addr_in;
+ sockaddr_in6 addr_in6;
+ 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.
+
+ case '4':
+ addr = reinterpret_cast<sockaddr*>(&addr_in);
+ addr_len = sizeof(addr_in);
+ memset(&addr_in, 0, sizeof(addr_in));
+ addr_in.sin_family = AF_INET;
+ read_message(input_fd, static_cast<void*>(&addr_in.sin_port),
+ sizeof(addr_in.sin_port));
+ read_message(input_fd, static_cast<void*>(&addr_in.sin_addr.s_addr),
+ sizeof(addr_in.sin_addr.s_addr));
+ break;
+
+ case '6':
+ addr = reinterpret_cast<sockaddr*>(&addr_in6);
+ addr_len = sizeof addr_in6;
+ memset(&addr_in6, 0, sizeof(addr_in6));
+ addr_in6.sin6_family = AF_INET6;
+ read_message(input_fd, static_cast<void*>(&addr_in6.sin6_port),
+ sizeof(addr_in6.sin6_port));
+ read_message(input_fd, static_cast<void*>(&addr_in6.sin6_addr.s6_addr),
+ sizeof(addr_in6.sin6_addr.s6_addr));
+ break;
+
+ default:
+ protocol_error(output_fd);
+ }
+
+ // Obtain the socket
+ int result = get_sock(sock_type, addr, addr_len);
+ if (result >= 0) {
+ // Got the socket, send it to the client.
+ write_message(output_fd, "S", 1);
+ if (send_fd_fun(output_fd, result) != 0) {
+ // Error. Close the socket (ignore any error from that operation)
+ // and abort.
+ close_fun(result);
+ isc_throw(InternalError, "Error sending descriptor");
+ }
+
+ // Successfully sent the socket, so free up resources we still hold
+ // for it.
+ if (close_fun(result) == -1) {
+ isc_throw(InternalError, "Error closing socket");
+ }
+ } else {
+ // Error. Tell the client.
+ write_message(output_fd, "E", 1); // Error occurred...
+ switch (result) {
+ case -1:
+ write_message(output_fd, "S", 1); // ... in the socket() call
+ break;
+
+ case -2:
+ write_message(output_fd, "B", 1); // ... in the bind() call
+ break;
+
+ default:
+ isc_throw(InternalError, "Error creating socket");
+ }
+
+ // ...and append the reason code to the error message
+ int error = errno;
+ write_message(output_fd, static_cast<void *>(&error), sizeof error);
+ }
+}
+
+} // Anonymous namespace
namespace isc {
namespace socket_creator {
+// Get the socket and bind to it.
int
get_sock(const int type, struct sockaddr *bind_addr, const socklen_t addr_len)
{
@@ -49,117 +193,25 @@ get_sock(const int type, struct sockaddr *bind_addr, const socklen_t addr_len)
return sock;
}
-// These are macros so they can exit the function
-#define READ(WHERE, HOW_MANY) do { \
- size_t how_many = (HOW_MANY); \
- if (read_data(input_fd, (WHERE), how_many) < how_many) { \
- return 1; \
- } \
- } while (0)
-
-#define WRITE(WHAT, HOW_MANY) do { \
- if (!write_data(output_fd, (WHAT), (HOW_MANY))) { \
- return 2; \
- } \
- } while (0)
-
-#define DEFAULT \
- default: /* Unrecognized part of protocol */ \
- WRITE("FI", 2); \
- return 3;
-
-int
+// Main run loop.
+void
run(const int input_fd, const int output_fd, const get_sock_t get_sock,
const send_fd_t send_fd_fun, const close_t close_fun)
{
for (;;) {
- // Read the command
char command;
- READ(&command, 1);
+ read_message(input_fd, &command, sizeof(command));
switch (command) {
- case 'T': // The "terminate" command
- return 0;
- case 'S': { // Create a socket
- // Read what type of socket they want
- char type[2];
- READ(type, 2);
- // Read the address they ask for
- struct sockaddr *addr(NULL);
- size_t addr_len(0);
- struct sockaddr_in addr_in;
- struct sockaddr_in6 addr_in6;
- switch (type[1]) { // The address family
- /*
- * Here are some casts. They are required by C++ and
- * the low-level interface (they are implicit in C).
- */
- case '4':
- addr = static_cast<struct sockaddr *>(
- static_cast<void *>(&addr_in));
- addr_len = sizeof addr_in;
- memset(&addr_in, 0, sizeof addr_in);
- addr_in.sin_family = AF_INET;
- READ(static_cast<char *>(static_cast<void *>(
- &addr_in.sin_port)), 2);
- READ(static_cast<char *>(static_cast<void *>(
- &addr_in.sin_addr.s_addr)), 4);
- break;
- case '6':
- addr = static_cast<struct sockaddr *>(
- static_cast<void *>(&addr_in6));
- addr_len = sizeof addr_in6;
- memset(&addr_in6, 0, sizeof addr_in6);
- addr_in6.sin6_family = AF_INET6;
- READ(static_cast<char *>(static_cast<void *>(
- &addr_in6.sin6_port)), 2);
- READ(static_cast<char *>(static_cast<void *>(
- &addr_in6.sin6_addr.s6_addr)), 16);
- break;
- DEFAULT
- }
- int sock_type;
- switch (type[0]) { // Translate the type
- case 'T':
- sock_type = SOCK_STREAM;
- break;
- case 'U':
- sock_type = SOCK_DGRAM;
- break;
- DEFAULT
- }
- int result(get_sock(sock_type, addr, addr_len));
- if (result >= 0) { // We got the socket
- WRITE("S", 1);
- if (send_fd_fun(output_fd, result) != 0) {
- // We'll soon abort ourselves, but make sure we still
- // close the socket; don't bother if it fails as the
- // higher level result (abort) is the same.
- close_fun(result);
- return 3;
- }
- // Don't leak the socket
- if (close_fun(result) == -1) {
- return 4;
- }
- } else {
- WRITE("E", 1);
- switch (result) {
- case -1:
- WRITE("S", 1);
- break;
- case -2:
- WRITE("B", 1);
- break;
- default:
- return 4;
- }
- int error(errno);
- WRITE(static_cast<char *>(static_cast<void *>(&error)),
- sizeof error);
- }
+ case 'S': // The "get socket" command
+ handle_request(input_fd, output_fd, get_sock,
+ send_fd_fun, close_fun);
break;
- }
- DEFAULT
+
+ case 'T': // The "terminate" command
+ return;
+
+ default: // Don't recognise anything else
+ protocol_error(output_fd);
}
}
}
diff --git a/src/bin/sockcreator/sockcreator.h b/src/bin/sockcreator/sockcreator.h
index 216e486..3aad2d2 100644
--- a/src/bin/sockcreator/sockcreator.h
+++ b/src/bin/sockcreator/sockcreator.h
@@ -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,18 +12,17 @@
// OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR
// PERFORMANCE OF THIS SOFTWARE.
-/**
- * \file sockcreator.h
- * \short Socket creator functionality.
- *
- * This module holds the functionality of the socket creator. It is
- * a separate module from main to ease up the tests.
- */
+/// \file sockcreator.h
+/// \short Socket creator functionality.
+///
+/// This module holds the functionality of the socket creator. It is a separate
+/// module from main to make testing easier.
#ifndef __SOCKCREATOR_H
#define __SOCKCREATOR_H 1
#include <util/io/fd_share.h>
+#include <exceptions/exceptions.h>
#include <sys/types.h>
#include <sys/socket.h>
@@ -32,78 +31,115 @@
namespace isc {
namespace socket_creator {
-/**
- * \short Create a socket and bind it.
- *
- * This is just a bundle of socket() and bind() calls. The sa_family of
- * bind_addr is used to determine the domain of the socket.
- *
- * \return The file descriptor of the newly created socket, if everything
- * goes well. A negative number is returned if an error occurs -
- * -1 if the socket() call fails or -2 if bind() fails. In case of error,
- * errno is set (or better, left intact from socket() or bind()).
- * \param type The type of socket to create (SOCK_STREAM, SOCK_DGRAM, etc).
- * \param bind_addr The address to bind.
- * \param addr_len The actual length of bind_addr.
- */
+// Exception classes - the base class exception SocketCreatorError is caught
+// by main() and holds an exit code returned to the environment. The code
+// depends on the exact exception raised.
+class SocketCreatorError : public isc::Exception {
+public:
+ SocketCreatorError(const char* file, size_t line, const char* what,
+ int exit_code) :
+ isc::Exception(file, line, what), exit_code_(exit_code) {}
+
+ int getExitCode() const {
+ return (exit_code_);
+ }
+
+private:
+ const int exit_code_; // Code returned to exit()
+};
+
+class ReadError : public SocketCreatorError {
+public:
+ ReadError(const char* file, size_t line, const char* what) :
+ SocketCreatorError(file, line, what, 1) {}
+};
+
+class WriteError : public SocketCreatorError {
+public:
+ WriteError(const char* file, size_t line, const char* what) :
+ SocketCreatorError(file, line, what, 2) {}
+};
+
+class ProtocolError : public SocketCreatorError {
+public:
+ ProtocolError(const char* file, size_t line, const char* what) :
+ SocketCreatorError(file, line, what, 3) {}
+};
+
+class InternalError : public SocketCreatorError {
+public:
+ InternalError(const char* file, size_t line, const char* what) :
+ SocketCreatorError(file, line, what, 4) {}
+};
+
+
+
+/// \short Create a socket and bind it.
+///
+/// This is just a bundle of socket() and bind() calls. The sa_family of
+/// bind_addr is used to determine the domain of the socket.
+///
+/// \param type The type of socket to create (SOCK_STREAM, SOCK_DGRAM, etc).
+/// \param bind_addr The address to bind.
+/// \param addr_len The actual length of bind_addr.
+///
+/// \return The file descriptor of the newly created socket, if everything
+/// goes well. A negative number is returned if an error occurs -
+/// -1 if the socket() call fails or -2 if bind() fails. In case of
+/// error, errno is set (or left intact from socket() or bind()).
int
get_sock(const int type, struct sockaddr *bind_addr, const socklen_t addr_len);
-/**
- * Type of the get_sock function, to pass it as parameter.
- */
-typedef
-int
-(*get_sock_t)(const int, struct sockaddr *, const socklen_t);
+// Define some types for functions used to perform socket-related operations.
+// These are typedefed so that alternatives can be passed through to the
+// main functions for testing purposes.
-/**
- * Type of the send_fd() function, so it can be passed as a parameter.
- */
-typedef
-int
-(*send_fd_t)(const int, const int);
+// Type of the get_sock function, to pass it as parameter. Arguments are
+// those described above for get_sock().
+typedef int (*get_sock_t)(const int, struct sockaddr *, const socklen_t);
-/// \brief Type of the close() function, so it can be passed as a parameter.
-typedef
-int
-(*close_t)(int);
-
-/**
- * \short Infinite loop parsing commands and returning the sockets.
- *
- * This reads commands and socket descriptions from the input_fd
- * file descriptor, creates sockets and writes the results (socket or
- * error) to output_fd.
- *
- * Current errors are:
- * - 1: Read error
- * - 2: Write error
- * - 3: Protocol error (unknown command, etc)
- * - 4: Some internal inconsistency detected
- *
- * It terminates either if a command asks it to or when unrecoverable
- * error happens.
- *
- * \return Like a return value of a main - 0 means everything OK, anything
- * else is error.
- * \param input_fd Here is where it reads the commads.
- * \param output_fd Here is where it writes the results.
- * \param get_sock_fun The function that is used to create the sockets.
- * This should be left on the default value, the parameter is here
- * for testing purposes.
- * \param send_fd_fun The function that is used to send the socket over
- * a file descriptor. This should be left on the default value, it is
- * here for testing purposes.
- * \param close_fun The close function used to close sockets, coming from
- * unistd.h. It can be overriden in tests.
- */
-int
+// Type of the send_fd() function, so it can be passed as a parameter.
+// Arguments are the same as those of the send_fd() function.
+typedef int (*send_fd_t)(const int, const int);
+
+// Type of the close() function, so it can be passed as a parameter.
+// Argument is the same as that for close(2).
+typedef int (*close_t)(int);
+
+
+/// \brief Infinite loop parsing commands and returning the sockets.
+///
+/// This reads commands and socket descriptions from the input_fd file
+/// descriptor, creates sockets and writes the results (socket or error) to
+/// output_fd.
+///
+/// It terminates either if a command asks it to or when unrecoverable error
+/// happens.
+///
+/// \param input_fd File descriptor of the stream from which the input commands
+/// are read.
+/// \param output_fd File descriptor of the stream to which the output
+/// (message/socket or error message) is written.
+/// \param get_sock_fun The function that is used to create the sockets.
+/// This should be left on the default value, the parameter is here
+/// for testing purposes.
+/// \param send_fd_fun The function that is used to send the socket over
+/// a file descriptor. This should be left on the default value, it is
+/// here for testing purposes.
+/// \param close_fun The close function used to close sockets, coming from
+/// unistd.h. It can be overriden in tests.
+///
+/// \exception isc::socket_creator::ReadError Error reading from input
+/// \exception isc::socket_creator::WriteError Error writing to output
+/// \exception isc::socket_creator::ProtocolError Unrecognised command received
+/// \exception isc::socket_creator::InternalError Other error
+void
run(const int input_fd, const int output_fd,
const get_sock_t get_sock_fun = get_sock,
const send_fd_t send_fd_fun = isc::util::io::send_fd,
const close_t close_fun = close);
-} // End of the namespaces
-}
+} // namespace socket_creator
+} // NAMESPACE ISC
#endif // __SOCKCREATOR_H
diff --git a/src/bin/sockcreator/tests/sockcreator_tests.cc b/src/bin/sockcreator/tests/sockcreator_tests.cc
index 360c750..22151d3 100644
--- a/src/bin/sockcreator/tests/sockcreator_tests.cc
+++ b/src/bin/sockcreator/tests/sockcreator_tests.cc
@@ -223,16 +223,18 @@ void run_test(const char *input_data, const size_t input_size,
ASSERT_NE(-1, input) << "Couldn't start input feeder";
ASSERT_NE(-1, output) << "Couldn't start output checker";
// Run the body
- int result(run(input_fd, output_fd, get_sock_dummy, send_fd, test_close));
+ if (should_succeed) {
+ EXPECT_NO_THROW(run(input_fd, output_fd, get_sock_dummy, send_fd,
+ test_close));
+ } else {
+ EXPECT_THROW(run(input_fd, output_fd, get_sock_dummy, send_fd,
+ test_close), isc::socket_creator::SocketCreatorError);
+ }
+
// Close the pipes
close(input_fd);
close(output_fd);
// Did it run well?
- if (should_succeed) {
- EXPECT_EQ(0, result);
- } else {
- EXPECT_NE(0, result);
- }
// Check the subprocesses say everything is OK too
EXPECT_TRUE(process_ok(input));
EXPECT_TRUE(process_ok(output));
More information about the bind10-changes
mailing list