Checkpoint commit of the logging code.

A checkpoint of various modifications to the logging code in trac438.

src/lib/log/Makefile.am

@@ -12,6 +12,7 @@ liblog_la_SOURCES += dbglevels.h
 liblog_la_SOURCES += dummylog.h dummylog.cc Message.h
 liblog_la_SOURCES += filename.h filename.cc
 liblog_la_SOURCES += logger.cc logger.h
+liblog_la_SOURCES += logger_support.cc logger_support.h
 liblog_la_SOURCES += messagedef.cc messagedef.h
 liblog_la_SOURCES += message_dictionary.cc message_dictionary.h
 liblog_la_SOURCES += message_exception.h message_exception.cc

src/lib/log/compiler/message.cc

@@ -239,7 +239,7 @@ void writeHeaderFile(const string& file, const string& prefix,
             "\n" <<
             "} // Anonymous namespace\n" <<
             "\n" <<
-            "#endif // " << sentinel_text;
+            "#endif // " << sentinel_text << "\n";
 
         // Report errors (if any) and exit
         if (hfile.fail()) {
@@ -348,21 +348,21 @@ void writeProgramFile(const string& file, MessageDictionary* dictionary)
 /// If the input file contained duplicate message IDs, only the first will be
 /// processed.  However, we should warn about it.
 ///
-/// \param dictionary Dictionary containing the message IDs and text.
+/// \param reader Message Reader used to read the file
 
-static void warnDuplicates(MessageDictionary* dictionary) {
+static void warnDuplicates(MessageReader& reader) {
 
     // Get the duplicates (the overflow) and, if present, sort them into some
     // order and remove those which occur more than once (which mean that they
     // occur more than twice in the input file).
-    vector<MessageID> duplicates = dictionary->getOverflow();
+    MessageReader::MessageIDCollection duplicates = reader.getNotAdded();
     if (duplicates.size() > 0) {
         cout << "Warning: the following duplicate IDs were found:\n";
 
         sort(duplicates.begin(), duplicates.end());
-        vector<MessageID>::iterator new_end =
+        MessageReader::MessageIDCollection::iterator new_end =
             unique(duplicates.begin(), duplicates.end());
-        for (vector<MessageID>::iterator i = duplicates.begin();
+        for (MessageReader::MessageIDCollection::iterator i = duplicates.begin();
             i != new_end; ++i) {
             cout << "    " << *i << "\n";
         }
@@ -379,15 +379,13 @@ int main(int argc, char** argv) {
     
     const struct option loptions[] = {          // Long options
         {"help",    no_argument, NULL, 'h'},
-        {"python",  no_argument, NULL, 'p'},
         {"version", no_argument, NULL, 'v'},
         {NULL,      0,           NULL, 0  }
     };
-    const char* soptions = "hpv";               // Short options
+    const char* soptions = "hv";               // Short options
 
     optind = 1;             // Ensure we start a new scan
     int  opt;               // Value of the option
-    bool python = false;    // Set true if the -p flag is detected
 
     while ((opt = getopt_long(argc, argv, soptions, loptions, NULL)) != -1) {
         switch (opt) {
@@ -399,10 +397,6 @@ int main(int argc, char** argv) {
                 version();
                 return 0;
 
-            case 'p':
-                python = true;
-                break;
-
             default:
                 // A message will have already been output about the error.
                 return 1;
@@ -437,7 +431,7 @@ int main(int argc, char** argv) {
         writeProgramFile(message_file, &dictionary);
 
         // Finally, warn of any duplicates encountered.
-        warnDuplicates(&dictionary);
+        warnDuplicates(reader);
     }
     catch (MessageException& e) {
         // Create an error message from the ID and the text

src/lib/log/logger.cc

@@ -10,12 +10,20 @@
 // 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 SOFTWAR
+// PERFORMANCE OF THIS SOFTWARE
 
 // $Id$
 
 #include <iostream>
 
+#include <stdarg.h>
+#include <stdio.h>
+
+#include <log4cxx/appender.h>
+#include <log4cxx/basicconfigurator.h>
+#include <log4cxx/patternlayout.h>
+#include <log4cxx/consoleappender.h>
+
 #include <log/root_logger_name.h>
 #include <log/logger.h>
 #include <log/message_dictionary.h>
@@ -23,36 +31,69 @@
 #include <log/strutil.h>
 #include <log/xdebuglevel.h>
 
-#include <log4cxx/basicconfigurator.h>
-
 using namespace std;
 
 namespace isc {
 namespace log {
 
+// Static initializations
+
 bool Logger::init_ = false;
 
-// Constructor - create a logger as a child of the root logger.  With log4cxx
-// this is assured by naming the logger <parent>.<child>.
+// Destructor.  Delete log4cxx stuff if "don't delete" is clear.
 
-Logger::Logger(const std::string& name) : loggerptr_()
-{
-    string root_name = RootLoggerName::getName();
-    if (root_name.empty() || (name == root_name)) {
-        fullname_ = name;
+Logger::~Logger() {
+    if (exit_delete_) {
+        delete loggerptr_;
     }
-    else {
-        fullname_ = root_name + "." + name;
-    }
-    loggerptr_ = log4cxx::Logger::getLogger(fullname_);
+}
 
-    // Initialize basic logging if not already done
+// Initialize logger - create a logger as a child of the root logger.  With
+// log4cxx this is assured by naming the logger <parent>.<child>.
+
+void Logger::initLogger() {
+
+    // Initialize basic logging if not already done.  This is a one-off for
+    // all loggers.
     if (! init_) {
-        log4cxx::BasicConfigurator::configure();
+
+        // TEMPORARY
+        // Add a suitable console logger to the log4cxx root logger.  (This
+        // is the logger at the root of the log4cxx tree, not the BIND-10 root
+        // logger, which is one level down.)  The chosen format is:
+        //
+        // YYYY-MM-DD hh:mm:ss.sss [logger] SEVERITY: text
+        //
+        // As noted, this is a temporary hack: it is done here to ensure that
+        // a suitable output and output pattern is set.  Future versions of the
+        // software will set this based on configuration data.
+
+        log4cxx::LayoutPtr layout(
+            new log4cxx::PatternLayout(
+                "%d{yyyy-MM-DD HH:mm:ss.SSS} %-5p [%c] %m\n"));
+        log4cxx::AppenderPtr console(
+            new log4cxx::ConsoleAppender(layout));
+        log4cxx::LoggerPtr sys_root_logger = log4cxx::Logger::getRootLogger();
+        sys_root_logger->addAppender(console);
+        
+        // All static stuff initialized
         init_ = true;
     }
+
+    // Initialize this logger.  Name this as to whether the BIND-10 root logger
+    // name has beens set.
+    string root_name = RootLoggerName::getName();
+    if (root_name.empty() || (name_ == root_name)) {
+        loggerptr_ = new log4cxx::LoggerPtr(log4cxx::Logger::getLogger(name_));
+    }
+    else {
+        loggerptr_ = new log4cxx::LoggerPtr(
+            log4cxx::Logger::getLogger(root_name + "." + name_)
+        );
+    }
 }
 
+
 // Set the severity for logging.  There is a 1:1 mapping between the logging
 // severity and the log4cxx logging levels, apart from DEBUG.
 //
@@ -73,45 +114,45 @@ Logger::Logger(const std::string& name) : loggerptr_()
 // level set is 10000 - 25 = 99975.
 //
 // Attempting to log a debug message of level 26 is an attempt to log a message
-// of level 10000 - 26 = 99974.  As 99974 !>= 99975, it is not logged.  A
-// message of level 25 is, because 99975 >= 99975.
+// of level 10000 - 26 = 9974.  As 9974 !>= 9975, it is not logged.  A
+// message of level 25 is, because 9975 >= 9975.
 //
 // The extended set of logging levels is implemented by the XDebugLevel class.
 
 void Logger::setSeverity(Severity severity, int dbglevel) {
     switch (severity) {
         case NONE:
-            loggerptr_->setLevel(
+            getLogger()->setLevel(
                 log4cxx::Level::toLevel(
                 log4cxx::Level::OFF_INT));
             break;
 
         case FATAL:
-            loggerptr_->setLevel(
+            getLogger()->setLevel(
                 log4cxx::Level::toLevel(
                 log4cxx::Level::FATAL_INT));
             break;
 
         case ERROR:
-            loggerptr_->setLevel(
+            getLogger()->setLevel(
                 log4cxx::Level::toLevel(
                 log4cxx::Level::ERROR_INT));
             break;
 
         case WARNING:
-            loggerptr_->setLevel(
+            getLogger()->setLevel(
                 log4cxx::Level::toLevel(
                 log4cxx::Level::WARN_INT));
             break;
 
         case INFO:
-            loggerptr_->setLevel(
+            getLogger()->setLevel(
                 log4cxx::Level::toLevel(
                 log4cxx::Level::INFO_INT));
             break;
 
         case DEBUG:
-            loggerptr_->setLevel(
+            getLogger()->setLevel(
                 log4cxx::XDebugLevel::getExtendedDebug(dbglevel));
             break;
 
@@ -119,7 +160,7 @@ void Logger::setSeverity(Severity severity, int dbglevel) {
         // logger's own severity and it defaults to the severity of the parent
         // logger.
         default:
-            loggerptr_->setLevel(0);
+            getLogger()->setLevel(0);
     }
 }
 
@@ -174,11 +215,13 @@ Logger::Severity Logger::getSeverityCommon(const log4cxx::LoggerPtr& ptrlogger,
         return convertLevel(level->toInt());
     }
 }
+
+
 // Get the debug level.  This returns 0 unless the severity is DEBUG.
 
-int Logger::getDebugLevel() const {
+int Logger::getDebugLevel() {
 
-    log4cxx::LevelPtr level = loggerptr_->getLevel();
+    log4cxx::LevelPtr level = getLogger()->getLevel();
     if (level == log4cxx::LevelPtr()) {
 
         // Null pointer returned, logging should be that of the parent.
@@ -195,78 +238,64 @@ int Logger::getDebugLevel() const {
     }
 }
 
-// Return formatted message
-// THIS IS A PLACE HOLDER
-
-string
-Logger::formatMessage(MessageID ident, vector<string>* args) {
-
-    // Return the format string
-    MessageDictionary* global = MessageDictionary::globalDictionary();
-    string text = global->getText(ident);
-
-    // Do argument substitution if there are any
-    if (args) {
-        text = isc::strutil::format(text, *args);
+// Log an error message:
+// Common code.  Owing to the use of variable arguments, this must be inline
+// (hence the definition of the macro).  Also note that it expects that the
+// message buffer "message" is declared in the compilation unit.
+
+#define MESSAGE_SIZE (256)
+
+#define FORMAT_MESSAGE(message) \
+    { \
+    MessageDictionary* global = MessageDictionary::globalDictionary(); \
+    string format = global->getText(ident); \
+    va_list ap; \
+    va_start(ap, ident); \
+    vsnprintf(message, sizeof(message), format.c_str(), ap); \
+    message[sizeof(message) - 1] = '\0'; \
+    va_end(ap); \
     }
+    
 
-    return ident + ", " + text;
-}
-
-string
-Logger::formatMessage(MessageID ident, const string& composite) {
-    vector<string> args = isc::strutil::tokens(composite, "\0");
-    return formatMessage(ident, &args);
-}
-
-
-// Debug methods
+// Output methods
 
-void Logger::debugCommon(MessageID ident, const std::string* args) {
-    if (args) {
-        LOG4CXX_DEBUG(loggerptr_, formatMessage(ident, *args));
-    } else {
-        LOG4CXX_DEBUG(loggerptr_, formatMessage(ident));
+void Logger::debug(int dbglevel, isc::log::MessageID ident, ...) {
+    if (isDebugEnabled(dbglevel)) {
+        char message[MESSAGE_SIZE];
+        FORMAT_MESSAGE(message);
+        LOG4CXX_DEBUG(getLogger(), message);
     }
 }
 
-// Info
-
-void Logger::infoCommon(MessageID ident, const std::string* args) {
-    if (args) {
-        LOG4CXX_INFO(loggerptr_, formatMessage(ident, *args));
-    } else {
-        LOG4CXX_INFO(loggerptr_, formatMessage(ident));
+void Logger::info(isc::log::MessageID ident, ...) {
+    if (isInfoEnabled()) {
+        char message[MESSAGE_SIZE];
+        FORMAT_MESSAGE(message);
+        LOG4CXX_INFO(getLogger(), message);
     }
 }
 
-// Warning
-
-void Logger::warnCommon(MessageID ident, const std::string* args) {
-    if (args) {
-        LOG4CXX_WARN(loggerptr_, formatMessage(ident, *args));
-    } else {
-        LOG4CXX_WARN(loggerptr_, formatMessage(ident));
+void Logger::warn(isc::log::MessageID ident, ...) {
+    if (isWarnEnabled()) {
+        char message[MESSAGE_SIZE];
+        FORMAT_MESSAGE(message);
+        LOG4CXX_WARN(getLogger(), message);
     }
 }
 
-// Error
-
-void Logger::errorCommon(MessageID ident, const std::string* args) {
-    if (args) {
-        LOG4CXX_ERROR(loggerptr_, formatMessage(ident, *args));
-    } else {
-        LOG4CXX_ERROR(loggerptr_, formatMessage(ident));
+void Logger::error(isc::log::MessageID ident, ...) {
+    if (isErrorEnabled()) {
+        char message[MESSAGE_SIZE];
+        FORMAT_MESSAGE(message);
+        LOG4CXX_ERROR(getLogger(), message);
     }
 }
 
-// Fatal
-
-void Logger::fatalCommon(MessageID ident, const std::string* args) {
-    if (args) {
-        LOG4CXX_FATAL(loggerptr_, formatMessage(ident, *args));
-    } else {
-        LOG4CXX_FATAL(loggerptr_, formatMessage(ident));
+void Logger::fatal(isc::log::MessageID ident, ...) {
+    if (isFatalEnabled()) {
+        char message[MESSAGE_SIZE];
+        FORMAT_MESSAGE(message);
+        LOG4CXX_FATAL(getLogger(), message);
     }
 }
 

src/lib/log/logger.h

@@ -17,6 +17,7 @@
 #ifndef __LOGGER_H
 #define __LOGGER_H
 
+#include <cstdlib>
 #include <string>
 #include <boost/lexical_cast.hpp>
 #include <log4cxx/logger.h>
@@ -44,25 +45,6 @@ public:
         FATAL = CRITICAL
     } Severity;
 
-    /// \brief Return a logger of a given name
-    ///
-    /// Returns a logger with the specified name. 
-    ///
-    /// \param name Name of the logger.  Unless specified as a root logger
-    /// (with a call to setRootLoggerName), the returned logger is a child
-    /// of the root logger.
-    ///
-    /// \return Pointer to Logger object
-    // static Logger* getLogger(const char* name) {}
-
-    /// \brief Set Root Logger Name
-    ///
-    /// One of the first calls in the program, this sets the name of the
-    /// root logger.  (The name appears in logging messages.)
-    ///
-    /// \param name Name of the root logger.
-    // static void setRootLoggerName(const char* name);
-
     /// \brief Constructor
     ///
     /// Creates/attaches to a logger of a specific name.
@@ -70,15 +52,64 @@ public:
     /// \param name Name of the logger.  If the name is that of the root name,
     /// this creates an instance of the root logger; otherwise it creates a
     /// child of the root logger.
-    Logger(const std::string& name);
+    /// \param exit_delete This argument is present to get round a bug in
+    /// log4cxx.  If a log4cxx logger is declared outside an execution unit, it
+    /// is not deleted until the program runs down.  At that point all such
+    /// objects - including internal log4cxx objects - are deleted.  However,
+    /// there seems to be a bug in log4cxx where the way that such objects are
+    /// destroyed causes a MutexException to be thrown (this is described in
+    /// https://issues.apache.org/jira/browse/LOGCXX-322).  As this only occurs
+    /// during program rundown, the issue is not serious - it just looks bad to
+    /// have the program crash instead of shut down cleanly.<BR>
+    /// The original implementation of the isc::log::Logger had as a member a
+    /// log4cxx logger (actually a LoggerPtr).  If the isc:: Logger was declared
+    /// statically, when it was destroyed at the end of the program the internal
+    /// LoggerPtr was destroyed, which triggered the problem.  The problem did
+    /// not occur if the isc::log::Logger was created on the stack.  To get
+    /// round this, the internal LoggerPtr is now created dynamically.  The
+    /// exit_delete argument controls its destruction: if true, it is destroyed
+    /// in the ISC Logger destructor.  If false, it is not.<BR>
+    /// When creating an isc::log::Logger on the stack, the argument should be
+    /// false (the default); when the Logger is destroyed, all the internal
+    /// log4cxx objects are destroyed.  As only the logger (and not the internal
+    /// log4cxx data structures are being destroyed), all is well.  However,
+    /// when creating the logger statically, the argument should be false.  This
+    /// means that the log4cxx objects are not destroyed at program rundown;
+    /// instead memory is reclaimed and files are closed when the process is
+    /// destroyed, something that does not trigger the bug.
+    Logger(const std::string& name, bool exit_delete = true) :
+        loggerptr_(), name_(name), exit_delete_(exit_delete)
+    {}
+
+
+    /// \brief Destructor
+    virtual ~Logger();
+
+
+    /// \brief Configure Options
+    ///
+    /// TEMPORARY: Pass in the command-line options to set the logging severity
+    /// for the root logger.  Future versions of the logger will get this
+    /// information from the configuration database.
+    ///
+    /// \param severity Severity level to log
+    /// \param dbglevel If the severity is DEBUG, this is the debug level.
+    /// This can be in the range 1 to 100 and controls the verbosity.  A value
+    /// outside these limits is silently coerced to the nearest boundary.
+    /// \param local_file If provided, the name of a message file to read in and
+    /// supersede one or more of the current messages.
+    static void runTimeInit(Severity severity = INFO, int dbglevel = 1,
+        const char* local_file = NULL);
+
 
     /// \brief Get Name of Logger
     ///
     /// \return The full name of the logger (including the root name)
-    virtual std::string getName() const {
-        return loggerptr_->getName();
+    virtual std::string getName() {
+        return getLogger()->getName();
     }
 
+
     /// \brief Set Severity Level for Logger
     ///
     /// Sets the level at which this logger will log messages.  If none is set,
@@ -90,12 +121,13 @@ public:
     /// outside these limits is silently coerced to the nearest boundary.
     virtual void setSeverity(Severity severity, int dbglevel = 1);
 
+
     /// \brief Get Severity Level for Logger
     ///
     /// \return The current logging level of this logger.  In most cases though,
     /// the effective logging level is what is required.
-    virtual Severity getSeverity() const {
-        return getSeverityCommon(loggerptr_, false);
+    virtual Severity getSeverity() {
+        return getSeverityCommon(getLogger(), false);
     }
 
     /// \brief Get Effective Severity Level for Logger
@@ -103,15 +135,17 @@ public:
     /// \return The effective severity level of the logger.  This is the same
     /// as getSeverity() if the logger has a severity level set, but otherwise
     /// is the severity of the parent.
-    virtual Severity getEffectiveSeverity() const {
-        return getSeverityCommon(loggerptr_, true);
+    virtual Severity getEffectiveSeverity() {
+        return getSeverityCommon(getLogger(), true);
     }
 
+
     /// \brief Return DEBUG Level
     ///
     /// \return Current setting of debug level.  This is returned regardless of
     /// whether the 
-    virtual int getDebugLevel() const;
+    virtual int getDebugLevel();
+
 
     /// \brief Returns if Debug Message Should Be Output
     ///
@@ -119,364 +153,86 @@ public:
     /// enabled only if the logger has DEBUG enabled and if the dbglevel
     /// checked is less than or equal to the debug level set for the logger.
     virtual bool
-    isDebugEnabled(int dbglevel = MIN_DEBUG_LEVEL) const {
-        return (loggerptr_->getEffectiveLevel()->toInt() <=
+    isDebugEnabled(int dbglevel = MIN_DEBUG_LEVEL) {
+        return (getLogger()->getEffectiveLevel()->toInt() <=
             (log4cxx::Level::DEBUG_INT - dbglevel));
     }
 
+
     /// \brief Is INFO Enabled?
-    virtual bool isInfoEnabled() const {
-        return (loggerptr_->isInfoEnabled());
+    virtual bool isInfoEnabled() {
+        return (getLogger()->isInfoEnabled());
     }
 
+
     /// \brief Is WARNING Enabled?
-    virtual bool isWarnEnabled() const {
-        return (loggerptr_->isWarnEnabled());
+    virtual bool isWarnEnabled() {
+        return (getLogger()->isWarnEnabled());
     }
 
+
     /// \brief Is WARNING Enabled?
-    virtual bool isWarningEnabled() const {
-        return (loggerptr_->isWarnEnabled());
+    virtual bool isWarningEnabled() {
+        return (getLogger()->isWarnEnabled());
     }
 
+
     /// \brief Is ERROR Enabled?
-    virtual bool isErrorEnabled() const {
-        return (loggerptr_->isErrorEnabled());
+    virtual bool isErrorEnabled() {
+        return (getLogger()->isErrorEnabled());
     }
 
+
     /// \brief Is CRITICAL Enabled?
-    virtual bool isCriticalEnabled() const {
-        return (loggerptr_->isFatalEnabled());
+    virtual bool isCriticalEnabled() {
+        return (getLogger()->isFatalEnabled());
     }
 
+
     /// \brief Is FATAL Enabled?
     ///
     /// FATAL is a synonym for CRITICAL.
-    virtual bool isFatalEnabled() const {
-        return (loggerptr_->isFatalEnabled());
+    virtual bool isFatalEnabled() {
+        return (getLogger()->isFatalEnabled());
     }
-/*
-    /// \brief Add Appender
-    ///
-    /// Adds an appender to the list of appenders for this logger.  The
-    /// appender is assumed to have an independent existence so although
-    /// a pointer to the appender is added here, the logger does not
-    /// assume responsibility for its destruction.
-    ///
-    /// \param appender Pointer to the appender that should be added.
-    /// If the appender is already added to this logger, a duplicate
-    /// is not added.
-    ///
-    /// \return true if the logger was added, false if it was already in the
-    /// list of appenders for this logger.
-    virtual bool addAppender(AbstractAppender* appender);
 
-    /// \brief Remove Appender
-    ///
-    /// Removes the appender from the list of appenders for this logger.
-    /// 
-    /// \param appender Pointer to the appender that should be removed.
-    ///
-    /// \return true if the appender was removed, false if it could not be
-    /// found in the list.
-    virtual bool removeAppender(AbstractAppender* appender);
 
-    /// \brief Get Effective Level for Logger
+    /// \brief Output Debug Message
     ///
-    /// Gets the current effective logging level.  If the current logger does
-    /// not have a level set, the inheritance tree is traversed until a level
-    /// is found.
-    virtual Level getEffectiveLevel() const;
-*/
-
-    // NOTE - THE FOLLOWING ARE TEMPORARY
-    //
-    // Until properly integrated into log4cxx, the following formatting methods
-    // are used. (It is this that explains why the arguments to the logging
-    // messages are concatenated into a single string only to be broken up
-    // again in the formatting code.)
+    /// \param dbglevel Debug level, ranging between 0 and 99.  Higher numbers
+    /// are used for more verbose output.
+    /// \param ident Message identification.
+    /// \param ... Optional arguments for the message.
+    void debug(int dbglevel, MessageID ident, ...);
 
 
-    /// \brief Basic Message Formatting
-    ///
-    /// Extracts a message from the global dictionary and substitutes
-    /// arguments (if any).
-    ///
-    /// \param ident Message identifier
-    /// \param args Pointer to an argument vector or NULL if none.
-    ///
-    /// \return Formatted message.
-    std::string formatMessage(MessageID ident,
-        std::vector<std::string>* args = NULL);
-
-    /// \brief Basic Message Formatting
+    /// \brief Output Informational Message
     ///
-    /// Extracts a message from the global dictionary and substitutes
-    /// arguments (if any).
-    ///
-    /// \param ident Message identifier
-    /// \param args Set of arguments as a single string separated by the NULL
-    /// character.
-    ///
-    /// \return Formatted message.
-    std::string formatMessage(MessageID ident, const std::string& args);
-
-
-    /// \brief Debug Messages
-    ///
-    /// A set of functions that control the output of the message and up to
-    /// four parameters.
-
-    void debugCommon(MessageID ident, const std::string* args = NULL);
-
-    void debug(MessageID ident) {
-        if (isDebugEnabled()) {
-            debugCommon(ident);
-        }
-    }
-
-    template <typename T1>
-    void debug(MessageID ident, T1 arg1) {
-        if (isDebugEnabled()) {
-            std::string args = boost::lexical_cast<std::string>(arg1);
-            debugCommon(ident, &args);
-        }
-    }
-
-    template <typename T1, typename T2>
-    void debug(MessageID ident, T1 arg1, T2 arg2) {
-        if (isDebugEnabled()) {
-            std::string args = boost::lexical_cast<std::string>(arg1) +
-                std::string("\0") + boost::lexical_cast<std::string>(arg2);
-            debugCommon(ident, &args);
-        }
-    }
-
-    template <typename T1, typename T2, typename T3>
-    void debug(MessageID ident, T1 arg1, T2 arg2, T3 arg3) {
-        if (isDebugEnabled()) {
-            std::string args = boost::lexical_cast<std::string>(arg1) +
-                std::string("\0") + boost::lexical_cast<std::string>(arg2) +
-                std::string("\0") + boost::lexical_cast<std::string>(arg3);
-            debugCommon(ident, &args);
-        }
-    }
-
-    template <typename T1, typename T2, typename T3, typename T4>
-    void debug(MessageID ident, T1 arg1, T2 arg2, T3 arg3,
-        T4 arg4) {
-        if (isDebugEnabled()) {
-            std::string args = boost::lexical_cast<std::string>(arg1) +
-                std::string("\0") + boost::lexical_cast<std::string>(arg2) +
-                std::string("\0") + boost::lexical_cast<std::string>(arg3) +
-                std::string("\0") + boost::lexical_cast<std::string>(arg4);
-            debugCommon(ident, &args);
-        }
-    }
-
-    /// \brief Informational Messages
-    ///
-    /// A set of functions that control the output of the message and up to
-    /// four parameters.
-
-    void infoCommon(MessageID ident, const std::string* args = NULL);
-
-    void info(MessageID ident) {
-        if (isInfoEnabled()) {
-            infoCommon(ident);
-        }
-    }
-
-    template <typename T1>
-    void info(MessageID ident, T1 arg1) {
-        if (isInfoEnabled()) {
-            std::string args = boost::lexical_cast<std::string>(arg1);
-            infoCommon(ident, &args);
-        }
-    }
-
-    template <typename T1, typename T2>
-    void info(MessageID ident, T1 arg1, T2 arg2) {
-        if (isInfoEnabled()) {
-            std::string args = boost::lexical_cast<std::string>(arg1) +
-                std::string("\0") + boost::lexical_cast<std::string>(arg2);
-            infoCommon(ident, &args);
-        }
-    }
+    /// \param ident Message identification.
+    /// \param ... Optional arguments for the message.
+    void info(MessageID ident, ...);
 
-    template <typename T1, typename T2, typename T3>
-    void info(MessageID ident, T1 arg1, T2 arg2, T3 arg3) {
-        if (isInfoEnabled()) {
-            std::string args = boost::lexical_cast<std::string>(arg1) +
-                std::string("\0") + boost::lexical_cast<std::string>(arg2) +
-                std::string("\0") + boost::lexical_cast<std::string>(arg3);
-            infoCommon(ident, &args);
-        }
-    }
 
-    template <typename T1, typename T2, typename T3, typename T4>
-    void info(MessageID ident, T1 arg1, T2 arg2, T3 arg3, T4 arg4) {
-        if (isInfoEnabled()) {
-            std::string args = boost::lexical_cast<std::string>(arg1) +
-                std::string("\0") + boost::lexical_cast<std::string>(arg2) +
-                std::string("\0") + boost::lexical_cast<std::string>(arg3) +
-                std::string("\0") + boost::lexical_cast<std::string>(arg4);
-            infoCommon(ident, &args);
-        }
-    }
-
-    /// \brief Warning Messages
+    /// \brief Output Warning Message
     ///
-    /// A set of functions that control the output of the message and up to
-    /// four parameters.
-
-    void warnCommon(MessageID ident, const std::string* args = NULL);
+    /// \param ident Message identification.
+    /// \param ... Optional arguments for the message.
+    void warn(MessageID ident, ...);
 
-    void warn(MessageID ident) {
-        if (isWarnEnabled()) {
-            warnCommon(ident);
-        }
-    }
 
-    template <typename T1>
-    void warn(Severity severity, MessageID ident, T1 arg1) {
-        if (isWarnEnabled()) {
-            std::string args = boost::lexical_cast<std::string>(arg1);
-            warnCommon(ident, &args);
-        }
-    }
-
-    template <typename T1, typename T2>
-    void warn(MessageID ident, T1 arg1, T2 arg2) {
-        if (isWarnEnabled()) {
-            std::string args = boost::lexical_cast<std::string>(arg1) +
-                std::string("\0") + boost::lexical_cast<std::string>(arg2);
-            warnCommon(ident, &args);
-        }
-    }
-
-    template <typename T1, typename T2, typename T3>
-    void warn(MessageID ident, T1 arg1, T2 arg2, T3 arg3) {
-        if (isWarnEnabled()) {
-            std::string args = boost::lexical_cast<std::string>(arg1) +
-                std::string("\0") + boost::lexical_cast<std::string>(arg2) +
-                std::string("\0") + boost::lexical_cast<std::string>(arg3);
-            warnCommon(ident, &args);
-        }
-    }
-
-    template <typename T1, typename T2, typename T3, typename T4>
-    void warn(MessageID ident, T1 arg1, T2 arg2, T3 arg3, T4 arg4) {
-        if (isWarnEnabled()) {
-            std::string args = boost::lexical_cast<std::string>(arg1) +
-                std::string("\0") + boost::lexical_cast<std::string>(arg2) +
-                std::string("\0") + boost::lexical_cast<std::string>(arg3) +
-                std::string("\0") + boost::lexical_cast<std::string>(arg4);
-            warnCommon(ident, &args);
-        }
-    }
-
-    /// \brief Error Messages
+    /// \brief Output Error Message
     ///
-    /// A set of functions that control the output of the message and up to
-    /// four parameters.
-
-    void errorCommon(MessageID ident, const std::string* args = NULL);
-
-    void error(MessageID ident) {
-        if (isErrorEnabled()) {
-            errorCommon(ident);
-        }
-    }
+    /// \param ident Message identification.
+    /// \param ... Optional arguments for the message.
+    void error(MessageID ident, ...);
 
-    template <typename T1>
-    void error(Severity severity, MessageID ident, T1 arg1) {
-        if (isErrorEnabled()) {
-            std::string args = boost::lexical_cast<std::string>(arg1);
-            errorCommon(ident, &args);
-        }
-    }
-
-    template <typename T1, typename T2>
-    void error(MessageID ident, T1 arg1, T2 arg2) {
-        if (isErrorEnabled()) {
-            std::string args = boost::lexical_cast<std::string>(arg1) +
-                std::string("\0") + boost::lexical_cast<std::string>(arg2);
-            errorCommon(ident, &args);
-        }
-    }
-
-    template <typename T1, typename T2, typename T3>
-    void error(MessageID ident, T1 arg1, T2 arg2, T3 arg3) {
-        if (isErrorEnabled()) {
-            std::string args = boost::lexical_cast<std::string>(arg1) +
-                std::string("\0") + boost::lexical_cast<std::string>(arg2) +
-                std::string("\0") + boost::lexical_cast<std::string>(arg3);
-            errorCommon(ident, &args);
-        }
-    }
 
-    template <typename T1, typename T2, typename T3, typename T4>
-    void error(MessageID ident, T1 arg1, T2 arg2, T3 arg3, T4 arg4) {
-        if (isErrorEnabled()) {
-            std::string args = boost::lexical_cast<std::string>(arg1) +
-                std::string("\0") + boost::lexical_cast<std::string>(arg2) +
-                std::string("\0") + boost::lexical_cast<std::string>(arg3) +
-                std::string("\0") + boost::lexical_cast<std::string>(arg4);
-            errorCommon(ident, &args);
-        }
-    }
-
-    /// \brief Fatal Messages
+    /// \brief Output Fatal Message
     ///
-    /// A set of functions that control the output of the message and up to
-    /// four parameters.
-
-    void fatalCommon(MessageID ident, const std::string* args = NULL);
-
-    void fatal(MessageID ident) {
-        if (isFatalEnabled()) {
-            fatalCommon(ident);
-        }
-    }
-
-    template <typename T1>
-    void fatal(Severity severity, MessageID ident, T1 arg1) {
-        if (isFatalEnabled()) {
-            std::string args = boost::lexical_cast<std::string>(arg1);
-            fatalCommon(ident, &args);
-        }
-    }
-
-    template <typename T1, typename T2>
-    void fatal(MessageID ident, T1 arg1, T2 arg2) {
-        if (isFatalEnabled()) {
-            std::string args = boost::lexical_cast<std::string>(arg1) +
-                std::string("\0") + boost::lexical_cast<std::string>(arg2);
-            fatalCommon(ident, &args);
-        }
-    }
-
-    template <typename T1, typename T2, typename T3>
-    void fatal(MessageID ident, T1 arg1, T2 arg2, T3 arg3) {
-        if (isFatalEnabled()) {
-            std::string args = boost::lexical_cast<std::string>(arg1) +
-                std::string("\0") + boost::lexical_cast<std::string>(arg2) +
-                std::string("\0") + boost::lexical_cast<std::string>(arg3);
-            fatalCommon(ident, &args);
-        }
-    }
+    /// \param ident Message identification.
+    /// \param ... Optional arguments for the message.
+    void fatal(MessageID ident, ...);
 
-    template <typename T1, typename T2, typename T3, typename T4>
-    void fatal(MessageID ident, T1 arg1, T2 arg2, T3 arg3, T4 arg4) {
-        if (isFatalEnabled()) {
-            std::string args = boost::lexical_cast<std::string>(arg1) +
-                std::string("\0") + boost::lexical_cast<std::string>(arg2) +
-                std::string("\0") + boost::lexical_cast<std::string>(arg3) +
-                std::string("\0") + boost::lexical_cast<std::string>(arg4);
-            fatalCommon(ident, &args);
-        }
-    }
 
 protected:
 
@@ -487,9 +243,10 @@ protected:
     ///
     /// \return true if the logger objects are instances of the same logger.
     bool operator==(const Logger& other) const {
-        return (loggerptr_ == other.loggerptr_);
+        return (*loggerptr_ == *other.loggerptr_);
     }
 
+
     /// \brief Logger Initialized
     ///
     /// Check that the logger has been properly initialized.  (This method
@@ -497,9 +254,10 @@ protected:
     ///
     /// \return true if this logger object has been initialized.
     bool isInitialized() const {
-        return (loggerptr_ != log4cxx::LoggerPtr());
+        return (loggerptr_ != NULL);
     }
 
+
     /// \brief Get Severity Level for Logger
     ///
     /// This is common code for getSeverity() and getEffectiveSeverity() -
@@ -517,6 +275,7 @@ protected:
     Logger::Severity getSeverityCommon(const log4cxx::LoggerPtr& ptrlogger,
         bool check_parent) const;
 
+
     /// \brief Convert Between BIND-10 and log4cxx Logging Levels
     ///
     /// Converts between the numeric value of the log4cxx logging level
@@ -527,13 +286,44 @@ protected:
     /// \return BIND-10 logging severity
     Severity convertLevel(int value) const;
 
-    /// \brief Formats Message
+
+    /// \brief Initialize log4cxx Logger
+    ///
+    /// Creates the log4cxx logger used internally.  A function is provided for
+    /// this so that the creation does not take place when this Logger object
+    /// is created but when it is used.  As the latter occurs in executable
+    /// code but the former can occur during initialization, this order
+    /// guarantees that anything that is statically initialized has completed
+    /// its initialization by the time the logger is used.
+    void initLogger();
+
+
+    /// \brief Return log4cxx Logger
+    ///
+    /// Returns the log4cxx logger, initializing it if not already initialized.
+    ///
+    /// \return Loggerptr object
+    log4cxx::LoggerPtr& getLogger() {
+        if (loggerptr_ == NULL) {
+            initLogger();
+        }
+        return *loggerptr_;
+    }
+
+
+    /// \brief Read Local Message File
+    ///
+    /// Reads a local message file into the global dictionary, replacing any
+    /// definitions there.  Any messages found in the local file that do not
+    /// replace ones in the global dictionary are listed.
     ///
-    /// Receives a message in the form of 
+    /// \param file Local message file to be read.
+    static void readLocalMessageFile(const char* file);
 
 private:
-    log4cxx::LoggerPtr  loggerptr_; ///< Pointer to the underlying logger
-    std::string         fullname_;  ///< Full name of this logger
+    log4cxx::LoggerPtr*  loggerptr_;    ///< Pointer to the underlying logger
+    std::string          name_;         ///< Name of this logger]
+    bool                 exit_delete_;  ///< Delete loggerptr_ on exit?
 
     // NOTE - THIS IS A PLACE HOLDER
     static bool         init_;      ///< Set true when initialized

src/lib/log/logger_support.cc

@@ -0,0 +1,114 @@
+// Copyright (C) 2010  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
+
+// $Id$
+
+
+
+/// \brief Temporary Logger Support
+///
+/// Performs run-time initialization of the logger system.  In particular, it
+/// is passed information from the command line and:
+///
+/// a) Sets the severity of the messages being logged (and debug level if
+/// appropriate).
+/// b) Reads in the local message file is one has been supplied.
+///
+/// These functions will be replaced once the code has bneen written to obtain
+/// the logging parameters from the configuration database.
+
+#include <vector>
+
+#include <log/logger.h>
+#include <log/logger_support.h>
+#include <log/messagedef.h>
+#include <log/message_dictionary.h>
+#include <log/message_exception.h>
+#include <log/message_reader.h>
+#include <log/message_types.h>
+#include <log/root_logger_name.h>
+
+namespace isc {
+namespace log {
+
+using namespace std;
+
+// Declare a logger for the logging subsystem
+Logger logger("log");
+
+
+/// \brief Reads Local Message File
+///
+/// Reads the local message file into the global dictionary, overwriting
+/// existing messages.  If the file contained any message IDs not in the
+/// dictionary, they are listed in a warning message.
+///
+/// \param file Name of the local message file
+static void readLocalMessageFile(const char* file) {
+    
+    MessageDictionary* dictionary = MessageDictionary::globalDictionary();
+    MessageReader reader(dictionary);
+    try {
+        reader.readFile(file, MessageReader::REPLACE);
+
+        // File successfully read, list the duplicates
+        MessageReader::MessageIDCollection unknown = reader.getNotAdded();
+        for (MessageReader::MessageIDCollection::const_iterator
+            i = unknown.begin(); i != unknown.end(); ++i) {
+                logger.warn(MSG_IDNOTFND, (*i).c_str());
+        }
+    }
+    catch (MessageException& e) {
+        MessageID ident = e.id();
+        vector<MessageID> args = e.arguments();
+        switch (args.size()) {
+        case 0:
+            logger.error(ident);
+            break;
+
+        case 1:
+            logger.error(ident, args[0].c_str());
+            break;
+
+        default:    // 2 or more (2 should be the maximum)
+            logger.error(ident, args[0].c_str(), args[1].c_str());
+        }
+    }
+}
+
+/// Logger Run-Time Initialization
+
+void runTimeInit(Logger::Severity severity, int dbglevel, const char* file) {
+
+    // Create the application root logger.  This is the logger that has the
+    // name of the application (and is one level down from the log4cxx root
+    // logger).  All other loggers created in this application will be its
+    // child.
+    //
+    // The main purpose of the application root logger is to provide the root
+    // name in output message for all other loggers.
+    Logger logger(RootLoggerName::getName());
+
+    // Set the severity associated with it.  If no other logger has a severity,
+    // this will be the default.
+    logger.setSeverity(severity, dbglevel);
+
+    // Replace any messages with local ones (if given)
+    if (file) {
+        readLocalMessageFile(file);
+    }
+}
+
+} // namespace log
+} // namespace isc

src/lib/log/logger_support.h

@@ -0,0 +1,43 @@
+// Copyright (C) 2010  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.
+
+// $Id$
+
+#ifndef __LOGGER_SUPPORT_H
+#define __LOGGER_SUPPORT_H
+
+#include <log/logger.h>
+
+namespace isc {
+namespace log {
+
+/// \brief Run-Time Initialization
+///
+/// This code will be used until the logger is fully integrated into the BIND-10
+/// configuration database.  It performs run-time initialization of th logger,
+/// in particular supplying run-time choices to it:
+///
+/// * The severity (and if applicable, debug level) at which to log
+/// * Name of a local message file, containing localisation of message text.
+///
+/// \param severity Severity at which to log
+/// \param dbglevel Debug severiy (ignored if "severity" is not "DEBUG")
+/// \param file Name of the local message file.
+void runTimeInit(Logger::Severity severity, int dbglevel, const char* file);
+
+} // namespace log
+} // namespace isc
+
+
+#endif // __LOGGER_SUPPORT_H

src/lib/log/message_dictionary.cc

@@ -33,19 +33,14 @@ MessageDictionary::~MessageDictionary() {
 bool MessageDictionary::add(const MessageID& ident, const std::string& text)
 {
     map<MessageID, string>::iterator i = dictionary_.find(ident);
+    bool not_found = (i == dictionary_.end());
+    if (not_found) {
 
-    if (i == dictionary_.end()) {
-
-        // Not found, so add it
+        // Message not already in the dictionary, so add it.
         dictionary_[ident] = text;
-        return true;
-    }
-    else {
-
-        // Exists, so add the ID to the overflow vector.
-        overflow_.push_back(ident);
-        return false;
     }
+    
+    return (not_found);
 }
 
 // Add message and note if ID does not already exist
@@ -53,34 +48,39 @@ bool MessageDictionary::add(const MessageID& ident, const std::string& text)
 bool MessageDictionary::replace(const MessageID& ident, const std::string& text)
 {
     map<MessageID, string>::iterator i = dictionary_.find(ident);
-
-    if (i != dictionary_.end()) {
+    bool found = (i != dictionary_.end());
+    if (found) {
 
         // Exists, so replace it.
         dictionary_[ident] = text;
-        return true;
-    }
-    else {
-
-        // Not found, so add to the overflow vector.
-        overflow_.push_back(ident);
-        return false;
     }
+    
+    return (found);
 }
 
 // Load a set of messages
 
-void MessageDictionary::load(const char* messages[]) {
+vector<MessageID> MessageDictionary::load(const char* messages[]) {
+    vector<MessageID> duplicates;
     int i = 0;
     while (messages[i]) {
-        MessageID ident(messages[i]);
-        ++i;
+
+        // ID present, so note it and point to text.
+        MessageID ident(messages[i++]);
         if (messages[i]) {
-            string text(messages[i]);
-            add(ident, text);
-            ++i;
+
+            // Text not null, note it and point to next ident. 
+            string text(messages[i++]);
+
+            // Add ID and text to message dictionary, noting if the ID was
+            // already present.
+            bool added = add(ident, text);
+            if (! added) {
+                duplicates.push_back(ident);
+            }
         }
     }
+    return duplicates;
 }
 
 // Return message text or blank string

src/lib/log/message_dictionary.h

@@ -35,18 +35,12 @@ namespace log {
 /// Adding text occurs in two modes:
 ///
 /// Through the "Add" method, ID/text mappings are added to the dictionary
-/// unless the ID already exists.  If so, the ID is added to an "overflow"
-/// vector from where it can be retrieved later.
+/// unless the ID already exists.  This is designed for use during program
+/// initialization, where a local message may supplant a compiled-in message.
 ///
 /// Through the "Replace" method, ID/text mappings are added to the dictionary
-/// only if the ID already exists.  Otherwise the ID is added to the overflow
-/// vector.
-///
-/// The "Add" method is designed for initialization of the program with the
-/// text supplied by the developers.  Here the message IDs must be unique.
-/// The "Replace" method is for use when a message file is supplied to replace
-/// messages provided with the program.  The supplied messages in this case
-/// should replace the ones that come with the program.
+/// only if the ID already exists.  This is for use when a message file is
+/// supplied to replace messages provided with the program.
 ///
 /// Although the class can be used stand-alone, it does supply a static method
 /// to return a particular instance - the "global" dictionary.
@@ -68,7 +62,7 @@ public:
     /// \param text Message text
     ///
     /// \return true if the message was added to the dictionary, false if the
-    /// message existed and it was added to the overflow vector.
+    /// message existed and it was not added.
     virtual bool add(const MessageID& ident, const std::string& text);
 
 
@@ -81,7 +75,7 @@ public:
     /// \param text Message text
     ///
     /// \return true if the message was added to the dictionary, false if the
-    /// message did not exist and it was added to the overflow vector.
+    /// message did not exist and it was not added.
     virtual bool replace(const MessageID& ident, const std::string& text);
 
 
@@ -96,7 +90,11 @@ public:
     /// message text.  This should be an odd number of elements long, the last
     /// elemnent being NULL.  If it is an even number of elements long, the
     /// last ID is ignored.
-    virtual void load(const char* elements[]);
+    ///
+    /// \return Vector of message IDs that were not loaded because an ID of the
+    /// same name already existing in the dictionary.  This vector may be
+    /// empty.
+    virtual std::vector<MessageID> load(const char* elements[]);
 
 
     /// \brief Get Message Text
@@ -111,25 +109,6 @@ public:
     virtual std::string getText(const MessageID& ident) const;
 
 
-    /// \brief Clear Overflow
-    ///
-    /// Clears the overflow vector, perhaps because new definitions are going
-    /// to be added.
-    virtual void clearOverflow() {
-        overflow_.clear();
-    }
-
-
-    /// \brief Return Overflow Vector
-    ///
-    /// Returns the overflow vector.
-    ///
-    /// \return Overflow vector
-    virtual std::vector<MessageID> getOverflow() const {
-        return overflow_;
-    }
-
-
     /// \brief Number of Items in Dictionary
     ///
     /// \return Number of items in the dictionary
@@ -137,14 +116,17 @@ public:
         return dictionary_.size();
     }
 
+
     // Allow access to the internal map structure, but don't allow alteration.
     typedef std::map<MessageID, std::string>::const_iterator const_iterator;
 
+
     /// \brief Return begin() iterator of internal map
     const_iterator begin() const {
         return dictionary_.begin();
     }
 
+
     /// \brief Return end() iterator of internal map
     const_iterator end() const {
         return dictionary_.end();
@@ -160,7 +142,6 @@ public:
 
 private:
     std::map<MessageID, std::string>  dictionary_;
-    std::vector<MessageID>            overflow_;
 };
 
 } // namespace log

src/lib/log/message_reader.cc

@@ -34,24 +34,16 @@ namespace log {
 MessageReader::~MessageReader() {
 }
 
-// Get/Set the dictionary.
-
-MessageDictionary* MessageReader::getDictionary() const {
-    return dictionary_;
-}
-
-void
-MessageReader::setDictionary(MessageDictionary* dictionary) {
-    dictionary_ = dictionary;
-    dictionary_->clearOverflow();
-}
-
 
 // Read the file.
 
 void
 MessageReader::readFile(const string& file, MessageReader::Mode mode) {
 
+    // Ensure the non-added collection is empty: this object might be
+    // being reused.
+    not_added_.clear();
+
     // Open the file
     ifstream infile(file.c_str());
     if (infile.fail()) {
@@ -174,12 +166,17 @@ MessageReader::parseMessage(const std::string& text, MessageReader::Mode mode) {
         throw MessageException(MSG_ONETOKEN, text);
     }
 
-    // Add the result to the dictionary.
+    // Add the result to the dictionary and to the non-added list if the add to
+    // the dictionary fails.
+    bool added;
     if (mode == ADD) {
-        (void) dictionary_->add(ident, text.substr(first_text));
+        added = dictionary_->add(ident, text.substr(first_text));
     }
     else {
-        (void) dictionary_->replace(ident, text.substr(first_text));
+        added = dictionary_->replace(ident, text.substr(first_text));
+    }
+    if (! added) {
+        not_added_.push_back(ident);
     }
 }
 

src/lib/log/message_reader.h

@@ -49,9 +49,8 @@ public:
         REPLACE
     } Mode;
 
-    /// \brief Other Types
-    typedef std::map<std::string, std::string>  MessageMap;
-    typedef std::vector<std::string>            MessageDuplicates;
+    /// \brief Visible collection types
+    typedef std::vector<MessageID>   MessageIDCollection;
 
     /// \brief Constructor
     ///
@@ -79,7 +78,9 @@ public:
     /// not transferred - the caller should not delete it.
     ///
     /// \return Pointer to current dictionary object
-    virtual MessageDictionary* getDictionary() const;
+    MessageDictionary* getDictionary() const {
+        return dictionary_;
+    }
 
 
     /// \brief Set Dictionary
@@ -89,7 +90,9 @@ public:
     /// \param dictionary New dictionary object. The ownership of the dictionary
     /// object is not transferred - the caller is responsible for managing the
     /// lifetime of the dictionary.
-    virtual void setDictionary(MessageDictionary* dictionary);
+    void setDictionary(MessageDictionary* dictionary) {
+        dictionary_ = dictionary;
+    }
 
 
     /// \brief Read File
@@ -128,6 +131,17 @@ public:
         prefix_ = "";
     }
 
+
+    /// \brief Get Not-Added List
+    ///
+    /// Returns the list of IDs that were not added during the last
+    /// read of the file.
+    ///
+    /// \return Collection of messages not added
+    MessageIDCollection getNotAdded() const {
+        return not_added_;
+    }
+
 private:
 
     /// \brief Handle a Message Definition
@@ -150,8 +164,9 @@ private:
     void parseDirective(const std::string& line);
 
     /// Attributes
-    MessageDictionary*  dictionary_;    // Dictionary to add messages to
-    std::string         prefix_;        // Input of $PREFIX statement
+    MessageDictionary*  dictionary_;    ///< Dictionary to add messages to
+    MessageIDCollection not_added_;     ///< List of IDs not added
+    std::string         prefix_;        ///< Input of $PREFIX statement
 };
 
 } // namespace log

src/lib/log/messagedef.cc

@@ -1,4 +1,4 @@
-// File created from messagedef.mes on Fri Jan  7 17:26:42 2011
+// File created from messagedef.mes on Mon Jan 17 15:25:32 2011
 
 #include <cstddef>
 #include <log/message_initializer.h>
@@ -9,8 +9,9 @@ namespace {
 
 const char* values[] = {
     "DUPLPRFX", "duplicate $PREFIX directive found",
+    "IDNOTFND", "could not replace message for '%s': no such message identification",
     "ONETOKEN", "a line containing a message ID ('%s') and nothing else was found",
-    "OPENIN", "unable to open %s for input: %s",
+    "OPENIN", "unable to open message file %s for input: %s",
     "OPENOUT", "unable to open %s for output: %s",
     "PRFEXTRARG", "$PREFIX directive has too many arguments",
     "PRFINVARG", "$PREFIX directive has an invalid argument ('%s')",
@@ -23,4 +24,4 @@ const char* values[] = {
 
 } // Anonymous namespace
 
-MessageInitializer messagedef_cc_Fri_Jan__7_17_26_42_2011(values);
+MessageInitializer messagedef_cc_Mon_Jan_17_15_25_32_2011(values);

src/lib/log/messagedef.h

@@ -1,4 +1,4 @@
-// File created from messagedef.mes on Fri Jan  7 17:26:42 2011
+// File created from messagedef.mes on Mon Jan 17 15:25:32 2011
 
 #ifndef __MESSAGEDEF_H
 #define __MESSAGEDEF_H
@@ -8,6 +8,7 @@
 namespace {
 
 isc::log::MessageID MSG_DUPLPRFX = "DUPLPRFX";
+isc::log::MessageID MSG_IDNOTFND = "IDNOTFND";
 isc::log::MessageID MSG_ONETOKEN = "ONETOKEN";
 isc::log::MessageID MSG_OPENIN = "OPENIN";
 isc::log::MessageID MSG_OPENOUT = "OPENOUT";
@@ -20,4 +21,4 @@ isc::log::MessageID MSG_WRITERR = "WRITERR";
 
 } // Anonymous namespace
 
-#endif // __MESSAGEDEF_H
+#endif // __MESSAGEDEF_H

src/lib/log/messagedef.mes

@@ -29,15 +29,26 @@ DUPLPRFX    duplicate $PREFIX directive found
 + this version of the code, such a condition is regarded as an error and the
 + read will be abandonded.
 
+IDNOTFND    could not replace message for '%s': no such message identification
++ During start-up a local message file was read.  A line with the listed
++ message identification was found in the file, but the identification is not
++ one contained in the compiled-in message dictionary.  Either the message
++ identification has been mis-spelled in the file, or the local file was used
++ for an earlier version of the software and the message with that
++ identification has been removed.
++
++ This message may appear a number of times in the file, once for every such
++ unknown mnessage identification.
+
 ONETOKEN    a line containing a message ID ('%s') and nothing else was found
 + Message definitions comprise lines starting with a message identification (a
 + symbolic name for the message) and followed by the text of the message.  This
 + error is generated when a line is found in the message file that contains just
 + the message identification.
 
-OPENIN      unable to open %s for input: %s
-+ The program was not able to open the specified input file for the reason
-+ given.
+OPENIN      unable to open message file %s for input: %s
++ The program was not able to open the specified input message file for the
++ reason given.
 
 OPENOUT     unable to open %s for output: %s
 + The program was not able to open the specified output file for the reason

src/lib/log/root_logger_name.h

@@ -32,6 +32,15 @@ namespace log {
 class RootLoggerName {
 public:
 
+    /// \brief Constructor
+    ///
+    /// Sets the root logger name.  Although the name is static, setting the
+    /// name in the constructor allows static initialization of the name by
+    /// declaring an external instance of the class in the main execution unit.
+    RootLoggerName(const std::string& name) {
+        setName(name);
+    } 
+
     /// \brief Set Root Logger Name
     ///
     /// \param name Name of the root logger.  This should be the program

src/lib/log/tests/logger_unittest.cc

@@ -97,7 +97,15 @@ TEST_F(LoggerTest, GetLogger) {
     TestLogger logger1(name1);
     TestLogger logger2(name1);
 
-    // And check they are equal and non-null
+    // And check they are null at this point.
+    EXPECT_FALSE(logger1.isInitialized());
+    EXPECT_FALSE(logger2.isInitialized());
+
+    // Do some random operation
+    EXPECT_TRUE(logger1.isFatalEnabled());
+    EXPECT_TRUE(logger2.isFatalEnabled());
+
+    // And check they initialized and equal
     EXPECT_TRUE(logger1.isInitialized());
     EXPECT_TRUE(logger2.isInitialized());
     EXPECT_TRUE(logger1 == logger2);
@@ -105,6 +113,8 @@ TEST_F(LoggerTest, GetLogger) {
     // Instantiate another logger with another name and check that it
     // is different to the previously instantiated ones.
     TestLogger logger3(name2);
+    EXPECT_FALSE(logger3.isInitialized());
+    EXPECT_TRUE(logger3.isFatalEnabled());
     EXPECT_TRUE(logger3.isInitialized());
     EXPECT_FALSE(logger1 == logger3);
 }
@@ -383,50 +393,3 @@ TEST_F(LoggerTest, IsDebugEnabledLevel) {
     EXPECT_TRUE(logger.isDebugEnabled(MID_LEVEL));
     EXPECT_TRUE(logger.isDebugEnabled(MAX_DEBUG_LEVEL));
 }
-
-// Check that the message formatting is correct.  As this test program is
-// linking with the logger library - which includes messages from the logger
-// itself - we'll use those messages for testing.
-
-TEST_F(LoggerTest, Format) {
-    RootLoggerName::setName("test9");
-    Logger logger("test9");
-
-// Individual arguments
-    string result = logger.formatMessage(MSG_OPENIN);
-    EXPECT_EQ(string("OPENIN, unable to open %s for input: %s"), result);
-
-    vector<string> args;
-    args.push_back("alpha.txt");
-
-    result = logger.formatMessage(MSG_OPENIN, &args);
-    EXPECT_EQ(string("OPENIN, unable to open alpha.txt for input: %s"), result);
-
-    args.push_back("test");
-    result = logger.formatMessage(MSG_OPENIN, &args);
-    EXPECT_EQ(string("OPENIN, unable to open alpha.txt for input: test"), result);
-
-    // Excess arguments should be ignored
-    args.push_back("ignore me");
-    result = logger.formatMessage(MSG_OPENIN, &args);
-    EXPECT_EQ(string("OPENIN, unable to open alpha.txt for input: test"), result);
-
-    // Try the same using concatenated arguments
-    string strarg = "alpha.txt";
-    result = logger.formatMessage(MSG_OPENIN, strarg);
-    EXPECT_EQ(string("OPENIN, unable to open alpha.txt for input: %s"), result);
-
-    strarg += "\0test";
-    result = logger.formatMessage(MSG_OPENIN, &args);
-    EXPECT_EQ(string("OPENIN, unable to open alpha.txt for input: test"), result);
-
-    // With the latter method, try a few "unusual" argument strings
-    strarg = "";
-    result = logger.formatMessage(MSG_OPENIN, strarg);
-    EXPECT_EQ(string("OPENIN, unable to open %s for input: %s"), result);
-    
-    strarg="\0";
-    result = logger.formatMessage(MSG_OPENIN, strarg);
-    EXPECT_EQ(string("OPENIN, unable to open %s for input: %s"), result);
-
-}

src/lib/log/tests/message_dictionary_unittest.cc

@@ -59,18 +59,12 @@ TEST_F(MessageDictionaryTest, Add) {
     MessageDictionary dictionary;
     EXPECT_EQ(0, dictionary.size());
 
-    vector<MessageID> overflow = dictionary.getOverflow();
-    EXPECT_EQ(0, overflow.size());
-
     // Add a few messages and check that we can look them up and that there is
     // nothing in the overflow vector.
     EXPECT_TRUE(dictionary.add(alpha_id, alpha_text));
     EXPECT_TRUE(dictionary.add(beta_id, beta_text));
     EXPECT_EQ(2, dictionary.size());
 
-    overflow = dictionary.getOverflow();
-    EXPECT_EQ(0, overflow.size());
-
     EXPECT_EQ(alpha_text, dictionary.getText(alpha_id));
     EXPECT_EQ(beta_text, dictionary.getText(beta_id));
     EXPECT_EQ(string(""), dictionary.getText(gamma_id));
@@ -79,35 +73,6 @@ TEST_F(MessageDictionaryTest, Add) {
     // current text and the ID should be in the overflow section.
     EXPECT_FALSE(dictionary.add(alpha_id, gamma_text));
     EXPECT_EQ(2, dictionary.size());
-
-    overflow = dictionary.getOverflow();
-    ASSERT_EQ(1, overflow.size());
-    EXPECT_EQ(alpha_id, overflow[0]);
-    EXPECT_EQ(alpha_text, dictionary.getText(alpha_id));
-}
-
-// Check that clearing the overflow vector works
-
-TEST_F(MessageDictionaryTest, ClearOverflow) {
-    MessageDictionary dictionary;
-    EXPECT_EQ(0, dictionary.size());
-
-    vector<MessageID> overflow = dictionary.getOverflow();
-
-    // Add one message twice to get an overflow
-    EXPECT_TRUE(dictionary.add(alpha_id, alpha_text));
-    EXPECT_FALSE(dictionary.add(alpha_id, alpha_text));
-    EXPECT_EQ(1, dictionary.size());
-
-    // Check the overflow
-    overflow = dictionary.getOverflow();
-    ASSERT_EQ(1, overflow.size());
-    EXPECT_EQ(alpha_id, overflow[0]);
-
-    // ... and check that clearing it works
-    dictionary.clearOverflow();
-    overflow = dictionary.getOverflow();
-    ASSERT_EQ(0, overflow.size());
 }
 
 // Check that replacing messages works.
@@ -116,21 +81,11 @@ TEST_F(MessageDictionaryTest, Replace) {
     MessageDictionary dictionary;
     EXPECT_EQ(0, dictionary.size());
 
-    vector<MessageID> overflow = dictionary.getOverflow();
-    EXPECT_EQ(0, overflow.size());
-
     // Try to replace a non-existent message
     EXPECT_FALSE(dictionary.replace(alpha_id, alpha_text));
     EXPECT_EQ(0, dictionary.size());
-    overflow = dictionary.getOverflow();
-    ASSERT_EQ(1, overflow.size());
-    EXPECT_EQ(alpha_id, overflow[0]);
-
-    // Clear the overflow and add a couple of messages.
-    dictionary.clearOverflow();
-    overflow = dictionary.getOverflow();
-    ASSERT_EQ(0, overflow.size());
 
+    // Add a couple of messages.
     EXPECT_TRUE(dictionary.add(alpha_id, alpha_text));
     EXPECT_TRUE(dictionary.add(beta_id, beta_text));
     EXPECT_EQ(2, dictionary.size());
@@ -139,8 +94,6 @@ TEST_F(MessageDictionaryTest, Replace) {
     EXPECT_TRUE(dictionary.replace(alpha_id, gamma_text));
     EXPECT_EQ(2, dictionary.size());
     EXPECT_EQ(gamma_text, dictionary.getText(alpha_id));
-    overflow = dictionary.getOverflow();
-    ASSERT_EQ(0, overflow.size());
 
     // ... and replace non-existent message (but now the dictionary has some
     // items in it).
@@ -169,27 +122,26 @@ TEST_F(MessageDictionaryTest, LoadTest) {
     EXPECT_EQ(0, dictionary1.size());
 
     // Load a dictionary1.
-    dictionary1.load(data1);
+    vector<MessageID> duplicates = dictionary1.load(data1);
     EXPECT_EQ(3, dictionary1.size());
     EXPECT_EQ(string(data1[1]), dictionary1.getText(data1[0]));
     EXPECT_EQ(string(data1[3]), dictionary1.getText(data1[2]));
     EXPECT_EQ(string(data1[5]), dictionary1.getText(data1[4]));
-    vector<MessageID> overflow = dictionary1.getOverflow();
-    EXPECT_EQ(0, overflow.size());
+    EXPECT_EQ(0, duplicates.size());
 
     // Attempt an overwrite
-    dictionary1.load(data1);
+    duplicates = dictionary1.load(data1);
     EXPECT_EQ(3, dictionary1.size());
-    overflow = dictionary1.getOverflow();
-    EXPECT_EQ(3, overflow.size());
+    EXPECT_EQ(3, duplicates.size());
 
     // Try a new dictionary but with an incorrect number of elements
     MessageDictionary dictionary2;
     EXPECT_EQ(0, dictionary2.size());
 
-    dictionary2.load(data2);
+    duplicates = dictionary2.load(data2);
     EXPECT_EQ(2, dictionary2.size());
     EXPECT_EQ(string(data2[1]), dictionary2.getText(data2[0]));
     EXPECT_EQ(string(data2[3]), dictionary2.getText(data2[2]));
     EXPECT_EQ(string(""), dictionary2.getText(data2[4]));
+    EXPECT_EQ(0, duplicates.size());
 }

src/lib/log/tests/message_reader_unittest.cc

@@ -62,8 +62,6 @@ TEST_F(MessageReaderTest, BlanksAndComments) {
 
     // Ensure that the dictionary is empty.
     EXPECT_EQ(0, dictionary_->size());
-    vector<MessageID> overflow = dictionary_->getOverflow();
-    EXPECT_EQ(0, overflow.size());
 
     // Add a number of blank lines and comments and check that (a) they are
     // parsed successfully ...
@@ -76,10 +74,10 @@ TEST_F(MessageReaderTest, BlanksAndComments) {
     EXPECT_NO_THROW(reader_.processLine("#+ A comment"));
     EXPECT_NO_THROW(reader_.processLine("  +# A description line"));
 
-    // ... and (b) nothing gets added to either the map or the duplicates
+    // ... and (b) nothing gets added to either the map or the not-added section.
     EXPECT_EQ(0, dictionary_->size());
-    overflow = dictionary_->getOverflow();
-    EXPECT_EQ(0, overflow.size());
+    vector<MessageID> not_added = reader_.getNotAdded();
+    EXPECT_EQ(0, not_added.size());
 }
 
 
@@ -149,9 +147,9 @@ TEST_F(MessageReaderTest, ValidMessageAddDefault) {
         dictionary_->getText("GLOBAL2"));
     EXPECT_EQ(2, dictionary_->size());
 
-    // ... and ensure no overflows
-    vector<MessageID> overflow = dictionary_->getOverflow();
-    EXPECT_EQ(0, overflow.size());
+    // ... and ensure no messages were not added
+    vector<MessageID> not_added = reader_.getNotAdded();
+    EXPECT_EQ(0, not_added.size());
 }
 
 TEST_F(MessageReaderTest, ValidMessageAdd) {
@@ -169,9 +167,9 @@ TEST_F(MessageReaderTest, ValidMessageAdd) {
         dictionary_->getText("GLOBAL2"));
     EXPECT_EQ(2, dictionary_->size());
 
-    // ... and ensure no overflows
-    vector<MessageID> overflow = dictionary_->getOverflow();
-    EXPECT_EQ(0, overflow.size());
+    // ... and ensure no messages were not added
+    vector<MessageID> not_added = reader_.getNotAdded();
+    EXPECT_EQ(0, not_added.size());
 }
 
 TEST_F(MessageReaderTest, ValidMessageReplace) {
@@ -192,9 +190,9 @@ TEST_F(MessageReaderTest, ValidMessageReplace) {
         dictionary_->getText("GLOBAL2"));
     EXPECT_EQ(2, dictionary_->size());
 
-    // ... and ensure no overflows
-    vector<MessageID> overflow = dictionary_->getOverflow();
-    EXPECT_EQ(0, overflow.size());
+    // ... and ensure no messages were not added
+    vector<MessageID> not_added = reader_.getNotAdded();
+    EXPECT_EQ(0, not_added.size());
 }
 
 // Do checks on overflows, although this essentially duplicates the checks
@@ -220,11 +218,11 @@ TEST_F(MessageReaderTest, Overflows) {
         dictionary_->getText("GLOBAL2"));
     EXPECT_EQ(2, dictionary_->size());
 
-    // Check what is in the overflow
-    vector<MessageID> overflow = dictionary_->getOverflow();
-    ASSERT_EQ(2, overflow.size());
+    // ... and ensure no overflows
+    vector<MessageID> not_added = reader_.getNotAdded();
+    ASSERT_EQ(2, not_added.size());
 
-    sort(overflow.begin(), overflow.end());
-    EXPECT_EQ(string("GLOBAL1"), overflow[0]);
-    EXPECT_EQ(string("LOCAL"), overflow[1]);
+    sort(not_added.begin(), not_added.end());
+    EXPECT_EQ(string("GLOBAL1"), not_added[0]);
+    EXPECT_EQ(string("LOCAL"), not_added[1]);
 }