From 9c7132e60db51286c7499a8194c5b85a5762019b Mon Sep 17 00:00:00 2001 From: "akalin@chromium.org" Date: Tue, 8 Feb 2011 07:39:08 +0000 Subject: [Logging] Remove unneeded CheckOpString struct for CHECKs Using string* directly works. Made LogMessage delete any passed in string* after it uses it. Removed suppressions for fixed memory leak. BUG=57683 TEST=Existing unit tests Review URL: http://codereview.chromium.org/6413032 git-svn-id: svn://svn.chromium.org/chrome/trunk/src@74100 0039d316-1c4b-4281-b951-d872f2087c98 --- base/logging.cc | 10 ++++++---- base/logging.h | 26 +++++++++----------------- tools/heapcheck/suppressions.txt | 8 -------- tools/valgrind/memcheck/suppressions.txt | 8 -------- 4 files changed, 15 insertions(+), 37 deletions(-) diff --git a/base/logging.cc b/base/logging.cc index d66f01b..0a808b4 100644 --- a/base/logging.cc +++ b/base/logging.cc @@ -540,17 +540,19 @@ LogMessage::LogMessage(const char* file, int line, LogSeverity severity) Init(file, line); } -LogMessage::LogMessage(const char* file, int line, const CheckOpString& result) +LogMessage::LogMessage(const char* file, int line, std::string* result) : severity_(LOG_FATAL), file_(file), line_(line) { Init(file, line); - stream_ << "Check failed: " << (*result.str_); + stream_ << "Check failed: " << *result; + delete result; } LogMessage::LogMessage(const char* file, int line, LogSeverity severity, - const CheckOpString& result) + std::string* result) : severity_(severity), file_(file), line_(line) { Init(file, line); - stream_ << "Check failed: " << (*result.str_); + stream_ << "Check failed: " << *result; + delete result; } LogMessage::~LogMessage() { diff --git a/base/logging.h b/base/logging.h index 771aa73..b1bc0b0 100644 --- a/base/logging.h +++ b/base/logging.h @@ -436,19 +436,10 @@ const LogSeverity LOG_0 = LOG_ERROR; LAZY_STREAM(PLOG_STREAM(FATAL), !(condition)) \ << "Check failed: " #condition ". " -// A container for a string pointer which can be evaluated to a bool - -// true iff the pointer is NULL. -struct CheckOpString { - CheckOpString(std::string* str) : str_(str) { } - // No destructor: if str_ is non-NULL, we're about to LOG(FATAL), - // so there's no point in cleaning up str_. - operator bool() const { return str_ != NULL; } - std::string* str_; -}; - // Build the error message string. This is separate from the "Impl" // function template because it is not performance critical and so can -// be out of line, while the "Impl" code should be inline. +// be out of line, while the "Impl" code should be inline. Caller +// takes ownership of the returned string. template std::string* MakeCheckOpString(const t1& v1, const t2& v2, const char* names) { std::ostringstream ss; @@ -479,7 +470,7 @@ extern template std::string* MakeCheckOpString( // TODO(akalin): Rewrite this so that constructs like if (...) // CHECK_EQ(...) else { ... } work properly. #define CHECK_OP(name, op, val1, val2) \ - if (logging::CheckOpString _result = \ + if (std::string* _result = \ logging::Check##name##Impl((val1), (val2), \ #val1 " " #op " " #val2)) \ logging::LogMessage(__FILE__, __LINE__, _result).stream() @@ -655,7 +646,7 @@ const LogSeverity LOG_DCHECK = LOG_INFO; // Don't use this macro directly in your code, use DCHECK_EQ et al below. #define DCHECK_OP(name, op, val1, val2) \ if (DCHECK_IS_ON()) \ - if (logging::CheckOpString _result = \ + if (std::string* _result = \ logging::Check##name##Impl((val1), (val2), \ #val1 " " #op " " #val2)) \ logging::LogMessage( \ @@ -723,14 +714,15 @@ class LogMessage { // saves a couple of bytes per call site. LogMessage(const char* file, int line, LogSeverity severity); - // A special constructor used for check failures. + // A special constructor used for check failures. Takes ownership + // of the given string. // Implied severity = LOG_FATAL - LogMessage(const char* file, int line, const CheckOpString& result); + LogMessage(const char* file, int line, std::string* result); // A special constructor used for check failures, with the option to - // specify severity. + // specify severity. Takes ownership of the given string. LogMessage(const char* file, int line, LogSeverity severity, - const CheckOpString& result); + std::string* result); ~LogMessage(); diff --git a/tools/heapcheck/suppressions.txt b/tools/heapcheck/suppressions.txt index 3e04c6d..bf4ef65 100644 --- a/tools/heapcheck/suppressions.txt +++ b/tools/heapcheck/suppressions.txt @@ -949,14 +949,6 @@ fun:::MessageLoopRelay::ProcessOnTargetThread } { - bug_57683 - Heapcheck:Leak - ... - fun:logging::MakeCheckOpString - fun:logging::Check*Impl - fun:logging::::LoggingTest_*_Test::TestBody -} -{ bug_57799 Heapcheck:Leak fun:cricket::Transport::OnRemoteCandidate diff --git a/tools/valgrind/memcheck/suppressions.txt b/tools/valgrind/memcheck/suppressions.txt index 6e35331..789213a 100644 --- a/tools/valgrind/memcheck/suppressions.txt +++ b/tools/valgrind/memcheck/suppressions.txt @@ -2872,14 +2872,6 @@ fun:_ZN14RunnableMethodIN3IPC12ChannelProxy7ContextEMS2_FvRKNS0_7MessageEE6Tuple1IS3_EE3RunEv } { - bug_57683 - Memcheck:Leak - fun:_Znw* - fun:_ZN7logging17MakeCheckOpString* - fun:_ZN7logging11Check*Impl* - fun:_ZN7logging*LoggingTest_*_Test8TestBodyEv -} -{ bug_57799 Memcheck:Leak fun:_Znw* -- cgit v1.1