[2445] Rename new files, and assorted cleanups

including docstring updates

src/bin/dhcp4/main.cc

@@ -17,7 +17,6 @@
 #include <dhcp4/ctrl_dhcp4_srv.h>
 #include <dhcp4/dhcp4_log.h>
 #include <log/logger_support.h>
-#include <log/logger_manager.h>
 
 #include <boost/lexical_cast.hpp>
 
@@ -116,7 +115,6 @@ main(int argc, char* argv[]) {
                 // DHCP server in stand-alone mode, e.g. for testing
             }
         } else {
-            std::cout << "[XX] STANDALONE" << std::endl;
             LOG_DEBUG(dhcp4_logger, DBG_DHCP4_START, DHCP4_STANDALONE);
         }
         server.run();

src/lib/log/Makefile.am

@@ -31,7 +31,7 @@ libb10_log_la_SOURCES += message_initializer.cc message_initializer.h
 libb10_log_la_SOURCES += message_reader.cc message_reader.h
 libb10_log_la_SOURCES += message_types.h
 libb10_log_la_SOURCES += output_option.cc output_option.h
-libb10_log_la_SOURCES += buffer_appender.cc buffer_appender.h
+libb10_log_la_SOURCES += log_buffer.cc log_buffer.h
 
 EXTRA_DIST  = README
 EXTRA_DIST += logimpl_messages.mes

src/lib/log/log_buffer.cc

@@ -0,0 +1,89 @@
+// Copyright (C) 2012  Internet Systems Consortium, Inc. ("ISC")
+//
+// Permission to use, copy, modify, and/or distribute this software for any
+// purpose with or without fee is hereby granted, provided that the above
+// copyright notice and this permission notice appear in all copies.
+//
+// THE SOFTWARE IS PROVIDED "AS IS" AND ISC DISCLAIMS ALL WARRANTIES WITH
+// REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF MERCHANTABILITY
+// AND FITNESS.  IN NO EVENT SHALL ISC BE LIABLE FOR ANY SPECIAL, DIRECT,
+// INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES WHATSOEVER RESULTING FROM
+// LOSS OF USE, DATA OR PROFITS, WHETHER IN AN ACTION OF CONTRACT, NEGLIGENCE
+// OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR
+// PERFORMANCE OF THIS SOFTWARE.
+
+#include <log/log_buffer.h>
+#include <log4cplus/loglevel.h>
+
+namespace isc {
+namespace log {
+
+LogBuffer& getLogBuffer() {
+    static boost::scoped_ptr<LogBuffer> log_buffer(NULL);
+    if (!log_buffer) {
+        log_buffer.reset(new LogBuffer);
+    }
+    return (*log_buffer);
+}
+
+LogBuffer::LogBuffer() {
+    flushed_ = false;
+}
+
+LogBuffer::~LogBuffer() {
+    // If there is anything left in the buffer,
+    // it means no reconfig has been done, and
+    // we can assume the logging system was either
+    // never setup, or broke while doing so.
+    // So dump all that is left to stdout
+    flush_stdout();
+}
+
+void
+LogBuffer::add(const log4cplus::spi::InternalLoggingEvent& event) {
+    if (flushed_) {
+        isc_throw(LogBufferAddAfterFlush,
+                  "Internal log buffer has been flushed already");
+    }
+    stored_.push_back(log4cplus::spi::InternalLoggingEvent(event));
+}
+
+void
+LogBuffer::flush_stdout() {
+    // This does not show a bit of information normal log messages
+    // do, so perhaps we should try and setup a new logger here
+    // However, as this is called from a destructor, it may not
+    // be a good idea; as we can't reliably know whether in what
+    // state the logger instance is now (or what the specific logger's
+    // settings were).
+    // So we print a slightly shortened format (it really only excludes
+    // the time and the pid)
+    std::vector<log4cplus::spi::InternalLoggingEvent>::const_iterator it;
+    log4cplus::LogLevelManager& manager = log4cplus::getLogLevelManager();
+    for (it = stored_.begin(); it != stored_.end(); ++it) {
+        std::cout << manager.toString(it->getLogLevel()) << " " <<
+                     "[" << it->getLoggerName() << "] " <<
+                     it->getMessage() << std::endl;
+    }
+    stored_.clear();
+}
+
+void
+LogBuffer::flush() {
+    for (size_t i = 0; i < stored_.size(); ++i) {
+        const log4cplus::spi::InternalLoggingEvent event(stored_.at(i));
+        log4cplus::Logger logger = log4cplus::Logger::getInstance(event.getLoggerName());
+
+        logger.log(event.getLogLevel(), event.getMessage());
+    }
+    stored_.clear();
+    flushed_ = true;
+}
+
+void
+BufferAppender::append(const log4cplus::spi::InternalLoggingEvent& event) {
+    buffer_.add(event);
+}
+
+} // end namespace log
+} // end namespace isc

