From eb1bea222322385e6e5b05206f09f21bb891f3f7 Mon Sep 17 00:00:00 2001
From: Nat Goodspeed <nat@lindenlab.com>
Date: Mon, 23 Apr 2012 11:26:18 -0400
Subject: [PATCH] IQA-463: LLError::addRecorder() claims ownership of passed
Recorder*. That is, when the underlying LLError::Settings object is destroyed
-- possibly at termination, possibly on LLError::restoreSettings() -- the
passed Recorder* is deleted. There was much existing code that seemed as
unaware of this alarming fact as I was myself. Passing to addRecorder() a
pointer to a stack object, or to a member of some other object, is just Bad.
It might be preferable to make addRecorder() accept std::auto_ptr<Recorder>
to make the ownership transfer more explicit -- or even
boost::shared_ptr<Recorder> instead, which would allow the caller to either
forget or retain the passed Recorder. This preliminary pass retains the
Recorder* dumb pointer API, but documents the ownership issue, and eliminates
known instances of passing pointers to anything but a standalone heap
Recorder subclass object.
---
indra/llcommon/llerrorcontrol.h | 77 +++++----
indra/llcommon/tests/llerror_test.cpp | 226 +++++++++++++-------------
indra/llcommon/tests/wrapllerrs.h | 46 +++++-
indra/test/test.cpp | 10 +-
4 files changed, 206 insertions(+), 153 deletions(-)
diff --git a/indra/llcommon/llerrorcontrol.h b/indra/llcommon/llerrorcontrol.h
index 1be49cebc8b..d53a819d883 100644
--- a/indra/llcommon/llerrorcontrol.h
+++ b/indra/llcommon/llerrorcontrol.h
@@ -1,4 +1,4 @@
-/**
+/**
* @file llerrorcontrol.h
* @date December 2006
* @brief error message system control
@@ -6,21 +6,21 @@
* $LicenseInfo:firstyear=2007&license=viewerlgpl$
* Second Life Viewer Source Code
* Copyright (C) 2010, Linden Research, Inc.
- *
+ *
* This library is free software; you can redistribute it and/or
* modify it under the terms of the GNU Lesser General Public
* License as published by the Free Software Foundation;
* version 2.1 of the License only.
- *
+ *
* This library is distributed in the hope that it will be useful,
* but WITHOUT ANY WARRANTY; without even the implied warranty of
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
* Lesser General Public License for more details.
- *
+ *
* You should have received a copy of the GNU Lesser General Public
* License along with this library; if not, write to the Free Software
* Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
- *
+ *
* Linden Research, Inc., 945 Battery Street, San Francisco, CA 94111 USA
* $/LicenseInfo$
*/
@@ -38,7 +38,7 @@ class LLSD;
This is the part of the LLError namespace that manages the messages
produced by the logging. The logging support is defined in llerror.h.
Most files do not need to include this.
-
+
These implementations are in llerror.cpp.
*/
@@ -72,7 +72,7 @@ namespace LLError
Settings that control what is logged.
Setting a level means log messages at that level or above.
*/
-
+
LL_COMMON_API void setPrintLocation(bool);
LL_COMMON_API void setDefaultLevel(LLError::ELevel);
LL_COMMON_API ELevel getDefaultLevel();
@@ -101,31 +101,31 @@ namespace LLError
// (by, for example, setting a class level to LEVEL_NONE), will keep
// the that message from causing the fatal funciton to be invoked.
- LL_COMMON_API FatalFunction getFatalFunction();
- // Retrieve the previously-set FatalFunction
-
- /// temporarily override the FatalFunction for the duration of a
- /// particular scope, e.g. for unit tests
- class LL_COMMON_API OverrideFatalFunction
- {
- public:
- OverrideFatalFunction(const FatalFunction& func):
- mPrev(getFatalFunction())
- {
- setFatalFunction(func);
- }
- ~OverrideFatalFunction()
- {
- setFatalFunction(mPrev);
- }
-
- private:
- FatalFunction mPrev;
- };
+ LL_COMMON_API FatalFunction getFatalFunction();
+ // Retrieve the previously-set FatalFunction
+
+ /// temporarily override the FatalFunction for the duration of a
+ /// particular scope, e.g. for unit tests
+ class LL_COMMON_API OverrideFatalFunction
+ {
+ public:
+ OverrideFatalFunction(const FatalFunction& func):
+ mPrev(getFatalFunction())
+ {
+ setFatalFunction(func);
+ }
+ ~OverrideFatalFunction()
+ {
+ setFatalFunction(mPrev);
+ }
+
+ private:
+ FatalFunction mPrev;
+ };
typedef std::string (*TimeFunction)();
LL_COMMON_API std::string utcTime();
-
+
LL_COMMON_API void setTimeFunction(TimeFunction);
// The function is use to return the current time, formatted for
// display by those error recorders that want the time included.
@@ -137,19 +137,27 @@ namespace LLError
// An object that handles the actual output or error messages.
public:
virtual ~Recorder();
-
+
virtual void recordMessage(LLError::ELevel, const std::string& message) = 0;
// use the level for better display, not for filtering
-
+
virtual bool wantsTime(); // default returns false
// override and return true if the recorder wants the time string
// included in the text of the message
};
-
+
+ /**
+ * @NOTE: addRecorder() conveys ownership to the underlying Settings
+ * object -- when destroyed, it will @em delete the passed Recorder*!
+ */
LL_COMMON_API void addRecorder(Recorder*);
+ /**
+ * @NOTE: removeRecorder() reclaims ownership of the Recorder*: its
+ * lifespan becomes the caller's problem.
+ */
LL_COMMON_API void removeRecorder(Recorder*);
// each error message is passed to each recorder via recordMessage()
-
+
LL_COMMON_API void logToFile(const std::string& filename);
LL_COMMON_API void logToFixedBuffer(LLLineBuffer*);
// Utilities to add recorders for logging to a file or a fixed buffer
@@ -167,10 +175,9 @@ namespace LLError
class Settings;
LL_COMMON_API Settings* saveAndResetSettings();
LL_COMMON_API void restoreSettings(Settings *);
-
+
LL_COMMON_API std::string abbreviateFile(const std::string& filePath);
LL_COMMON_API int shouldLogCallCount();
-
};
#endif // LL_LLERRORCONTROL_H
diff --git a/indra/llcommon/tests/llerror_test.cpp b/indra/llcommon/tests/llerror_test.cpp
index 09a20231dea..279a90e51b5 100644
--- a/indra/llcommon/tests/llerror_test.cpp
+++ b/indra/llcommon/tests/llerror_test.cpp
@@ -1,4 +1,4 @@
-/**
+/**
* @file llerror_test.cpp
* @date December 2006
* @brief error unit tests
@@ -6,21 +6,21 @@
* $LicenseInfo:firstyear=2006&license=viewerlgpl$
* Second Life Viewer Source Code
* Copyright (C) 2010, Linden Research, Inc.
- *
+ *
* This library is free software; you can redistribute it and/or
* modify it under the terms of the GNU Lesser General Public
* License as published by the Free Software Foundation;
* version 2.1 of the License only.
- *
+ *
* This library is distributed in the hope that it will be useful,
* but WITHOUT ANY WARRANTY; without even the implied warranty of
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
* Lesser General Public License for more details.
- *
+ *
* You should have received a copy of the GNU Lesser General Public
* License along with this library; if not, write to the Free Software
* Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
- *
+ *
* Linden Research, Inc., 945 Battery Street, San Francisco, CA 94111 USA
* $/LicenseInfo$
*/
@@ -49,7 +49,7 @@ namespace
static bool fatalWasCalled;
void fatalCall(const std::string&) { fatalWasCalled = true; }
}
-
+
namespace tut
{
class TestRecorder : public LLError::Recorder
@@ -57,59 +57,65 @@ namespace tut
public:
TestRecorder() : mWantsTime(false) { }
~TestRecorder() { LLError::removeRecorder(this); }
-
+
void recordMessage(LLError::ELevel level,
const std::string& message)
{
mMessages.push_back(message);
}
-
+
int countMessages() { return (int) mMessages.size(); }
void clearMessages() { mMessages.clear(); }
-
+
void setWantsTime(bool t) { mWantsTime = t; }
bool wantsTime() { return mWantsTime; }
-
+
std::string message(int n)
{
std::ostringstream test_name;
test_name << "testing message " << n << ", not enough messages";
-
+
tut::ensure(test_name.str(), n < countMessages());
return mMessages[n];
}
-
+
private:
typedef std::vector<std::string> MessageVector;
MessageVector mMessages;
-
+
bool mWantsTime;
};
struct ErrorTestData
{
- TestRecorder mRecorder;
+ // addRecorder() expects to be able to later delete the passed
+ // Recorder*. Even though removeRecorder() reclaims ownership, passing
+ // a pointer to a data member rather than a heap Recorder subclass
+ // instance would just be Wrong.
+ TestRecorder* mRecorder;
LLError::Settings* mPriorErrorSettings;
-
- ErrorTestData()
+
+ ErrorTestData():
+ mRecorder(new TestRecorder)
{
fatalWasCalled = false;
-
+
mPriorErrorSettings = LLError::saveAndResetSettings();
LLError::setDefaultLevel(LLError::LEVEL_DEBUG);
LLError::setFatalFunction(fatalCall);
- LLError::addRecorder(&mRecorder);
+ LLError::addRecorder(mRecorder);
}
-
+
~ErrorTestData()
{
- LLError::removeRecorder(&mRecorder);
+ LLError::removeRecorder(mRecorder);
+ delete mRecorder;
LLError::restoreSettings(mPriorErrorSettings);
}
-
+
void ensure_message_count(int expectedCount)
{
- ensure_equals("message count", mRecorder.countMessages(), expectedCount);
+ ensure_equals("message count", mRecorder->countMessages(), expectedCount);
}
void ensure_message_contains(int n, const std::string& expectedText)
@@ -117,7 +123,7 @@ namespace tut
std::ostringstream test_name;
test_name << "testing message " << n;
- ensure_contains(test_name.str(), mRecorder.message(n), expectedText);
+ ensure_contains(test_name.str(), mRecorder->message(n), expectedText);
}
void ensure_message_does_not_contain(int n, const std::string& expectedText)
@@ -125,22 +131,22 @@ namespace tut
std::ostringstream test_name;
test_name << "testing message " << n;
- ensure_does_not_contain(test_name.str(), mRecorder.message(n), expectedText);
+ ensure_does_not_contain(test_name.str(), mRecorder->message(n), expectedText);
}
};
-
+
typedef test_group<ErrorTestData> ErrorTestGroup;
typedef ErrorTestGroup::object ErrorTestObject;
-
+
ErrorTestGroup errorTestGroup("error");
-
+
template<> template<>
void ErrorTestObject::test<1>()
// basic test of output
{
llinfos << "test" << llendl;
llinfos << "bob" << llendl;
-
+
ensure_message_contains(0, "test");
ensure_message_contains(1, "bob");
}
@@ -159,7 +165,7 @@ namespace
};
namespace tut
-{
+{
template<> template<>
void ErrorTestObject::test<2>()
// messages are filtered based on default level
@@ -172,7 +178,7 @@ namespace tut
ensure_message_contains(3, "error");
ensure_message_contains(4, "four");
ensure_message_count(5);
-
+
LLError::setDefaultLevel(LLError::LEVEL_INFO);
writeSome();
ensure_message_contains(5, "two");
@@ -180,20 +186,20 @@ namespace tut
ensure_message_contains(7, "error");
ensure_message_contains(8, "four");
ensure_message_count(9);
-
+
LLError::setDefaultLevel(LLError::LEVEL_WARN);
writeSome();
ensure_message_contains(9, "three");
ensure_message_contains(10, "error");
ensure_message_contains(11, "four");
ensure_message_count(12);
-
+
LLError::setDefaultLevel(LLError::LEVEL_ERROR);
writeSome();
ensure_message_contains(12, "error");
ensure_message_contains(13, "four");
ensure_message_count(14);
-
+
LLError::setDefaultLevel(LLError::LEVEL_NONE);
writeSome();
ensure_message_count(14);
@@ -218,14 +224,14 @@ namespace tut
{
std::string thisFile = __FILE__;
std::string abbreviateFile = LLError::abbreviateFile(thisFile);
-
+
ensure_ends_with("file name abbreviation",
abbreviateFile,
"llcommon/tests/llerror_test.cpp"
);
ensure_does_not_contain("file name abbreviation",
abbreviateFile, "indra");
-
+
std::string someFile =
#if LL_WINDOWS
"C:/amy/bob/cam.cpp"
@@ -234,12 +240,12 @@ namespace tut
#endif
;
std::string someAbbreviation = LLError::abbreviateFile(someFile);
-
+
ensure_equals("non-indra file abbreviation",
someAbbreviation, someFile);
}
}
-
+
namespace
{
std::string locationString(int line)
@@ -247,22 +253,22 @@ namespace
std::ostringstream location;
location << LLError::abbreviateFile(__FILE__)
<< "(" << line << ") : ";
-
+
return location.str();
}
-
+
std::string writeReturningLocation()
{
llinfos << "apple" << llendl; int this_line = __LINE__;
return locationString(this_line);
}
-
+
std::string writeReturningLocationAndFunction()
{
llinfos << "apple" << llendl; int this_line = __LINE__;
return locationString(this_line) + __FUNCTION__;
}
-
+
std::string errorReturningLocation()
{
llerrs << "die" << llendl; int this_line = __LINE__;
@@ -271,20 +277,20 @@ 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_contains(1, location);
ensure_message_does_not_contain(2, location);
@@ -297,7 +303,7 @@ namespace tut
existing log messages often do.) The functions all return their C++
name so that test can be substantial mechanized.
*/
-
+
std::string logFromGlobal(bool id)
{
llinfos << (id ? "logFromGlobal: " : "") << "hi" << llendl;
@@ -345,7 +351,7 @@ namespace
return "ClassWithNoLogType::logFromStatic";
}
};
-
+
class ClassWithLogType {
LOG_CLASS(ClassWithLogType);
public:
@@ -360,13 +366,13 @@ namespace
return "ClassWithLogType::logFromStatic";
}
};
-
+
std::string logFromNamespace(bool id) { return Foo::logFromNamespace(id); }
std::string logFromClassWithNoLogTypeMember(bool id) { ClassWithNoLogType c; return c.logFromMember(id); }
std::string logFromClassWithNoLogTypeStatic(bool id) { return ClassWithNoLogType::logFromStatic(id); }
std::string logFromClassWithLogTypeMember(bool id) { ClassWithLogType c; return c.logFromMember(id); }
std::string logFromClassWithLogTypeStatic(bool id) { return ClassWithLogType::logFromStatic(id); }
-
+
void ensure_has(const std::string& message,
const std::string& actual, const std::string& expected)
{
@@ -379,18 +385,18 @@ namespace
throw tut::failure(ss.str().c_str());
}
}
-
+
typedef std::string (*LogFromFunction)(bool);
- void testLogName(tut::TestRecorder& recorder, LogFromFunction f,
+ void testLogName(tut::TestRecorder* recorder, LogFromFunction f,
const std::string& class_name = "")
{
- recorder.clearMessages();
+ recorder->clearMessages();
std::string name = f(false);
f(true);
-
- std::string messageWithoutName = recorder.message(0);
- std::string messageWithName = recorder.message(1);
-
+
+ std::string messageWithoutName = recorder->message(0);
+ std::string messageWithName = recorder->message(1);
+
ensure_has(name + " logged without name",
messageWithoutName, name);
ensure_has(name + " logged with name",
@@ -431,18 +437,18 @@ namespace
llinfos << "inside" << llendl;
return "moo";
}
-
+
std::string outerLogger()
{
llinfos << "outside(" << innerLogger() << ")" << llendl;
return "bar";
}
-
+
void uberLogger()
{
llinfos << "uber(" << outerLogger() << "," << innerLogger() << ")" << llendl;
}
-
+
class LogWhileLogging
{
public:
@@ -461,11 +467,11 @@ namespace
LogWhileLogging l;
llinfos << "meta(" << l << ")" << llendl;
}
-
+
}
namespace tut
-{
+{
template<> template<>
// handle nested logging
void ErrorTestObject::test<7>()
@@ -474,31 +480,31 @@ namespace tut
ensure_message_contains(0, "inside");
ensure_message_contains(1, "outside(moo)");
ensure_message_count(2);
-
+
uberLogger();
ensure_message_contains(2, "inside");
ensure_message_contains(3, "inside");
ensure_message_contains(4, "outside(moo)");
ensure_message_contains(5, "uber(bar,moo)");
ensure_message_count(6);
-
+
metaLogger();
ensure_message_contains(6, "logging");
ensure_message_contains(7, "meta(baz)");
ensure_message_count(8);
}
-
+
template<> template<>
// special handling of llerrs calls
void ErrorTestObject::test<8>()
{
LLError::setPrintLocation(false);
std::string location = errorReturningLocation();
-
+
ensure_message_contains(0, location + "error");
ensure_message_contains(1, "die");
ensure_message_count(2);
-
+
ensure("fatal callback called", fatalWasCalled);
}
}
@@ -509,7 +515,7 @@ namespace
{
return "1947-07-08T03:04:05Z";
}
-
+
void ufoSighting()
{
llinfos << "ufo" << llendl;
@@ -517,35 +523,35 @@ namespace
}
namespace tut
-{
+{
template<> template<>
// time in output (for recorders that need it)
void ErrorTestObject::test<9>()
{
LLError::setTimeFunction(roswell);
- mRecorder.setWantsTime(false);
+ mRecorder->setWantsTime(false);
ufoSighting();
ensure_message_contains(0, "ufo");
ensure_message_does_not_contain(0, roswell());
-
- mRecorder.setWantsTime(true);
+
+ mRecorder->setWantsTime(true);
ufoSighting();
ensure_message_contains(1, "ufo");
ensure_message_contains(1, roswell());
}
-
+
template<> template<>
// output order
void ErrorTestObject::test<10>()
{
LLError::setPrintLocation(true);
LLError::setTimeFunction(roswell);
- mRecorder.setWantsTime(true);
+ mRecorder->setWantsTime(true);
std::string locationAndFunction = writeReturningLocationAndFunction();
-
+
ensure_equals("order is time type location function message",
- mRecorder.message(0),
+ mRecorder->message(0),
roswell() + " INFO: " + locationAndFunction + ": apple");
}
@@ -553,30 +559,30 @@ namespace tut
// multiple recorders
void ErrorTestObject::test<11>()
{
- TestRecorder altRecorder;
- LLError::addRecorder(&altRecorder);
-
+ TestRecorder* altRecorder(new TestRecorder);
+ LLError::addRecorder(altRecorder);
+
llinfos << "boo" << llendl;
ensure_message_contains(0, "boo");
- ensure_equals("alt recorder count", altRecorder.countMessages(), 1);
- ensure_contains("alt recorder message 0", altRecorder.message(0), "boo");
-
+ ensure_equals("alt recorder count", altRecorder->countMessages(), 1);
+ ensure_contains("alt recorder message 0", altRecorder->message(0), "boo");
+
LLError::setTimeFunction(roswell);
- TestRecorder anotherRecorder;
- anotherRecorder.setWantsTime(true);
- LLError::addRecorder(&anotherRecorder);
-
+ TestRecorder* anotherRecorder(new TestRecorder);
+ anotherRecorder->setWantsTime(true);
+ LLError::addRecorder(anotherRecorder);
+
llinfos << "baz" << llendl;
std::string when = roswell();
-
+
ensure_message_does_not_contain(1, when);
- ensure_equals("alt recorder count", altRecorder.countMessages(), 2);
- ensure_does_not_contain("alt recorder message 1", altRecorder.message(1), when);
- ensure_equals("another recorder count", anotherRecorder.countMessages(), 1);
- ensure_contains("another recorder message 0", anotherRecorder.message(0), when);
+ ensure_equals("alt recorder count", altRecorder->countMessages(), 2);
+ ensure_does_not_contain("alt recorder message 1", altRecorder->message(1), when);
+ ensure_equals("another recorder count", anotherRecorder->countMessages(), 1);
+ ensure_contains("another recorder message 0", anotherRecorder->message(0), when);
}
}
@@ -610,10 +616,10 @@ namespace tut
{
LLError::setDefaultLevel(LLError::LEVEL_WARN);
LLError::setClassLevel("TestBeta", LLError::LEVEL_INFO);
-
+
TestAlpha::doAll();
TestBeta::doAll();
-
+
ensure_message_contains(0, "aim west");
ensure_message_contains(1, "error");
ensure_message_contains(2, "ate eels");
@@ -623,7 +629,7 @@ namespace tut
ensure_message_contains(6, "big easy");
ensure_message_count(7);
}
-
+
template<> template<>
// filtering by function, and that it will override class filtering
void ErrorTestObject::test<13>()
@@ -632,13 +638,13 @@ namespace tut
LLError::setClassLevel("TestBeta", LLError::LEVEL_WARN);
LLError::setFunctionLevel("TestBeta::doInfo", LLError::LEVEL_DEBUG);
LLError::setFunctionLevel("TestBeta::doError", LLError::LEVEL_NONE);
-
+
TestBeta::doAll();
ensure_message_contains(0, "buy iron");
ensure_message_contains(1, "bad word");
ensure_message_count(2);
}
-
+
template<> template<>
// filtering by file
// and that it is overridden by both class and function filtering
@@ -652,7 +658,7 @@ namespace tut
LLError::LEVEL_NONE);
LLError::setFunctionLevel("TestBeta::doError",
LLError::LEVEL_NONE);
-
+
TestAlpha::doAll();
TestBeta::doAll();
ensure_message_contains(0, "any idea");
@@ -660,7 +666,7 @@ namespace tut
ensure_message_contains(2, "bad word");
ensure_message_count(3);
}
-
+
template<> template<>
// proper cached, efficient lookup of filtering
void ErrorTestObject::test<15>()
@@ -690,7 +696,7 @@ namespace tut
ensure_message_count(2);
ensure_equals("sixth check", LLError::shouldLogCallCount(), 3);
}
-
+
template<> template<>
// configuration from LLSD
void ErrorTestObject::test<16>()
@@ -699,26 +705,26 @@ namespace tut
LLSD config;
config["print-location"] = true;
config["default-level"] = "DEBUG";
-
+
LLSD set1;
set1["level"] = "WARN";
set1["files"][0] = this_file;
-
+
LLSD set2;
set2["level"] = "INFO";
set2["classes"][0] = "TestAlpha";
-
+
LLSD set3;
set3["level"] = "NONE";
set3["functions"][0] = "TestAlpha::doError";
set3["functions"][1] = "TestBeta::doError";
-
+
config["settings"][0] = set1;
config["settings"][1] = set2;
config["settings"][2] = set3;
-
+
LLError::configure(config);
-
+
TestAlpha::doAll();
TestBeta::doAll();
ensure_message_contains(0, "any idea");
@@ -726,13 +732,13 @@ namespace tut
ensure_message_contains(1, "aim west");
ensure_message_contains(2, "bad word");
ensure_message_count(3);
-
+
// make sure reconfiguring works
LLSD config2;
config2["default-level"] = "WARN";
-
+
LLError::configure(config2);
-
+
TestAlpha::doAll();
TestBeta::doAll();
ensure_message_contains(3, "aim west");
@@ -744,13 +750,13 @@ namespace tut
ensure_message_contains(8, "big easy");
ensure_message_count(9);
}
-}
+}
/* Tests left:
handling of classes without LOG_CLASS
- live update of filtering from file
-
+ live update of filtering from file
+
syslog recorder
file recorder
cerr/stderr recorder
diff --git a/indra/llcommon/tests/wrapllerrs.h b/indra/llcommon/tests/wrapllerrs.h
index f79acacb22f..a4d3a4e0260 100644
--- a/indra/llcommon/tests/wrapllerrs.h
+++ b/indra/llcommon/tests/wrapllerrs.h
@@ -29,10 +29,15 @@
#if ! defined(LL_WRAPLLERRS_H)
#define LL_WRAPLLERRS_H
+#if LL_WINDOWS
+#pragma warning (disable : 4355) // 'this' used in initializer list: yes, intentionally
+#endif
+
#include <tut/tut.hpp>
#include "llerrorcontrol.h"
#include "stringize.h"
#include <boost/bind.hpp>
+#include <boost/noncopyable.hpp>
#include <list>
#include <string>
#include <stdexcept>
@@ -80,11 +85,39 @@ struct WrapLL_ERRS
LLError::FatalFunction mPriorFatal;
};
+/**
+ * LLError::addRecorder() accepts ownership of the passed Recorder* -- it
+ * expects to be able to delete it later. CaptureLog isa Recorder whose
+ * pointer we want to be able to pass without any ownership implications.
+ * For such cases, instantiate a new RecorderProxy(yourRecorder) and pass
+ * that. Your heap RecorderProxy might later be deleted, but not yourRecorder.
+ */
+class RecorderProxy: public LLError::Recorder
+{
+public:
+ RecorderProxy(LLError::Recorder* recorder):
+ mRecorder(recorder)
+ {}
+
+ virtual void recordMessage(LLError::ELevel level, const std::string& message)
+ {
+ mRecorder->recordMessage(level, message);
+ }
+
+ virtual bool wantsTime()
+ {
+ return mRecorder->wantsTime();
+ }
+
+private:
+ LLError::Recorder* mRecorder;
+};
+
/**
* Capture log messages. This is adapted (simplified) from the one in
* llerror_test.cpp.
*/
-class CaptureLog : public LLError::Recorder
+class CaptureLog : public LLError::Recorder, public boost::noncopyable
{
public:
CaptureLog(LLError::ELevel level=LLError::LEVEL_DEBUG):
@@ -97,16 +130,18 @@ class CaptureLog : public LLError::Recorder
// with that output. If it turns out that saveAndResetSettings() has
// some bad effect, give up and just let the DEBUG level log messages
// display.
- mOldSettings(LLError::saveAndResetSettings())
+ mOldSettings(LLError::saveAndResetSettings()),
+ mProxy(new RecorderProxy(this))
{
LLError::setFatalFunction(wouldHaveCrashed);
LLError::setDefaultLevel(level);
- LLError::addRecorder(this);
+ LLError::addRecorder(mProxy);
}
~CaptureLog()
{
- LLError::removeRecorder(this);
+ LLError::removeRecorder(mProxy);
+ delete mProxy;
LLError::restoreSettings(mOldSettings);
}
@@ -133,7 +168,7 @@ class CaptureLog : public LLError::Recorder
throw tut::failure(STRINGIZE("failed to find '" << search
<< "' in captured log messages:\n"
- << *this));
+ << boost::ref(*this)));
}
std::ostream& streamto(std::ostream& out) const
@@ -155,6 +190,7 @@ class CaptureLog : public LLError::Recorder
typedef std::list<std::string> MessageList;
MessageList mMessages;
LLError::Settings* mOldSettings;
+ LLError::Recorder* mProxy;
};
inline
diff --git a/indra/test/test.cpp b/indra/test/test.cpp
index 128d84e4282..06128e09022 100644
--- a/indra/test/test.cpp
+++ b/indra/test/test.cpp
@@ -37,6 +37,7 @@
#include "linden_common.h"
#include "llerrorcontrol.h"
#include "lltut.h"
+#include "tests/wrapllerrs.h" // RecorderProxy
#include "stringize.h"
#include "namedtempfile.h"
@@ -91,22 +92,24 @@ class LLReplayLog
virtual void replay(std::ostream&) {}
};
-class LLReplayLogReal: public LLReplayLog, public LLError::Recorder
+class LLReplayLogReal: public LLReplayLog, public LLError::Recorder, public boost::noncopyable
{
public:
LLReplayLogReal(LLError::ELevel level, apr_pool_t* pool):
mOldSettings(LLError::saveAndResetSettings()),
+ mProxy(new RecorderProxy(this)),
mTempFile("log", "", pool), // create file
mFile(mTempFile.getName().c_str()) // open it
{
LLError::setFatalFunction(wouldHaveCrashed);
LLError::setDefaultLevel(level);
- LLError::addRecorder(this);
+ LLError::addRecorder(mProxy);
}
virtual ~LLReplayLogReal()
{
- LLError::removeRecorder(this);
+ LLError::removeRecorder(mProxy);
+ delete mProxy;
LLError::restoreSettings(mOldSettings);
}
@@ -134,6 +137,7 @@ class LLReplayLogReal: public LLReplayLog, public LLError::Recorder
private:
LLError::Settings* mOldSettings;
+ LLError::Recorder* mProxy;
NamedTempFile mTempFile;
std::ofstream mFile;
};
--
GitLab