[1593] Miscellaneous cleanup of comments etc

src/bin/sockcreator/sockcreator.cc

@@ -31,14 +31,14 @@ namespace {
 
 // Simple wrappers for read_data/write_data that throw an exception on error.
 void
-read_message(int fd, void* where, const size_t length) {
+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(int fd, const void* what, const size_t length) {
+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");
     }
@@ -46,7 +46,7 @@ write_message(int fd, const void* what, const size_t length) {
 
 // Exit on a protocol error after informing the client of the problem.
 void
-protocol_error(int fd, const char reason = 'I') {
+protocol_error(const int fd, const char reason = 'I') {
 
     // Tell client we have a problem
     char message[2];
@@ -60,10 +60,11 @@ protocol_error(int fd, const char reason = 'I') {
 
 // Handle the request from the client.
 //
-// Reads the type and family of socket required, creates the socket, then
-// returns it to the client.
+// Reads the type and family of socket required, creates the socket and returns
+// it to the client.
 //
-// The arguments are the same as those passed to run().
+// 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,
@@ -103,22 +104,22 @@ handle_request(const int input_fd, const int output_fd,
         case '4':
             addr = reinterpret_cast<sockaddr*>(&addr_in);
             addr_len = sizeof(addr_in);
-            memset(&addr_in, 0, 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),
+            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),
+            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);
+            memset(&addr_in6, 0, sizeof(addr_in6));
             addr_in6.sin6_family = AF_INET6;
-            read_message(input_fd, static_cast<void *>(&addr_in6.sin6_port),
+            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),
+            read_message(input_fd, static_cast<void*>(&addr_in6.sin6_addr.s6_addr),
                          sizeof(addr_in6.sin6_addr.s6_addr));
             break;
 
@@ -132,35 +133,34 @@ handle_request(const int input_fd, const int output_fd,
         // Got the socket, send it to the client.
         write_message(output_fd, "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.
+            // Error.  Close the socket (ignore any error from that operation)
+            // and abort.
             close_fun(result);
             isc_throw(InternalError, "Error sending descriptor");
         }
 
-        // Don't leak the socket used to send the acquired socket back to the
-        // client.
+        // 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);
+        write_message(output_fd, "E", 1);           // Error occurred...
         switch (result) {
             case -1:
-                write_message(output_fd, "S", 1);
+                write_message(output_fd, "S", 1);   // ... in the socket() call
                 break;
 
             case -2:
-                write_message(output_fd, "B", 1);
+                write_message(output_fd, "B", 1);   // ... in the bind() call
                 break;
 
             default:
                 isc_throw(InternalError, "Error creating socket");
         }
 
-        // Error reason code.
+        // ...and append the reason code to the error message
         int error = errno;
         write_message(output_fd, static_cast<void *>(&error), sizeof error);
     }

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,13 +12,11 @@
 // 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
@@ -34,9 +32,9 @@ namespace isc {
 namespace socket_creator {
 
 // Exception classes - the base class exception SocketCreatorError is caught
-// by main(), and holds a reason code returned to the environment.  The code
-// depends on the exception raised.
-class SocketCreatorError : public Exception {
+// 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) :
@@ -47,7 +45,7 @@ public:
     }
 
 private:
-    int     exit_code_;     // Code returned to exit()
+    const int exit_code_;   // Code returned to exit()
 };
 
 class ReadError : public SocketCreatorError {
@@ -76,75 +74,72 @@ public:
 
 
 
-/**
- * \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.
- */
+/// \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);
-
-/**
- * Type of the send_fd() function, so it can be passed as a parameter.
- */
-typedef
-int
-(*send_fd_t)(const int, const int);
-
-/// \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.
- *
- * It terminates either if a command asks it to or when unrecoverable
- * error happens.
- *
- * \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.
- *
- * \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
- */
+// 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 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);
+
+// 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