Extract APR and temp-fixture-file helper code to indra/test.

Specifically:
Introduce ManageAPR class in indra/test/manageapr.h. This is useful for a
simple test program without lots of static constructors.
Extract NamedTempFile from llsdserialize_test.cpp to indra/test/
namedtempfile.h. Refactor to use APR file operations rather than platform-
dependent APIs.
Use NamedTempFile for llprocesslauncher_test.cpp.

4 files changed, 184 insertions, 308 deletions

diff --git a/indra/llcommon/tests/llprocesslauncher_test.cpp b/indra/llcommon/tests/llprocesslauncher_test.cpp
index 4d8f850d92..3935c64a94 100644
--- a/indra/llcommon/tests/llprocesslauncher_test.cpp
+++ b/indra/llcommon/tests/llprocesslauncher_test.cpp
@@ -18,14 +18,14 @@
 #include <vector>
 #include <list>
 // std headers
-#include <errno.h>
 // external library headers
 #include "llapr.h"
 #include "apr_thread_proc.h"
-#include "apr_file_io.h"
 #include <boost/foreach.hpp>
 // other Linden headers
 #include "../test/lltut.h"
+#include "../test/manageapr.h"
+#include "../test/namedtempfile.h"
 #include "stringize.h"
 
 #if defined(LL_WINDOWS)
@@ -36,30 +36,8 @@
 #include <sys/wait.h>
 #endif
 
-class APR
-{
-public:
-    APR():
-        pool(NULL)
-    {
-        apr_initialize();
-        apr_pool_create(&pool, NULL);
-    }
-
-    ~APR()
-    {
-        apr_terminate();
-    }
-
-    std::string strerror(apr_status_t rv)
-    {
-        char errbuf[256];
-        apr_strerror(rv, errbuf, sizeof(errbuf));
-        return errbuf;
-    }
-
-    apr_pool_t *pool;
-};
+// static instance of this manages APR init/cleanup
+static ManageAPR manager;
 
 #define ensure_equals_(left, right) \
         ensure_equals(STRINGIZE(#left << " != " << #right), (left), (right))
@@ -74,11 +52,11 @@ namespace tut
     {
         void aprchk_(const char* call, apr_status_t rv, apr_status_t expected=APR_SUCCESS)
         {
-            ensure_equals(STRINGIZE(call << " => " << rv << ": " << apr.strerror(rv)),
+            ensure_equals(STRINGIZE(call << " => " << rv << ": " << manager.strerror(rv)),
                           rv, expected);
         }
 
-        APR apr;
+        LLAPRPool pool;
     };
     typedef test_group<llprocesslauncher_data> llprocesslauncher_group;
     typedef llprocesslauncher_group::object object;
@@ -186,19 +164,7 @@ namespace tut
         set_test_name("raw APR nonblocking I/O");
 
         // Create a script file in a temporary place.
-        const char* tempdir = NULL;
-        aprchk(apr_temp_dir_get(&tempdir, apr.pool));
-
-        // Construct a temp filename template in that directory.
-        char *tempname = NULL;
-        aprchk(apr_filepath_merge(&tempname, tempdir, "testXXXXXX", 0, apr.pool));
-
-        // Create a temp file from that template.
-        apr_file_t* fp = NULL;
-        aprchk(apr_file_mktemp(&fp, tempname, APR_CREATE | APR_WRITE | APR_EXCL, apr.pool));
-
-        // Write it.
-        const char script[] =
+        NamedTempFile script("py",
             "import sys" EOL
             "import time" EOL
             EOL
@@ -208,10 +174,7 @@ namespace tut
             "time.sleep(2)" EOL
             "print >>sys.stderr, 'stderr after wait'" EOL
             "sys.stderr.flush()" EOL
-            ;
-        apr_size_t len(sizeof(script)-1);
-        aprchk(apr_file_write(fp, script, &len));
-        aprchk(apr_file_close(fp));
+            );
 
         // Arrange to track the history of our interaction with child: what we
         // fetched, which pipe it came from, how many tries it took before we
@@ -221,30 +184,33 @@ namespace tut
 
         // Run the child process.
         apr_procattr_t *procattr = NULL;
-        aprchk(apr_procattr_create(&procattr, apr.pool));
+        aprchk(apr_procattr_create(&procattr, pool.getAPRPool()));
         aprchk(apr_procattr_io_set(procattr, APR_CHILD_BLOCK, APR_CHILD_BLOCK, APR_CHILD_BLOCK));
         aprchk(apr_procattr_cmdtype_set(procattr, APR_PROGRAM_PATH));
 
         std::vector<const char*> argv;
         apr_proc_t child;
         argv.push_back("python");
-        argv.push_back(tempname);
+        // Have to have a named copy of this std::string so its c_str() value
+        // will persist.
+        std::string scriptname(script.getName());
+        argv.push_back(scriptname.c_str());
         argv.push_back(NULL);
 
         aprchk(apr_proc_create(&child, argv[0],
                                &argv[0],
                                NULL, // if we wanted to pass explicit environment
                                procattr,
-                               apr.pool));
+                               pool.getAPRPool()));
 
         // We do not want this child process to outlive our APR pool. On
         // destruction of the pool, forcibly kill the process. Tell APR to try
         // SIGTERM and wait 3 seconds. If that didn't work, use SIGKILL.
-        apr_pool_note_subprocess(apr.pool, &child, APR_KILL_AFTER_TIMEOUT);
+        apr_pool_note_subprocess(pool.getAPRPool(), &child, APR_KILL_AFTER_TIMEOUT);
 
         // arrange to call child_status_callback()
         WaitInfo wi(&child);
-        apr_proc_other_child_register(&child, child_status_callback, &wi, child.in, apr.pool);
+        apr_proc_other_child_register(&child, child_status_callback, &wi, child.in, pool.getAPRPool());
 
         // TODO:
         // Stuff child.in until it (would) block to verify EWOULDBLOCK/EAGAIN.
@@ -289,7 +255,7 @@ namespace tut
                 }
                 if (rv == EWOULDBLOCK || rv == EAGAIN)
                 {
-//                  std::cout << "(waiting; apr_file_gets(" << dfli->first << ") => " << rv << ": " << apr.strerror(rv) << ")\n";
+//                  std::cout << "(waiting; apr_file_gets(" << dfli->first << ") => " << rv << ": " << manager.strerror(rv) << ")\n";
                     ++history.back().tries;
                     continue;
                 }
