BIND 10 trac555, updated. a61bc91e4109168327481710f17affe728af80d5 [trac555] Remove redundant test class from OutputOption

BIND 10 source code commits bind10-changes at lists.isc.org
Wed Jun 1 13:48:35 UTC 2011


The branch, trac555 has been updated
       via  a61bc91e4109168327481710f17affe728af80d5 (commit)
       via  c58ee77ef68b2cf49db6af85ede886580ad2b6bc (commit)
       via  601d7f0170b1f9f929acadd3c37d60c5876ca7be (commit)
       via  ef8b3f326fe9ab9aab0050071c6f8975b8ecd354 (commit)
       via  51f6bda14d754aa6aea474300c8c51f15f32f0be (commit)
       via  711a621387d48993712031be9164510de7b27054 (commit)
       via  4948469dafe0a5ff130706ff2fb13676c417a538 (commit)
       via  2626464bbbb35e29f01a7e5a532d06da8feef837 (commit)
       via  83a82bdac7711760eed682b91f6be4435606a0dd (commit)
       via  616019706c0101316835f85843c59439e20a1c8e (commit)
       via  cb6be8450ae82e705dc4c65ca4415b1ed77abd6b (commit)
      from  99a107f4ab33c0791df351408c76c07f5820cde5 (commit)

Those revisions listed above that are new to this repository have
not appeared on any other notification email; so we list those
revisions in full, below.

- Log -----------------------------------------------------------------
commit a61bc91e4109168327481710f17affe728af80d5
Author: Stephen Morris <stephen at isc.org>
Date:   Wed Jun 1 14:47:30 2011 +0100

    [trac555] Remove redundant test class from OutputOption
    
    ... and convert from using TEST_F to TEST.

commit c58ee77ef68b2cf49db6af85ede886580ad2b6bc
Author: Stephen Morris <stephen at isc.org>
Date:   Wed Jun 1 14:43:44 2011 +0100

    [trac555] Remove redundant test class from LoggerSpecification unit tests
    
    ... and correct error in previous commit.

commit 601d7f0170b1f9f929acadd3c37d60c5876ca7be
Author: Stephen Morris <stephen at isc.org>
Date:   Wed Jun 1 14:32:46 2011 +0100

    [trac555] Don't automatically unlink temporary file after creation

commit ef8b3f326fe9ab9aab0050071c6f8975b8ecd354
Author: Stephen Morris <stephen at isc.org>
Date:   Wed Jun 1 14:18:48 2011 +0100

    [trac555] Move definition of UnknownLoggingDestination to header file

commit 51f6bda14d754aa6aea474300c8c51f15f32f0be
Author: Stephen Morris <stephen at isc.org>
Date:   Wed Jun 1 13:33:15 2011 +0100

    [trac555] Comment update.

commit 711a621387d48993712031be9164510de7b27054
Author: Stephen Morris <stephen at isc.org>
Date:   Wed Jun 1 13:28:48 2011 +0100

    [trac555] Clarify comments

commit 4948469dafe0a5ff130706ff2fb13676c417a538
Author: Stephen Morris <stephen at isc.org>
Date:   Wed Jun 1 12:27:15 2011 +0100

    [trac555] Simplfy and clarify header comment for process()

commit 2626464bbbb35e29f01a7e5a532d06da8feef837
Author: Stephen Morris <stephen at isc.org>
Date:   Wed Jun 1 12:14:50 2011 +0100

    [trac555] Change to message formatting if catching MessageException
    
    Also add test to check that an exception is generated if there is
    a failure to access a local message file.

commit 83a82bdac7711760eed682b91f6be4435606a0dd
Author: Stephen Morris <stephen at isc.org>
Date:   Wed Jun 1 11:03:52 2011 +0100

    [trac555] Updated comments concerining duplicate/unknown messages

commit 616019706c0101316835f85843c59439e20a1c8e
Author: Stephen Morris <stephen at isc.org>
Date:   Wed Jun 1 10:48:33 2011 +0100

    [trac555] Move LoggerManagerImpl::processEnd() to the correct file

