Cleanups and library splitting

* Some of the code might be useful in other places, so it is split into
  libraries (isc::util::io and isc::util::unittests).
* Fixing flags in the sockcreator, leading to new warnings, fixing these
  warnings.
* TODO: read_data and write_data were tested implicitly with run(), but
  as they moved out to other library, they should have their own
  testcase.

git-svn-id: svn://bind10.isc.org/svn/bind10/branches/vorner-sockcreator@3163 e5f2f494-b856-4b98-b285-d166d9295462

configure.ac

@@ -508,6 +508,9 @@ AC_CONFIG_FILES([Makefile
                  src/lib/datasrc/Makefile
                  src/lib/datasrc/tests/Makefile
                  src/lib/xfr/Makefile
+                 src/lib/util/Makefile
+                 src/lib/util/io/Makefile
+                 src/lib/util/unittests/Makefile
                ])
 AC_OUTPUT([src/bin/cfgmgr/b10-cfgmgr.py
            src/bin/cfgmgr/tests/b10-cfgmgr_test.py

src/bin/sockcreator/Makefile.am

 SUBDIRS = tests
 
+AM_CPPFLAGS = -I$(top_srcdir)/src/lib -I$(top_builddir)/src/lib
+
+AM_CXXFLAGS = $(B10_CXXFLAGS)
+
+if USE_STATIC_LINK
+AM_LDFLAGS = -static
+endif
+
+pkglibexecdir = $(libexecdir)/@PACKAGE@
+
 CLEANFILES = *.gcno *.gcda
 
 pkglibexec_PROGRAMS = b10-sockcreator
 
 b10_sockcreator_SOURCES = sockcreator.cc sockcreator.h main.cc
+b10_sockcreator_LDADD = $(top_builddir)/src/lib/util/io/libutil_io.la

src/bin/sockcreator/sockcreator.cc

@@ -14,6 +14,8 @@
 
 #include "sockcreator.h"
 
+#include <util/io/fd.h>
+
 #include <unistd.h>
 #include <cerrno>
 #include <cstring>
@@ -21,6 +23,8 @@
 #include <sys/socket.h>
 #include <netinet/in.h>
 
+using namespace isc::util::io;
+
 namespace isc {
 namespace socket_creator {
 
@@ -38,8 +42,8 @@ get_sock(const int type, struct sockaddr *bind_addr, const socklen_t addr_len)
 }
 
 int
-send_fd(const int destination, const int payload) {
-    // TODO Steal
+send_fd(const int, const int) {
+    return 0;
 }
 
 int
@@ -54,7 +58,6 @@ run(const int input_fd, const int output_fd, const get_sock_t get_sock,
         } \
     } while (0)
 #define WRITE(WHAT, HOW_MANY) do { \
-        size_t how_many = (HOW_MANY); \
         if (!write_data(output_fd, (WHAT), (HOW_MANY))) { \
             return 2; \
         } \
@@ -121,6 +124,7 @@ run(const int input_fd, const int output_fd, const get_sock_t get_sock,
                 int result(get_sock(sock_type, addr, addr_len));
                 if (result >= 0) { // We got the socket
                     WRITE("S", 1);
+                    // FIXME: Check the output and write a test for it
                     send_fd(output_fd, result);
                 } else {
                     WRITE("E", 1);
@@ -145,47 +149,5 @@ run(const int input_fd, const int output_fd, const get_sock_t get_sock,
     }
 }
 
-bool
-write_data(const int fd, const char *buffer, const size_t length) {
-    size_t rest(length);
-    // Just keep writing until all is written
-    while (rest) {
-        ssize_t written(write(fd, buffer, rest));
-        if (rest == -1) {
-            if (errno == EINTR) { // Just keep going
-                continue;
-            } else {
-                return false;
-            }
-        } else { // Wrote something
-            rest -= written;
-            buffer += written;
-        }
-    }
-    return true;
-}
-
-ssize_t
-read_data(const int fd, char *buffer, const size_t length) {
-    size_t rest(length), already(0);
-    while (rest) { // Stil something to read
-        ssize_t amount(read(fd, buffer, rest));
-        if (rest == -1) {
-            if (errno == EINTR) { // Continue on interrupted call
-                continue;
-            } else {
-                return -1;
-            }
-        } else if (amount) {
-            already += amount;
-            rest -= amount;
-            buffer += amount;
-        } else { // EOF
-            return already;
-        }
-    }
-    return already;
-}
-
 } // End of the namespaces
 }

src/bin/sockcreator/sockcreator.h

@@ -102,38 +102,6 @@ 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 = send_fd);
 
-/*
- * \short write() that writes everything.
- * Wrapper around write(). The difference is, it never writes less data
- * and looks successfull (eg. it blocks until all data are written).
- * Retries on signals.
- *
- * \return True if sucessfull, false otherwise. The errno variable is left
- *     intact.
- * \param fd Where to write.
- * \param data The buffer to write.
- * \param length How much data is there to write.
- *
- * TODO: Shouldn't this be in some kind of library?
- */
-bool
-write_data(const int fd, const char *data, const size_t length);
-
-/*
- * \short read() that reads everything.
- * Wrapper around read(). It does not do short reads, if it returns less,
- * it means there was EOF. It retries on signals.
- *
- * \return Number of bytes read or -1 on error.
- * \param fd Where to read data from.
- * \param data Where to put the data.
- * \param length How many of them.
- *
- * TODO: Shouldn't this be in some kind of library?
- */
-ssize_t
-read_data(const int fd, char *buffer, const size_t length);
-
 } // End of the namespaces
 }
 

