SL-18837: Use Boost.Filesystem for NamedTempFile, instead of APR.

indra/test/namedtempfile.h

@@ -13,15 +13,15 @@
 #define LL_NAMEDTEMPFILE_H
 
 #include "llerror.h"
-#include "llapr.h"
-#include "apr_file_io.h"
+#include "stringize.h"
 #include <string>
-#include <boost/function.hpp>
-#include <boost/phoenix/core/argument.hpp>
-#include <boost/phoenix/operator/bitwise.hpp>
 #include <boost/noncopyable.hpp>
+#include <boost/filesystem.hpp>
+#include <boost/filesystem/fstream.hpp>
+#include <functional>
 #include <iostream>
 #include <sstream>
+#include <string_view>
 
 /**
  * Create a text file with specified content "somewhere in the
@@ -31,41 +31,36 @@ class NamedTempFile: public boost::noncopyable
 {
     LOG_CLASS(NamedTempFile);
 public:
-    NamedTempFile(const std::string& pfx, const std::string& content, apr_pool_t* pool=gAPRPoolp):
-        mPool(pool)
+    NamedTempFile(const std::string_view& pfx,
+                  const std::string_view& content,
+                  const std::string_view& sfx=std::string_view(""))
     {
-        createFile(pfx, boost::phoenix::placeholders::arg1 << content);
-    }
-
-    // Disambiguate when passing string literal
-    NamedTempFile(const std::string& pfx, const char* content, apr_pool_t* pool=gAPRPoolp):
-        mPool(pool)
-    {
-        createFile(pfx, boost::phoenix::placeholders::arg1 << content);
+        createFile(pfx, [&content](std::ostream& out){ out << content; }, sfx);
     }
 
     // Function that accepts an ostream ref and (presumably) writes stuff to
     // it, e.g.:
     // (boost::phoenix::placeholders::arg1 << "the value is " << 17 << '\n')
-    typedef boost::function<void(std::ostream&)> Streamer;
+    typedef std::function<void(std::ostream&)> Streamer;
 
-    NamedTempFile(const std::string& pfx, const Streamer& func, apr_pool_t* pool=gAPRPoolp):
-        mPool(pool)
+    NamedTempFile(const std::string_view& pfx,
+                  const Streamer& func,
+                  const std::string_view& sfx=std::string_view(""))
     {
-        createFile(pfx, func);
+        createFile(pfx, func, sfx);
     }
 
     virtual ~NamedTempFile()
     {
-        ll_apr_assert_status(apr_file_remove(mPath.c_str(), mPool));
+        boost::filesystem::remove(mPath);
     }
 
-    virtual std::string getName() const { return mPath; }
+    virtual std::string getName() const { return mPath.native(); }
 
     void peep()
     {
         std::cout << "File '" << mPath << "' contains:\n";
-        std::ifstream reader(mPath.c_str());
+        boost::filesystem::ifstream reader(mPath);
         std::string line;
         while (std::getline(reader, line))
             std::cout << line << '\n';
@@ -73,92 +68,40 @@ class NamedTempFile: public boost::noncopyable
     }
 
 protected:
-    void createFile(const std::string& pfx, const Streamer& func)
+    void createFile(const std::string_view& pfx,
+                    const Streamer& func,
+                    const std::string_view& sfx)
     {
         // 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;
+        boost::filesystem::path tempname{ boost::filesystem::temp_directory_path() };
+        // unique_path() recommended model
+        tempname += stringize(pfx, "%%%%-%%%%-%%%%-%%%%", sfx);
+        mPath = boost::filesystem::unique_path(tempname);
+        boost::filesystem::ofstream out{ mPath };
 
         // 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());
     }
 
-    std::string mPath;
-    apr_pool_t* mPool;
+    boost::filesystem::path mPath;
 };
 
 /**
  * Create a NamedTempFile with a specified filename extension. This is useful
  * when, for instance, you must be able to use the file in a Python import
  * statement.
- *
- * A NamedExtTempFile actually has two different names. We retain the original
- * no-extension name as a placeholder in the temp directory to ensure
- * uniqueness; to that we link the name plus the desired extension. Naturally,
- * both must be removed on destruction.
  */
 class NamedExtTempFile: public NamedTempFile
 {
     LOG_CLASS(NamedExtTempFile);
 public:
-    NamedExtTempFile(const std::string& ext, const std::string& content, apr_pool_t* pool=gAPRPoolp):
-        NamedTempFile(remove_dot(ext), content, pool),
-        mLink(mPath + ensure_dot(ext))
-    {
-        linkto(mLink);
-    }
+    NamedExtTempFile(const std::string& ext, const std::string_view& content):
+        NamedTempFile(remove_dot(ext), content, ensure_dot(ext))
+    {}
 
-    // Disambiguate when passing string literal
-    NamedExtTempFile(const std::string& ext, const char* content, apr_pool_t* pool=gAPRPoolp):
-        NamedTempFile(remove_dot(ext), content, pool),
-        mLink(mPath + ensure_dot(ext))
-    {
-        linkto(mLink);
-    }
-
-    NamedExtTempFile(const std::string& ext, const Streamer& func, apr_pool_t* pool=gAPRPoolp):
-        NamedTempFile(remove_dot(ext), func, pool),
-        mLink(mPath + ensure_dot(ext))
-    {
-        linkto(mLink);
-    }
-
-    virtual ~NamedExtTempFile()
-    {
-        ll_apr_assert_status(apr_file_remove(mLink.c_str(), mPool));
-    }
-
-    // Since the caller has gone to the trouble to create the name with the
-    // extension, that should be the name we return. In this class, mPath is
-    // just a placeholder to ensure that future createFile() calls won't
-    // collide.
-    virtual std::string getName() const { return mLink; }
+    NamedExtTempFile(const std::string& ext, const Streamer& func):
+        NamedTempFile(remove_dot(ext), func, ensure_dot(ext))
+    {}
 
     static std::string ensure_dot(const std::string& ext)
     {
@@ -175,7 +118,7 @@ class NamedExtTempFile: public NamedTempFile
         {
             return ext;
         }
-        return std::string(".") + ext;
+        return "." + ext;
     }
 
     static std::string remove_dot(const std::string& ext)
@@ -187,19 +130,6 @@ class NamedExtTempFile: public NamedTempFile
         }
         return ext.substr(found);
     }
