From 3753c6278355308b280ce630b82fcf29d6d139c5 Mon Sep 17 00:00:00 2001 From: James Turner Date: Sun, 6 Sep 2020 14:56:55 +0100 Subject: [PATCH] Add MANDATORY_INFO log priority. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit New log-level to avoid using ‘ALERT’ for expected mandatory messages, since I want to do error collection on real alerts in a future commit. As part of this, split logstream.hxx into separate header files, and add a new virtual hook to LogCallback which takes the complete log-entry --- simgear/debug/BufferedLogCallback.hxx | 2 +- simgear/debug/CMakeLists.txt | 7 +- simgear/debug/LogCallback.cxx | 116 +++++++++++++++++++ simgear/debug/LogCallback.hxx | 56 +++++++++ simgear/debug/LogEntry.cxx | 45 ++++++++ simgear/debug/LogEntry.hxx | 54 +++++++++ simgear/debug/OsgIoCapture.hxx | 3 +- simgear/debug/debug_types.h | 21 ++-- simgear/debug/logstream.cxx | 157 +++++--------------------- simgear/debug/logstream.hxx | 23 +--- simgear/scene/tsync/terrasync.cxx | 6 +- 11 files changed, 319 insertions(+), 171 deletions(-) create mode 100644 simgear/debug/LogCallback.cxx create mode 100644 simgear/debug/LogCallback.hxx create mode 100644 simgear/debug/LogEntry.cxx create mode 100644 simgear/debug/LogEntry.hxx diff --git a/simgear/debug/BufferedLogCallback.hxx b/simgear/debug/BufferedLogCallback.hxx index bda7fd31..51e11532 100644 --- a/simgear/debug/BufferedLogCallback.hxx +++ b/simgear/debug/BufferedLogCallback.hxx @@ -26,7 +26,7 @@ #include #include // for std::unique_ptr -#include +#include namespace simgear { diff --git a/simgear/debug/CMakeLists.txt b/simgear/debug/CMakeLists.txt index e494eb92..0b2a2bab 100644 --- a/simgear/debug/CMakeLists.txt +++ b/simgear/debug/CMakeLists.txt @@ -1,7 +1,10 @@ include (SimGearComponent) -set(HEADERS debug_types.h logstream.hxx BufferedLogCallback.hxx OsgIoCapture.hxx) -set(SOURCES logstream.cxx BufferedLogCallback.cxx) +set(HEADERS debug_types.h + logstream.hxx BufferedLogCallback.hxx OsgIoCapture.hxx + LogCallback.hxx LogEntry.hxx) +set(SOURCES logstream.cxx BufferedLogCallback.cxx + LogCallback.cxx LogEntry.cxx) simgear_component(debug debug "${SOURCES}" "${HEADERS}") diff --git a/simgear/debug/LogCallback.cxx b/simgear/debug/LogCallback.cxx new file mode 100644 index 00000000..ce584719 --- /dev/null +++ b/simgear/debug/LogCallback.cxx @@ -0,0 +1,116 @@ +// This library is free software; you can redistribute it and/or +// modify it under the terms of the GNU Library General Public +// License as published by the Free Software Foundation; either +// version 2 of the License, or (at your option) any later version. +// +// 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 +// Library General Public License for more details. +// +// You should have received a copy of the GNU General Public License +// along with this program; if not, write to the Free Software +// Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. +// + +#include + +#include "LogCallback.hxx" + +using namespace simgear; + +LogCallback::LogCallback(sgDebugClass c, sgDebugPriority p) : m_class(c), + m_priority(p) +{ +} + +void LogCallback::operator()(sgDebugClass c, sgDebugPriority p, + const char* file, int line, const std::string& aMessage) +{ + // override me +} + +bool LogCallback::doProcessEntry(const LogEntry& e) +{ + return false; +} + +void LogCallback::processEntry(const LogEntry& e) +{ + if (doProcessEntry(e)) + return; // derived class used the new API + + // call the old API + (*this)(e.debugClass, e.debugPriority, e.file, e.line, e.message); +} + + +bool LogCallback::shouldLog(sgDebugClass c, sgDebugPriority p) const +{ + if ((c & m_class) != 0 && p >= m_priority) + return true; + if (c == SG_OSG) // always have OSG logging as it OSG logging is configured separately. + return true; + return false; +} + +void LogCallback::setLogLevels(sgDebugClass c, sgDebugPriority p) +{ + m_priority = p; + m_class = c; +} +const char* LogCallback::debugPriorityToString(sgDebugPriority p) +{ + switch (p) { + case SG_DEV_ALERT: + case SG_ALERT: + return "ALRT"; + case SG_BULK: return "BULK"; + case SG_DEBUG: return "DBUG"; + case SG_MANDATORY_INFO: + case SG_INFO: + return "INFO"; + case SG_POPUP: return "POPU"; + case SG_DEV_WARN: + case SG_WARN: + return "WARN"; + + default: return "UNKN"; + } +} + +const char* LogCallback::debugClassToString(sgDebugClass c) +{ + switch (c) { + case SG_NONE: return "none"; + case SG_TERRAIN: return "terrain"; + case SG_ASTRO: return "astro"; + case SG_FLIGHT: return "flight"; + case SG_INPUT: return "input"; + case SG_GL: return "opengl"; + case SG_VIEW: return "view"; + case SG_COCKPIT: return "cockpit"; + case SG_GENERAL: return "general"; + case SG_MATH: return "math"; + case SG_EVENT: return "event"; + case SG_AIRCRAFT: return "aircraft"; + case SG_AUTOPILOT: return "autopilot"; + case SG_IO: return "io"; + case SG_CLIPPER: return "clipper"; + case SG_NETWORK: return "network"; + case SG_ATC: return "atc"; + case SG_NASAL: return "nasal"; + case SG_INSTR: return "instruments"; + case SG_SYSTEMS: return "systems"; + case SG_AI: return "ai"; + case SG_ENVIRONMENT: return "environment"; + case SG_SOUND: return "sound"; + case SG_NAVAID: return "navaid"; + case SG_GUI: return "gui"; + case SG_TERRASYNC: return "terrasync"; + case SG_PARTICLES: return "particles"; + case SG_HEADLESS: return "headless"; + case SG_OSG: return "OSG"; + default: return "unknown"; + } +} diff --git a/simgear/debug/LogCallback.hxx b/simgear/debug/LogCallback.hxx new file mode 100644 index 00000000..3c9a38b3 --- /dev/null +++ b/simgear/debug/LogCallback.hxx @@ -0,0 +1,56 @@ +// This library is free software; you can redistribute it and/or +// modify it under the terms of the GNU Library General Public +// License as published by the Free Software Foundation; either +// version 2 of the License, or (at your option) any later version. +// +// 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 +// Library General Public License for more details. +// +// You should have received a copy of the GNU General Public License +// along with this program; if not, write to the Free Software +// Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. +// + +#pragma once + +#include + +#include "LogEntry.hxx" +#include "debug_types.h" + +namespace simgear { + +class LogCallback +{ +public: + virtual ~LogCallback() = default; + + // newer API: return true if you handled the message, otherwise + // the old API will be called + virtual bool doProcessEntry(const LogEntry& e); + + // old API, kept for compatability + virtual void operator()(sgDebugClass c, sgDebugPriority p, + const char* file, int line, const std::string& aMessage); + + void setLogLevels(sgDebugClass c, sgDebugPriority p); + + void processEntry(const LogEntry& e); + +protected: + LogCallback(sgDebugClass c, sgDebugPriority p); + + bool shouldLog(sgDebugClass c, sgDebugPriority p) const; + + static const char* debugClassToString(sgDebugClass c); + static const char* debugPriorityToString(sgDebugPriority p); + +private: + sgDebugClass m_class; + sgDebugPriority m_priority; +}; + + +} // namespace simgear diff --git a/simgear/debug/LogEntry.cxx b/simgear/debug/LogEntry.cxx new file mode 100644 index 00000000..5f8c5357 --- /dev/null +++ b/simgear/debug/LogEntry.cxx @@ -0,0 +1,45 @@ + +// This library is free software; you can redistribute it and/or +// modify it under the terms of the GNU Library General Public +// License as published by the Free Software Foundation; either +// version 2 of the License, or (at your option) any later version. +// +// 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 +// Library General Public License for more details. +// +// You should have received a copy of the GNU General Public License +// along with this program; if not, write to the Free Software +// Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. +// + +#include + +#include "LogEntry.hxx" + +#include // for strdup + +namespace simgear { + +LogEntry::~LogEntry() +{ + if (freeFilename) { + free(const_cast(file)); + } +} + +LogEntry::LogEntry(const LogEntry& c) : debugClass(c.debugClass), + debugPriority(c.debugPriority), + originalPriority(c.originalPriority), + file(c.file), + line(c.line), + message(c.message), + freeFilename(c.freeFilename) +{ + if (c.freeFilename) { + file = strdup(c.file); + } +} + +} // namespace simgear \ No newline at end of file diff --git a/simgear/debug/LogEntry.hxx b/simgear/debug/LogEntry.hxx new file mode 100644 index 00000000..9a4cce63 --- /dev/null +++ b/simgear/debug/LogEntry.hxx @@ -0,0 +1,54 @@ +// This library is free software; you can redistribute it and/or +// modify it under the terms of the GNU Library General Public +// License as published by the Free Software Foundation; either +// version 2 of the License, or (at your option) any later version. +// +// 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 +// Library General Public License for more details. +// +// You should have received a copy of the GNU General Public License +// along with this program; if not, write to the Free Software +// Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. +// + +#pragma once + +#include + +#include "debug_types.h" + +namespace simgear { +/** + * storage of a single log entry. This is used to pass log entries from + * the various threads to the logging thread, and also to store the startup + * entries + */ +class LogEntry +{ +public: + LogEntry(sgDebugClass c, sgDebugPriority p, + sgDebugPriority op, + const char* f, int l, const std::string& msg) : debugClass(c), debugPriority(p), originalPriority(op), + file(f), line(l), + message(msg) + { + } + + LogEntry(const LogEntry& c); + LogEntry& operator=(const LogEntry& c) = delete; + + ~LogEntry(); + + const sgDebugClass debugClass; + const sgDebugPriority debugPriority; + const sgDebugPriority originalPriority; + const char* file; + const int line; + const std::string message; + + bool freeFilename = false; ///< if true, we own, and therefore need to free, the memory pointed to by 'file' +}; + +} // namespace simgear \ No newline at end of file diff --git a/simgear/debug/OsgIoCapture.hxx b/simgear/debug/OsgIoCapture.hxx index 9f14e441..611778fa 100644 --- a/simgear/debug/OsgIoCapture.hxx +++ b/simgear/debug/OsgIoCapture.hxx @@ -2,8 +2,7 @@ #include -using namespace osg; - +#include /** * merge OSG output into our logging system, so it gets recorded to file, diff --git a/simgear/debug/debug_types.h b/simgear/debug/debug_types.h index 9289cb48..968ea868 100644 --- a/simgear/debug/debug_types.h +++ b/simgear/debug/debug_types.h @@ -1,3 +1,5 @@ +#pragma once + /** \file debug_types.h * Define the various logging classes and priorities */ @@ -52,16 +54,17 @@ typedef enum { * appended, or the priority Nasal reports to compiled code will change. */ typedef enum { - SG_BULK = 1, // For frequent messages - SG_DEBUG, // Less frequent debug type messages - SG_INFO, // Informatory messages - SG_WARN, // Possible impending problem - SG_ALERT, // Very possible impending problem - SG_POPUP, // Severe enough to alert using a pop-up window + SG_BULK = 1, // For frequent messages + SG_DEBUG, // Less frequent debug type messages + SG_INFO, // Informatory messages + SG_WARN, // Possible impending problem + SG_ALERT, // Very possible impending problem + SG_POPUP, // Severe enough to alert using a pop-up window // SG_EXIT, // Problem (no core) // SG_ABORT // Abandon ship (core) - SG_DEV_WARN, // Warning for developers, translated to other priority - SG_DEV_ALERT // Alert for developers, translated -} sgDebugPriority; + SG_DEV_WARN, // Warning for developers, translated to other priority + SG_DEV_ALERT, // Alert for developers, translated + SG_MANDATORY_INFO // information, but should always be shown +} sgDebugPriority; diff --git a/simgear/debug/logstream.cxx b/simgear/debug/logstream.cxx index 5610973c..7900bb65 100644 --- a/simgear/debug/logstream.cxx +++ b/simgear/debug/logstream.cxx @@ -35,6 +35,7 @@ #include #include +#include "LogCallback.hxx" #include #include #include @@ -47,86 +48,9 @@ #include #endif - - ////////////////////////////////////////////////////////////////////////////// -namespace simgear -{ -LogCallback::LogCallback(sgDebugClass c, sgDebugPriority p) : - m_class(c), - m_priority(p) -{ -} - -bool LogCallback::shouldLog(sgDebugClass c, sgDebugPriority p) const -{ - - if ((c & m_class) != 0 && p >= m_priority) - return true; - if (c == SG_OSG) // always have OSG logging as it OSG logging is configured separately. - return true; - return false; -} - -void LogCallback::setLogLevels( sgDebugClass c, sgDebugPriority p ) -{ - m_priority = p; - m_class = c; -} -const char* LogCallback::debugPriorityToString(sgDebugPriority p) -{ - switch (p) { - case SG_ALERT: return "ALRT"; - case SG_BULK: return "BULK"; - case SG_DEBUG: return "DBUG"; - case SG_INFO: return "INFO"; - case SG_POPUP: return "POPU"; - case SG_WARN: return "WARN"; - default: return "UNKN"; - } -} - -const char* LogCallback::debugClassToString(sgDebugClass c) -{ - switch (c) { - case SG_NONE: return "none"; - case SG_TERRAIN: return "terrain"; - case SG_ASTRO: return "astro"; - case SG_FLIGHT: return "flight"; - case SG_INPUT: return "input"; - case SG_GL: return "opengl"; - case SG_VIEW: return "view"; - case SG_COCKPIT: return "cockpit"; - case SG_GENERAL: return "general"; - case SG_MATH: return "math"; - case SG_EVENT: return "event"; - case SG_AIRCRAFT: return "aircraft"; - case SG_AUTOPILOT: return "autopilot"; - case SG_IO: return "io"; - case SG_CLIPPER: return "clipper"; - case SG_NETWORK: return "network"; - case SG_ATC: return "atc"; - case SG_NASAL: return "nasal"; - case SG_INSTR: return "instruments"; - case SG_SYSTEMS: return "systems"; - case SG_AI: return "ai"; - case SG_ENVIRONMENT:return "environment"; - case SG_SOUND: return "sound"; - case SG_NAVAID: return "navaid"; - case SG_GUI: return "gui"; - case SG_TERRASYNC: return "terrasync"; - case SG_PARTICLES: return "particles"; - case SG_HEADLESS: return "headless"; - case SG_OSG: return "OSG"; - default: return "unknown"; - } -} - -} // of namespace simgear - -////////////////////////////////////////////////////////////////////////////// class FileLogCallback : public simgear::LogCallback { @@ -196,8 +120,8 @@ public: } #endif - virtual void operator()(sgDebugClass c, sgDebugPriority p, - const char* file, int line, const std::string& aMessage) + void operator()(sgDebugClass c, sgDebugPriority p, + const char* file, int line, const std::string& aMessage) override { if (!shouldLog(c, p)) return; //fprintf(stderr, "%s\n", aMessage.c_str()); @@ -226,8 +150,8 @@ public: { } - virtual void operator()(sgDebugClass c, sgDebugPriority p, - const char* file, int line, const std::string& aMessage) + void operator()(sgDebugClass c, sgDebugPriority p, + const char* file, int line, const std::string& aMessage) override { if (!shouldLog(c, p)) return; @@ -242,29 +166,6 @@ public: class logstream::LogStreamPrivate : public SGThread { private: - /** - * storage of a single log entry. This is used to pass log entries from - * the various threads to the logging thread, and also to store the startup - * entries - */ - class LogEntry - { - public: - LogEntry(sgDebugClass c, sgDebugPriority p, - const char* f, int l, const std::string& msg) : - debugClass(c), debugPriority(p), file(f), line(l), - message(msg) - { - } - - const sgDebugClass debugClass; - const sgDebugPriority debugPriority; - const char* file; - const int line; - const std::string message; - bool freeFilename = false; - }; - /** * RAII object to pause the logging thread if it's running, and restart it. * used to safely make configuration changes. @@ -412,10 +313,10 @@ public: } std::mutex m_lock; - SGBlockingQueue m_entries; + SGBlockingQueue m_entries; // log entries posted during startup - std::vector m_startupEntries; + std::vector m_startupEntries; bool m_startupLogging = false; typedef std::vector CallbackVec; @@ -438,6 +339,8 @@ public: // test suite mode. bool m_testMode = false; + std::vector _popupMessages; + void startLog() { std::lock_guard g(m_lock); @@ -461,16 +364,13 @@ public: void clearStartupEntriesLocked() { - std::for_each(m_startupEntries.begin(), m_startupEntries.end(), [](const LogEntry& e) { - if (e.freeFilename) free(const_cast(e.file)); - }); m_startupEntries.clear(); } void run() override { while (1) { - LogEntry entry(m_entries.pop()); + simgear::LogEntry entry(m_entries.pop()); // special marker entry detected, terminate the thread since we are // making a configuration change or quitting the app if ((entry.debugClass == SG_NONE) && entry.file && !strcmp(entry.file, "done")) { @@ -486,14 +386,7 @@ public: } // submit to each installed callback in turn for (simgear::LogCallback* cb : m_callbacks) { - (*cb)(entry.debugClass, entry.debugPriority, - entry.file, entry.line, entry.message); - } - - // frrr the filename if required. For startup entries - // we wait and do this later - if (!m_startupLogging && entry.freeFilename) { - free(const_cast(entry.file)); + cb->processEntry(entry); } } // of main thread loop } @@ -523,9 +416,8 @@ public: // we clear startup entries not using this, so always safe to run // this code, container will simply be empty - for (auto entry : m_startupEntries) { - (*cb)(entry.debugClass, entry.debugPriority, - entry.file, entry.line, entry.message); + for (const auto& entry : m_startupEntries) { + cb->processEntry(entry); } } @@ -576,12 +468,13 @@ public: const char* fileName, int line, const std::string& msg, bool freeFilename) { - p = translatePriority(p); + auto tp = translatePriority(p); if (!m_fileLine) { /* This prevents output of file:line in StderrLogCallback. */ line = -line; } - LogEntry entry(c, p, fileName, line, msg); + + simgear::LogEntry entry(c, tp, p, fileName, line, msg); entry.freeFilename = freeFilename; m_entries.push(entry); } @@ -613,8 +506,8 @@ logstream::logstream() logstream::~logstream() { - popup_msgs.clear(); d->stop(); + d.reset(); } void @@ -717,17 +610,18 @@ void logstream::hexdump(sgDebugClass c, sgDebugPriority p, const char* fileName, void logstream::popup( const std::string& msg) { - popup_msgs.push_back(msg); + std::lock_guard g(d->m_lock); + d->_popupMessages.push_back(msg); } std::string logstream::get_popup() { - std::string rv = ""; - if (!popup_msgs.empty()) - { - rv = popup_msgs.front(); - popup_msgs.erase(popup_msgs.begin()); + std::string rv; + std::lock_guard g(d->m_lock); + if (!d->_popupMessages.empty()) { + rv = d->_popupMessages.front(); + d->_popupMessages.erase(d->_popupMessages.begin()); } return rv; } @@ -735,7 +629,8 @@ logstream::get_popup() bool logstream::has_popup() { - return (popup_msgs.size() > 0) ? true : false; + std::lock_guard g(d->m_lock); + return !d->_popupMessages.empty(); } bool diff --git a/simgear/debug/logstream.hxx b/simgear/debug/logstream.hxx index 3c2ef1ad..4c52b935 100644 --- a/simgear/debug/logstream.hxx +++ b/simgear/debug/logstream.hxx @@ -37,27 +37,8 @@ class SGPath; namespace simgear { - -class LogCallback -{ -public: - virtual ~LogCallback() {} - virtual void operator()(sgDebugClass c, sgDebugPriority p, - const char* file, int line, const std::string& aMessage) = 0; - - void setLogLevels(sgDebugClass c, sgDebugPriority p); -protected: - LogCallback(sgDebugClass c, sgDebugPriority p); - - bool shouldLog(sgDebugClass c, sgDebugPriority p) const; - - static const char* debugClassToString(sgDebugClass c); - static const char* debugPriorityToString(sgDebugPriority p); -private: - sgDebugClass m_class; - sgDebugPriority m_priority; -}; +class LogCallback; /** * Helper force a console on platforms where it might optional, when * we need to show a console. This basically means Windows at the @@ -198,8 +179,6 @@ private: // constructor logstream(); - std::vector popup_msgs; - class LogStreamPrivate; std::unique_ptr d; diff --git a/simgear/scene/tsync/terrasync.cxx b/simgear/scene/tsync/terrasync.cxx index 209d0b81..4898081c 100644 --- a/simgear/scene/tsync/terrasync.cxx +++ b/simgear/scene/tsync/terrasync.cxx @@ -448,10 +448,8 @@ bool SGTerraSync::WorkerThread::start() _stop = false; _state = TerrasyncThreadState(); // clean state - // not really an alert - but we want to (always) see this message, so user is - // aware we're downloading scenery (and using bandwidth). - SG_LOG(SG_TERRASYNC,SG_ALERT, - "Starting automatic scenery download/synchronization to '"<< _local_dir << "'."); + SG_LOG(SG_TERRASYNC, SG_MANDATORY_INFO, + "Starting automatic scenery download/synchronization to '" << _local_dir << "'."); SGThread::start(); return true;