src/bin/sockcreator/tests/Makefile.am

 CLEANFILES = *.gcno *.gcda
 
+AM_CPPFLAGS = -I$(top_srcdir)/src/lib -I$(top_builddir)/src/lib
+# This is here because of juggling with sockaddr pointer.
+# It is somehow required by the BSD socket interface, but it
+# breaks C++ strict aliasing rules, so we need to ask the compiler
+# not to use them.
+AM_CPPFLAGS += -fno-strict-aliasing
+AM_CXXFLAGS = $(B10_CXXFLAGS)
+
+if USE_STATIC_LINK
+AM_LDFLAGS = -static
+endif
+
 TESTS =
 if HAVE_GTEST
 TESTS += run_unittests
@@ -10,6 +22,9 @@ run_unittests_SOURCES += run_unittests.cc
 run_unittests_CPPFLAGS = $(AM_CPPFLAGS) $(GTEST_INCLUDES)
 run_unittests_LDFLAGS = $(AM_LDFLAGS) $(GTEST_LDFLAGS)
 run_unittests_LDADD = $(GTEST_LDADD)
+run_unittests_LDADD += $(top_builddir)/src/lib/util/io/libutil_io.la
+run_unittests_LDADD += \
+	$(top_builddir)/src/lib/util/unittests/libutil_unittests.la
 endif
 
 noinst_PROGRAMS = $(TESTS)

src/bin/sockcreator/tests/sockcreator_tests.cc

@@ -14,19 +14,20 @@
 
 #include "../sockcreator.h"
 
+#include <util/unittests/fork.h>
+#include <util/io/fd.h>
+
 #include <gtest/gtest.h>
 #include <sys/types.h>
 #include <sys/socket.h>
-#include <sys/wait.h>
-#include <signal.h>
 #include <netinet/in.h>
 #include <unistd.h>
 #include <cstring>
-#include <cstdlib>
 #include <cerrno>
-#include <cstdio>
 
 using namespace isc::socket_creator;
