From d87cc1859f3f96b98a627fdc674e297e78438681 Mon Sep 17 00:00:00 2001
From: Oz Linden <oz@lindenlab.com>
Date: Thu, 11 Oct 2018 14:17:52 -0400
Subject: Modify logging so that the in-viewer console and stderr do not escape
line breaks Improve the implementation so that escaping is computed only once
---
indra/llcommon/llerror.cpp | 149 ++++++++++++++++++++++------------
indra/llcommon/llerrorcontrol.h | 19 +++--
indra/llcommon/tests/llerror_test.cpp | 68 ++++++++--------
3 files changed, 148 insertions(+), 88 deletions(-)
(limited to 'indra/llcommon')
diff --git a/indra/llcommon/llerror.cpp b/indra/llcommon/llerror.cpp
index 6dfb4bf028..668ea1f7d2 100644
--- a/indra/llcommon/llerror.cpp
+++ b/indra/llcommon/llerror.cpp
@@ -119,8 +119,6 @@ namespace {
{
LL_INFOS() << "Error setting log file to " << filename << LL_ENDL;
}
- mWantsTime = true;
- mWantsTags = true;
}
~RecordToFile()
@@ -146,7 +144,7 @@ namespace {
public:
RecordToStderr(bool timestamp) : mUseANSI(ANSI_PROBE)
{
- mWantsTime = timestamp;
+ this->showMultiline(true);
}
virtual void recordMessage(LLError::ELevel level,
@@ -207,7 +205,13 @@ namespace {
class RecordToFixedBuffer : public LLError::Recorder
{
public:
- RecordToFixedBuffer(LLLineBuffer* buffer) : mBuffer(buffer) { }
+ RecordToFixedBuffer(LLLineBuffer* buffer)
+ : mBuffer(buffer)
+ {
+ this->showMultiline(true);
+ this->showTags(false);
+ this->showLocation(false);
+ }
virtual void recordMessage(LLError::ELevel level,
const std::string& message)
@@ -224,7 +228,11 @@ namespace {
{
public:
RecordToWinDebug()
- {}
+ {
+ this->showMultiline(true);
+ this->showTags(false);
+ this->showLocation(false);
+ }
virtual void recordMessage(LLError::ELevel level,
const std::string& message)
@@ -411,8 +419,6 @@ namespace LLError
public:
virtual ~SettingsConfig();
- bool mPrintLocation;
-
LLError::ELevel mDefaultLevel;
LevelMap mFunctionLevelMap;
@@ -453,7 +459,6 @@ namespace LLError
SettingsConfig::SettingsConfig()
: LLRefCount(),
- mPrintLocation(false),
mDefaultLevel(LLError::LEVEL_DEBUG),
mFunctionLevelMap(),
mClassLevelMap(),
@@ -655,12 +660,6 @@ namespace LLError
commonInit(user_dir, app_dir, log_to_stderr);
}
- void setPrintLocation(bool print)
- {
- SettingsConfigPtr s = Settings::getInstance()->getSettingsConfig();
- s->mPrintLocation = print;
- }
-
void setFatalFunction(const FatalFunction& f)
{
SettingsConfigPtr s = Settings::getInstance()->getSettingsConfig();
@@ -775,7 +774,6 @@ namespace LLError
s->mTagLevelMap.clear();
s->mUniqueLogMessages.clear();
- setPrintLocation(config["print-location"]);
setDefaultLevel(decodeLevel(config["default-level"]));
LLSD sets = config["settings"];
@@ -798,11 +796,12 @@ namespace LLError
namespace LLError
{
Recorder::Recorder()
- : mWantsTime(false),
- mWantsTags(false),
- mWantsLevel(true),
- mWantsLocation(false),
- mWantsFunctionName(true)
+ : mWantsTime(true)
+ , mWantsTags(true)
+ , mWantsLevel(true)
+ , mWantsLocation(true)
+ , mWantsFunctionName(true)
+ , mWantsMultiline(false)
{
}
@@ -839,6 +838,42 @@ namespace LLError
return mWantsFunctionName;
}
+ // virtual
+ bool Recorder::wantsMultiline()
+ {
+ return mWantsMultiline;
+ }
+
+ void Recorder::showTime(bool show)
+ {
+ mWantsTime = show;
+ }
+
+ void Recorder::showTags(bool show)
+ {
+ mWantsTags = show;
+ }
+
+ void Recorder::showLevel(bool show)
+ {
+ mWantsLevel = show;
+ }
+
+ void Recorder::showLocation(bool show)
+ {
+ mWantsLocation = show;
+ }
+
+ void Recorder::showFunctionName(bool show)
+ {
+ mWantsFunctionName = show;
+ }
+
+ void Recorder::showMultiline(bool show)
+ {
+ mWantsMultiline = show;
+ }
+
void addRecorder(RecorderPtr recorder)
{
if (!recorder)
@@ -871,17 +906,15 @@ namespace LLError
s->mFileRecorder.reset();
s->mFileRecorderFileName.clear();
- if (file_name.empty())
- {
- return;
- }
-
- RecorderPtr recordToFile(new RecordToFile(file_name));
- if (boost::dynamic_pointer_cast<RecordToFile>(recordToFile)->okay())
+ if (!file_name.empty())
{
- s->mFileRecorderFileName = file_name;
- s->mFileRecorder = recordToFile;
- addRecorder(recordToFile);
+ RecorderPtr recordToFile(new RecordToFile(file_name));
+ if (boost::dynamic_pointer_cast<RecordToFile>(recordToFile)->okay())
+ {
+ s->mFileRecorderFileName = file_name;
+ s->mFileRecorder = recordToFile;
+ addRecorder(recordToFile);
+ }
}
}
@@ -892,14 +925,12 @@ namespace LLError
removeRecorder(s->mFixedBufferRecorder);
s->mFixedBufferRecorder.reset();
- if (!fixedBuffer)
+ if (fixedBuffer)
{
- return;
- }
-
- RecorderPtr recordToFixedBuffer(new RecordToFixedBuffer(fixedBuffer));
- s->mFixedBufferRecorder = recordToFixedBuffer;
- addRecorder(recordToFixedBuffer);
+ RecorderPtr recordToFixedBuffer(new RecordToFixedBuffer(fixedBuffer));
+ s->mFixedBufferRecorder = recordToFixedBuffer;
+ addRecorder(recordToFixedBuffer);
+ }
}
std::string logFileName()
@@ -911,8 +942,9 @@ namespace LLError
namespace
{
- void addEscapedMessage(std::ostream& out, const std::string& message)
+ std::string escapedMessageLines(const std::string& message)
{
+ std::ostringstream out;
size_t written_out = 0;
size_t all_content = message.length();
size_t escape_char_index; // always relative to start of message
@@ -948,13 +980,16 @@ namespace
// write whatever was left
out << message.substr(written_out, std::string::npos);
}
+ return out.str();
}
- void writeToRecorders(const LLError::CallSite& site, const std::string& escaped_message, bool show_location = true, bool show_time = true, bool show_tags = true, bool show_level = true, bool show_function = true)
+ void writeToRecorders(const LLError::CallSite& site, const std::string& message)
{
LLError::ELevel level = site.mLevel;
LLError::SettingsConfigPtr s = LLError::Settings::getInstance()->getSettingsConfig();
-
+
+ std::string escaped_message;
+
for (Recorders::const_iterator i = s->mRecorders.begin();
i != s->mRecorders.end();
++i)
@@ -969,7 +1004,7 @@ namespace
}
message_stream << " ";
- if (show_level && r->wantsLevel())
+ if (r->wantsLevel())
{
message_stream << site.mLevelString;
}
@@ -981,19 +1016,30 @@ namespace
}
message_stream << " ";
- if (r->wantsLocation() || level == LLError::LEVEL_ERROR || s->mPrintLocation)
+ if (r->wantsLocation() || level == LLError::LEVEL_ERROR)
{
message_stream << site.mLocationString;
}
message_stream << " ";
- if (show_function && r->wantsFunctionName())
+ if (r->wantsFunctionName())
{
message_stream << site.mFunctionString;
}
message_stream << " : ";
- message_stream << escaped_message;
+ if (r->wantsMultiline())
+ {
+ message_stream << message;
+ }
+ else
+ {
+ if (escaped_message.empty())
+ {
+ escaped_message = escapedMessageLines(message);
+ }
+ message_stream << escaped_message;
+ }
r->recordMessage(level, message_stream.str());
}
@@ -1236,10 +1282,11 @@ namespace LLError
delete out;
}
- std::ostringstream message_stream;
if (site.mPrintOnce)
{
+ std::ostringstream message_stream;
+
std::map<std::string, unsigned int>::iterator messageIter = s->mUniqueLogMessages.find(message);
if (messageIter != s->mUniqueLogMessages.end())
{
@@ -1259,19 +1306,18 @@ namespace LLError
message_stream << "ONCE: ";
s->mUniqueLogMessages[message] = 1;
}
+ message_stream << message;
+ message = message_stream.str();
}
- addEscapedMessage(message_stream, message);
- std::string message_line(message_stream.str());
-
- writeToRecorders(site, message_line);
+ writeToRecorders(site, message);
if (site.mLevel == LEVEL_ERROR)
{
- g->mFatalMessage = message_line;
+ g->mFatalMessage = message;
if (s->mCrashFunction)
{
- s->mCrashFunction(message_line);
+ s->mCrashFunction(message);
}
}
}
@@ -1579,3 +1625,4 @@ bool debugLoggingEnabled(const std::string& tag)
}
+
diff --git a/indra/llcommon/llerrorcontrol.h b/indra/llcommon/llerrorcontrol.h
index ddbcdc94a0..a6278b3e50 100644
--- a/indra/llcommon/llerrorcontrol.h
+++ b/indra/llcommon/llerrorcontrol.h
@@ -148,13 +148,22 @@ namespace LLError
bool wantsLevel();
bool wantsLocation();
bool wantsFunctionName();
+ bool wantsMultiline();
+
+ void showTime(bool show);
+ void showTags(bool show);
+ void showLevel(bool show);
+ void showLocation(bool show);
+ void showFunctionName(bool show);
+ void showMultiline(bool show);
protected:
- bool mWantsTime,
- mWantsTags,
- mWantsLevel,
- mWantsLocation,
- mWantsFunctionName;
+ bool mWantsTime;
+ bool mWantsTags;
+ bool mWantsLevel;
+ bool mWantsLocation;
+ bool mWantsFunctionName;
+ bool mWantsMultiline;
};
typedef boost::shared_ptr<Recorder> RecorderPtr;
diff --git a/indra/llcommon/tests/llerror_test.cpp b/indra/llcommon/tests/llerror_test.cpp
index ce0dbce075..bd0357e4bf 100644
--- a/indra/llcommon/tests/llerror_test.cpp
+++ b/indra/llcommon/tests/llerror_test.cpp
@@ -78,8 +78,12 @@ namespace tut
class TestRecorder : public LLError::Recorder
{
public:
- TestRecorder() { mWantsTime = false; mWantsTags = true; }
- virtual ~TestRecorder() { }
+ TestRecorder()
+ {
+ showTime(false);
+ }
+ virtual ~TestRecorder()
+ {}
virtual void recordMessage(LLError::ELevel level,
const std::string& message)
@@ -90,8 +94,6 @@ namespace tut
int countMessages() { return (int) mMessages.size(); }
void clearMessages() { mMessages.clear(); }
- void setWantsTime(bool t) { mWantsTime = t; }
-
std::string message(int n)
{
std::ostringstream test_name;
@@ -139,9 +141,14 @@ namespace tut
}
void setWantsTime(bool t)
- {
- boost::dynamic_pointer_cast<TestRecorder>(mRecorder)->setWantsTime(t);
- }
+ {
+ boost::dynamic_pointer_cast<TestRecorder>(mRecorder)->showTime(t);
+ }
+
+ void setWantsMultiline(bool t)
+ {
+ boost::dynamic_pointer_cast<TestRecorder>(mRecorder)->showMultiline(t);
+ }
std::string message(int n)
{
@@ -378,27 +385,6 @@ namespace
}
}
-namespace tut
-{
- template<> template<>
- void ErrorTestObject::test<5>()
- // file and line information in log messages
- {
- std::string location = writeReturningLocation();
- // expecting default to not print location information
-
- LLError::setPrintLocation(true);
- writeReturningLocation();
-
- LLError::setPrintLocation(false);
- writeReturningLocation();
-
- ensure_message_does_not_contain(0, location);
- ensure_message_field_equals(1, LOCATION_FIELD, location);
- ensure_message_does_not_contain(2, location);
- }
-}
-
/* The following helper functions and class members all log a simple message
from some particular function scope. Each function takes a bool argument
that indicates if it should log its own name or not (in the manner that
@@ -583,7 +569,6 @@ namespace tut
// special handling of LL_ERRS() calls
void ErrorTestObject::test<8>()
{
- LLError::setPrintLocation(false);
std::string location = errorReturningLocation();
ensure_message_field_equals(0, LOCATION_FIELD, location);
@@ -630,15 +615,15 @@ namespace tut
// output order
void ErrorTestObject::test<10>()
{
- LLError::setPrintLocation(true);
LLError::setTimeFunction(roswell);
setWantsTime(true);
+
std::string location,
function;
writeReturningLocationAndFunction(location, function);
ensure_equals("order is time level tags location function message",
- message(0),
+ message(0),
roswell() + " INFO " + "# " /* no tag */ + location + " " + function + " : " + "apple");
}
@@ -658,7 +643,7 @@ namespace tut
LLError::setTimeFunction(roswell);
LLError::RecorderPtr anotherRecorder(new TestRecorder());
- boost::dynamic_pointer_cast<TestRecorder>(anotherRecorder)->setWantsTime(true);
+ boost::dynamic_pointer_cast<TestRecorder>(anotherRecorder)->showTime(true);
LLError::addRecorder(anotherRecorder);
LL_INFOS() << "baz" << LL_ENDL;
@@ -896,6 +881,25 @@ namespace tut
}
}
+namespace tut
+{
+ template<> template<>
+ void ErrorTestObject::test<19>()
+ // backslash, return, and newline are not escaped with backslashes
+ {
+ LLError::setDefaultLevel(LLError::LEVEL_DEBUG);
+ setWantsMultiline(true);
+ writeMsgNeedsEscaping(); // but should not be now
+ ensure_message_field_equals(0, MSG_FIELD, "backslash\\");
+ ensure_message_field_equals(1, MSG_FIELD, "newline\nafternewline");
+ ensure_message_field_equals(2, MSG_FIELD, "return\rafterreturn");
+ ensure_message_field_equals(3, MSG_FIELD, "backslash\\backslash\\");
+ ensure_message_field_equals(4, MSG_FIELD, "backslash\\newline\nanothernewline\nafternewline");
+ ensure_message_field_equals(5, MSG_FIELD, "backslash\\returnnewline\r\n\\afterbackslash");
+ ensure_message_count(6);
+ }
+}
+
/* Tests left:
handling of classes without LOG_CLASS
--
cgit v1.2.3
From 00a839d66590de1204af5fa295f66abcff87e477 Mon Sep 17 00:00:00 2001
From: Oz Linden <oz@lindenlab.com>
Date: Tue, 16 Oct 2018 16:18:31 -0400
Subject: renumber the new test to replace the one that was removed
---
indra/llcommon/tests/llerror_test.cpp | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
(limited to 'indra/llcommon')
diff --git a/indra/llcommon/tests/llerror_test.cpp b/indra/llcommon/tests/llerror_test.cpp
index bd0357e4bf..e7084fffc6 100644
--- a/indra/llcommon/tests/llerror_test.cpp
+++ b/indra/llcommon/tests/llerror_test.cpp
@@ -884,7 +884,7 @@ namespace tut
namespace tut
{
template<> template<>
- void ErrorTestObject::test<19>()
+ void ErrorTestObject::test<5>()
// backslash, return, and newline are not escaped with backslashes
{
LLError::setDefaultLevel(LLError::LEVEL_DEBUG);
--
cgit v1.2.3
From cd9d051b9024e4e0fc16a4aca28601d2a88a4045 Mon Sep 17 00:00:00 2001
From: Nat Goodspeed <nat@lindenlab.com>
Date: Wed, 17 Oct 2018 16:42:59 -0400
Subject: DRTVWR-447: Move test<5> and writeMsgNeedsEscaping() into sequence.
---
indra/llcommon/tests/llerror_test.cpp | 66 +++++++++++++++++------------------
1 file changed, 33 insertions(+), 33 deletions(-)
(limited to 'indra/llcommon')
diff --git a/indra/llcommon/tests/llerror_test.cpp b/indra/llcommon/tests/llerror_test.cpp
index e7084fffc6..8e1f4c14ac 100644
--- a/indra/llcommon/tests/llerror_test.cpp
+++ b/indra/llcommon/tests/llerror_test.cpp
@@ -498,6 +498,39 @@ namespace
}
}
+namespace
+{
+ void writeMsgNeedsEscaping()
+ {
+ LL_DEBUGS("WriteTag") << "backslash\\" << LL_ENDL;
+ LL_INFOS("WriteTag") << "newline\nafternewline" << LL_ENDL;
+ LL_WARNS("WriteTag") << "return\rafterreturn" << LL_ENDL;
+
+ LL_DEBUGS("WriteTag") << "backslash\\backslash\\" << LL_ENDL;
+ LL_INFOS("WriteTag") << "backslash\\newline\nanothernewline\nafternewline" << LL_ENDL;
+ LL_WARNS("WriteTag") << "backslash\\returnnewline\r\n\\afterbackslash" << LL_ENDL;
+ }
+};
+
+namespace tut
+{
+ template<> template<>
+ void ErrorTestObject::test<5>()
+ // backslash, return, and newline are not escaped with backslashes
+ {
+ LLError::setDefaultLevel(LLError::LEVEL_DEBUG);
+ setWantsMultiline(true);
+ writeMsgNeedsEscaping(); // but should not be now
+ ensure_message_field_equals(0, MSG_FIELD, "backslash\\");
+ ensure_message_field_equals(1, MSG_FIELD, "newline\nafternewline");
+ ensure_message_field_equals(2, MSG_FIELD, "return\rafterreturn");
+ ensure_message_field_equals(3, MSG_FIELD, "backslash\\backslash\\");
+ ensure_message_field_equals(4, MSG_FIELD, "backslash\\newline\nanothernewline\nafternewline");
+ ensure_message_field_equals(5, MSG_FIELD, "backslash\\returnnewline\r\n\\afterbackslash");
+ ensure_message_count(6);
+ }
+}
+
namespace tut
{
template<> template<>
@@ -820,20 +853,6 @@ namespace tut
}
}
-namespace
-{
- void writeMsgNeedsEscaping()
- {
- LL_DEBUGS("WriteTag") << "backslash\\" << LL_ENDL;
- LL_INFOS("WriteTag") << "newline\nafternewline" << LL_ENDL;
- LL_WARNS("WriteTag") << "return\rafterreturn" << LL_ENDL;
-
- LL_DEBUGS("WriteTag") << "backslash\\backslash\\" << LL_ENDL;
- LL_INFOS("WriteTag") << "backslash\\newline\nanothernewline\nafternewline" << LL_ENDL;
- LL_WARNS("WriteTag") << "backslash\\returnnewline\r\n\\afterbackslash" << LL_ENDL;
- }
-};
-
namespace tut
{
template<> template<>
@@ -881,25 +900,6 @@ namespace tut
}
}
-namespace tut
-{
- template<> template<>
- void ErrorTestObject::test<5>()
- // backslash, return, and newline are not escaped with backslashes
- {
- LLError::setDefaultLevel(LLError::LEVEL_DEBUG);
- setWantsMultiline(true);
- writeMsgNeedsEscaping(); // but should not be now
- ensure_message_field_equals(0, MSG_FIELD, "backslash\\");
- ensure_message_field_equals(1, MSG_FIELD, "newline\nafternewline");
- ensure_message_field_equals(2, MSG_FIELD, "return\rafterreturn");
- ensure_message_field_equals(3, MSG_FIELD, "backslash\\backslash\\");
- ensure_message_field_equals(4, MSG_FIELD, "backslash\\newline\nanothernewline\nafternewline");
- ensure_message_field_equals(5, MSG_FIELD, "backslash\\returnnewline\r\n\\afterbackslash");
- ensure_message_count(6);
- }
-}
-
/* Tests left:
handling of classes without LOG_CLASS
--
cgit v1.2.3