commit cb6be8450ae82e705dc4c65ca4415b1ed77abd6b
Author: Stephen Morris <stephen at isc.org>
Date:   Wed Jun 1 10:39:28 2011 +0100

    [trac555] Log message if error converting a DEBUG level from a string

-----------------------------------------------------------------------

Summary of changes:
 src/lib/log/Makefile.am                            |    2 +
 src/lib/log/impldef.cc                             |   29 ++++++++++
 src/lib/log/impldef.h                              |   18 ++++++
 src/lib/log/impldef.mes                            |   38 ++++++++++++++
 src/lib/log/log_formatter.h                        |   30 ++++++++++-
 src/lib/log/logger_level_impl.cc                   |   31 ++++++++++--
 src/lib/log/logger_manager.cc                      |   55 +++++++++-----------
 src/lib/log/logger_manager.h                       |   27 +++++++---
 src/lib/log/logger_manager_impl.cc                 |   27 +++-------
 src/lib/log/logger_manager_impl.h                  |   14 ++++--
 src/lib/log/tests/local_file_test.sh.in            |   14 +++++-
 src/lib/log/tests/logger_manager_unittest.cc       |   13 +++--
 src/lib/log/tests/logger_specification_unittest.cc |   18 ++-----
 src/lib/log/tests/output_option_unittest.cc        |   12 +----
 14 files changed, 231 insertions(+), 97 deletions(-)
 create mode 100644 src/lib/log/impldef.cc
 create mode 100644 src/lib/log/impldef.h
 create mode 100644 src/lib/log/impldef.mes

-----------------------------------------------------------------------
diff --git a/src/lib/log/Makefile.am b/src/lib/log/Makefile.am
index a750559..0e0ef87 100644
--- a/src/lib/log/Makefile.am
+++ b/src/lib/log/Makefile.am
@@ -8,6 +8,7 @@ CLEANFILES = *.gcno *.gcda
 lib_LTLIBRARIES = liblog.la
 liblog_la_SOURCES  =
 liblog_la_SOURCES += dummylog.h dummylog.cc
+liblog_la_SOURCES += impldef.cc impldef.h
 liblog_la_SOURCES += log_formatter.h log_formatter.cc
 liblog_la_SOURCES += logger.cc logger.h
 liblog_la_SOURCES += logger_impl.cc logger_impl.h
@@ -29,6 +30,7 @@ liblog_la_SOURCES += output_option.cc output_option.h
 liblog_la_SOURCES += root_logger_name.cc root_logger_name.h
 
 EXTRA_DIST  = README
+EXTRA_DIST += impldef.mes
 EXTRA_DIST += messagedef.mes
 
 # Note: the ordering matters: -Wno-... must follow -Wextra (defined in