@@ -334,14 +300,11 @@ namespace tut
             std::cout << "child_status_callback(APR_OC_REASON_DEATH) wasn't called" << std::endl;
             wi.rv = apr_proc_wait(wi.child, &wi.rc, &wi.why, APR_NOWAIT);
         }
-//      std::cout << "child done: rv = " << rv << " (" << apr.strerror(rv) << "), why = " << why << ", rc = " << rc << '\n';
+//      std::cout << "child done: rv = " << rv << " (" << manager.strerror(rv) << "), why = " << why << ", rc = " << rc << '\n';
         aprchk_("apr_proc_wait(wi->child, &wi->rc, &wi->why, APR_NOWAIT)", wi.rv, APR_CHILD_DONE);
         ensure_equals_(wi.why, APR_PROC_EXIT);
         ensure_equals_(wi.rc, 0);
 
-        // Remove temp script file
-        aprchk(apr_file_remove(tempname, apr.pool));
-
         // Beyond merely executing all the above successfully, verify that we
         // obtained expected output -- and that we duly got control while
         // waiting, proving the non-blocking nature of these pipes.
diff --git a/indra/llcommon/tests/llsdserialize_test.cpp b/indra/llcommon/tests/llsdserialize_test.cpp
index 72322c3b72..4359e9afb9 100644
--- a/indra/llcommon/tests/llsdserialize_test.cpp
+++ b/indra/llcommon/tests/llsdserialize_test.cpp
@@ -43,38 +43,12 @@ typedef U32 uint32_t;
 #include "llprocesslauncher.h"
 #endif
 
