Add an API to clear the saved LogBacktraceAt location, and call it when setting an empty or invalid flag value.
PiperOrigin-RevId: 525065479
Change-Id: I3c0822db8301e0999b0669394b415df82edd445f
diff --git a/absl/log/flags.cc b/absl/log/flags.cc
index b530888..215b7bd 100644
--- a/absl/log/flags.cc
+++ b/absl/log/flags.cc
@@ -90,19 +90,27 @@
.OnUpdate([] {
const std::string log_backtrace_at =
absl::GetFlag(FLAGS_log_backtrace_at);
- if (log_backtrace_at.empty()) return;
+ if (log_backtrace_at.empty()) {
+ absl::ClearLogBacktraceLocation();
+ return;
+ }
const size_t last_colon = log_backtrace_at.rfind(':');
- if (last_colon == log_backtrace_at.npos) return;
+ if (last_colon == log_backtrace_at.npos) {
+ absl::ClearLogBacktraceLocation();
+ return;
+ }
const absl::string_view file =
absl::string_view(log_backtrace_at).substr(0, last_colon);
int line;
- if (absl::SimpleAtoi(
+ if (!absl::SimpleAtoi(
absl::string_view(log_backtrace_at).substr(last_colon + 1),
&line)) {
- absl::SetLogBacktraceLocation(file, line);
+ absl::ClearLogBacktraceLocation();
+ return;
}
+ absl::SetLogBacktraceLocation(file, line);
});
ABSL_FLAG(bool, log_prefix, true,
diff --git a/absl/log/flags_test.cc b/absl/log/flags_test.cc
index a0f6d76..1080ea1 100644
--- a/absl/log/flags_test.cc
+++ b/absl/log/flags_test.cc
@@ -92,23 +92,23 @@
TEST_F(LogFlagsTest, EmptyBacktraceAtFlag) {
absl::SetMinLogLevel(absl::LogSeverityAtLeast::kInfo);
- absl::SetFlag(&FLAGS_log_backtrace_at, "");
absl::ScopedMockLog test_sink(absl::MockLogDefault::kDisallowUnexpected);
EXPECT_CALL(test_sink, Send(TextMessage(Not(HasSubstr("(stacktrace:")))));
test_sink.StartCapturingLogs();
+ absl::SetFlag(&FLAGS_log_backtrace_at, "");
LOG(INFO) << "hello world";
}
TEST_F(LogFlagsTest, BacktraceAtNonsense) {
absl::SetMinLogLevel(absl::LogSeverityAtLeast::kInfo);
- absl::SetFlag(&FLAGS_log_backtrace_at, "gibberish");
absl::ScopedMockLog test_sink(absl::MockLogDefault::kDisallowUnexpected);
EXPECT_CALL(test_sink, Send(TextMessage(Not(HasSubstr("(stacktrace:")))));
test_sink.StartCapturingLogs();
+ absl::SetFlag(&FLAGS_log_backtrace_at, "gibberish");
LOG(INFO) << "hello world";
}
@@ -116,13 +116,13 @@
absl::SetMinLogLevel(absl::LogSeverityAtLeast::kInfo);
const int log_line = __LINE__ + 1;
auto do_log = [] { LOG(INFO) << "hello world"; };
- absl::SetFlag(&FLAGS_log_backtrace_at,
- absl::StrCat("some_other_file.cc:", log_line));
absl::ScopedMockLog test_sink(absl::MockLogDefault::kDisallowUnexpected);
EXPECT_CALL(test_sink, Send(TextMessage(Not(HasSubstr("(stacktrace:")))));
test_sink.StartCapturingLogs();
+ absl::SetFlag(&FLAGS_log_backtrace_at,
+ absl::StrCat("some_other_file.cc:", log_line));
do_log();
}
@@ -130,13 +130,13 @@
absl::SetMinLogLevel(absl::LogSeverityAtLeast::kInfo);
const int log_line = __LINE__ + 1;
auto do_log = [] { LOG(INFO) << "hello world"; };
- absl::SetFlag(&FLAGS_log_backtrace_at,
- absl::StrCat("flags_test.cc:", log_line + 1));
absl::ScopedMockLog test_sink(absl::MockLogDefault::kDisallowUnexpected);
EXPECT_CALL(test_sink, Send(TextMessage(Not(HasSubstr("(stacktrace:")))));
test_sink.StartCapturingLogs();
+ absl::SetFlag(&FLAGS_log_backtrace_at,
+ absl::StrCat("flags_test.cc:", log_line + 1));
do_log();
}
@@ -144,12 +144,12 @@
absl::SetMinLogLevel(absl::LogSeverityAtLeast::kInfo);
const int log_line = __LINE__ + 1;
auto do_log = [] { LOG(INFO) << "hello world"; };
- absl::SetFlag(&FLAGS_log_backtrace_at, absl::StrCat(__FILE__, ":", log_line));
absl::ScopedMockLog test_sink(absl::MockLogDefault::kDisallowUnexpected);
EXPECT_CALL(test_sink, Send(TextMessage(Not(HasSubstr("(stacktrace:")))));
test_sink.StartCapturingLogs();
+ absl::SetFlag(&FLAGS_log_backtrace_at, absl::StrCat(__FILE__, ":", log_line));
do_log();
}
@@ -157,13 +157,13 @@
absl::SetMinLogLevel(absl::LogSeverityAtLeast::kInfo);
const int log_line = __LINE__ + 1;
auto do_log = [] { LOG(INFO) << "hello world"; };
- absl::SetFlag(&FLAGS_log_backtrace_at,
- absl::StrCat("flags_test.cc:", log_line, "gibberish"));
absl::ScopedMockLog test_sink(absl::MockLogDefault::kDisallowUnexpected);
EXPECT_CALL(test_sink, Send(TextMessage(Not(HasSubstr("(stacktrace:")))));
test_sink.StartCapturingLogs();
+ absl::SetFlag(&FLAGS_log_backtrace_at,
+ absl::StrCat("flags_test.cc:", log_line, "gibberish"));
do_log();
}
@@ -171,13 +171,17 @@
absl::SetMinLogLevel(absl::LogSeverityAtLeast::kInfo);
const int log_line = __LINE__ + 1;
auto do_log = [] { LOG(INFO) << "hello world"; };
- absl::SetFlag(&FLAGS_log_backtrace_at,
- absl::StrCat("flags_test.cc:", log_line));
absl::ScopedMockLog test_sink(absl::MockLogDefault::kDisallowUnexpected);
+ testing::InSequence seq;
EXPECT_CALL(test_sink, Send(TextMessage(HasSubstr("(stacktrace:"))));
+ EXPECT_CALL(test_sink, Send(TextMessage(Not(HasSubstr("(stacktrace:")))));
test_sink.StartCapturingLogs();
+ absl::SetFlag(&FLAGS_log_backtrace_at,
+ absl::StrCat("flags_test.cc:", log_line));
+ do_log();
+ absl::SetFlag(&FLAGS_log_backtrace_at, "");
do_log();
}
diff --git a/absl/log/globals.cc b/absl/log/globals.cc
index 6dfe81f..f7c2ec4 100644
--- a/absl/log/globals.cc
+++ b/absl/log/globals.cc
@@ -123,7 +123,7 @@
bool ShouldLogBacktraceAt(absl::string_view file, int line) {
const size_t flag_hash =
- log_backtrace_at_hash.load(std::memory_order_acquire);
+ log_backtrace_at_hash.load(std::memory_order_relaxed);
return flag_hash != 0 && flag_hash == HashSiteForLogBacktraceAt(file, line);
}
@@ -132,7 +132,11 @@
void SetLogBacktraceLocation(absl::string_view file, int line) {
log_backtrace_at_hash.store(HashSiteForLogBacktraceAt(file, line),
- std::memory_order_release);
+ std::memory_order_relaxed);
+}
+
+void ClearLogBacktraceLocation() {
+ log_backtrace_at_hash.store(0, std::memory_order_relaxed);
}
bool ShouldPrependLogPrefix() {
diff --git a/absl/log/globals.h b/absl/log/globals.h
index 32b87db..d56622a 100644
--- a/absl/log/globals.h
+++ b/absl/log/globals.h
@@ -110,8 +110,8 @@
// Log Backtrace At
//------------------------------------------------------------------------------
//
-// Users can request backtrace to be logged at specific locations, specified
-// by file and line number.
+// Users can request an existing `LOG` statement, specified by file and line
+// number, to also include a backtrace when logged.
// ShouldLogBacktraceAt()
//
@@ -123,9 +123,16 @@
// SetLogBacktraceLocation()
//
-// Sets the location the backtrace should be logged at.
+// Sets the location the backtrace should be logged at. If the specified
+// location isn't a `LOG` statement, the effect will be the same as
+// `ClearLogBacktraceLocation` (but less efficient).
void SetLogBacktraceLocation(absl::string_view file, int line);
+// ClearLogBacktraceLocation()
+//
+// Clears the set location so that backtraces will no longer be logged at it.
+void ClearLogBacktraceLocation();
+
//------------------------------------------------------------------------------
// Prepend Log Prefix
//------------------------------------------------------------------------------
