From 6b70493ddb1b95a2d3527e2189f5b94f5a2b606f Mon Sep 17 00:00:00 2001
From: Nat Goodspeed <nat@lindenlab.com>
Date: Mon, 14 Oct 2019 15:41:09 -0400
Subject: [PATCH] DRTVWR-476: Make test program --debug switch work like
LOGTEST=DEBUG.
The comments within indra/test/test.cpp promise that --debug is, in fact, like
LOGTEST=DEBUG. Until now, that was a lie. LOGTEST=level displayed log output
on stderr as well as in testprogram.log, while --debug did not.
Add LLError::logToStderr() function, and make initForApplication() (i.e.
commonInit()) call that instead of instantiating RecordToStderr inline. Also
call it when test.cpp recognizes --debug switch.
Remove the mFileRecorder, mFixedBufferRecorder and mFileRecorderFileName
members from SettingsConfig. That tactic doesn't scale.
Instead, add findRecorder<RECORDER>() and removeRecorder<RECORDER>() template
functions to locate (or remove) a RecorderPtr to an object of the specified
subclass. Both are based on an underlying findRecorderPos<RECORDER>() template
function. Since we never expect to manage more than a handful of RecorderPtrs,
and since access to the deleted members is very much application setup rather
than any kind of ongoing access, a search loop suffices.
logToFile() uses removeRecorder<RecordToFile>() rather than removing
mFileRecorder (the only use of mFileRecorder).
logToFixedBuffer() uses removeRecorder<RecordToFixedBuffer>() rather than
removing mFixedBufferRecorder (the only use of mFixedBufferRecorder).
Make RecordToFile store the filename with which it was instantiated. Add a
getFilename() method to retrieve it. logFileName() is now based on
findRecorder<RecordToFile>() instead of mFileRecorderFileName (the only use of
mFileRecorderFileName).
Make RecordToStderr::mUseANSI a simple bool rather than a three-state enum,
and set it immediately on construction. Apparently the reason it was set
lazily was because it consults its own checkANSI() method, and of course
'this' doesn't acquire the leaf class type until the constructor has completed
successfully. But since nothing in checkANSI() depends on anything else in
RecordToStderr, making it static solves that problem.
---
indra/llcommon/llerror.cpp | 196 ++++++++++++++++++++------------
indra/llcommon/llerrorcontrol.h | 1 +
indra/test/test.cpp | 3 +
3 files changed, 127 insertions(+), 73 deletions(-)
diff --git a/indra/llcommon/llerror.cpp b/indra/llcommon/llerror.cpp
index 2d4898f7be1..acd863a3165 100644
--- a/indra/llcommon/llerror.cpp
+++ b/indra/llcommon/llerror.cpp
@@ -118,27 +118,28 @@ namespace {
class RecordToFile : public LLError::Recorder
{
public:
- RecordToFile(const std::string& filename)
+ RecordToFile(const std::string& filename):
+ mName(filename)
{
mFile.open(filename.c_str(), std::ios_base::out | std::ios_base::app);
if (!mFile)
{
LL_INFOS() << "Error setting log file to " << filename << LL_ENDL;
}
- else
- {
- if (!LLError::getAlwaysFlush())
- {
- mFile.sync_with_stdio(false);
- }
- }
+ else
+ {
+ if (!LLError::getAlwaysFlush())
+ {
+ mFile.sync_with_stdio(false);
+ }
+ }
}
-
+
~RecordToFile()
{
mFile.close();
}
-
+
virtual bool enabled() override
{
#ifdef LL_RELEASE_FOR_DOWNLOAD
@@ -148,11 +149,13 @@ namespace {
#endif
}
- bool okay() { return mFile.good(); }
-
- virtual void recordMessage(LLError::ELevel level,
- const std::string& message) override
- {
+ bool okay() const { return mFile.good(); }
+
+ std::string getFilename() const { return mName; }
+
+ virtual void recordMessage(LLError::ELevel level,
+ const std::string& message) override
+ {
if (LLError::getAlwaysFlush())
{
mFile << message << std::endl;
@@ -161,9 +164,10 @@ namespace {
{
mFile << message << "\n";
}
- }
-
+ }
+
private:
+ const std::string mName;
llofstream mFile;
};
@@ -171,7 +175,7 @@ namespace {
class RecordToStderr : public LLError::Recorder
{
public:
- RecordToStderr(bool timestamp) : mUseANSI(ANSI_PROBE)
+ RecordToStderr(bool timestamp) : mUseANSI(checkANSI())
{
this->showMultiline(true);
}
@@ -184,10 +188,7 @@ namespace {
virtual void recordMessage(LLError::ELevel level,
const std::string& message) override
{
- if (ANSI_PROBE == mUseANSI)
- mUseANSI = (checkANSI() ? ANSI_YES : ANSI_NO);
-
- if (ANSI_YES == mUseANSI)
+ if (mUseANSI)
{
// Default all message levels to bold so we can distinguish our own messages from those dumped by subprocesses and libraries.
colorANSI("1"); // bold
@@ -206,16 +207,11 @@ namespace {
}
}
fprintf(stderr, "%s\n", message.c_str());
- if (ANSI_YES == mUseANSI) colorANSI("0"); // reset
+ if (mUseANSI) colorANSI("0"); // reset
}
private:
- enum ANSIState
- {
- ANSI_PROBE,
- ANSI_YES,
- ANSI_NO
- } mUseANSI;
+ bool mUseANSI;
void colorANSI(const std::string color)
{
@@ -223,7 +219,7 @@ namespace {
fprintf(stderr, "\033[%sm", color.c_str() );
};
- bool checkANSI(void)
+ static bool checkANSI(void)
{
#if LL_LINUX || LL_DARWIN
// Check whether it's okay to use ANSI; if stderr is
@@ -491,14 +487,11 @@ namespace LLError
LLError::FatalFunction mCrashFunction;
LLError::TimeFunction mTimeFunction;
-
+
Recorders mRecorders;
- RecorderPtr mFileRecorder;
- RecorderPtr mFixedBufferRecorder;
- std::string mFileRecorderFileName;
-
- int mShouldLogCallCounter;
-
+
+ int mShouldLogCallCounter;
+
private:
SettingsConfig();
};
@@ -532,9 +525,6 @@ namespace LLError
mCrashFunction(NULL),
mTimeFunction(NULL),
mRecorders(),
- mFileRecorder(),
- mFixedBufferRecorder(),
- mFileRecorderFileName(),
mShouldLogCallCounter(0)
{
}
@@ -686,20 +676,19 @@ namespace
void commonInit(const std::string& user_dir, const std::string& app_dir, bool log_to_stderr = true)
{
LLError::Settings::getInstance()->reset();
-
+
LLError::setDefaultLevel(LLError::LEVEL_INFO);
- LLError::setAlwaysFlush(true);
- LLError::setEnabledLogTypesMask(0xFFFFFFFF);
+ LLError::setAlwaysFlush(true);
+ LLError::setEnabledLogTypesMask(0xFFFFFFFF);
LLError::setFatalFunction(LLError::crashAndLoop);
LLError::setTimeFunction(LLError::utcTime);
// log_to_stderr is only false in the unit and integration tests to keep builds quieter
if (log_to_stderr && shouldLogToStderr())
{
- LLError::RecorderPtr recordToStdErr(new RecordToStderr(stderrLogWantsTime()));
- LLError::addRecorder(recordToStdErr);
+ LLError::logToStderr();
}
-
+
#if LL_WINDOWS
LLError::RecorderPtr recordToWinDebug(new RecordToWinDebug());
LLError::addRecorder(recordToWinDebug);
@@ -997,49 +986,110 @@ namespace LLError
s->mRecorders.erase(std::remove(s->mRecorders.begin(), s->mRecorders.end(), recorder),
s->mRecorders.end());
}
+
+ // Find an entry in SettingsConfig::mRecorders whose RecorderPtr points to
+ // a Recorder subclass of type RECORDER. Return, not a RecorderPtr (which
+ // points to the Recorder base class), but a shared_ptr<RECORDER> which
+ // specifically points to the concrete RECORDER subclass instance, along
+ // with a Recorders::iterator indicating the position of that entry in
+ // mRecorders. The shared_ptr might be empty (operator!() returns true) if
+ // there was no such RECORDER subclass instance in mRecorders.
+ template <typename RECORDER>
+ std::pair<boost::shared_ptr<RECORDER>, Recorders::iterator>
+ findRecorderPos()
+ {
+ SettingsConfigPtr s = Settings::instance().getSettingsConfig();
+ // Since we promise to return an iterator, use a classic iterator
+ // loop.
+ auto end{s->mRecorders.end()};
+ for (Recorders::iterator it{s->mRecorders.begin()}; it != end; ++it)
+ {
+ // *it is a RecorderPtr, a shared_ptr<Recorder>. Use a
+ // dynamic_pointer_cast to try to downcast to test if it's also a
+ // shared_ptr<RECORDER>.
+ auto ptr = boost::dynamic_pointer_cast<RECORDER>(*it);
+ if (ptr)
+ {
+ // found the entry we want
+ return { ptr, it };
+ }
+ }
+ // dropped out of the loop without finding any such entry -- instead
+ // of default-constructing Recorders::iterator (which might or might
+ // not be valid), return a value that is valid but not dereferenceable.
+ return { {}, end };
+ }
+
+ // Find an entry in SettingsConfig::mRecorders whose RecorderPtr points to
+ // a Recorder subclass of type RECORDER. Return, not a RecorderPtr (which
+ // points to the Recorder base class), but a shared_ptr<RECORDER> which
+ // specifically points to the concrete RECORDER subclass instance. The
+ // shared_ptr might be empty (operator!() returns true) if there was no
+ // such RECORDER subclass instance in mRecorders.
+ template <typename RECORDER>
+ boost::shared_ptr<RECORDER> findRecorder()
+ {
+ return findRecorderPos<RECORDER>().first;
+ }
+
+ // Remove an entry from SettingsConfig::mRecorders whose RecorderPtr
+ // points to a Recorder subclass of type RECORDER. Return true if there
+ // was one and we removed it, false if there wasn't one to start with.
+ template <typename RECORDER>
+ bool removeRecorder()
+ {
+ auto found = findRecorderPos<RECORDER>();
+ if (found.first)
+ {
+ SettingsConfigPtr s = Settings::instance().getSettingsConfig();
+ s->mRecorders.erase(found.second);
+ }
+ return bool(found.first);
+ }
}
namespace LLError
{
void logToFile(const std::string& file_name)
{
- SettingsConfigPtr s = Settings::getInstance()->getSettingsConfig();
+ // remove any previous Recorder filling this role
+ removeRecorder<RecordToFile>();
- removeRecorder(s->mFileRecorder);
- s->mFileRecorder.reset();
- s->mFileRecorderFileName.clear();
-
if (!file_name.empty())
{
- RecorderPtr recordToFile(new RecordToFile(file_name));
- if (boost::dynamic_pointer_cast<RecordToFile>(recordToFile)->okay())
- {
- s->mFileRecorderFileName = file_name;
- s->mFileRecorder = recordToFile;
- addRecorder(recordToFile);
- }
+ boost::shared_ptr<RecordToFile> recordToFile(new RecordToFile(file_name));
+ if (recordToFile->okay())
+ {
+ addRecorder(recordToFile);
+ }
}
}
-
- void logToFixedBuffer(LLLineBuffer* fixedBuffer)
+
+ std::string logFileName()
{
- SettingsConfigPtr s = Settings::getInstance()->getSettingsConfig();
+ auto found = findRecorder<RecordToFile>();
+ return found? found->getFilename() : std::string();
+ }
- removeRecorder(s->mFixedBufferRecorder);
- s->mFixedBufferRecorder.reset();
-
- if (fixedBuffer)
- {
- RecorderPtr recordToFixedBuffer(new RecordToFixedBuffer(fixedBuffer));
- s->mFixedBufferRecorder = recordToFixedBuffer;
- addRecorder(recordToFixedBuffer);
+ void logToStderr()
+ {
+ if (! findRecorder<RecordToStderr>())
+ {
+ RecorderPtr recordToStdErr(new RecordToStderr(stderrLogWantsTime()));
+ addRecorder(recordToStdErr);
}
- }
+ }
- std::string logFileName()
+ void logToFixedBuffer(LLLineBuffer* fixedBuffer)
{
- SettingsConfigPtr s = Settings::getInstance()->getSettingsConfig();
- return s->mFileRecorderFileName;
+ // remove any previous Recorder filling this role
+ removeRecorder<RecordToFixedBuffer>();
+
+ if (fixedBuffer)
+ {
+ RecorderPtr recordToFixedBuffer(new RecordToFixedBuffer(fixedBuffer));
+ addRecorder(recordToFixedBuffer);
+ }
}
}
diff --git a/indra/llcommon/llerrorcontrol.h b/indra/llcommon/llerrorcontrol.h
index 276d22fc361..bfa22690252 100644
--- a/indra/llcommon/llerrorcontrol.h
+++ b/indra/llcommon/llerrorcontrol.h
@@ -183,6 +183,7 @@ namespace LLError
// each error message is passed to each recorder via recordMessage()
LL_COMMON_API void logToFile(const std::string& filename);
+ LL_COMMON_API void logToStderr();
LL_COMMON_API void logToFixedBuffer(LLLineBuffer*);
// Utilities to add recorders for logging to a file or a fixed buffer
// A second call to the same function will remove the logger added
diff --git a/indra/test/test.cpp b/indra/test/test.cpp
index b14c2eb255a..51f0e800439 100644
--- a/indra/test/test.cpp
+++ b/indra/test/test.cpp
@@ -611,6 +611,9 @@ int main(int argc, char **argv)
wait_at_exit = true;
break;
case 'd':
+ // this is what LLError::initForApplication() does internally
+ // when you pass log_to_stderr=true
+ LLError::logToStderr();
LLError::setDefaultLevel(LLError::LEVEL_DEBUG);
break;
case 'x':
--
GitLab