src/lib/log/log_buffer.h

@@ -0,0 +1,111 @@
+// Copyright (C) 2012  Internet Systems Consortium, Inc. ("ISC")
+//
+// Permission to use, copy, modify, and/or distribute this software for any
+// purpose with or without fee is hereby granted, provided that the above
+// copyright notice and this permission notice appear in all copies.
+//
+// THE SOFTWARE IS PROVIDED "AS IS" AND ISC DISCLAIMS ALL WARRANTIES WITH
+// REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF MERCHANTABILITY
+// AND FITNESS.  IN NO EVENT SHALL ISC BE LIABLE FOR ANY SPECIAL, DIRECT,
+// INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES WHATSOEVER RESULTING FROM
+// LOSS OF USE, DATA OR PROFITS, WHETHER IN AN ACTION OF CONTRACT, NEGLIGENCE
+// OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR
+// PERFORMANCE OF THIS SOFTWARE.
+
+#ifndef LOG_BUFFER_H
+#define LOG_BUFFER_H
+
+#include <exceptions/exceptions.h>
+
+#include <log4cplus/logger.h>
+#include <boost/scoped_ptr.hpp>
+
+namespace isc {
+namespace log {
+
+/// \brief Buffer add after flush
+///
+/// This exception is thrown if the log buffer's add() method
+/// is called after the log buffer has been flushed; the buffer
+/// is only supposed to be used once (until the first time a
+/// logger specification is processed)
+class LogBufferAddAfterFlush : public isc::Exception {
+public:
+    LogBufferAddAfterFlush(const char* file, size_t line, const char* what) :
+        isc::Exception(file, line, what)
+    {}
+};
+
+/// \brief Buffering class for logging event
+///
+/// This class is used to store logging events; it simply keeps any
+/// event that is passed to \c add(), and will replay them to the
+/// logger that they were originally created for when \c flush() is
+/// called.
+///
+/// The idea is that initially, a program may want to do some logging,
+/// but does not know where to yet (for instance because it has yet to
+/// read and parse its configuration). Any log messages before this time
+/// would normally go to some default (say, stdout), and be lost in the
+/// real logging destination. By buffering them (and flushing them once
+/// the logger has been configured), these log messages are kept in a
+/// consistent place, and are not lost.
+///
+/// Given this goal, this class has an extra check; it will raise
+/// an exception if \c add() is called after flush().
+///
+/// If the LogBuffer instance is destroyed before being flushed, it will
+/// dump any event it has left to stdout.
+class LogBuffer {
+    // For testing purposes, we make this a friend class, so that
+    // it can inspect the stored_ vector
+    friend class LogBufferTest;
+
+public:
+    LogBuffer();
+    ~LogBuffer();
+    /// \brief add the given event to the list of stored events
+    ///
+    /// This is called by the BufferAppender.
+    ///
+    /// \param event The event to store
+    /// \exception LogBufferAddAfterFlush if this method is called
+    ///            when \c flush() has been called previously
+    void add(const log4cplus::spi::InternalLoggingEvent& event);
+    void flush();
+private:
+    void flush_stdout();
+    std::vector<log4cplus::spi::InternalLoggingEvent> stored_;
+    bool flushed_;
+};
+
+/// \brief Log4cplus appender for our buffer
+///
+/// This class can be set as an Appender for log4cplus loggers
+///
+/// When logging an event, it will not actually log anything, but
+/// merely add it to the singleton LogBuffer instance
+class BufferAppender : public log4cplus::Appender {
+public:
+    /// \brief Constructor
+    ///
+    /// \note Only a reference to the LogBuffer is stored, so
+    /// this buffer must remain in scope during the lifetime of
+    /// the appender. In general, only one buffer would be needed,
+    /// and for that purpose there is the singleton instance
+    /// that can be reached using \c getLogBuffer()
+    BufferAppender(LogBuffer& buffer) : buffer_(buffer) {}
+    virtual void close() {}
+    virtual void append(const log4cplus::spi::InternalLoggingEvent& event);
+private:
+    LogBuffer& buffer_;
+};
+
+/// \brief Getter for the singleton instance of the log buffer
+LogBuffer& getLogBuffer();
+
+} // end namespace log
+} // end namespace isc
+
+#endif // LOG_BUFFER_H
+

