Logging: merge Logger and LoggerBase

Having two separate classes had little (no?!) benefit and just
caused confusion.

10 files changed, 88 insertions, 84 deletions

diff --git a/src/syncevo/Cmdline.cpp b/src/syncevo/Cmdline.cpp
index ad04e3f0..b0e1cda1 100644
--- a/src/syncevo/Cmdline.cpp
+++ b/src/syncevo/Cmdline.cpp
@@ -4362,7 +4362,7 @@ private:
      * vararg constructor with NULL termination,
      * out and error stream into stringstream members
      */
-    class TestCmdline : private LoggerBase {
+    class TestCmdline : private Logger {
         void init() {
             pushLogger(this);
 
diff --git a/src/syncevo/ForkExec.cpp b/src/syncevo/ForkExec.cpp
index 99a71e35..649b5b0c 100644
--- a/src/syncevo/ForkExec.cpp
+++ b/src/syncevo/ForkExec.cpp
@@ -128,10 +128,10 @@ void ForkExecParent::forked(gpointer data) throw()
     // any output is printed directly, instead of going through
     // the parent's output processing in LogRedirect.
     if (getenv("SYNCEVOLUTION_DEBUG")) {
-        int index = LoggerBase::numLoggers();
+        int index = Logger::numLoggers();
         LogRedirect *redirect = NULL;
         while (--index >= 0 &&
-               !(redirect = dynamic_cast<LogRedirect *>(LoggerBase::loggerAt(index)))) {
+               !(redirect = dynamic_cast<LogRedirect *>(Logger::loggerAt(index)))) {
         }
         if (redirect) {
             redirect->reset();
diff --git a/src/syncevo/LocalTransportAgent.cpp b/src/syncevo/LocalTransportAgent.cpp
index 59089a18..08b20d45 100644
--- a/src/syncevo/LocalTransportAgent.cpp
+++ b/src/syncevo/LocalTransportAgent.cpp
@@ -177,7 +177,7 @@ void LocalTransportAgent::logChildOutput(const std::string &level, const std::st
 {
     Logger::MessageOptions options(Logger::strToLevel(level.c_str()));
     options.m_processName = &m_clientContext;
-    SyncEvo::LoggerBase::instance().messageWithOptions(options, "%s", message.c_str());
+    SyncEvo::Logger::instance().messageWithOptions(options, "%s", message.c_str());
 }
 
 void LocalTransportAgent::onChildConnect(const GDBusCXX::DBusConnectionPtr &conn)
@@ -550,7 +550,7 @@ public:
                           true /* ignore transmission failures */> m_logOutput;
 };
 
-class LocalTransportAgentChild : public TransportAgent, private LoggerBase
+class LocalTransportAgentChild : public TransportAgent, private Logger
 {
     /** final return code of our main(): non-zero indicates that we need to shut down */
     int m_ret;
@@ -862,7 +862,7 @@ public:
         m_reportSent(false),
         m_status(INACTIVE)
     {
-        LoggerBase::pushLogger(this);
+        Logger::pushLogger(this);
 
         m_forkexec->m_onConnect.connect(boost::bind(&LocalTransportAgentChild::onConnect, this, _1));
         m_forkexec->m_onFailure.connect(boost::bind(&LocalTransportAgentChild::onFailure, this, _1, _2));
@@ -882,7 +882,7 @@ public:
 
     ~LocalTransportAgentChild()
     {
-        LoggerBase::popLogger();
+        Logger::popLogger();
     }
 
     void run()
@@ -1119,7 +1119,7 @@ int LocalTransportMain(int argc, char **argv)
 
     try {
         if (getenv("SYNCEVOLUTION_DEBUG")) {
-            LoggerBase::instance().setLevel(Logger::DEBUG);
+            Logger::instance().setLevel(Logger::DEBUG);
         }
         // process name will be set to target config name once it is known
         Logger::setProcessName("syncevo-local-sync");
diff --git a/src/syncevo/LogRedirect.cpp b/src/syncevo/LogRedirect.cpp
index 2dca16eb..89475c7f 100644
--- a/src/syncevo/LogRedirect.cpp
+++ b/src/syncevo/LogRedirect.cpp
@@ -132,7 +132,7 @@ LogRedirect::LogRedirect(bool both, const char *filename) throw()
                            fileno(m_out) :
                            m_stderr.m_copy), "w");
     }
-    LoggerBase::pushLogger(this);
+    Logger::pushLogger(this);
     m_redirect = this;
 
     if (!getenv("SYNCEVOLUTION_DEBUG")) {
@@ -187,7 +187,7 @@ LogRedirect::~LogRedirect() throw()
         free(m_buffer);
     }
     if (pop) {
-        LoggerBase::popLogger();
+        Logger::popLogger();
     }
 }
 
@@ -440,9 +440,9 @@ bool LogRedirect::process(FDs &fds) throw()
                     if (eol) {
                         m_stdoutData.append(text, eol - text);
                         text = eol + 1;
-                        LoggerBase::instance().message(level, prefix.empty() ? NULL : &prefix,
-                                                       NULL, 0, NULL,
-                                                       "%s", m_stdoutData.c_str());
+                        Logger::instance().message(level, prefix.empty() ? NULL : &prefix,
+                                                   NULL, 0, NULL,
+                                                   "%s", m_stdoutData.c_str());
                         m_stdoutData.clear();
                     }
                 }
@@ -502,9 +502,9 @@ bool LogRedirect::process(FDs &fds) throw()
             if (len > 0 && text[len - 1] == '\n') {
                 text[len - 1] = 0;
             }
-            LoggerBase::instance().message(level, prefix.empty() ? NULL : &prefix,
-                                           NULL, 0, NULL,
-                                           "%s", text);
+            Logger::instance().message(level, prefix.empty() ? NULL : &prefix,
+                                       NULL, 0, NULL,
+                                       "%s", text);
             available = 0;
         }
     } while(have_message);
@@ -617,9 +617,9 @@ void LogRedirect::flush() throw()
     if (!m_stdoutData.empty()) {
         std::string buffer;
         std::swap(buffer, m_stdoutData);
-        LoggerBase::instance().message(Logger::SHOW, NULL,
-                                       NULL, 0, NULL,
-                                       "%s", buffer.c_str());
+        Logger::instance().message(Logger::SHOW, NULL,
+                                   NULL, 0, NULL,
+                                   "%s", buffer.c_str());
     }
 }
 
