Browse Source

[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.
Stephen Morris 14 years ago
parent
commit
2626464bbb
3 changed files with 46 additions and 19 deletions
  1. 28 2
      src/lib/log/log_formatter.h
  2. 5 16
      src/lib/log/logger_manager.cc
  3. 13 1
      src/lib/log/tests/local_file_test.sh.in

+ 28 - 2
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);
     }
+
 };
 
 }

+ 5 - 16
src/lib/log/logger_manager.cc

@@ -114,8 +114,6 @@ 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 {
@@ -137,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]);
         }
     }
 }

+ 13 - 1
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