src/lib/log/logger_impl.cc

@@ -60,7 +60,6 @@ LoggerImpl::LoggerImpl(const string& name) :
 // Destructor. (Here because of virtual declaration.)
 
 LoggerImpl::~LoggerImpl() {
-    logger_.shutdown();
     delete sync_;
 }
 

src/lib/log/logger_manager.cc

@@ -189,9 +189,9 @@ LoggerManager::readLocalMessageFile(const char* file) {
 
 // Reset logging to settings passed to init()
 void
-LoggerManager::reset(bool buffer) {
+LoggerManager::reset() {
     setRootLoggerName(initRootName());
-    LoggerManagerImpl::reset(initSeverity(), initDebugLevel(), buffer);
+    LoggerManagerImpl::reset(initSeverity(), initDebugLevel());
 }
 
 } // namespace log

src/lib/log/logger_manager.h

@@ -121,12 +121,9 @@ public:
 
     /// \brief Reset logging
     ///
-    /// Resets logging to whatever was set in the call to init().
-    ///
-    /// \param buffer If true, all log messages will be buffered until one of
-    ///        the \c process() methods is called. If false, initial logging
-    ///        shall go to the default output (i.e. stdout)
-    static void reset(bool buffer = false);
+    /// Resets logging to whatever was set in the call to init(), expect for
+    /// the buffer option.
+    static void reset();
 
     /// \brief Read local message file
     ///

src/lib/log/logger_manager_impl.cc

@@ -30,7 +30,7 @@
 #include <log/log_messages.h>
 #include <log/logger_name.h>
 #include <log/logger_specification.h>
-#include <log/buffer_appender.h>
+#include <log/log_buffer.h>
 
 #include <boost/scoped_ptr.hpp>
 
@@ -70,12 +70,12 @@ LoggerManagerImpl::processSpecification(const LoggerSpecification& spec) {
     // Set the additive flag.
     logger.setAdditivity(spec.getAdditive());
 
+    // Replace all appenders for this logger.
+    logger.removeAllAppenders();
+
     // Output options given?
     if (spec.optionCount() > 0) {
 
-        // Yes, so replace all appenders for this logger.
-        logger.removeAllAppenders();
-
         // Now process output specifications.
         for (LoggerSpecification::const_iterator i = spec.begin();
              i != spec.end(); ++i) {
@@ -104,8 +104,6 @@ LoggerManagerImpl::processSpecification(const LoggerSpecification& spec) {
         }
     } else {
         // If no output options are given, use a default appender
-        // Yes, so replace all appenders for this logger.
-        logger.removeAllAppenders();
         OutputOption opt;
         createConsoleAppender(logger, opt);
     }
@@ -152,7 +150,7 @@ LoggerManagerImpl::createFileAppender(log4cplus::Logger& logger,
 void
 LoggerManagerImpl::createBufferAppender(log4cplus::Logger& logger)
 {
-    log4cplus::SharedAppenderPtr bufferapp(new BufferAppender());
+    log4cplus::SharedAppenderPtr bufferapp(new BufferAppender(getLogBuffer()));
     bufferapp->setName("buffer");
     logger.addAppender(bufferapp);
     // Since we do not know at what level the loggers will end up
@@ -187,18 +185,17 @@ LoggerManagerImpl::init(isc::log::Severity severity, int dbglevel,
     // Add the additional debug levels
     LoggerLevelImpl::init();
 
-    reset(severity, dbglevel, buffer);
+    initRootLogger(severity, dbglevel, buffer);
 }
 
 // Reset logging to default configuration.  This closes all appenders
 // and resets the root logger to output INFO messages to the console.
 // It is principally used in testing.
 void
-LoggerManagerImpl::reset(isc::log::Severity severity, int dbglevel,
-                         bool buffer)
+LoggerManagerImpl::reset(isc::log::Severity severity, int dbglevel)
 {
     // Initialize the root logger
-    initRootLogger(severity, dbglevel, buffer);
+    initRootLogger(severity, dbglevel);
 }
 
 // Initialize the root logger

src/lib/log/logger_manager_impl.h

@@ -53,8 +53,6 @@ public:
     /// \brief Constructor
     LoggerManagerImpl() {}
 
-    ~LoggerManagerImpl() {}
-
     /// \brief Initialize Processing
     ///
     /// This resets the hierachy of loggers back to their defaults.  This means
@@ -101,11 +99,8 @@ public:
     ///
     /// \param severity Severity to be associated with this logger
     /// \param dbglevel Debug level associated with the root logger
-    /// \param buffer If true, all log messages will be buffered until one of
-    ///        the \c process() methods is called. If false, initial logging
-    ///        shall go to the default output (i.e. stdout)
     static void reset(isc::log::Severity severity = isc::log::INFO,
-                      int dbglevel = 0, bool buffer = false);
+                      int dbglevel = 0);
 
 private:
     /// \brief Create console appender
@@ -139,12 +134,21 @@ private:
     static void createSyslogAppender(log4cplus::Logger& logger,
                                      const OutputOption& opt);
 
+    /// \brief Create buffered appender
+    ///
+    /// Appends an object to the logger that will store the log events sent
+    /// to the logger in the singleton \c LogBuffer instance. These log
+    /// messages are replayed to the logger when the LogBuffer instance is
+    /// flushed (which is done at the end of \c ProcessSpecification().
+    ///
+    /// \param logger Log4cplus logger to which the appender must be attached.
+    /// \param opt Output options for this appender.
     static void createBufferAppender(log4cplus::Logger& logger);
 
     /// \brief Set default layout and severity for root logger
     ///
-    /// Initializes the root logger to BIND 10 defaults - console output and
-    /// the passed severity/debug level.
+    /// Initializes the root logger to BIND 10 defaults - console or buffered
+    /// output and the passed severity/debug level.
     ///
     /// \param severity Severity of messages that the logger should output.
     /// \param dbglevel Debug level if severity = DEBUG

src/lib/log/tests/Makefile.am

@@ -91,7 +91,7 @@ run_unittests_SOURCES += logger_specification_unittest.cc
 run_unittests_SOURCES += message_dictionary_unittest.cc
 run_unittests_SOURCES += message_reader_unittest.cc
 run_unittests_SOURCES += output_option_unittest.cc
-run_unittests_SOURCES += buffer_appender_unittest.cc
+run_unittests_SOURCES += log_buffer_unittest.cc
 nodist_run_unittests_SOURCES = log_test_messages.cc log_test_messages.h
 
 run_unittests_CPPFLAGS = $(AM_CPPFLAGS)

src/lib/log/tests/buffer_logger_test.sh.in

@@ -46,9 +46,9 @@ passfail $?
 
 echo -n  "   - Buffer excluding process() call: "
 cat > $tempfile << .
-LOG_BAD_SEVERITY unrecognized log severity: info
-LOG_BAD_DESTINATION unrecognized log destination: debug-50
-LOG_BAD_SEVERITY unrecognized log severity: info
+INFO [buffertest.log] LOG_BAD_SEVERITY unrecognized log severity: info
+DEBUG [buffertest.log] LOG_BAD_DESTINATION unrecognized log destination: debug-50
+INFO [buffertest.log] LOG_BAD_SEVERITY unrecognized log severity: info
 .
 ./buffer_logger_test -n 2>&1 | diff $tempfile -
 passfail $?

src/lib/log/tests/buffer_appender_unittest.cc

@@ -1,4 +1,4 @@
-// Copyright (C) 2011  Internet Systems Consortium, Inc. ("ISC")
+// Copyright (C) 2012  Internet Systems Consortium, Inc. ("ISC")
 //
 // Permission to use, copy, modify, and/or distribute this software for any
 // purpose with or without fee is hereby granted, provided that the above
@@ -16,9 +16,9 @@
 #include <gtest/gtest.h>
 
 #include <log/macros.h>
-#include <log/buffer_appender.h>
 #include <log/logger_support.h>
 #include <log/log_messages.h>
+#include <log/log_buffer.h>
 
 #include <log4cplus/logger.h>
 #include <log4cplus/nullappender.h>
@@ -28,23 +28,10 @@ using namespace isc::log;
 namespace isc {
 namespace log {
 
-// In these tests we are testing the LogBuffer class itself,
-// and not the singleton usage of one, so we need another
-// special appender
-class TestLogBufferAppender : public log4cplus::Appender {
-public:
-    TestLogBufferAppender(LogBuffer& buffer) : buffer_(buffer) {}
-    virtual void close() {}
-    virtual void append(const log4cplus::spi::InternalLoggingEvent& event) {
-        buffer_.add(event);
-    }
-    LogBuffer& buffer_;
-};
-
 class LogBufferTest : public ::testing::Test {
 public:
-    LogBufferTest() : appender1(new TestLogBufferAppender(buffer1)),
-                      appender2(new TestLogBufferAppender(buffer2)),
+    LogBufferTest() : appender1(new BufferAppender(buffer1)),
+                      appender2(new BufferAppender(buffer2)),
                       logger(log4cplus::Logger::getInstance("buffer"))
     {
         logger.setLogLevel(log4cplus::TRACE_LOG_LEVEL);
@@ -71,6 +58,8 @@ public:
     log4cplus::Logger logger;
 };
 
+// Test that log events are indeed stored, and that they are
+// flushed to the new appenders of their logger
 TEST_F(LogBufferTest, flush) {
     checkBufferedSize(buffer1, 0);
     checkBufferedSize(buffer2, 0);
@@ -96,8 +85,8 @@ TEST_F(LogBufferTest, flush) {
     checkBufferedSize(buffer2, 3);
 }
 
+// Once flushed, logging new messages with the same buffer should fail
 TEST_F(LogBufferTest, addAfterFlush) {
-    // Once flushed, logging new messages with the same buffer should fail
     logger.addAppender(appender1);
     buffer1.flush();
     EXPECT_THROW(LOG4CPLUS_INFO(logger, "Foo"), LogBufferAddAfterFlush);
@@ -113,7 +102,3 @@ TEST_F(LogBufferTest, addAfterFlush) {
 
 }
 }
-
-namespace {
-
-} // end unnamed namespace

src/lib/python/isc/config/cfgmgr.py

@@ -34,7 +34,7 @@ import bind10_config
 import isc.log
 from isc.log_messages.cfgmgr_messages import *
 
-logger = isc.log.Logger("cfgmgr")
+logger = isc.log.Logger("cfgmgr", buffer=True)
 
 class ConfigManagerDataReadError(Exception):
     """This exception is thrown when there is an error while reading