+using namespace isc::util::unittests;
+using namespace isc::util::io;
 
 namespace {
 
@@ -115,103 +116,14 @@ TEST(get_sock, fail_with_nonsense) {
     ASSERT_LT(get_sock(0, &addr, sizeof addr), 0);
 }
 
-/*
- * This creates a pipe, forks and feeds the pipe with given data.
- * Used to provide the input in non-blocking/asynchronous way.
- */
-pid_t
-provide_input(int *read_pipe, const char *input, const size_t length) {
-    int pipes[2];
-    if (pipe(pipes)) {
-        return -1;
-    }
-    *read_pipe = pipes[0];
-    pid_t pid(fork());
-    if (pid) { // We are in the parent
-        return pid;
-    } else { // This is in the child, just puth the data there
-        close(pipes[0]);
-        if (!write_data(pipes[1], input, length)) {
-            exit(1);
-        } else {
-            close(pipes[1]);
-            exit(0);
-        }
-    }
-}
-
-/*
- * This creates a pipe, forks and reads the pipe and compares it
- * with given data. Used to check output of run in asynchronous way.
- */
-pid_t
-check_output(int *write_pipe, const char *output, const size_t length) {
-    int pipes[2];
-    if (pipe(pipes)) {
-        return -1;
-    }
-    *write_pipe = pipes[1];
-    pid_t pid(fork());
-    if (pid) { // We are in parent
-        return pid;
-    } else {
-        close(pipes[1]);
-        char buffer[length + 1];
-        // Try to read one byte more to see if the output ends here
-        size_t got_length(read_data(pipes[0], buffer, length + 1));
-        bool ok(true);
-        if (got_length != length) {
-            fprintf(stderr, "Different length (expected %u, got %u)\n",
-                static_cast<unsigned>(length),
-                static_cast<unsigned>(got_length));
-            ok = false;
-        }
-        if(!ok || memcmp(buffer, output, length)) {
-            // If the differ, print what we have
-            for(size_t i(0); i != got_length; ++ i) {
-                fprintf(stderr, "%02hhx", buffer[i]);
-            }
-            fprintf(stderr, "\n");
-            for(size_t i(0); i != length; ++ i) {
-                fprintf(stderr, "%02hhx", output[i]);
-            }
-            fprintf(stderr, "\n");
-            exit(1);
-        } else {
-            exit(0);
-        }
-    }
-}
-
-/*
- * Waits for pid to terminate and checks it terminates successfully (with 0).
- */
-bool
-process_ok(pid_t process) {
-    int status;
-    // Make sure it does terminate when the output is shorter than expected
-    /*
-     * FIXME: if the timeout is reached, this kill the whole test, not just
-     * the waitpid call. Should have signal handler. This is no show-stopper,
-     * but we might have another tests to run.
-     */
-    alarm(3);
-    if (waitpid(process, &status, 0) == -1) {
-        if (errno == EINTR)
-            kill(process, SIGTERM);
-        return false;
-    }
-    return WIFEXITED(status) && WEXITSTATUS(status) == 0;
-}
-
 /*
  * Helper functions to pass to run during testing.
  */
 int
-get_sock_dummy(const int type, struct sockaddr *addr, const socklen_t addr_len)
+get_sock_dummy(const int type, struct sockaddr *addr, const socklen_t)
 {
     int result(0);
-    int port;
+    int port(0);
     /*
      * We encode the type and address family into the int and return it.
      * Lets ignore the port and address for now
@@ -265,8 +177,11 @@ send_fd_dummy(const int destination, const int what)
      * the test anyway.
      */
     char fd_data(what);
-    if (!write_data(destination, &fd_data, 1))
+    if (!write_data(destination, &fd_data, 1)) {
         return -1;
+    } else {
+        return 0;
+    }
 }
 
 /*
@@ -280,7 +195,7 @@ void run_test(const char *input_data, const size_t input_size,
     bool should_succeed = true)
 {
     // Prepare the input feeder and output checker processes
-    int input_fd, output_fd;
+    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));
     ASSERT_NE(-1, input) << "Couldn't start input feeder";

src/lib/Makefile.am

-SUBDIRS = exceptions dns cc config datasrc python xfr bench
+SUBDIRS = exceptions dns cc config datasrc python xfr bench util

src/lib/util/Makefile.am

+SUBDIRS = io unittests

src/lib/util/io/Makefile.am

+AM_CXXFLAGS = $(B10_CXXFLAGS)
+
+lib_LTLIBRARIES = libutil_io.la
+libutil_io_la_SOURCES = fd.h fd.cc
+
+CLEANFILES = *.gcno *.gcda

src/lib/util/io/fd.cc

+// Copyright (C) 2010  CZ NIC
+//
+// Permission to use, copy, modify, and/or distribute this software for any
+// purpose with or without fee is hereby granted, provided that the above
+// copyright notice and this permission notice appear in all copies.
+//
+// THE SOFTWARE IS PROVIDED "AS IS" AND ISC DISCLAIMS ALL WARRANTIES WITH
+// REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF MERCHANTABILITY
+// AND FITNESS.  IN NO EVENT SHALL ISC BE LIABLE FOR ANY SPECIAL, DIRECT,
+// INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES WHATSOEVER RESULTING FROM
+// LOSS OF USE, DATA OR PROFITS, WHETHER IN AN ACTION OF CONTRACT, NEGLIGENCE
+// OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR
+// PERFORMANCE OF THIS SOFTWARE.
+
+#include "fd.h"
+
+#include <unistd.h>
+#include <cerrno>
+
+namespace isc {
+namespace util {
+namespace io {
+
+bool
+write_data(const int fd, const char *buffer, const size_t length) {
+    size_t rest(length);
+    // Just keep writing until all is written
+    while (rest) {
+        ssize_t written(write(fd, buffer, rest));
+        if (rest == -1) {
+            if (errno == EINTR) { // Just keep going
+                continue;
+            } else {
+                return false;
+            }
+        } else { // Wrote something
+            rest -= written;
+            buffer += written;
+        }
+    }
+    return true;
+}
+
+ssize_t
+read_data(const int fd, char *buffer, const size_t length) {
+    size_t rest(length), already(0);
+    while (rest) { // Stil something to read
+        ssize_t amount(read(fd, buffer, rest));
+        if (rest == -1) {
+            if (errno == EINTR) { // Continue on interrupted call
+                continue;
+            } else {
+                return -1;
+            }
+        } else if (amount) {
+            already += amount;
+            rest -= amount;
+            buffer += amount;
+        } else { // EOF
+            return already;
+        }
+    }
+    return already;
+}
+
+}
+}
+}

src/lib/util/io/fd.h

+// Copyright (C) 2010  CZ NIC
+//
+// Permission to use, copy, modify, and/or distribute this software for any
+// purpose with or without fee is hereby granted, provided that the above
+// copyright notice and this permission notice appear in all copies.
+//
+// THE SOFTWARE IS PROVIDED "AS IS" AND ISC DISCLAIMS ALL WARRANTIES WITH
+// REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF MERCHANTABILITY
+// AND FITNESS.  IN NO EVENT SHALL ISC BE LIABLE FOR ANY SPECIAL, DIRECT,
+// INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES WHATSOEVER RESULTING FROM
+// LOSS OF USE, DATA OR PROFITS, WHETHER IN AN ACTION OF CONTRACT, NEGLIGENCE
+// OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR
+// PERFORMANCE OF THIS SOFTWARE.
+
+#ifndef __UTIL_IO_FD_H
+#define __UTIL_IO_FD_H 1
+
+#include <unistd.h>
+
+/**
+ * @file fd.h
+ * @short Wrappers around common unix fd manipulation functions.
+ */
+
+namespace isc {
+namespace util {
+namespace io {
+
+/*
+ * \short write() that writes everything.
+ * Wrapper around write(). The difference is, it never writes less data
+ * and looks successfull (eg. it blocks until all data are written).
+ * Retries on signals.
+ *
+ * \return True if sucessfull, false otherwise. The errno variable is left
+ *     intact.
+ * \param fd Where to write.
+ * \param data The buffer to write.
+ * \param length How much data is there to write.
+ */
+bool
+write_data(const int fd, const char *data, const size_t length);
+
+/*
+ * \short read() that reads everything.
+ * Wrapper around read(). It does not do short reads, if it returns less,
+ * it means there was EOF. It retries on signals.
+ *
+ * \return Number of bytes read or -1 on error.
+ * \param fd Where to read data from.
+ * \param data Where to put the data.
+ * \param length How many of them.
+ */
+ssize_t
+read_data(const int fd, char *buffer, const size_t length);
+
+}
+}
+}
+
+#endif // __UTIL_IO_FD_H

src/lib/util/unittests/Makefile.am

+AM_CPPFLAGS = -I$(top_srcdir)/src/lib -I$(top_builddir)/src/lib
+AM_CXXFLAGS = $(B10_CXXFLAGS)
+
+lib_LTLIBRARIES = libutil_unittests.la
+libutil_unittests_la_SOURCES = fork.h fork.cc
+
+CLEANFILES = *.gcno *.gcda

src/lib/util/unittests/README

+This directory contains some code that is useful while writing various
+unittest code. It doesn't contain any code that would actually run in
+bind10.
+
+Because this is a test code, we do not test it explicitly.

src/lib/util/unittests/fork.cc

+// Copyright (C) 2010  CZ NIC
+//
+// Permission to use, copy, modify, and/or distribute this software for any
+// purpose with or without fee is hereby granted, provided that the above
+// copyright notice and this permission notice appear in all copies.
+//
+// THE SOFTWARE IS PROVIDED "AS IS" AND ISC DISCLAIMS ALL WARRANTIES WITH
+// REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF MERCHANTABILITY
+// AND FITNESS.  IN NO EVENT SHALL ISC BE LIABLE FOR ANY SPECIAL, DIRECT,
+// INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES WHATSOEVER RESULTING FROM
+// LOSS OF USE, DATA OR PROFITS, WHETHER IN AN ACTION OF CONTRACT, NEGLIGENCE
+// OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR
+// PERFORMANCE OF THIS SOFTWARE.
+
+#include "fork.h"
+
+#include <util/io/fd.h>
+
+#include <sys/types.h>
+#include <sys/wait.h>
+#include <unistd.h>
+#include <signal.h>
+#include <cstring>
+#include <cerrno>
+#include <cstdlib>
+#include <cstdio>
+
+using namespace isc::util::io;
+
+namespace {
+
+// Just a NOP function to ignore a signal but let it interrupt function.
+void no_handler(int) { }
+
+};
+
+namespace isc {
+namespace util {
+namespace unittests {
+
+bool
+process_ok(pid_t process) {
+    // Create a timeout
+    struct sigaction ignored, original;
+    memset(&ignored, 0, sizeof ignored);
+    ignored.sa_handler = no_handler;
+    if (sigaction(SIGALRM, &ignored, &original)) {
+        return false;
+    }
+    alarm(3);
+    int status;
+    int result(waitpid(process, &status, 0) == -1);
+    // Cancel the alarm and return the original handler
+    alarm(0);
+    if (sigaction(SIGALRM, &original, NULL)) {
+        return false;
+    }
+    // Check what we found out
+    if (result) {
+        if (errno == EINTR)
+            kill(process, SIGTERM);
+        return false;
+    }
+    return WIFEXITED(status) && WEXITSTATUS(status) == 0;
+}
+
+/*
+ * This creates a pipe, forks and feeds the pipe with given data.
+ * Used to provide the input in non-blocking/asynchronous way.
+ */
+pid_t
+provide_input(int *read_pipe, const char *input, const size_t length) {
+    int pipes[2];
+    if (pipe(pipes)) {
+        return -1;
+    }
+    *read_pipe = pipes[0];
+    pid_t pid(fork());
+    if (pid) { // We are in the parent
+        return pid;
+    } else { // This is in the child, just puth the data there
+        close(pipes[0]);
+        if (!write_data(pipes[1], input, length)) {
+            exit(1);
+        } else {
+            close(pipes[1]);
+            exit(0);
+        }
+    }
+}
+
+/*
+ * This creates a pipe, forks and reads the pipe and compares it
+ * with given data. Used to check output of run in asynchronous way.
+ */
+pid_t
+check_output(int *write_pipe, const char *output, const size_t length) {
+    int pipes[2];
+    if (pipe(pipes)) {
+        return -1;
+    }
+    *write_pipe = pipes[1];
+    pid_t pid(fork());
+    if (pid) { // We are in parent
+        return pid;
+    } else {
+        close(pipes[1]);
+        char buffer[length + 1];
+        // Try to read one byte more to see if the output ends here
+        size_t got_length(read_data(pipes[0], buffer, length + 1));
+        bool ok(true);
+        if (got_length != length) {
+            fprintf(stderr, "Different length (expected %u, got %u)\n",
+                static_cast<unsigned>(length),
+                static_cast<unsigned>(got_length));
+            ok = false;
+        }
+        if(!ok || memcmp(buffer, output, length)) {
+            // If the differ, print what we have
+            for(size_t i(0); i != got_length; ++ i) {