-#include <sstream>
-
-/*==========================================================================*|
-// Whoops, seems Linden's Boost package and the viewer are built with
-// different settings of VC's /Zc:wchar_t switch! Using Boost.Filesystem
-// pathname operations produces Windows link errors:
-// unresolved external symbol "private: static class std::codecvt<unsigned short,
-// char,int> const * & __cdecl boost::filesystem3::path::wchar_t_codecvt_facet()"
-// unresolved external symbol "void __cdecl boost::filesystem3::path_traits::convert()"
-// See:
-// http://boost.2283326.n4.nabble.com/filesystem-v3-unicode-and-std-codecvt-linker-error-td3455549.html
-// which points to:
-// http://msdn.microsoft.com/en-us/library/dh8che7s%28v=VS.100%29.aspx
-
-// As we're not trying to preserve compatibility with old Boost.Filesystem
-// code, but rather writing brand-new code, use the newest available
-// Filesystem API.
-#define BOOST_FILESYSTEM_VERSION 3
-#include "boost/filesystem.hpp"
-#include "boost/filesystem/v3/fstream.hpp"
-|*==========================================================================*/
 #include "boost/range.hpp"
 #include "boost/foreach.hpp"
 #include "boost/function.hpp"
 #include "boost/lambda/lambda.hpp"
 #include "boost/lambda/bind.hpp"
 namespace lambda = boost::lambda;
-/*==========================================================================*|
-// Aaaarrgh, Linden's Boost package doesn't even include Boost.Iostreams!
-#include "boost/iostreams/stream.hpp"
-#include "boost/iostreams/device/file_descriptor.hpp"
-|*==========================================================================*/
 
 #include "../llsd.h"
 #include "../llsdserialize.h"
@@ -82,236 +56,17 @@ namespace lambda = boost::lambda;
 #include "../llformat.h"
 
 #include "../test/lltut.h"
+#include "../test/manageapr.h"
+#include "../test/namedtempfile.h"
 #include "stringize.h"
 
+static ManageAPR manager;
+
 std::vector<U8> string_to_vector(const std::string& str)
 {
 	return std::vector<U8>(str.begin(), str.end());
 }
 