diff --git a/src/lib/log/impldef.cc b/src/lib/log/impldef.cc
new file mode 100644
index 0000000..087ebea
--- /dev/null
+++ b/src/lib/log/impldef.cc
@@ -0,0 +1,29 @@
+// File created from impldef.mes on Wed Jun  1 10:32:57 2011
+
+#include <cstddef>
+#include <log/message_types.h>
+#include <log/message_initializer.h>
+
+namespace isc {
+namespace log {
+
+extern const isc::log::MessageID LOGIMPL_ABOVEDBGMAX = "LOGIMPL_ABOVEDBGMAX";
+extern const isc::log::MessageID LOGIMPL_BADDEBUG = "LOGIMPL_BADDEBUG";
+extern const isc::log::MessageID LOGIMPL_BELOWDBGMIN = "LOGIMPL_BELOWDBGMIN";
+
+} // namespace log
+} // namespace isc
+
+namespace {
+
+const char* values[] = {
+    "LOGIMPL_ABOVEDBGMAX", "debug level of %1 is too high and will be set to the maximum of %2",
+    "LOGIMPL_BADDEBUG", "debug string is '%1': must be of the form DEBUGn",
+    "LOGIMPL_BELOWDBGMIN", "debug level of %1 is too low and will be set to the minimum of %2",
+    NULL
+};
+
+const isc::log::MessageInitializer initializer(values);
+
+} // Anonymous namespace
+
diff --git a/src/lib/log/impldef.h b/src/lib/log/impldef.h
new file mode 100644
index 0000000..7c70996
--- /dev/null
+++ b/src/lib/log/impldef.h
@@ -0,0 +1,18 @@
+// File created from impldef.mes on Wed Jun  1 10:32:57 2011
+
+#ifndef __IMPLDEF_H
+#define __IMPLDEF_H
+
+#include <log/message_types.h>
+
+namespace isc {
+namespace log {
+
+extern const isc::log::MessageID LOGIMPL_ABOVEDBGMAX;
+extern const isc::log::MessageID LOGIMPL_BADDEBUG;
+extern const isc::log::MessageID LOGIMPL_BELOWDBGMIN;
+
+} // namespace log
+} // namespace isc
+
+#endif // __IMPLDEF_H
diff --git a/src/lib/log/impldef.mes b/src/lib/log/impldef.mes
new file mode 100644
index 0000000..93e9fab
--- /dev/null
+++ b/src/lib/log/impldef.mes
@@ -0,0 +1,38 @@
+# Copyright (C) 2011  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.
+
+# \brief Logger Implementation Messages
+#
+# This holds messages generated by the underlying logger implementation.  They
+# are likely to be specific to that implementation, and may well change if the
+# underlying implementation is changed.  For that reason, they have been put
+# in a separate file.
+
+$PREFIX LOGIMPL_
+$NAMESPACE isc::log
+
+% ABOVEDBGMAX   debug level of %1 is too high and will be set to the maximum of %2
+A message from the underlying logger implementation code, the debug level
+(as set by the string DEBGUGn) is above the maximum allowed value and has
+been reduced to that value.
+
+% BADDEBUG      debug string is '%1': must be of the form DEBUGn
+The string indicating the extended logging level (used by the underlying
+logger implementation code) is not of the stated form.  In particular,
+it starts DEBUG but does not end with an integer.
+
+% BELOWDBGMIN   debug level of %1 is too low and will be set to the minimum of %2
+A message from the underlying logger implementation code, the debug level
+(as set by the string DEBGUGn) is below the minimum allowed value and has
+been increased to that value.
diff --git a/src/lib/log/log_formatter.h b/src/lib/log/log_formatter.h
index 14531bd..85890d0 100644
--- a/src/lib/log/log_formatter.h
+++ b/src/lib/log/log_formatter.h
@@ -15,7 +15,9 @@
 #ifndef __LOG_FORMATTER_H
 #define __LOG_FORMMATER_H
 
+#include <cstddef>
 #include <string>
+#include <iostream>
 #include <boost/lexical_cast.hpp>
 #include <log/logger_level.h>
 
@@ -84,7 +86,6 @@ private:
     /// \brief Which will be the next placeholder to replace
     unsigned nextPlaceholder_;
 
-    Formatter& operator =(const Formatter& other);
 
 public:
     /// \brief Constructor of "active" formatter
@@ -108,12 +109,18 @@ public:
     {
     }
 
+    /// \brief Copy constructor
+    ///
+    /// "Control" is passed to the created object in that it is the created object
+    /// that will have responsibility for outputting the formatted message - the
+    /// object being copied relinquishes that responsibility.
     Formatter(const Formatter& other) :
         logger_(other.logger_), severity_(other.severity_),
         message_(other.message_), nextPlaceholder_(other.nextPlaceholder_)
     {
-        other.logger_ = false;
+        other.logger_ = NULL;
     }
+
     /// \brief Destructor.
     //
     /// This is the place where output happens if the formatter is active.
@@ -123,6 +130,23 @@ public:
             delete message_;
         }
     }
+
+    /// \brief Assignment operator
+    ///
+    /// Essentially the same function as the assignment operator - the object being
+    /// assigned to takes responsibility for outputting the message.
+    Formatter& operator =(const Formatter& other) {
+        if (&other != this) {
+            logger_ = other.logger_;
+            severity_ = other.severity_;
+            message_ = other.message_;
+            nextPlaceholder_ = other.nextPlaceholder_;
+            other.logger_ = NULL;
+        }
+
+        return *this;
+    }
+
     /// \brief Replaces another placeholder
     ///
     /// Replaces another placeholder and returns a new formatter with it.
