DRTVWR-476: Encapsulate dup()/dup2() fd saving as LLTempRedirect.

4 files changed, 241 insertions, 38 deletions

diff --git a/indra/llcommon/CMakeLists.txt b/indra/llcommon/CMakeLists.txt
index 035b379246..d17ee4c70a 100644
--- a/indra/llcommon/CMakeLists.txt
+++ b/indra/llcommon/CMakeLists.txt
@@ -104,6 +104,7 @@ set(llcommon_SOURCE_FILES
     llstring.cpp
     llstringtable.cpp
     llsys.cpp
+    lltempredirect.cpp
     llthread.cpp
     llthreadlocalstorage.cpp
     llthreadsafequeue.cpp
@@ -228,6 +229,7 @@ set(llcommon_HEADER_FILES
     llstaticstringtable.h
     llstatsaccumulator.h
     llsys.h
+    lltempredirect.h
     llthread.h
     llthreadlocalstorage.h
     llthreadsafequeue.h
diff --git a/indra/llcommon/llerror.cpp b/indra/llcommon/llerror.cpp
index 457965b1fd..2ae2cb6cbc 100644
--- a/indra/llcommon/llerror.cpp
+++ b/indra/llcommon/llerror.cpp
@@ -39,8 +39,6 @@
 #if !LL_WINDOWS
 # include <syslog.h>
 # include <unistd.h>
-#else
-# include <io.h>
 #endif // !LL_WINDOWS
 #include <vector>
 #include "string.h"
@@ -54,20 +52,7 @@
 #include "llsingleton.h"
 #include "llstl.h"
 #include "lltimer.h"
-
-#if LL_WINDOWS
-#define fhclose  _close
-#define fhdup    _dup
-#define fhdup2   _dup2
-#define fhfdopen _fdopen
-#define fhfileno _fileno
-#else
-#define fhclose  ::close
-#define fhdup    ::dup
-#define fhdup2   ::dup2
-#define fhfdopen ::fdopen
-#define fhfileno ::fileno
-#endif
+#include "lltempredirect.h"
 
 namespace LLError
 {
@@ -80,7 +65,6 @@ namespace LLError
 		LLSINGLETON(Settings);
 	public:
 		SettingsConfigPtr getSettingsConfig();
-		~Settings();
 
 		void reset();
 		SettingsStoragePtr saveAndReset();
@@ -90,7 +74,7 @@ namespace LLError
 
 	private:
 		SettingsConfigPtr mSettingsConfig;
-		int mDupStderr;
+		LLTempRedirect mRedirect;
 	};
 
 } // namespace LLError
@@ -162,8 +146,7 @@ namespace {
 	public:
 		RecordToFile(const std::string& filename):
 			mName(filename),
-			mFile(LLFile::fopen(filename, "a")),
-			mSavedStderr(LLError::Settings::instance().getDupStderr())
+			mFile(LLFile::fopen(filename, "a"))
 		{
 			if (!mFile)
 			{
@@ -174,16 +157,13 @@ namespace {
 				// We use a number of classic-C libraries, some of which write
 				// log output to stderr. The trouble with that is that unless
 				// you launch the viewer from a console, stderr output is
-				// lost. Redirect STDERR_FILENO to write into this log file.
-				fhdup2(fhfileno(mFile), fhfileno(stderr));
+				// lost. Redirect stderr to write into this log file.
+				mRedirect = LLTempRedirect(mFile, stderr);
 			}
 		}
 
 		~RecordToFile()
 		{
-			// restore stderr to its original fileno so any subsequent output
-			// to stderr goes to original stream
-			fhdup2(mSavedStderr, fhfileno(stderr));
 			mFile.close();
 		}
 
@@ -214,7 +194,7 @@ namespace {
 	private:
 		const std::string mName;
 		LLUniqueFile mFile;
-		int mSavedStderr;
+		LLTempRedirect mRedirect;
 	};
 
 
@@ -225,7 +205,7 @@ namespace {
 			mUseANSI(checkANSI()),
 			// use duplicate stderr file handle so THIS output isn't affected
 			// by our internal redirection of all (other) stderr output
-			mStderr(fhfdopen(LLError::Settings::instance().getDupStderr(), "a"))
+			mStderr(llfd::open(LLError::Settings::instance().getDupStderr(), "a"))
 		{
 			this->showMultiline(true);
 		}
@@ -276,7 +256,7 @@ namespace {
 			// Check whether it's okay to use ANSI; if stderr is
 			// a tty then we assume yes.  Can be turned off with
 			// the LL_NO_ANSI_COLOR env var.
-			return (0 != isatty(fhfileno(stderr))) &&
+			return (0 != isatty(fileno(stderr))) &&
 				(NULL == getenv("LL_NO_ANSI_COLOR"));
 #endif // LL_LINUX
 			return false;
@@ -572,16 +552,8 @@ namespace LLError
 	Settings::Settings():
 		mSettingsConfig(new SettingsConfig()),
 		// duplicate stderr file handle right away
-		mDupStderr(fhdup(fhfileno(stderr)))
-	{
-	}
-
-	Settings::~Settings()
+		mRedirect(NULL, stderr)
 	{
-		// restore original stderr
-		fhdup2(mDupStderr, fhfileno(stderr));
-		// and close the duplicate
-		fhclose(mDupStderr);
 	}
 
 	SettingsConfigPtr Settings::getSettingsConfig()
@@ -611,7 +583,7 @@ namespace LLError
 
 	int Settings::getDupStderr() const
 	{
-		return mDupStderr;
+		return mRedirect.getOriginalTarget();
 	}
 
 	bool is_available()
diff --git a/indra/llcommon/lltempredirect.cpp b/indra/llcommon/lltempredirect.cpp
new file mode 100644
index 0000000000..1ae3116b77
--- /dev/null
+++ b/indra/llcommon/lltempredirect.cpp
@@ -0,0 +1,138 @@
+/**
+ * @file   lltempredirect.cpp
+ * @author Nat Goodspeed
+ * @date   2019-10-31
+ * @brief  Implementation for lltempredirect.
+ * 
+ * $LicenseInfo:firstyear=2019&license=viewerlgpl$
+ * Copyright (c) 2019, Linden Research, Inc.
+ * $/LicenseInfo$
+ */
+
+// Precompiled header
+#include "linden_common.h"
+// associated header
+#include "lltempredirect.h"
+// STL headers
+// std headers
+#if !LL_WINDOWS
+# include <unistd.h>
+#else
+# include <io.h>
+#endif // !LL_WINDOWS
+// external library headers
+// other Linden headers
+
+/*****************************************************************************
+*   llfd
+*****************************************************************************/
+// We could restate the implementation of each of llfd::close(), etc., but
+// this is way more succinct.
+#if LL_WINDOWS
+#define fhclose  _close
+#define fhdup    _dup
+#define fhdup2   _dup2
+#define fhfdopen _fdopen
+#define fhfileno _fileno
+#else
+#define fhclose  ::close
+#define fhdup    ::dup
+#define fhdup2   ::dup2
+#define fhfdopen ::fdopen
+#define fhfileno ::fileno
+#endif
+
+int llfd::close(int fd)
+{
+    return fhclose(fd);
+}
+
+int llfd::dup(int target)
+{
+    return fhdup(target);
+}
+
+int llfd::dup2(int target, int reference)
+{
+    return fhdup2(target, reference);
+}
+
+FILE* llfd::open(int fd, const char* mode)
+{
+    return fhfdopen(fd, mode);
+}
+
+int llfd::fileno(FILE* stream)
+{
+    return fhfileno(stream);
+}
+
+/*****************************************************************************
+*   LLTempRedirect
+*****************************************************************************/
+LLTempRedirect::LLTempRedirect():
+    mOrigTarget(-1),                // -1 is an invalid file descriptor
+    mReference(-1)
+{}
+
+LLTempRedirect::LLTempRedirect(FILE* target, FILE* reference):
+    LLTempRedirect((target?    fhfileno(target)    : -1),
+                   (reference? fhfileno(reference) : -1))
+{}
+
+LLTempRedirect::LLTempRedirect(int target, int reference):
+    // capture a duplicate file descriptor for the file originally targeted by
+    // 'reference'
+    mOrigTarget((reference >= 0)? fhdup(reference) : -1),
+    mReference(reference)
+{
+    if (target >= 0 && reference >= 0)
+    {
+        // As promised, force 'reference' to refer to 'target'. This first
+        // implicitly closes 'reference', which is why we first capture a
+        // duplicate so the original target file stays open.
+        fhdup2(target, reference);
+    }
+}
+
+LLTempRedirect::LLTempRedirect(LLTempRedirect&& other)
+{
+    mOrigTarget = other.mOrigTarget;
+    mReference  = other.mReference;
+    // other LLTempRedirect must be in moved-from state so its destructor
+    // won't repeat the same operations as ours!
+    other.mOrigTarget = -1;
+    other.mReference  = -1;
+}
+
+LLTempRedirect::~LLTempRedirect()
+{
+    reset();
+}
+
+void LLTempRedirect::reset()
+{
+    // If this instance was default-constructed (or constructed with an
+    // invalid file descriptor), skip the following.
+    if (mOrigTarget >= 0)
+    {
+        // Restore mReference to point to mOrigTarget. This implicitly closes
+        // the duplicate created by our constructor of its 'target' file
+        // descriptor.
+        fhdup2(mOrigTarget, mReference);
+        // mOrigTarget has served its purpose
+        fhclose(mOrigTarget);
+        // assign these because reset() is also responsible for a "moved from"
+        // instance
+        mOrigTarget = -1;
+        mReference  = -1;
+    }
+}
+
+LLTempRedirect& LLTempRedirect::operator=(LLTempRedirect&& other)
+{
+    reset();
+    std::swap(mOrigTarget, other.mOrigTarget);
+    std::swap(mReference,  other.mReference);
+    return *this;
+}
diff --git a/indra/llcommon/lltempredirect.h b/indra/llcommon/lltempredirect.h
new file mode 100644
index 0000000000..33e05dc06b
--- /dev/null
+++ b/indra/llcommon/lltempredirect.h
@@ -0,0 +1,91 @@
+/**
+ * @file   lltempredirect.h
+ * @author Nat Goodspeed
+ * @date   2019-10-31
+ * @brief  RAII low-level file-descriptor redirection
+ * 
+ * $LicenseInfo:firstyear=2019&license=viewerlgpl$
+ * Copyright (c) 2019, Linden Research, Inc.
+ * $/LicenseInfo$
+ */
+
+#if ! defined(LL_LLTEMPREDIRECT_H)
+#define LL_LLTEMPREDIRECT_H
+
+// Functions in this namespace are intended to insulate the caller from the
+// aggravating distinction between ::close() and Microsoft _close().
+namespace llfd
+{
+
+int close(int fd);
+int dup(int target);
+int dup2(int target, int reference);
+FILE* open(int fd, const char* mode);
+int fileno(FILE* stream);
+
+} // namespace llfd
+
+/**
+ * LLTempRedirect is an RAII class that performs file redirection on low-level
+ * file descriptors, expressed as ints. (Use llfd::fileno() to obtain the file
+ * descriptor from a classic-C FILE*. There is no portable way to obtain the
+ * file descriptor from a std::fstream.)
+ *
+ * Instantiate LLTempRedirect with a target file descriptor (e.g. for some
+ * open file) and a reference file descriptor (e.g. for stderr). From that
+ * point until the LLTempRedirect instance is destroyed, all OS-level writes
+ * to the reference file descriptor will be redirected to the target file.
+ *
+ * Because dup2() is used for redirection, the original passed target file
+ * descriptor remains open. If you want LLTempRedirect's destructor to close
+ * the target file, close() the target file descriptor after passing it to
+ * LLTempRedirect's constructor.
+ *
+ * LLTempRedirect's constructor saves the original target of the reference
+ * file descriptor. Its destructor restores the reference file descriptor to
+ * point once again to its original target.
+ */
+class LLTempRedirect
+{
+public:
+    LLTempRedirect();
+    /**
+     * For the lifespan of this LLTempRedirect instance, all writes to
+     * 'reference' will be redirected to 'target'. When this LLTempRedirect is
+     * destroyed, the original target for 'reference' will be restored.
+     *
+     * Pass 'target' as NULL if you simply want to save and restore
+     * 'reference' against possible redirection in the meantime.
+     */
+    LLTempRedirect(FILE* target, FILE* reference);
+    /**
+     * For the lifespan of this LLTempRedirect instance, all writes to
+     * 'reference' will be redirected to 'target'. When this LLTempRedirect is
+     * destroyed, the original target for 'reference' will be restored.
+     *
+     * Pass 'target' as -1 if you simply want to save and restore
+     * 'reference' against possible redirection in the meantime.
+     */
+    LLTempRedirect(int target,   int reference);
+    LLTempRedirect(const LLTempRedirect&) = delete;
+    LLTempRedirect(LLTempRedirect&& other);
+
+    ~LLTempRedirect();
+
+    LLTempRedirect& operator=(const LLTempRedirect&) = delete;
+    LLTempRedirect& operator=(LLTempRedirect&& other);
+
+    /// returns (duplicate file descriptor for) the original target of the
+    /// 'reference' file descriptor passed to our constructor
+    int getOriginalTarget() const { return mOrigTarget; }
+    /// returns the original 'reference' file descriptor passed to our
+    /// constructor
+    int getReference()      const { return mReference; }
+
+private:
+    void reset();
+
+    int mOrigTarget, mReference;
+};
+
+#endif /* ! defined(LL_LLTEMPREDIRECT_H) */