-#if ! LL_WINDOWS
-// We want to call strerror_r(), but alarmingly, there are two different
-// variants. The one that returns int always populates the passed buffer
-// (except in case of error), whereas the other one always returns a valid
-// char* but might or might not populate the passed buffer. How do we know
-// which one we're getting? Define adapters for each and let the compiler
-// select the applicable adapter.
-
-// strerror_r() returns char*
-std::string message_from(int /*orig_errno*/, const char* /*buffer*/, const char* strerror_ret)
-{
-    return strerror_ret;
-}
-
-// strerror_r() returns int
-std::string message_from(int orig_errno, const char* buffer, int strerror_ret)
-{
-    if (strerror_ret == 0)
-    {
-        return buffer;
-    }
-    // Here strerror_r() has set errno. Since strerror_r() has already failed,
-    // seems like a poor bet to call it again to diagnose its own error...
-    int stre_errno = errno;
-    if (stre_errno == ERANGE)
-    {
-        return STRINGIZE("strerror_r() can't explain errno " << orig_errno
-                         << " (buffer too small)");
-    }
-    if (stre_errno == EINVAL)
-    {
-        return STRINGIZE("unknown errno " << orig_errno);
-    }
-    // Here we don't even understand the errno from strerror_r()!
-    return STRINGIZE("strerror_r() can't explain errno " << orig_errno
-                     << " (error " << stre_errno << ')');
-}
-#endif  // ! LL_WINDOWS
-
-// boost::filesystem::temp_directory_path() isn't yet in Boost 1.45! :-(
-std::string temp_directory_path()
-{
-#if LL_WINDOWS
-    char buffer[4096];
-    GetTempPathA(sizeof(buffer), buffer);
-    return buffer;
-
-#else  // LL_DARWIN, LL_LINUX
-    static const char* vars[] = { "TMPDIR", "TMP", "TEMP", "TEMPDIR" };
-    BOOST_FOREACH(const char* var, vars)
-    {
-        const char* found = getenv(var);
-        if (found)
-            return found;
-    }
-    return "/tmp";
-#endif // LL_DARWIN, LL_LINUX
-}
-
-// Windows presents a kinda sorta compatibility layer. Code to the yucky
-// Windows names because they're less likely than the Posix names to collide
-// with any other names in this source.
-#if LL_WINDOWS
-#define _remove   DeleteFileA
-#else  // ! LL_WINDOWS
-#define _open     open
-#define _write    write
-#define _close    close
-#define _remove   remove
-#endif  // ! LL_WINDOWS
-
-// Create a text file with specified content "somewhere in the
-// filesystem," cleaning up when it goes out of scope.
-class NamedTempFile
-{
-public:
-    // Function that accepts an ostream ref and (presumably) writes stuff to
-    // it, e.g.:
-    // (lambda::_1 << "the value is " << 17 << '\n')
-    typedef boost::function<void(std::ostream&)> Streamer;
-
-    NamedTempFile(const std::string& ext, const std::string& content):
-        mPath(temp_directory_path())
-    {
-        createFile(ext, lambda::_1 << content);
-    }
-
-    // Disambiguate when passing string literal
-    NamedTempFile(const std::string& ext, const char* content):
-        mPath(temp_directory_path())
-    {
-        createFile(ext, lambda::_1 << content);
-    }
-
-    NamedTempFile(const std::string& ext, const Streamer& func):
-        mPath(temp_directory_path())
-    {
-        createFile(ext, func);
-    }
-
-    ~NamedTempFile()
-    {
-        _remove(mPath.c_str());
-    }
-
-    std::string getName() const { return mPath; }
-
-private:
-    void createFile(const std::string& ext, const Streamer& func)
-    {
-        // Silly maybe, but use 'ext' as the name prefix. Strip off a leading
-        // '.' if present.
-        int pfx_offset = ((! ext.empty()) && ext[0] == '.')? 1 : 0;
-
-#if ! LL_WINDOWS
-        // Make sure mPath ends with a directory separator, if it doesn't already.
-        if (mPath.empty() ||
-            ! (mPath[mPath.length() - 1] == '\\' || mPath[mPath.length() - 1] == '/'))
-        {
-            mPath.append("/");
-        }
-
-        // mkstemp() accepts and modifies a char* template string. Generate
-        // the template string, then copy to modifiable storage.
-        // mkstemp() requires its template string to end in six X's.
-        mPath += ext.substr(pfx_offset) + "XXXXXX";
-        // Copy to vector<char>
-        std::vector<char> pathtemplate(mPath.begin(), mPath.end());
-        // append a nul byte for classic-C semantics
-        pathtemplate.push_back('\0');
-        // std::vector promises that a pointer to the 0th element is the same
-        // as a pointer to a contiguous classic-C array
-        int fd(mkstemp(&pathtemplate[0]));
-        if (fd == -1)
-        {
-            // The documented errno values (http://linux.die.net/man/3/mkstemp)
-            // are used in a somewhat unusual way, so provide context-specific
-            // errors.
-            if (errno == EEXIST)
-            {
-                LL_ERRS("NamedTempFile") << "mkstemp(\"" << mPath
-                                         << "\") could not create unique file " << LL_ENDL;
-            }
-            if (errno == EINVAL)
-            {
-                LL_ERRS("NamedTempFile") << "bad mkstemp() file path template '"
-                                         << mPath << "'" << LL_ENDL;
-            }
-            // Shrug, something else
-            int mkst_errno = errno;
-            char buffer[256];
-            LL_ERRS("NamedTempFile") << "mkstemp(\"" << mPath << "\") failed: "
-                                     << message_from(mkst_errno, buffer,
-                                                     strerror_r(mkst_errno, buffer, sizeof(buffer)))
-                                     << LL_ENDL;
-        }
-        // mkstemp() seems to have worked! Capture the modified filename.
-        // Avoid the nul byte we appended.
-        mPath.assign(pathtemplate.begin(), (pathtemplate.end()-1));
-
-/*==========================================================================*|
-        // Define an ostream on the open fd. Tell it to close fd on destruction.
-        boost::iostreams::stream<boost::iostreams::file_descriptor_sink>
-            out(fd, boost::iostreams::close_handle);
-|*==========================================================================*/
-
-        // Write desired content.
-        std::ostringstream out;
-        // Stream stuff to it.
-        func(out);
-
-        std::string data(out.str());
-        int written(_write(fd, data.c_str(), data.length()));
-        int closed(_close(fd));
-        llassert_always(written == data.length() && closed == 0);
-
-#else // LL_WINDOWS
-        // GetTempFileName() is documented to require a MAX_PATH buffer.
-        char tempname[MAX_PATH];
-        // Use 'ext' as filename prefix, but skip leading '.' if any.
-        // The 0 param is very important: requests iterating until we get a
-        // unique name.
-        if (0 == GetTempFileNameA(mPath.c_str(), ext.c_str() + pfx_offset, 0, tempname))
-        {
-            // I always have to look up this call...  :-P
-            LPSTR msgptr;
-            FormatMessageA(
-                FORMAT_MESSAGE_ALLOCATE_BUFFER | 
-                FORMAT_MESSAGE_FROM_SYSTEM |
-                FORMAT_MESSAGE_IGNORE_INSERTS,
-                NULL,
-                GetLastError(),
-                MAKELANGID(LANG_NEUTRAL, SUBLANG_DEFAULT),
-                LPSTR(&msgptr),     // have to cast (char**) to (char*)
-                0, NULL );
-            LL_ERRS("NamedTempFile") << "GetTempFileName(\"" << mPath << "\", \""
-                                     << (ext.c_str() + pfx_offset) << "\") failed: "
-                                     << msgptr << LL_ENDL;
-            LocalFree(msgptr);
-        }
-        // GetTempFileName() appears to have worked! Capture the actual
-        // filename.
-        mPath = tempname;
-        // Open the file and stream content to it. Destructor will close.
-        std::ofstream out(tempname);
-        func(out);
-
-#endif  // LL_WINDOWS
-    }
-
-    void peep()
-    {
-        std::cout << "File '" << mPath << "' contains:\n";
-        std::ifstream reader(mPath.c_str());
-        std::string line;
-        while (std::getline(reader, line))
-            std::cout << line << '\n';
-        std::cout << "---\n";
-    }
-
-    std::string mPath;
-};
-
 namespace tut
 {
 	struct sd_xml_data
@@ -1783,7 +1538,7 @@ namespace tut
             const char* PYTHON(getenv("PYTHON"));
             ensure("Set $PYTHON to the Python interpreter", PYTHON);
 
-            NamedTempFile scriptfile(".py", script);
+            NamedTempFile scriptfile("py", script);
 
 #if LL_WINDOWS
             std::string q("\"");
@@ -1888,12 +1643,12 @@ namespace tut
             "    else:\n"
             "        assert False, 'Too many data items'\n";
 
-        // Create a something.llsd file containing 'data' serialized to
+        // Create an llsdXXXXXX file containing 'data' serialized to
         // notation. It's important to separate with newlines because Python's
         // llsd module doesn't support parsing from a file stream, only from a
         // string, so we have to know how much of the file to read into a
         // string.
-        NamedTempFile file(".llsd",
+        NamedTempFile file("llsd",
                            // NamedTempFile's boost::function constructor
                            // takes a callable. To this callable it passes the
                            // std::ostream with which it's writing the
@@ -1926,7 +1681,7 @@ namespace tut
         // Create an empty data file. This is just a placeholder for our
         // script to write into. Create it to establish a unique name that
         // we know.
-        NamedTempFile file(".llsd", "");
+        NamedTempFile file("llsd", "");
 
         python("write Python notation",
                lambda::_1 <<
diff --git a/indra/test/manageapr.h b/indra/test/manageapr.h
new file mode 100644
index 0000000000..0c1ca7b7be
--- /dev/null
+++ b/indra/test/manageapr.h
@@ -0,0 +1,45 @@
+/**
+ * @file   manageapr.h
+ * @author Nat Goodspeed
+ * @date   2012-01-13
+ * @brief  ManageAPR class for simple test programs
+ * 
+ * $LicenseInfo:firstyear=2012&license=viewerlgpl$
+ * Copyright (c) 2012, Linden Research, Inc.
+ * $/LicenseInfo$
+ */
+
+#if ! defined(LL_MANAGEAPR_H)
+#define LL_MANAGEAPR_H
+
+#include "llapr.h"
+
+/**
+ * Declare a static instance of this class for dead-simple ll_init_apr() at
+ * program startup, ll_cleanup_apr() at termination. This is recommended for
+ * use only with simple test programs. Once you start introducing static
+ * instances of other classes that depend on APR already being initialized,
+ * the indeterminate static-constructor-order problem rears its ugly head.
+ */
+class ManageAPR
+{
+public:
+    ManageAPR()
+    {
+        ll_init_apr();
+    }
+
+    ~ManageAPR()
+    {
+        ll_cleanup_apr();
+    }
+
+    static std::string strerror(apr_status_t rv)
+    {
+        char errbuf[256];
+        apr_strerror(rv, errbuf, sizeof(errbuf));
+        return errbuf;
+    }
+};
+
+#endif /* ! defined(LL_MANAGEAPR_H) */
diff --git a/indra/test/namedtempfile.h b/indra/test/namedtempfile.h
new file mode 100644
index 0000000000..9670d4db53
--- /dev/null
+++ b/indra/test/namedtempfile.h
@@ -0,0 +1,113 @@
+/**
+ * @file   namedtempfile.h
+ * @author Nat Goodspeed
+ * @date   2012-01-13
+ * @brief  NamedTempFile class for tests that need disk files as fixtures.
+ * 
+ * $LicenseInfo:firstyear=2012&license=viewerlgpl$
+ * Copyright (c) 2012, Linden Research, Inc.
+ * $/LicenseInfo$
+ */
+
+#if ! defined(LL_NAMEDTEMPFILE_H)
+#define LL_NAMEDTEMPFILE_H
+
+#include "llapr.h"
+#include "apr_file_io.h"
+#include <string>
+#include <boost/function.hpp>
+#include "boost/lambda/lambda.hpp"
+#include "boost/lambda/bind.hpp"
+#include <iostream>
+#include <sstream>
+
+/**
+ * Create a text file with specified content "somewhere in the
+ * filesystem," cleaning up when it goes out of scope.
+ */
+class NamedTempFile
+{
+public:
+    NamedTempFile(const std::string& pfx, const std::string& content, apr_pool_t* pool=gAPRPoolp):
+        mPool(pool)
+    {
+        createFile(pfx, boost::lambda::_1 << content);
+    }
+
+    // Disambiguate when passing string literal
+    NamedTempFile(const std::string& pfx, const char* content, apr_pool_t* pool=gAPRPoolp):
+        mPool(pool)
+    {
+        createFile(pfx, boost::lambda::_1 << content);
+    }
+
+    // Function that accepts an ostream ref and (presumably) writes stuff to
+    // it, e.g.:
+    // (boost::lambda::_1 << "the value is " << 17 << '\n')
+    typedef boost::function<void(std::ostream&)> Streamer;
+
+    NamedTempFile(const std::string& pfx, const Streamer& func, apr_pool_t* pool=gAPRPoolp):
+        mPool(pool)
+    {
+        createFile(pfx, func);
+    }
+
+    ~NamedTempFile()
+    {
+        ll_apr_assert_status(apr_file_remove(mPath.c_str(), mPool));
+    }
+
+    std::string getName() const { return mPath; }
+
+private:
+    void createFile(const std::string& pfx, const Streamer& func)
+    {
+        // Create file in a temporary place.
+        const char* tempdir = NULL;
+        ll_apr_assert_status(apr_temp_dir_get(&tempdir, mPool));
+
+        // Construct a temp filename template in that directory.
+        char *tempname = NULL;
+        ll_apr_assert_status(apr_filepath_merge(&tempname,
+                                                tempdir,
+                                                (pfx + "XXXXXX").c_str(),
+                                                0,
+                                                mPool));
+
+        // Create a temp file from that template.
+        apr_file_t* fp = NULL;
+        ll_apr_assert_status(apr_file_mktemp(&fp,
+                                             tempname,
+                                             APR_CREATE | APR_WRITE | APR_EXCL,
+                                             mPool));
+        // apr_file_mktemp() alters tempname with the actual name. Not until
+        // now is it valid to capture as our mPath.
+        mPath = tempname;
+
+        // Write desired content.
+        std::ostringstream out;
+        // Stream stuff to it.
+        func(out);
+
+        std::string data(out.str());
+        apr_size_t writelen(data.length());
+        ll_apr_assert_status(apr_file_write(fp, data.c_str(), &writelen));
+        ll_apr_assert_status(apr_file_close(fp));
+        llassert_always(writelen == data.length());
+    }
+
+    void peep()
+    {
+        std::cout << "File '" << mPath << "' contains:\n";
+        std::ifstream reader(mPath.c_str());
+        std::string line;
+        while (std::getline(reader, line))
+            std::cout << line << '\n';
+        std::cout << "---\n";
+    }
+
+    std::string mPath;
+    apr_pool_t* mPool;
+};
+
+#endif /* ! defined(LL_NAMEDTEMPFILE_H) */