@@ -137,6 +161,7 @@ public:
             return (*this);
         }
     }
+
     /// \brief String version of arg.
     Formatter& arg(const std::string& arg) {
         if (logger_) {
@@ -147,6 +172,7 @@ public:
         }
         return (*this);
     }
+
 };
 
 }
diff --git a/src/lib/log/logger_level_impl.cc b/src/lib/log/logger_level_impl.cc
index 7c98977..d6d8ed7 100644
--- a/src/lib/log/logger_level_impl.cc
+++ b/src/lib/log/logger_level_impl.cc
@@ -18,12 +18,19 @@
 #include <boost/lexical_cast.hpp>
 
 #include <log4cplus/logger.h>
+
+#include <log/impldef.h>
 #include <log/logger_level.h>
 #include <log/logger_level_impl.h>
+#include <log/macros.h>
 
 using namespace log4cplus;
 using namespace std;
 
+namespace {
+isc::log::Logger logger("log");
+}
+
 namespace isc {
 namespace log {
 
@@ -87,7 +94,8 @@ LoggerLevelImpl::convertToBindLevel(const log4cplus::LogLevel loglevel) {
     } else if (loglevel <= log4cplus::DEBUG_LOG_LEVEL) {
 
         // Debug severity, so extract the debug level from the numeric value.
-        // If outside the limits, change the severity to the level above or below.
+        // If outside the limits, change the severity to the level above or
+        // below.
         int dbglevel = MIN_DEBUG_LEVEL +
                        static_cast<int>(log4cplus::DEBUG_LOG_LEVEL) -
                        static_cast<int>(loglevel);
@@ -148,16 +156,28 @@ LoggerLevelImpl::logLevelFromString(const log4cplus::tstring& level) {
                 // if DEBUG99 has been specified.
                 try {
                     int dbglevel = boost::lexical_cast<int>(name.substr(5));
+                    if (dbglevel < MIN_DEBUG_LEVEL) {
+                        LOG_WARN(logger, LOGIMPL_BELOWDBGMIN).arg(dbglevel)
+                            .arg(MIN_DEBUG_LEVEL);
+                        dbglevel = MIN_DEBUG_LEVEL;
+
+                    } else if (dbglevel > MAX_DEBUG_LEVEL) {
+                        LOG_WARN(logger, LOGIMPL_ABOVEDBGMAX).arg(dbglevel)
+                            .arg(MAX_DEBUG_LEVEL);
+                        dbglevel = MAX_DEBUG_LEVEL;
+
+                    }
                     return convertFromBindLevel(Level(DEBUG, dbglevel));
                 }
                 catch (boost::bad_lexical_cast&) {
+                    LOG_ERROR(logger, LOGIMPL_BADDEBUG).arg(name);
                     return (NOT_SET_LOG_LEVEL);
                 }
             }
-        }
-        else {
+        } else {
 
-            // Unknown string - return default. 
+            // Unknown string - return default.  Log4cplus will call any other
+            // registered conversion functions to interpret it.
             return (NOT_SET_LOG_LEVEL);
         }
     }
@@ -175,6 +195,9 @@ LoggerLevelImpl::logLevelToString(log4cplus::LogLevel level) {
         ((dbglevel >= MIN_DEBUG_LEVEL) && (dbglevel <= MAX_DEBUG_LEVEL))) {
         return (tstring("DEBUG"));
     }
+
+    // Unknown, so return empty string for log4cplus to try other conversion
+    // functions.
     return (tstring());
 }
 
diff --git a/src/lib/log/logger_manager.cc b/src/lib/log/logger_manager.cc
index 5f4770d..78bb7f1 100644
--- a/src/lib/log/logger_manager.cc
+++ b/src/lib/log/logger_manager.cc
@@ -31,11 +31,15 @@
 
 using namespace std;
 