@@ -641,7 +641,7 @@ class LogRedirectTest : public CppUnit::TestFixture {
      * redirect stdout/stderr, then intercept the log messages and
      * store them for inspection
      */
-    class LogBuffer : public LoggerBase, private boost::noncopyable
+    class LogBuffer : public Logger, private boost::noncopyable
     {
     public:
         std::stringstream m_streams[DEBUG + 1];
diff --git a/src/syncevo/LogStdout.h b/src/syncevo/LogStdout.h
index b9a25f01..d875920e 100644
--- a/src/syncevo/LogStdout.h
+++ b/src/syncevo/LogStdout.h
@@ -33,7 +33,7 @@ SE_BEGIN_CXX
 /**
  * A logger which writes to stdout or a file.
  */
-class LoggerStdout : public LoggerBase
+class LoggerStdout : public Logger
 {
     FILE *m_file;
     bool m_closeFile;
diff --git a/src/syncevo/LogSyslog.cpp b/src/syncevo/LogSyslog.cpp
index f2ac768f..a24d7737 100644
--- a/src/syncevo/LogSyslog.cpp
+++ b/src/syncevo/LogSyslog.cpp
@@ -30,18 +30,18 @@ SE_BEGIN_CXX
 
 LoggerSyslog::LoggerSyslog(const std::string &processName) :
     m_processName(processName),
-    m_parentLogger(LoggerBase::instance())
+    m_parentLogger(Logger::instance())
 {
     // valgrind tells us that openlog() does not copy the string.
     // Must provide pointer to a permanent copy.
     openlog(m_processName.c_str(), LOG_CONS | LOG_NDELAY | LOG_PID, LOG_USER);
-    LoggerBase::pushLogger(this);
+    Logger::pushLogger(this);
 }
 
 LoggerSyslog::~LoggerSyslog()
 {
     closelog();
-    LoggerBase::popLogger();
+    Logger::popLogger();
 }
 
 static void printToSyslog(int sysloglevel, std::string &chunk, size_t expectedTotal)
diff --git a/src/syncevo/LogSyslog.h b/src/syncevo/LogSyslog.h
index 255f3e55..3f72132c 100644
--- a/src/syncevo/LogSyslog.h
+++ b/src/syncevo/LogSyslog.h
@@ -32,10 +32,10 @@ SE_BEGIN_CXX
 /**
  * A logger which writes to syslog.
  */
-class LoggerSyslog : public LoggerBase
+class LoggerSyslog : public Logger
 {
     const std::string m_processName;
-    LoggerBase &m_parentLogger;
+    Logger &m_parentLogger;
 
 public:
     /**
diff --git a/src/syncevo/Logging.cpp b/src/syncevo/Logging.cpp
index 6193c057..82a063a5 100644
--- a/src/syncevo/Logging.cpp
+++ b/src/syncevo/Logging.cpp
@@ -29,15 +29,24 @@ SE_BEGIN_CXX
 
 std::string Logger::m_processName;
 
-static std::vector<LoggerBase *> &loggers()
+Logger::Logger() :
+    m_level(INFO)
+{
+}
+
+Logger::~Logger()
+{
+}
+
+static std::vector<Logger *> &loggers()
 {
     // allocate array once and never free it because it might be needed till
     // the very end of the application life cycle
-    static std::vector<LoggerBase *> *loggers = new std::vector<LoggerBase *>;
+    static std::vector<Logger *> *loggers = new std::vector<Logger *>;
     return *loggers;
 }
 
-LoggerBase &LoggerBase::instance()
+Logger &Logger::instance()
 {
     // prevent destructing this instance as part of the executable's
     // shutdown by allocating it dynamically, because it may be
@@ -52,12 +61,12 @@ LoggerBase &LoggerBase::instance()
     }
 }
 
-void LoggerBase::pushLogger(LoggerBase *logger)
+void Logger::pushLogger(Logger *logger)
 {
     loggers().push_back(logger);
 }
 
-void LoggerBase::popLogger()
+void Logger::popLogger()
 {
     if (loggers().empty()) {
         throw "too many popLogger() calls";
@@ -66,19 +75,19 @@ void LoggerBase::popLogger()
     }
 }
 
-int LoggerBase::numLoggers()
+int Logger::numLoggers()
 {
     return (int)loggers().size();
 }
 
-LoggerBase *LoggerBase::loggerAt(int index)
+Logger *Logger::loggerAt(int index)
 {
     return index < 0 || index >= (int)loggers().size() ?
         NULL :
         loggers()[index];
 }
 
-void LoggerBase::formatLines(Level msglevel,
+void Logger::formatLines(Level msglevel,
                              Level outputlevel,
                              const std::string *processName,
                              const std::string *prefix,
@@ -293,18 +302,18 @@ void Logger::glogFunc(const gchar *logDomain,
                       const gchar *message,
                       gpointer userData)
 {
-    LoggerBase::instance().message((logLevel & (G_LOG_LEVEL_ERROR|G_LOG_LEVEL_CRITICAL)) ? ERROR :
-                                   (logLevel & G_LOG_LEVEL_WARNING) ? WARNING :
-                                   (logLevel & (G_LOG_LEVEL_MESSAGE|G_LOG_LEVEL_INFO)) ? SHOW :
-                                   DEBUG,
-                                   NULL,
-                                   NULL,
-                                   0,
-                                   NULL,
-                                   "%s%s%s",
-                                   logDomain ? logDomain : "",
-                                   logDomain ? ": " : "",
-                                   message);
+    Logger::instance().message((logLevel & (G_LOG_LEVEL_ERROR|G_LOG_LEVEL_CRITICAL)) ? ERROR :
+                               (logLevel & G_LOG_LEVEL_WARNING) ? WARNING :
+                               (logLevel & (G_LOG_LEVEL_MESSAGE|G_LOG_LEVEL_INFO)) ? SHOW :
+                               DEBUG,
+                               NULL,
+                               NULL,
+                               0,
+                               NULL,
+                               "%s%s%s",
+                               logDomain ? logDomain : "",
+                               logDomain ? ": " : "",
+                               message);
 }
 
 #endif
@@ -322,7 +331,7 @@ int Logger::sysyncPrintf(FILE *stream,
         // in a better way (= to each line) via the prefix parameter.
         format += prefix.size() + 1;
     }
-    LoggerBase::instance().messagev(MessageOptions(DEBUG, &prefix, NULL, 0, NULL), format, args);
+    Logger::instance().messagev(MessageOptions(DEBUG, &prefix, NULL, 0, NULL), format, args);
     va_end(args);
 
     return 0;
diff --git a/src/syncevo/Logging.h b/src/syncevo/Logging.h
index 7d1c878a..283efa8d 100644
--- a/src/syncevo/Logging.h
+++ b/src/syncevo/Logging.h
@@ -44,6 +44,14 @@ SE_BEGIN_CXX
  * implemented by other classes to add information (like a certain
  * prefix) before passing the message on to a global instance for the
  * actual processing.
+ *
+ * The static methods provide some common utility code and manage a
+ * global stack of loggers. The one pushed latest is called first to
+ * handle a new message. It can find its parent logger (= the one
+ * added just before it) and optionally pass the message up the chain
+ * before or after processing it itself.
+ *
+ * All methods must be thread-safe.
  */
 class Logger
 {
@@ -149,7 +157,8 @@ class Logger
                             const char *format,
                             ...);
 
-    virtual ~Logger() {}
+    Logger();
+    virtual ~Logger();
 
     /**
      * Collects all the parameters which may get passed to
@@ -221,28 +230,13 @@ class Logger
 #endif
         ;
 
- protected:
-    static std::string m_processName;
-};
-
-/**
- * Global logging, implemented as a singleton with one instance per
- * process.
- *
- * @TODO avoid global variable
- */
-class LoggerBase : public Logger
-{
- public:
-     LoggerBase() : m_level(INFO) {}
-
     /**
      * Grants access to the singleton which implements logging.
      * The implementation of this function and thus the Log
      * class itself is platform specific: if no Log instance
      * has been set yet, then this call has to create one.
      */
-    static LoggerBase &instance();
+    static Logger &instance();
 
     /**
      * Overrides the default Logger implementation. The Logger class
@@ -250,8 +244,8 @@ class LoggerBase : public Logger
      *
      * @param logger    will be used for all future logging activities
      */
+    static void pushLogger(Logger *logger);
 
-    static void pushLogger(LoggerBase *logger);
     /**
      * Remove the current logger and restore previous one.
      * Must match a pushLogger() call.
@@ -266,7 +260,7 @@ class LoggerBase : public Logger
      * @param index    0 for oldest (inner-most) logger
      * @return pointer or NULL for invalid index
      */
-    static LoggerBase *loggerAt(int index);
+    static Logger *loggerAt(int index);
 
     virtual void setLevel(Level level) { m_level = level; }
     virtual Level getLevel() { return m_level; }
@@ -294,6 +288,7 @@ class LoggerBase : public Logger
                      boost::function<void (std::string &chunk, size_t expectedTotal)> print);
 
  private:
+    static std::string m_processName;
     Level m_level;
 
     /**
@@ -317,13 +312,13 @@ class LoggerBase : public Logger
  * @TODO add function name (GCC extension)
  */
 #define SE_LOG(_prefix, _level, _format, _args...) \
-    SyncEvo::LoggerBase::instance().message(_level, \
-                                            _prefix, \
-                                            __FILE__, \
-                                            __LINE__, \
-                                            NULL, \
-                                            _format, \
-                                            ##_args); \
+    SyncEvo::Logger::instance().message(_level, \
+                                        _prefix, \
+                                        __FILE__, \
+                                        __LINE__, \
+                                        NULL, \
+                                        _format, \
+                                        ##_args);
 
 #define SE_LOG_SHOW(_prefix, _format, _args...) SE_LOG(_prefix, SyncEvo::Logger::SHOW, _format, ##_args)
 #define SE_LOG_ERROR(_prefix, _format, _args...) SE_LOG(_prefix, SyncEvo::Logger::ERROR, _format, ##_args)
diff --git a/src/syncevo/SyncContext.cpp b/src/syncevo/SyncContext.cpp
index 4950fdb3..fe8e9183 100644
--- a/src/syncevo/SyncContext.cpp
+++ b/src/syncevo/SyncContext.cpp
@@ -270,7 +270,7 @@ public:
 // this class owns the logging directory and is responsible
 // for redirecting output at the start and end of sync (even
 // in case of exceptions thrown!)
-class LogDir : public LoggerBase, private boost::noncopyable, private LogDirNames {
+class LogDir : public Logger, private boost::noncopyable, private LogDirNames {
     SyncContext &m_client;
     Logger &m_parentLogger;  /**< the logger which was active before we started to intercept messages */
     string m_logdir;         /**< configured backup root dir */
@@ -290,7 +290,7 @@ class LogDir : public LoggerBase, private boost::noncopyable, private LogDirName
     SyncReport *m_report;    /**< record start/end times here */
 
 public:
-    LogDir(SyncContext &client) : m_client(client), m_parentLogger(LoggerBase::instance()), m_info(NULL), m_readonly(false), m_report(NULL)
+    LogDir(SyncContext &client) : m_client(client), m_parentLogger(Logger::instance()), m_info(NULL), m_readonly(false), m_report(NULL)
     {
         // Set default log directory. This will be overwritten with a user-specified
         // location later on, if one was selected by the user. SyncEvolution >= 0.9 alpha
@@ -537,10 +537,10 @@ public:
             break;
         }
         if (mode != SESSION_USE_PATH) {
-            LoggerBase::instance().setLevel(level);
+            Logger::instance().setLevel(level);
         }
         setLevel(level);
-        LoggerBase::pushLogger(this);
+        Logger::pushLogger(this);
 
         time_t start = time(NULL);
         if (m_report) {
@@ -720,8 +720,8 @@ public:
 
     // remove redirection of logging (safe for destructor)
     void restore() {
-        if (&LoggerBase::instance() == this) {
-            LoggerBase::popLogger();
+        if (&Logger::instance() == this) {
+            Logger::popLogger();
         }
     }
 
@@ -3973,7 +3973,7 @@ void SyncContext::status()
                 out.str().c_str());
 
     sourceList.accessSession(getLogDir());
-    LoggerBase::instance().setLevel(Logger::INFO);
+    Logger::instance().setLevel(Logger::INFO);
     string prevLogdir = sourceList.getPrevLogdir();
     bool found = access(prevLogdir.c_str(), R_OK|X_OK) == 0;
 
@@ -4081,7 +4081,7 @@ void SyncContext::restore(const string &dirname, RestoreDatabase database)
 
     SourceList sourceList(*this, false);
     sourceList.accessSession(dirname.c_str());
-    LoggerBase::instance().setLevel(Logger::INFO);
+    Logger::instance().setLevel(Logger::INFO);
     initSources(sourceList);
     BOOST_FOREACH(SyncSource *source, sourceList) {
         ConfigPropertyRegistry& registry = SyncSourceConfig::getRegistry();
@@ -4157,7 +4157,7 @@ string SyncContext::readSessionInfo(const string &dir, SyncReport &report)
  * With that setup and a fake SyncContext it is possible to simulate
  * sessions and test the resulting logdirs.
  */
-class LogDirTest : public CppUnit::TestFixture, private SyncContext, private LoggerBase
+class LogDirTest : public CppUnit::TestFixture, private SyncContext, private Logger
 {
 public:
     LogDirTest() :
@@ -4305,7 +4305,7 @@ private:
      * @return logdir created for the session
      */
     string session(bool changeServer, SyncMLStatus status, ...) {
-        Logger::Level level = LoggerBase::instance().getLevel();
+        Logger::Level level = Logger::instance().getLevel();
         SourceList list(*this, true);
         list.setLogLevel(SourceList::LOGGING_QUIET);
         SyncReport report;
@@ -4355,7 +4355,7 @@ private:
         }
         list.syncDone(status, &report);
 
-        LoggerBase::instance().setLevel(level);
+        Logger::instance().setLevel(level);
         return list.getLogdir();
     }