[1593] Addressed comments from review

src/bin/sockcreator/main.cc

@@ -13,6 +13,7 @@
 // PERFORMANCE OF THIS SOFTWARE.
 
 #include "sockcreator.h"
+#include <unistd.h>
 
 using namespace isc::socket_creator;
 
@@ -22,11 +23,10 @@ main() {
      * TODO Maybe use some OS-specific caps interface and drop everything
      * but ability to bind ports? It would be nice.
      */
-    int status = 0;
     try {
-        run(0, 1); // Read commands from stdin, output to stdout
+        run(STDIN_FILENO, STDOUT_FILENO);
     } catch (const SocketCreatorError& ec) {
-        status = ec.getExitCode();
+        return (ec.getExitCode());
     }
-    return (status);
+    return (0);
 }

src/bin/sockcreator/sockcreator.cc

@@ -58,6 +58,45 @@ protocolError(const int fd, const char reason = 'I') {
     isc_throw(ProtocolError, "Fatal error, reason: " << reason);
 }
 
+// Return appropriate socket type constant for the socket type requested.
+// The output_fd argument is required to report a protocol error.
+int getSocketType(const char type_code, const int output_fd) {
+    int socket_type = 0;
+    switch (type_code) {
+        case 'T':
+            socket_type = SOCK_STREAM;
+            break;
+
+        case 'U':
+            socket_type = SOCK_DGRAM;
+            break;
+
+        default:
+            protocolError(output_fd);   // Does not return
+    }
+    return (socket_type);
+}
+
+// Convert return status from getSock() to a character to be sent back to
+// the caller.
+char getErrorCode(const int status) {
+    char error_code = ' ';
+    switch (status) {
+        case -1:
+            error_code = 'S';
+            break;
+
+        case -2:
+            error_code = 'B';
+            break;
+
+        default:
+            isc_throw(InternalError, "Error creating socket");
+    }
+    return (error_code);
+}
+
+
 // Handle the request from the client.
 //
 // Reads the type and family of socket required, creates the socket and returns
@@ -75,19 +114,7 @@ handleRequest(const int input_fd, const int output_fd,
     readMessage(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:
-            protocolError(output_fd);
-    }
+    const int sock_type = getSocketType(type[0], output_fd);
 
     // Read the address they ask for depending on what address family was
     // specified.
@@ -144,23 +171,14 @@ handleRequest(const int input_fd, const int output_fd,
         }
     } else {
         // Error.  Tell the client.
-        writeMessage(output_fd, "E", 1);           // Error occurred...
-        switch (result) {
-            case -1:
-                writeMessage(output_fd, "S", 1);   // ... in the socket() call
-                break;
-
-            case -2:
-                writeMessage(output_fd, "B", 1);   // ... in the bind() call
-                break;
-
-            default:
-                isc_throw(InternalError, "Error creating socket");
-        }
+        char error_message[2];
+        error_message[0] = 'E';
+        error_message[1] = getErrorCode(result);
+        writeMessage(output_fd, error_message, sizeof(error_message));
 
         // ...and append the reason code to the error message
-        const int error = errno;
-        writeMessage(output_fd, &error, sizeof(error));
+        const int error_number = errno;
+        writeMessage(output_fd, &error_number, sizeof(error_number));
     }
 }
 

src/bin/sockcreator/sockcreator.h

@@ -116,10 +116,11 @@ typedef int (*close_t)(int);
 /// 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
+/// \param input_fd File number 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 output_fd File number of the stream to which the response is
+///        written.  The socket is sent as part of a control message associated
+///        with that stream.
 /// \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.