Remove multiple logging implementations.
Change-Id: I7474c29b394f37918a35bba58b2fab58a7be930f
diff --git a/aos/dump_rtprio.cc b/aos/dump_rtprio.cc
index 12000f0..5d9a0b4 100644
--- a/aos/dump_rtprio.cc
+++ b/aos/dump_rtprio.cc
@@ -248,7 +248,7 @@
int main() {
::aos::logging::Init();
- ::aos::logging::AddImplementation(
+ ::aos::logging::SetImplementation(
new ::aos::logging::StreamLogImplementation(stdout));
const int pid_max = find_pid_max();
diff --git a/aos/ipc_lib/ipc_comparison.cc b/aos/ipc_lib/ipc_comparison.cc
index 1553159..cf5581e 100644
--- a/aos/ipc_lib/ipc_comparison.cc
+++ b/aos/ipc_lib/ipc_comparison.cc
@@ -955,7 +955,7 @@
::gflags::ParseCommandLineFlags(&argc, &argv, true);
::aos::InitNRT();
- ::aos::logging::AddImplementation(
+ ::aos::logging::SetImplementation(
new ::aos::logging::StreamLogImplementation(stdout));
return ::aos::Main(argc, argv);
diff --git a/aos/ipc_lib/raw_queue_test.cc b/aos/ipc_lib/raw_queue_test.cc
index 3048e1b..1c02c53 100644
--- a/aos/ipc_lib/raw_queue_test.cc
+++ b/aos/ipc_lib/raw_queue_test.cc
@@ -994,52 +994,52 @@
EXPECT_DEATH(
{
- logging::AddImplementation(new util::DeathTestLogImplementation());
+ logging::SetImplementation(new util::DeathTestLogImplementation());
queue->WriteMessage(nullptr, RawQueue::kPeek);
},
".*illegal write option.*");
EXPECT_DEATH(
{
- logging::AddImplementation(new util::DeathTestLogImplementation());
+ logging::SetImplementation(new util::DeathTestLogImplementation());
queue->WriteMessage(nullptr, RawQueue::kFromEnd);
},
".*illegal write option.*");
EXPECT_DEATH(
{
- logging::AddImplementation(new util::DeathTestLogImplementation());
+ logging::SetImplementation(new util::DeathTestLogImplementation());
queue->WriteMessage(nullptr, RawQueue::kPeek | RawQueue::kFromEnd);
},
".*illegal write option.*");
EXPECT_DEATH(
{
- logging::AddImplementation(new util::DeathTestLogImplementation());
+ logging::SetImplementation(new util::DeathTestLogImplementation());
queue->WriteMessage(nullptr, RawQueue::kNonBlock | RawQueue::kBlock);
},
".*invalid write option.*");
EXPECT_DEATH(
{
- logging::AddImplementation(new util::DeathTestLogImplementation());
+ logging::SetImplementation(new util::DeathTestLogImplementation());
queue->ReadMessageIndex(
RawQueue::kBlock | RawQueue::kFromEnd | RawQueue::kPeek, nullptr);
},
".*ReadMessageIndex.*is not allowed.*");
EXPECT_DEATH(
{
- logging::AddImplementation(new util::DeathTestLogImplementation());
+ logging::SetImplementation(new util::DeathTestLogImplementation());
queue->ReadMessageIndex(RawQueue::kOverride, nullptr);
},
".*illegal read option.*");
EXPECT_DEATH(
{
- logging::AddImplementation(new util::DeathTestLogImplementation());
+ logging::SetImplementation(new util::DeathTestLogImplementation());
queue->ReadMessageIndex(RawQueue::kOverride | RawQueue::kBlock,
nullptr);
},
".*illegal read option.*");
EXPECT_DEATH(
{
- logging::AddImplementation(new util::DeathTestLogImplementation());
+ logging::SetImplementation(new util::DeathTestLogImplementation());
queue->ReadMessage(RawQueue::kNonBlock | RawQueue::kBlock);
},
".*invalid read option.*");
diff --git a/aos/logging/implementations.cc b/aos/logging/implementations.cc
index b35414e..cfc14c0 100644
--- a/aos/logging/implementations.cc
+++ b/aos/logging/implementations.cc
@@ -23,30 +23,18 @@
// apply here (mostly the parts about being able to use AOS_LOG) because this is
// the root one.
class RootLogImplementation : public LogImplementation {
- public:
- void have_other_implementation() { only_implementation_ = false; }
-
protected:
virtual ::aos::monotonic_clock::time_point monotonic_now() const {
return ::aos::monotonic_clock::now();
}
private:
- void set_next(LogImplementation *) override {
- AOS_LOG(FATAL, "can't have a next logger from here\n");
- }
-
__attribute__((format(GOOD_PRINTF_FORMAT_TYPE, 3, 0))) void DoLog(
log_level level, const char *format, va_list ap) override {
LogMessage message;
internal::FillInMessage(level, monotonic_now(), format, ap, &message);
internal::PrintMessage(stderr, message);
- if (!only_implementation_) {
- fputs("root logger got used, see stderr for message\n", stdout);
- }
}
-
- bool only_implementation_ = true;
};
RootLogImplementation *root_implementation = nullptr;
@@ -140,22 +128,26 @@
internal::PrintMessage(stream_, message);
}
-void AddImplementation(LogImplementation *implementation) {
+void SetImplementation(LogImplementation *implementation, bool update_global) {
internal::Context *context = internal::Context::Get();
- if (implementation->next() != NULL) {
- AOS_LOG(FATAL,
- "%p already has a next implementation, but it's not"
- " being used yet\n",
- implementation);
+ if (implementation == nullptr) {
+ AOS_LOG(FATAL, "SetImplementation got invalid implementation");
}
- LogImplementation *old = context->implementation;
- if (old != NULL) {
- implementation->set_next(old);
+ context->implementation = implementation;
+ if (update_global) {
+ SetGlobalImplementation(implementation);
}
- SetGlobalImplementation(implementation);
- root_implementation->have_other_implementation();
+}
+
+LogImplementation *SwapImplementation(LogImplementation *implementation) {
+ internal::Context *context = internal::Context::Get();
+
+ LogImplementation *old = context->implementation;
+ context->implementation = implementation;
+
+ return old;
}
void Init() {
@@ -267,13 +259,14 @@
Die("logging: couldn't fetch queue\n");
}
- AddImplementation(new LinuxQueueLogImplementation());
+ SetImplementation(new LinuxQueueLogImplementation());
}
void RegisterCallbackImplementation(
- const ::std::function<void(const LogMessage &)> &callback) {
+ const ::std::function<void(const LogMessage &)> &callback,
+ bool update_global = true) {
Init();
- AddImplementation(new CallbackLogImplementation(callback));
+ SetImplementation(new CallbackLogImplementation(callback), update_global);
}
} // namespace logging
diff --git a/aos/logging/implementations.h b/aos/logging/implementations.h
index ce8271f..78980ff 100644
--- a/aos/logging/implementations.h
+++ b/aos/logging/implementations.h
@@ -132,7 +132,12 @@
// when needed or by calling Load()).
// The logging system takes ownership of implementation. It will delete it if
// necessary, so it must be created with new.
-void AddImplementation(LogImplementation *implementation);
+void SetImplementation(LogImplementation *implementation,
+ bool update_global = true);
+
+// Updates the log implementation for the current thread, returning the current
+// implementation.
+LogImplementation *SwapImplementation(LogImplementation *implementation);
// Must be called at least once per process/load before anything else is
// called. This function is safe to call multiple times from multiple
@@ -152,7 +157,7 @@
// The caller takes ownership.
RawQueue *GetLoggingQueue();
-// Calls AddImplementation to register the standard linux logging implementation
+// Calls SetImplementation to register the standard linux logging implementation
// which sends the messages through a queue. This implementation relies on
// another process(es) to read the log messages that it puts into the queue.
// This function is usually called by aos::Init*.
diff --git a/aos/logging/implementations_test.cc b/aos/logging/implementations_test.cc
index d97e165..a7b8b7d 100644
--- a/aos/logging/implementations_test.cc
+++ b/aos/logging/implementations_test.cc
@@ -1,17 +1,17 @@
+#include "aos/logging/implementations.h"
+
#include <inttypes.h>
#include <chrono>
#include <string>
+#include "aos/logging/printf_formats.h"
+#include "aos/time/time.h"
#include "gtest/gtest.h"
-#include "aos/logging/implementations.h"
-#include "aos/time/time.h"
-#include "aos/logging/printf_formats.h"
-
+using ::testing::AssertionFailure;
using ::testing::AssertionResult;
using ::testing::AssertionSuccess;
-using ::testing::AssertionFailure;
namespace aos {
namespace logging {
@@ -51,8 +51,8 @@
protected:
AssertionResult WasAnythingLogged() {
if (log_implementation->used()) {
- return AssertionSuccess() << "read message '" <<
- log_implementation->message().message << "'";
+ return AssertionSuccess() << "read message '"
+ << log_implementation->message().message << "'";
}
return AssertionFailure();
}
@@ -61,9 +61,9 @@
return AssertionFailure() << "nothing was logged";
}
if (log_implementation->message().level != level) {
- return AssertionFailure() << "a message with level " <<
- log_str(log_implementation->message().level) <<
- " was logged instead of " << log_str(level);
+ return AssertionFailure() << "a message with level "
+ << log_str(log_implementation->message().level)
+ << " was logged instead of " << log_str(level);
}
internal::Context *context = internal::Context::Get();
if (log_implementation->message().source != context->source) {
@@ -73,17 +73,16 @@
}
if (log_implementation->message().name_length != context->name_size ||
memcmp(log_implementation->message().name, context->name,
- context->name_size) !=
- 0) {
+ context->name_size) != 0) {
AOS_LOG(FATAL, "got a message from %.*s, but we're %s\n",
static_cast<int>(log_implementation->message().name_length),
log_implementation->message().name, context->name);
}
- if (strstr(log_implementation->message().message, message.c_str())
- == NULL) {
- return AssertionFailure() << "got a message of '" <<
- log_implementation->message().message <<
- "' but expected it to contain '" << message << "'";
+ if (strstr(log_implementation->message().message, message.c_str()) ==
+ NULL) {
+ return AssertionFailure()
+ << "got a message of '" << log_implementation->message().message
+ << "' but expected it to contain '" << message << "'";
}
return AssertionSuccess() << log_implementation->message().message;
@@ -96,14 +95,12 @@
first = false;
Init();
- AddImplementation(log_implementation = new TestLogImplementation());
+ SetImplementation(log_implementation = new TestLogImplementation());
}
log_implementation->reset_used();
}
- void TearDown() override {
- Cleanup();
- }
+ void TearDown() override { Cleanup(); }
static TestLogImplementation *log_implementation;
};
@@ -169,7 +166,7 @@
TEST_F(LoggingTest, Timing) {
// For writing only.
- //static const long kTimingCycles = 5000000;
+ // static const long kTimingCycles = 5000000;
static const long kTimingCycles = 5000;
monotonic_clock::time_point start = monotonic_clock::now();
diff --git a/aos/logging/interface.cc b/aos/logging/interface.cc
index b72f8e1..e6e9203 100644
--- a/aos/logging/interface.cc
+++ b/aos/logging/interface.cc
@@ -4,8 +4,8 @@
#include <stdio.h>
#include <string.h>
-#include <type_traits>
#include <functional>
+#include <type_traits>
#include "aos/die.h"
#include "aos/logging/context.h"
@@ -21,8 +21,8 @@
const int ret = vsnprintf(output, size, format, ap);
typedef ::std::common_type<int, size_t>::type RetType;
if (ret < 0) {
- AOS_PLOG(FATAL, "vsnprintf(%p, %zd, %s, args) failed",
- output, size, format);
+ AOS_PLOG(FATAL, "vsnprintf(%p, %zd, %s, args) failed", output, size,
+ format);
} else if (static_cast<RetType>(ret) >= static_cast<RetType>(size)) {
// Overwrite the '\0' at the end of the existing data and
// copy in the one on the end of continued.
@@ -32,30 +32,22 @@
}
void RunWithCurrentImplementation(
- int levels, ::std::function<void(LogImplementation *)> function) {
+ ::std::function<void(LogImplementation *)> function) {
Context *context = Context::Get();
- LogImplementation *const top_implementation = context->implementation;
- LogImplementation *new_implementation = top_implementation;
- LogImplementation *implementation = NULL;
- for (int i = 0; i < levels; ++i) {
- implementation = new_implementation;
- if (new_implementation == NULL) {
- Die("no logging implementation to use\n");
- }
- new_implementation = new_implementation->next();
+ LogImplementation *const implementation = context->implementation;
+ if (implementation == NULL) {
+ Die("no logging implementation to use\n");
}
- context->implementation = new_implementation;
function(implementation);
- context->implementation = top_implementation;
}
} // namespace internal
using internal::Context;
-void LogImplementation::DoVLog(log_level level, const char *format, va_list ap,
- int levels) {
+void LogImplementation::DoVLog(log_level level, const char *format,
+ va_list ap) {
auto log_impl = [&](LogImplementation *implementation) {
va_list ap1;
va_copy(ap1, ap);
@@ -66,11 +58,11 @@
VDie(format, ap);
}
};
- internal::RunWithCurrentImplementation(levels, ::std::ref(log_impl));
+ internal::RunWithCurrentImplementation(::std::ref(log_impl));
}
void VLog(log_level level, const char *format, va_list ap) {
- LogImplementation::DoVLog(level, format, ap, 1);
+ LogImplementation::DoVLog(level, format, ap);
}
void VCork(int line, const char *function, const char *format, va_list ap) {
@@ -101,9 +93,8 @@
VCork(line, function, format, ap);
- log_do(level, "%s: %d-%d: %s: %s", file,
- context->cork_data.line_min, context->cork_data.line_max, function,
- context->cork_data.message);
+ log_do(level, "%s: %d-%d: %s: %s", file, context->cork_data.line_min,
+ context->cork_data.line_max, function, context->cork_data.message);
context->cork_data.Reset();
}
diff --git a/aos/logging/interface.h b/aos/logging/interface.h
index 92f0b10..3695e40 100644
--- a/aos/logging/interface.h
+++ b/aos/logging/interface.h
@@ -3,8 +3,8 @@
#include <stdarg.h>
-#include <string>
#include <functional>
+#include <string>
#include "aos/logging/logging.h"
#include "aos/macros.h"
@@ -38,24 +38,19 @@
// overriden methods may end up logging through a given implementation's DoLog.
class LogImplementation {
public:
- LogImplementation() : next_(NULL) {}
+ LogImplementation() {}
- // The one that this one's implementation logs to.
- // NULL means that there is no next one.
- LogImplementation *next() { return next_; }
- // Virtual in case a subclass wants to perform checks. There will be a valid
- // logger other than this one available while this is called.
- virtual void set_next(LogImplementation *next) { next_ = next; }
+ virtual ~LogImplementation() {}
virtual bool fill_type_cache() { return true; }
protected:
// Actually logs the given message. Implementations should somehow create a
// LogMessage and then call internal::FillInMessage.
- __attribute__((format(GOOD_PRINTF_FORMAT_TYPE, 3, 0)))
- virtual void DoLog(log_level level, const char *format, va_list ap) = 0;
- __attribute__((format(GOOD_PRINTF_FORMAT_TYPE, 3, 4)))
- void DoLogVariadic(log_level level, const char *format, ...) {
+ __attribute__((format(GOOD_PRINTF_FORMAT_TYPE, 3, 0))) virtual void DoLog(
+ log_level level, const char *format, va_list ap) = 0;
+ __attribute__((format(GOOD_PRINTF_FORMAT_TYPE, 3, 4))) void DoLogVariadic(
+ log_level level, const char *format, ...) {
va_list ap;
va_start(ap, format);
DoLog(level, format, ap);
@@ -66,12 +61,10 @@
// These functions call similar methods on the "current" LogImplementation or
// Die if they can't find one.
// levels is how many LogImplementations to not use off the stack.
- static void DoVLog(log_level, const char *format, va_list ap, int levels)
+ static void DoVLog(log_level, const char *format, va_list ap)
__attribute__((format(GOOD_PRINTF_FORMAT_TYPE, 2, 0)));
friend void VLog(log_level, const char *, va_list);
-
- LogImplementation *next_;
};
namespace internal {
diff --git a/aos/logging/log_displayer.cc b/aos/logging/log_displayer.cc
index d3e5728..fa8fe6b 100644
--- a/aos/logging/log_displayer.cc
+++ b/aos/logging/log_displayer.cc
@@ -154,7 +154,7 @@
int32_t source_pid = -1;
::aos::logging::Init();
- ::aos::logging::AddImplementation(
+ ::aos::logging::SetImplementation(
new ::aos::logging::StreamLogImplementation(stdout));
while (true) {
diff --git a/aos/mutex/mutex_test.cc b/aos/mutex/mutex_test.cc
index 53b47ca..e7bd617 100644
--- a/aos/mutex/mutex_test.cc
+++ b/aos/mutex/mutex_test.cc
@@ -72,7 +72,7 @@
test_mutex_.Unlock();
EXPECT_DEATH(
{
- logging::AddImplementation(new util::DeathTestLogImplementation());
+ logging::SetImplementation(new util::DeathTestLogImplementation());
test_mutex_.Unlock();
},
".*multiple unlock.*");
@@ -83,7 +83,7 @@
logging::Init();
EXPECT_DEATH(
{
- logging::AddImplementation(new util::DeathTestLogImplementation());
+ logging::SetImplementation(new util::DeathTestLogImplementation());
test_mutex_.Unlock();
},
".*multiple unlock.*");
@@ -93,7 +93,7 @@
TEST_F(MutexDeathTest, RepeatLock) {
EXPECT_DEATH(
{
- logging::AddImplementation(new util::DeathTestLogImplementation());
+ logging::SetImplementation(new util::DeathTestLogImplementation());
ASSERT_FALSE(test_mutex_.Lock());
ASSERT_FALSE(test_mutex_.Lock());
},
@@ -297,7 +297,7 @@
});
EXPECT_DEATH(
{
- logging::AddImplementation(new util::DeathTestLogImplementation());
+ logging::SetImplementation(new util::DeathTestLogImplementation());
MutexLocker locker(mutex);
},
".*previous owner of mutex [^ ]+ died.*");
diff --git a/aos/testing/test_logging.cc b/aos/testing/test_logging.cc
index bc60081..1799cda 100644
--- a/aos/testing/test_logging.cc
+++ b/aos/testing/test_logging.cc
@@ -143,7 +143,7 @@
void *DoEnableTestLogging() {
logging::Init();
- logging::AddImplementation(TestLogImplementation::GetInstance());
+ logging::SetImplementation(TestLogImplementation::GetInstance());
::testing::UnitTest::GetInstance()->listeners().Append(
new MyTestEventListener());
diff --git a/aos/transaction/transaction_test.cc b/aos/transaction/transaction_test.cc
index f7551c8..5d1fe90 100644
--- a/aos/transaction/transaction_test.cc
+++ b/aos/transaction/transaction_test.cc
@@ -93,7 +93,7 @@
logging::Init();
EXPECT_DEATH(
{
- logging::AddImplementation(new util::DeathTestLogImplementation());
+ logging::SetImplementation(new util::DeathTestLogImplementation());
for (int i = 0; i < 1000; ++i) {
CreateWork(i);
}
diff --git a/aos/vision/debug/debug_framework.cc b/aos/vision/debug/debug_framework.cc
index 7b3ad76..47e0856 100644
--- a/aos/vision/debug/debug_framework.cc
+++ b/aos/vision/debug/debug_framework.cc
@@ -151,7 +151,7 @@
void DebugFrameworkMain(int argc, char **argv, FilterHarness *filter,
CameraParams camera_params) {
::aos::logging::Init();
- ::aos::logging::AddImplementation(
+ ::aos::logging::SetImplementation(
new ::aos::logging::StreamLogImplementation(stdout));
gtk_init(&argc, &argv);
diff --git a/aos/vision/tools/camera_primer.cc b/aos/vision/tools/camera_primer.cc
index 1ac96fd..e1d5fc5 100644
--- a/aos/vision/tools/camera_primer.cc
+++ b/aos/vision/tools/camera_primer.cc
@@ -27,7 +27,7 @@
// target_sender
int main(int argc, char **argv) {
::aos::logging::Init();
- ::aos::logging::AddImplementation(
+ ::aos::logging::SetImplementation(
new ::aos::logging::StreamLogImplementation(stdout));
aos::vision::CameraParams params;
diff --git a/aos/vision/tools/jpeg_vision_test.cc b/aos/vision/tools/jpeg_vision_test.cc
index 5267041..4ba290c 100644
--- a/aos/vision/tools/jpeg_vision_test.cc
+++ b/aos/vision/tools/jpeg_vision_test.cc
@@ -112,7 +112,7 @@
int main(int argc, char *argv[]) {
::aos::logging::Init();
- ::aos::logging::AddImplementation(
+ ::aos::logging::SetImplementation(
new ::aos::logging::StreamLogImplementation(stdout));
aos::events::EpollLoop loop;
gtk_init(&argc, &argv);
diff --git a/y2016/vision/target_sender.cc b/y2016/vision/target_sender.cc
index 3e29085..41da462 100644
--- a/y2016/vision/target_sender.cc
+++ b/y2016/vision/target_sender.cc
@@ -226,7 +226,7 @@
using namespace y2016::vision;
StereoGeometry stereo("./stereo_rig.calib");
::aos::logging::Init();
- ::aos::logging::AddImplementation(
+ ::aos::logging::SetImplementation(
new ::aos::logging::StreamLogImplementation(stdout));
std::thread cam0([stereo]() {
RunCamera(0, GetCameraParams(stereo.calibration()),
diff --git a/y2017/vision/target_sender.cc b/y2017/vision/target_sender.cc
index 261c397..88655c7 100644
--- a/y2017/vision/target_sender.cc
+++ b/y2017/vision/target_sender.cc
@@ -220,7 +220,7 @@
using namespace y2017::vision;
::aos::logging::Init();
- ::aos::logging::AddImplementation(
+ ::aos::logging::SetImplementation(
new ::aos::logging::StreamLogImplementation(stdout));
VisionConfig cfg;
if (ReadConfiguration("ConfigFile.pb.ascii", &cfg)) {
diff --git a/y2018/vision/image_streamer.cc b/y2018/vision/image_streamer.cc
index b0c1f86..fa5f4b5 100644
--- a/y2018/vision/image_streamer.cc
+++ b/y2018/vision/image_streamer.cc
@@ -289,7 +289,7 @@
int main(int argc, char ** argv) {
gflags::ParseCommandLineFlags(&argc, &argv, false);
::aos::logging::Init();
- ::aos::logging::AddImplementation(
+ ::aos::logging::SetImplementation(
new ::aos::logging::StreamLogImplementation(stderr));
TCPServer<MjpegDataSocket> tcp_server_(80);
diff --git a/y2019/image_streamer/image_streamer.cc b/y2019/image_streamer/image_streamer.cc
index 30bea8f..6f8027c 100644
--- a/y2019/image_streamer/image_streamer.cc
+++ b/y2019/image_streamer/image_streamer.cc
@@ -308,7 +308,7 @@
int main(int argc, char **argv) {
gflags::ParseCommandLineFlags(&argc, &argv, false);
::aos::logging::Init();
- ::aos::logging::AddImplementation(
+ ::aos::logging::SetImplementation(
new ::aos::logging::StreamLogImplementation(stderr));
TCPServer<MjpegDataSocket> tcp_server_(80);
aos::vision::CameraParams params0;
diff --git a/y2019/vision/debug_serial.cc b/y2019/vision/debug_serial.cc
index 857f20f..c707d1f 100644
--- a/y2019/vision/debug_serial.cc
+++ b/y2019/vision/debug_serial.cc
@@ -24,7 +24,7 @@
using namespace frc971::jevois;
// gflags::ParseCommandLineFlags(&argc, &argv, false);
::aos::logging::Init();
- ::aos::logging::AddImplementation(
+ ::aos::logging::SetImplementation(
new ::aos::logging::StreamLogImplementation(stderr));
int flags = fcntl(0, F_GETFL, 0);
diff --git a/y2019/vision/global_calibration.cc b/y2019/vision/global_calibration.cc
index e2c4490..52a6e52 100644
--- a/y2019/vision/global_calibration.cc
+++ b/y2019/vision/global_calibration.cc
@@ -103,7 +103,7 @@
};
::aos::logging::Init();
- ::aos::logging::AddImplementation(
+ ::aos::logging::SetImplementation(
new ::aos::logging::StreamLogImplementation(stderr));
TargetFinder target_finder;