[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.

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);
 }

src/bin/sockcreator/sockcreator.cc

@@ -49,40 +49,50 @@ 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)
+// Simple wrappers for read_data/write_data that thr0w an exception on error.
+void
+read_message(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) {
+    if (!write_data(fd, what, length)) {
+        isc_throw(WriteError, "Error writing to socket creator client");
+    }
+}
+
+// Exit on some protocol error after informing the client of the problem.
+void
+protocol_error(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));
 
-#define WRITE(WHAT, HOW_MANY) do { \
-        if (!write_data(output_fd, (WHAT), (HOW_MANY))) { \
-            return 2; \
-        } \
-    } while (0)
+    // ... and exit
+    isc_throw(ProtocolError, "Fatal error, reason: " << reason);
+}
 
-#define DEFAULT \
-    default: /* Unrecognized part of protocol */ \
-        WRITE("FI", 2); \
-        return 3;
 
-int
+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;
+                return;
             case 'S': { // Create a socket
                 // Read what type of socket they want
                 char type[2];
-                READ(type, 2);
+                read_message(input_fd, type, sizeof(type));
                 // Read the address they ask for
                 struct sockaddr *addr(NULL);
                 size_t addr_len(0);
@@ -99,9 +109,11 @@ run(const int input_fd, const int output_fd, const get_sock_t get_sock,
                         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 *>(
+                        read_message(input_fd,
+                            static_cast<char *>(static_cast<void *>(
                             &addr_in.sin_port)), 2);
-                        READ(static_cast<char *>(static_cast<void *>(
+                        read_message(input_fd,
+                            static_cast<char *>(static_cast<void *>(
                             &addr_in.sin_addr.s_addr)), 4);
                         break;
                     case '6':
@@ -110,14 +122,17 @@ run(const int input_fd, const int output_fd, const get_sock_t get_sock,
                         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 *>(
+                        read_message(input_fd,
+                            static_cast<char *>(static_cast<void *>(
                             &addr_in6.sin6_port)), 2);
-                        READ(static_cast<char *>(static_cast<void *>(
+                        read_message(input_fd,
+                            static_cast<char *>(static_cast<void *>(
                             &addr_in6.sin6_addr.s6_addr)), 16);
                         break;
-                    DEFAULT
+                    default:
+                        protocol_error(output_fd);
                 }
-                int sock_type;
+                int sock_type = 0;
                 switch (type[0]) { // Translate the type
                     case 'T':
                         sock_type = SOCK_STREAM;
@@ -125,41 +140,44 @@ run(const int input_fd, const int output_fd, const get_sock_t get_sock,
                     case 'U':
                         sock_type = SOCK_DGRAM;
                         break;
-                    DEFAULT
+                    default:
+                        protocol_error(output_fd);
                 }
                 int result(get_sock(sock_type, addr, addr_len));
                 if (result >= 0) { // We got the socket
-                    WRITE("S", 1);
+                    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.
                         close_fun(result);
-                        return 3;
+                        isc_throw(InternalError, "Error sending descriptor");
                     }
                     // Don't leak the socket
                     if (close_fun(result) == -1) {
-                        return 4;
+                        isc_throw(InternalError, "Error closing socket");
                     }
                 } else {
-                    WRITE("E", 1);
+                    write_message(output_fd, "E", 1);
                     switch (result) {
                         case -1:
-                            WRITE("S", 1);
+                            write_message(output_fd, "S", 1);
                             break;
                         case -2:
-                            WRITE("B", 1);
+                            write_message(output_fd, "B", 1);
                             break;
                         default:
-                            return 4;
+                            isc_throw(InternalError, "Error creating socket");
                     }
                     int error(errno);
-                    WRITE(static_cast<char *>(static_cast<void *>(&error)),
+                    write_message(output_fd,
+                        static_cast<char *>(static_cast<void *>(&error)),
                         sizeof error);
                 }
                 break;
             }
-            DEFAULT
+            default:
+                protocol_error(output_fd);
         }
     }
 }

src/bin/sockcreator/sockcreator.h

@@ -24,6 +24,7 @@
 #define __SOCKCREATOR_H 1
 
 #include <util/io/fd_share.h>
+#include <exceptions/exceptions.h>
 
 #include <sys/types.h>
 #include <sys/socket.h>
@@ -32,6 +33,49 @@
 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 {
+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:
+    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.
  *
@@ -75,17 +119,9 @@ int
  * 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.
@@ -96,8 +132,13 @@ int
  *     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
  */
-int
+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,

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));