-
-private:
-    void linkto(const std::string& path)
-    {
-        // This method assumes that since mPath (without extension) is
-        // guaranteed by apr_file_mktemp() to be unique, then (mPath + any
-        // extension) is also unique. This is likely, though not guaranteed:
-        // files could be created in the same temp directory other than by
-        // this class.
-        ll_apr_assert_status(apr_file_link(mPath.c_str(), path.c_str()));
-    }
-
-    std::string mLink;
 };
 
 #endif /* ! defined(LL_NAMEDTEMPFILE_H) */

indra/test/test.cpp

@@ -97,10 +97,10 @@ class LLReplayLog
 class RecordToTempFile : public LLError::Recorder, public boost::noncopyable
 {
 public:
-	RecordToTempFile(apr_pool_t* pPool)
+	RecordToTempFile()
 		: LLError::Recorder(),
 		boost::noncopyable(),
-		mTempFile("log", "", pPool),
+		mTempFile("log", ""),
 		mFile(mTempFile.getName().c_str())
 	{
 	}
@@ -141,11 +141,11 @@ class RecordToTempFile : public LLError::Recorder, public boost::noncopyable
 class LLReplayLogReal: public LLReplayLog, public boost::noncopyable
 {
 public:
-	LLReplayLogReal(LLError::ELevel level, apr_pool_t* pool)
+	LLReplayLogReal(LLError::ELevel level)
 		: LLReplayLog(),
 		boost::noncopyable(),
 		mOldSettings(LLError::saveAndResetSettings()),
-		mRecorder(new RecordToTempFile(pool))
+		mRecorder(new RecordToTempFile())
 	{
 		LLError::setFatalFunction(wouldHaveCrashed);
 		LLError::setDefaultLevel(level);
@@ -624,7 +624,7 @@ int main(int argc, char **argv)
 		if (LOGFAIL && *LOGFAIL)
 		{
 			LLError::ELevel level = LLError::decodeLevel(LOGFAIL);
-			replayer.reset(new LLReplayLogReal(level, gAPRPoolp));
+			replayer.reset(new LLReplayLogReal(level));
 		}
 	}
 	LLError::setFatalFunction(wouldHaveCrashed);