+namespace {
+
+// Logger used for logging messages within the logging code itself.
+isc::log::Logger logger("log");
+}
+
 namespace isc {
 namespace log {
 
-void LoggerManagerImpl::processEnd() {}
-
 // Constructor - create the implementation  class.
 LoggerManager::LoggerManager() {
     impl_ = new LoggerManagerImpl();
@@ -76,22 +80,22 @@ LoggerManager::init(const std::string& root, const char* file,
     // All other loggers created in this application will be its children.
     setRootLoggerName(root);
 
-    // Initialize the implementation logging.
+    // Initialize the implementation logging.  After this point, some basic
+    // logging has been set up and messages can be logged.
     LoggerManagerImpl::init(severity, dbglevel);
 
-    // TODO: sort out the names.
-    Logger logger("log");
-
     // Check if there were any duplicate message IDs in the default dictionary
-    // and if so, log them.  Log using the logging facility root logger.
+    // and if so, log them.  Log using the logging facility logger.
     vector<string>& duplicates = MessageInitializer::getDuplicates();
     if (!duplicates.empty()) {
 
-        // There are - sort and remove any duplicates.
+        // There are duplicates present.  This will be listed in alphabetic
+        // order of message ID, so they need to be sorted.  This list itself may
+        // contain duplicates; if so, the message ID is listed as many times as
+        // there are duplicates.
         sort(duplicates.begin(), duplicates.end());
-        vector<string>::iterator new_end =
-            unique(duplicates.begin(), duplicates.end());
-        for (vector<string>::iterator i = duplicates.begin(); i != new_end; ++i) {
+        for (vector<string>::iterator i = duplicates.begin();
+             i != duplicates.end(); ++i) {
             LOG_WARN(logger, MSG_DUPMSGID).arg(*i);
         }
 
@@ -110,17 +114,17 @@ LoggerManager::init(const std::string& root, const char* file,
 void
 LoggerManager::readLocalMessageFile(const char* file) {
 
-    Logger logger("log");
-
     MessageDictionary& dictionary = MessageDictionary::globalDictionary();
     MessageReader reader(&dictionary);
     try {
 
-        // FIXME: commented out for testing
-        // logger.info(MSG_RDLOCMES).arg(file);
+        logger.info(MSG_RDLOCMES).arg(file);
         reader.readFile(file, MessageReader::REPLACE);
 
-        // File successfully read, list the duplicates
+        // File successfully read.  As each message in the file is supposed to
+        // replace one in the dictionary, any ID read that can't be located in
+        // the dictionary will not be used.  To aid problem diagnosis, the
+        // unknown message IDs are listed.
         MessageReader::MessageIDCollection unknown = reader.getNotAdded();
         for (MessageReader::MessageIDCollection::const_iterator
             i = unknown.begin(); i != unknown.end(); ++i) {
@@ -131,21 +135,12 @@ LoggerManager::readLocalMessageFile(const char* file) {
     catch (MessageException& e) {
         MessageID ident = e.id();
         vector<string> args = e.arguments();
-        switch (args.size()) {
-        case 0:
-            LOG_ERROR(logger, ident);
-            break;
-
-        case 1:
-            LOG_ERROR(logger, ident).arg(args[0]);
-            break;
-
-        case 2:
-            LOG_ERROR(logger, ident).arg(args[0]).arg(args[1]);
-            break;
 
-        default:    // 3 or more (3 should be the maximum)
-            LOG_ERROR(logger, ident).arg(args[0]).arg(args[1]).arg(args[2]);
+        // Log the variable number of arguments.  The actual message will be
+        // logged when the error_message variable is destroyed.
+        Formatter<isc::log::Logger> error_message = logger.error(ident);
+        for (int i = 0; i < args.size(); ++i) {
+            error_message = error_message.arg(args[i]);
         }
     }
 }
diff --git a/src/lib/log/logger_manager.h b/src/lib/log/logger_manager.h
index 15bb1b2..fbcc9b0 100644
--- a/src/lib/log/logger_manager.h
+++ b/src/lib/log/logger_manager.h
@@ -15,8 +15,18 @@
 #ifndef __LOGGER_MANAGER_H
 #define __LOGGER_MANAGER_H
 
+#include "exceptions/exceptions.h"
 #include <log/logger_specification.h>
 
+// Generated if, when updating the logging specification, an unknown
+// destination is encountered.
+class UnknownLoggingDestination : public isc::Exception {
+public:
+    UnknownLoggingDestination(const char* file, size_t line, const char* what) :
+        isc::Exception(file, line, what)
+    {}
+};
+
 namespace isc {
 namespace log {
 
@@ -40,8 +50,7 @@ public:
 
     /// \brief Process Specifications
     ///
-    /// Given a list of logger specifications, disables all current logging
-    /// and resets the properties of each logger to that given.
+    /// Replaces the current logging configuration by the one given.
     ///
     /// \param start Iterator pointing to the start of the collection of
     ///        logging specifications.
@@ -72,10 +81,13 @@ public:
     /// Performs run-time initialization of the logger system, in particular
     /// supplying the root logger name and name of a replacement message file.
     ///
-    /// This must be the first logging function called in the program.
+    /// This must be the first logging function called in the program.  If
+    /// an attempt is made to log a message before this is function is called,
+    /// the results will be dependent on the underlying logging package.
     ///
-    /// \param root Name of the root logger
-    /// \param file Name of the local message file.
+    /// \param root Name of the root logger.  This can be NULL, but should be
+    ///        set to the name of the program.
+    /// \param file Name of the local message file (if there is one).
     /// \param severity Severity at which to log
     /// \param dbglevel Debug severity (ignored if "severity" is not "DEBUG")
     static void init(const std::string& root, const char* file = NULL,
@@ -101,8 +113,9 @@ private:
     /// \brief Initialize Processing
     ///
     /// Initializes the processing of a list of specifications by resetting all
-    /// loggers to their defaults.  Note that loggers aren't removed as unless
-    /// a previously-enabled logger is re-enabled, it becomes inactive.
+    /// loggers to their defaults, which is to pass the message to their
+    /// parent logger.  (Except for the root logger, where the default action is
+    /// to output the message.)
     void processInit();
 
     /// \brief Process Logging Specification
diff --git a/src/lib/log/logger_manager_impl.cc b/src/lib/log/logger_manager_impl.cc
index d718c66..510ffe5 100644
--- a/src/lib/log/logger_manager_impl.cc
+++ b/src/lib/log/logger_manager_impl.cc
@@ -21,6 +21,7 @@
 #include <log4cplus/fileappender.h>
 
 #include "log/logger_level_impl.h"
+#include "log/logger_manager.h"
 #include "log/logger_manager_impl.h"
 #include "log/logger_specification.h"
 #include "log/root_logger_name.h"
@@ -28,16 +29,10 @@
 #include "log/logger.h"
 #include "log/messagedef.h"
 
-#include "exceptions/exceptions.h"
+using namespace std;
 
-// Generated exceptions.  Methods in this file can't log exceptions as they may
-// occur when logging is disabled or in an inconsistent state.
-class UnknownLoggingDestination : public isc::Exception {
-public:
-    UnknownLoggingDestination(const char* file, size_t line, const char* what) :
-        isc::Exception(file, line, what)
-    {}
-};
+namespace isc {
+namespace log {
 
 // Reset hierarchy of loggers back to default settings.  This removes all
 // appenders from loggers, sets their severity to NOT_SET (so that events are
@@ -45,15 +40,6 @@ public:
 // informational messages.  (This last is not a log4cplus default, so we have to
 // explicitly reset the logging severity.)
 
-using namespace std;
-
-namespace isc {
-namespace log {
-
-// Reset hierarchy back to default.  Note that this does not delete existing
-// loggers, it makes them inactive.  (So a logger is never removed, even if a
-// configuration update removes the logger.)
-
 void
 LoggerManagerImpl::processInit() {
     log4cplus::Logger::getDefaultHierarchy().resetConfiguration();
@@ -66,7 +52,6 @@ LoggerManagerImpl::processInit() {
 void
 LoggerManagerImpl::processSpecification(const LoggerSpecification& spec) {
 
-    // Get/construct the logger for which this specification applies.
     log4cplus::Logger logger = (spec.getName() == getRootLoggerName()) ?
                                log4cplus::Logger::getRoot() :
                                log4cplus::Logger::getInstance(spec.getName());
@@ -101,6 +86,10 @@ LoggerManagerImpl::processSpecification(const LoggerSpecification& spec) {
                 break;
 
             default:
+                // Not a valid destination.  As we are in the middle of updating
+                // logging destinations, we could be in the situation where
+                // there are no valid appenders.  For this reason, throw an
+                // exception.
                 isc_throw(UnknownLoggingDestination,
                           "Unknown logging destination, code = " <<
                           i->destination);
diff --git a/src/lib/log/logger_manager_impl.h b/src/lib/log/logger_manager_impl.h
index 56ac105..e384112 100644
--- a/src/lib/log/logger_manager_impl.h
+++ b/src/lib/log/logger_manager_impl.h
@@ -73,13 +73,19 @@ public:
     /// \brief End Processing
     ///
     /// Terminates the processing of the logging specifications.
-    static void processEnd();
+    static void processEnd()
+    {}
 
     /// \brief Implementation-specific initialization
     ///
-    /// Performs any implementation-specific initialization.  It is assumed
-    /// that the name of the BIND 10 root logger can be obtained from the
-    /// global function getRootLoggerName().
+    /// Sets the basic configuration for logging (the root logger has INFO and
+    /// more severe messages routed to stdout).  Unless this function (or
+    /// process() with a valid specification for all loggers that will log
+    /// messages) is called before a message is logged, log4cplus will output
+    /// a message to stderr noting that logging has not been initialized.
+    ///
+    /// It is assumed here that the name of the BIND 10 root logger can be
+    /// obtained from the global function getRootLoggerName().
     ///
     /// \param severity Severity to be associated with this logger
     /// \param dbglevel Debug level associated with the root logger
diff --git a/src/lib/log/tests/local_file_test.sh.in b/src/lib/log/tests/local_file_test.sh.in
index 83f16d0..f370c5b 100755
--- a/src/lib/log/tests/local_file_test.sh.in
+++ b/src/lib/log/tests/local_file_test.sh.in
@@ -43,7 +43,7 @@ cat > $localmes << .
 % RDLOCMES    replacement read local message file, parameter is '%1'
 .
 
-echo "1. Local message replacement: "
+echo "1. Local message replacement:"
 cat > $tempfile << .
 WARN  [alpha.log] MSG_IDNOTFND, could not replace message text for 'MSG_NOTHERE': no such message
 FATAL [alpha.example] MSG_WRITERR, error writing to test1: 42
@@ -53,7 +53,19 @@ WARN  [alpha.dlm] MSG_READERR, replacement read error, parameters: 'a.txt' and '
 ./logger_example -c stdout -s warn $localmes | cut -d' ' -f3- | diff $tempfile -
 passfail $?
 
+echo "2. Report error if unable to read local message file:"
+cat > $tempfile << .
+ERROR [alpha.log] MSG_OPENIN, unable to open message file $localmes for input: No such file or directory
+FATAL [alpha.example] MSG_WRITERR, error writing to test1: 42
+ERROR [alpha.example] MSG_RDLOCMES, reading local message file dummy/file
+WARN  [alpha.dlm] MSG_READERR, error reading from message file a.txt: dummy reason
+.
 rm -f $localmes
+./logger_example -c stdout -s warn $localmes | cut -d' ' -f3- | diff $tempfile -
+passfail $?
+
+# Tidy up.
+
 rm -f $tempfile
 
 if [ $failcount -eq 0 ]; then
diff --git a/src/lib/log/tests/logger_manager_unittest.cc b/src/lib/log/tests/logger_manager_unittest.cc
index cf2486d..a9a3a5e 100644
--- a/src/lib/log/tests/logger_manager_unittest.cc
+++ b/src/lib/log/tests/logger_manager_unittest.cc
@@ -111,10 +111,6 @@ public:
     //
     // \return Temporary file name
     std::string createTempFilename() {
-
-        // Get prefix.  Note that in all copies, strncpy does not guarantee
-        // a null-terminated string, hence the explict setting of the last
-        // character to NULL.
         string filename = TEMP_DIR + "/bind10_logger_manager_test_XXXXXX";
 
         // Copy into writeable storage for the call to mkstemp
@@ -128,7 +124,6 @@ public:
             isc_throw(Exception, "Unable to obtain unique filename");
         }
         close(filenum);
-        unlink(tname.get());
 
         return (string(tname.get()));
     }
@@ -191,6 +186,13 @@ TEST_F(LoggerManagerTest, FileLogger) {
     // Create a specification for the file logger and use the manager to
     // connect the "filelogger" logger to it.
     SpecificationForFileLogger file_spec;
+
+    // For the first test, we want to check that the file is created
+    // if it does not already exist.  So delete the temporary file before
+    // logging the first message.
+    unlink(file_spec.getFileName().c_str());
+
+    // Set up the file appenders.
     LoggerManager manager;
     manager.process(file_spec.getSpecification());
 
@@ -199,6 +201,7 @@ TEST_F(LoggerManagerTest, FileLogger) {
     // put in the file for a later comparison.
     vector<MessageID> ids;
     {
+
         // Scope-limit the logger to ensure it is destroyed after the brief
         // check.  This adds weight to the idea that the logger will not
         // keep the file open.
diff --git a/src/lib/log/tests/logger_specification_unittest.cc b/src/lib/log/tests/logger_specification_unittest.cc
index 7f245a1..e416c32 100644
--- a/src/lib/log/tests/logger_specification_unittest.cc
+++ b/src/lib/log/tests/logger_specification_unittest.cc
@@ -22,18 +22,8 @@
 using namespace isc::log;
 using namespace std;
 
-/// \brief LoggerSpecification Test
-class LoggerSpecificationTest : public ::testing::Test {
-public:
-    LoggerSpecificationTest()
-    {}
-    ~LoggerSpecificationTest()
-    {}
-};
-
-
 // Check default initialization.
-TEST_F(LoggerSpecificationTest, DefaultInitialization) {
+TEST(LoggerSpecificationTest, DefaultInitialization) {
     LoggerSpecification spec;
 
     EXPECT_EQ(string(""), spec.getName());
@@ -44,7 +34,7 @@ TEST_F(LoggerSpecificationTest, DefaultInitialization) {
 }
 
 // Non-default initialization
-TEST_F(LoggerSpecificationTest, Initialization) {
+TEST(LoggerSpecificationTest, Initialization) {
     LoggerSpecification spec("alpha", isc::log::ERROR, 42, true);
 
     EXPECT_EQ(string("alpha"), spec.getName());
@@ -55,7 +45,7 @@ TEST_F(LoggerSpecificationTest, Initialization) {
 }
 
 // Get/Set tests
-TEST_F(LoggerSpecificationTest, SetGet) {
+TEST(LoggerSpecificationTest, SetGet) {
     LoggerSpecification spec;
 
     spec.setName("gamma");
@@ -75,7 +65,7 @@ TEST_F(LoggerSpecificationTest, SetGet) {
 }
 
 // Check option setting
-TEST_F(LoggerSpecificationTest, AddOption) {
+TEST(LoggerSpecificationTest, AddOption) {
     OutputOption option1;
     option1.destination = OutputOption::DEST_FILE;
     option1.filename = "/tmp/example.log";
diff --git a/src/lib/log/tests/output_option_unittest.cc b/src/lib/log/tests/output_option_unittest.cc
index f85e2cf..70dc4ba 100644
--- a/src/lib/log/tests/output_option_unittest.cc
+++ b/src/lib/log/tests/output_option_unittest.cc
@@ -21,20 +21,10 @@
 using namespace isc::log;
 using namespace std;
 
-/// \brief OutputOption Test
-class OutputOptionTest : public ::testing::Test {
-public:
-    OutputOptionTest()
-    {}
-    ~OutputOptionTest()
-    {}
-};
-
-
 // As OutputOption is a struct, the only meaningful test is to check that it
 // initializes correctly.
 
-TEST_F(OutputOptionTest, Initialization) {
+TEST(OutputOptionTest, Initialization) {
     OutputOption option;
 
     EXPECT_EQ(OutputOption::DEST_CONSOLE, option.destination);




More information about the bind10-changes mailing list