[trac558] Address some of initialization points

* Logger implementation now not created until logger is first used.
* Messages are now "const char*" to avoid problems if a reference
  to a message is made in a static initialization.
* Message compiler now recognises $NAMESPACE directive

src/lib/log/Makefile.am

@@ -20,7 +20,7 @@ liblog_la_SOURCES += message_dictionary.cc message_dictionary.h
 liblog_la_SOURCES += message_exception.h message_exception.cc
 liblog_la_SOURCES += message_initializer.cc message_initializer.h
 liblog_la_SOURCES += message_reader.cc message_reader.h
-liblog_la_SOURCES += message_types.h
+liblog_la_SOURCES += message_types.h message_types.cc
 liblog_la_SOURCES += root_logger_name.cc root_logger_name.h
 liblog_la_SOURCES += strutil.h strutil.cc
 # liblog_la_SOURCES += xdebuglevel.cc xdebuglevel.h

src/lib/log/compiler/message.cc

@@ -15,6 +15,7 @@
 // $Id$
 
 #include <cctype>
+#include <cstddef>
 #include <fstream>
 #include <iostream>
 #include <string>
@@ -84,8 +85,6 @@ static void usage() {
         "-h       Print this message and exit\n" <<
         "-p       Output a Python module holding the message definitions.\n" <<
         "         By default a C++ header file and implementation file are\n" <<
-
-
         "         written.\n" <<
         "-v       Print the program version and exit\n" <<
         "\n" <<
@@ -169,8 +168,8 @@ string quoteString(const string& instring) {
 ///
 /// \return Sorted list of message IDs
 
-vector<MessageID> sortedIdentifiers(MessageDictionary* dictionary) {
-    vector<MessageID> ident;
+vector<string> sortedIdentifiers(MessageDictionary* dictionary) {
+    vector<string> ident;
 
     for (MessageDictionary::const_iterator i = dictionary->begin();
          i != dictionary->end(); ++i) {
@@ -182,6 +181,34 @@ vector<MessageID> sortedIdentifiers(MessageDictionary* dictionary) {
 }
 
 
+/// \brief Split Namespace
+///
+/// The $NAMESPACE directive may well specify a namespace in the form a::b.
+/// Unfortunately, the C++ "namespace" statement can only accept a single
+/// string - to set up the namespace of "a::b" requires two statements, one
+/// for "namspace a" and the other for "namespace b".
+///
+/// This function returns the set of namespace components as a vector of
+/// strings.
+///
+/// \param ns Argument to $NAMESPACE (passed by valid, as we will be modifying
+/// it.)
+
+vector<string> splitNamespace(string ns) {
+
+    // Namespaces components are separated by double colon characters - convert
+    // to single colons.
+    size_t dcolon;
+    while ((dcolon = ns.find("::")) != string::npos) {
+        ns.replace(dcolon, 2, ":");
+    }
+
+    // ... and return the vector of namespace components split on the single
+    // colon.
+    return isc::strutil::tokens(ns, ":");
+}
+
+
 /// \brief Write Header File
 ///
 /// Writes the C++ header file containing the symbol definitions.
@@ -189,9 +216,12 @@ vector<MessageID> sortedIdentifiers(MessageDictionary* dictionary) {
 /// \param file Name of the message file.  The header file is written to a
 /// file of the same name but with a .h suffix.
 /// \param prefix Prefix string to use in symbols
+/// \param ns Namespace in which the definitions are to be placed.  An empty
+/// string indicates no namespace, the special string of "::" the anonymous
+/// namespace, and anything else is the namspace desired.
 /// \param dictionary Dictionary holding the message definitions.
 
-void writeHeaderFile(const string& file, const string& prefix,
+void writeHeaderFile(const string& file, const string& prefix, const string& ns,
     MessageDictionary* dictionary)
 {
     Filename message_file(file);
@@ -220,23 +250,60 @@ void writeHeaderFile(const string& file, const string& prefix,
              "#define "  << sentinel_text << "\n" <<
              "\n" <<
              "#include <log/message_types.h>\n" <<
-             "\n" <<
-             "namespace {\n" <<
              "\n";
 
-        vector<MessageID> idents = sortedIdentifiers(dictionary);
-        for (vector<MessageID>::const_iterator j = idents.begin();
+        // Namespaces
+        vector<string> ns_components;
+        if (ns == "") {
+
+            ; // Nothing given, so no namespace
+
+        } else if (ns == "::") {
+
+            // Use the anonymous namespace.
+            hfile << "namespace {\n" <<
+                "\n";
+        } else {
+
+            // Output one namespace statement for each namespace component.
+            ns_components = splitNamespace(ns);
+            for (int i = 0; i < ns_components.size(); ++i) {
+                hfile << "namespace " << ns_components[i] << " {\n";
+            }
+            hfile << "\n";
+        }
+
+        // Now the m,essage identifications themselves.
+        vector<string> idents = sortedIdentifiers(dictionary);
+        for (vector<string>::const_iterator j = idents.begin();
             j != idents.end(); ++j) {
-            hfile << "isc::log::MessageID " << prefix << *j <<
+            hfile << "const isc::log::MessageID " << prefix << *j <<
                 " = \"" << *j << "\";\n";
         }
+        hfile << "\n";
+
+        // Close off namespaces if appropriate.
+        if (ns == "") {
+
+            ; // Nothing given, so no namespace
+
+        } else if (ns == "::") {
+
+            // Use the anonymous namespace.
+            hfile << "}\n" <<
+                "\n";
+        } else {
+
+            // Output one namespace statement for each namespace component.
+            for (int i = (ns_components.size() - 1); i >= 0; --i) {
+                hfile << "} // namespace " << ns_components[i] << "\n";
+            }
+            hfile << "\n";
+        }
+
 
         // ... and finally the postamble
-        hfile <<
-            "\n" <<
-            "} // Anonymous namespace\n" <<
-            "\n" <<
-            "#endif // " << sentinel_text << "\n";
+        hfile << "#endif // " << sentinel_text << "\n";
 
         // Report errors (if any) and exit
         if (hfile.fail()) {
@@ -298,8 +365,8 @@ void writeProgramFile(const string& file, MessageDictionary* dictionary)
              "const char* values[] = {\n";
 
         // Output the identifiers and the associated text.
-        vector<MessageID> idents = sortedIdentifiers(dictionary);
-        for (vector<MessageID>::const_iterator i = idents.begin();
+        vector<string> idents = sortedIdentifiers(dictionary);
+        for (vector<string>::const_iterator i = idents.begin();
             i != idents.end(); ++i) {
                 ccfile << "    \"" << *i << "\", \"" <<
                     quoteString(dictionary->getText(*i)) << "\",\n";
@@ -422,7 +489,8 @@ int main(int argc, char** argv) {
         reader.readFile(message_file);
 
         // Now write the header file.
-        writeHeaderFile(message_file, reader.getPrefix(), &dictionary);
+        writeHeaderFile(message_file, reader.getPrefix(), reader.getNamespace(),
+            &dictionary);
 
         // ... and the message text file.
         writeProgramFile(message_file, &dictionary);
@@ -433,7 +501,9 @@ int main(int argc, char** argv) {
     catch (MessageException& e) {
         // Create an error message from the ID and the text
         MessageDictionary* global = MessageDictionary::globalDictionary();
-        string text = e.id() + ", " + global->getText(e.id());
+        string text = e.id();
+        text += ", ";
+        text += global->getText(e.id());
 
         // Format with arguments
         text = isc::strutil::format(text, e.arguments());

src/lib/log/logger.cc

@@ -31,11 +31,11 @@ using namespace std;
 namespace isc {
 namespace log {
 
-// Constructor
-
-Logger::Logger(const std::string& name, bool infunc) :
-    loggerptr_(new LoggerImpl(name, infunc))
-{}
+// Initialize Logger implementation.  Does not check whether the implementation
+// has already been ionitialized - that was done by the caller (getLoggerptr()).
+void Logger::initLoggerImpl() {
+    loggerptr_ = new LoggerImpl(name_, infunc_);
+}
 
 // Destructor.
 
@@ -47,62 +47,62 @@ Logger::~Logger() {
 
 std::string
 Logger::getName() {
-    return (loggerptr_->getName());
+    return (getLoggerptr()->getName());
 }
 
 // Set the severity for logging.
 
 void
 Logger::setSeverity(isc::log::Severity severity, int dbglevel) {
-    loggerptr_->setSeverity(severity, dbglevel);
+    getLoggerptr()->setSeverity(severity, dbglevel);
 }
 
 // Return the severity of the logger.
 
 isc::log::Severity
 Logger::getSeverity() {
-    return (loggerptr_->getSeverity());
+    return (getLoggerptr()->getSeverity());
 }
 
 // Get Effective Severity Level for Logger
 
 isc::log::Severity
 Logger::getEffectiveSeverity() {
-    return (loggerptr_->getEffectiveSeverity());
+    return (getLoggerptr()->getEffectiveSeverity());
 }
 
 // Debug level (only relevant if messages of severity DEBUG are being logged).
 
 int
 Logger::getDebugLevel() {
-    return (loggerptr_->getDebugLevel());
+    return (getLoggerptr()->getDebugLevel());
 }
 
 // Check on the current severity settings
 
 bool
 Logger::isDebugEnabled(int dbglevel) {
-    return (loggerptr_->isDebugEnabled(dbglevel));
+    return (getLoggerptr()->isDebugEnabled(dbglevel));
 }
 
 bool
 Logger::isInfoEnabled() {
-    return (loggerptr_->isInfoEnabled());
+    return (getLoggerptr()->isInfoEnabled());
 }
 
 bool
 Logger::isWarnEnabled() {
-    return (loggerptr_->isWarnEnabled());
+    return (getLoggerptr()->isWarnEnabled());
 }
 
 bool
 Logger::isErrorEnabled() {
-    return (loggerptr_->isErrorEnabled());
+    return (getLoggerptr()->isErrorEnabled());
 }
 
 bool
 Logger::isFatalEnabled() {
-    return (loggerptr_->isFatalEnabled());
+    return (getLoggerptr()->isFatalEnabled());
 }
 
 // Format a message: looks up the message text in the dictionary and formats
@@ -129,52 +129,52 @@ Logger::isFatalEnabled() {
 // Output methods
 
 void
-Logger::debug(int dbglevel, isc::log::MessageID ident, ...) {
+Logger::debug(int dbglevel, const isc::log::MessageID& ident, ...) {
     if (isDebugEnabled(dbglevel)) {
         char message[MESSAGE_SIZE];
         FORMAT_MESSAGE(message);
-        loggerptr_->debug(ident, message);
+        getLoggerptr()->debug(ident, message);
     }
 }
 
 void
-Logger::info(isc::log::MessageID ident, ...) {
+Logger::info(const isc::log::MessageID& ident, ...) {
     if (isInfoEnabled()) {
         char message[MESSAGE_SIZE];
         FORMAT_MESSAGE(message);
-        loggerptr_->info(ident, message);
+        getLoggerptr()->info(ident, message);
     }
 }
 
 void
-Logger::warn(isc::log::MessageID ident, ...) {
+Logger::warn(const isc::log::MessageID& ident, ...) {
     if (isWarnEnabled()) {
         char message[MESSAGE_SIZE];
         FORMAT_MESSAGE(message);
-        loggerptr_->warn(ident, message);
+        getLoggerptr()->warn(ident, message);
     }
 }
 
 void
-Logger::error(isc::log::MessageID ident, ...) {
+Logger::error(const isc::log::MessageID& ident, ...) {
     if (isErrorEnabled()) {
         char message[MESSAGE_SIZE];
         FORMAT_MESSAGE(message);
-        loggerptr_->error(ident, message);
+        getLoggerptr()->error(ident, message);
     }
 }
 
 void
-Logger::fatal(isc::log::MessageID ident, ...) {
+Logger::fatal(const isc::log::MessageID& ident, ...) {
     if (isFatalEnabled()) {
         char message[MESSAGE_SIZE];
         FORMAT_MESSAGE(message);
-        loggerptr_->fatal(ident, message);
+        getLoggerptr()->fatal(ident, message);
     }
 }
 
-bool Logger::operator==(const Logger& other) {
-    return (*loggerptr_ == *other.loggerptr_);
+bool Logger::operator==(Logger& other) {
+    return (*getLoggerptr() == *other.getLoggerptr());
 }
 
 // Protected methods (used for testing)

src/lib/log/logger.h

@@ -81,7 +81,9 @@ public:
     /// manifests itself during program rundown.
     /// \n
     /// The flag has no effect on non-log4cxx implementations.
-    Logger(const std::string& name, bool infunc = false);
+    Logger(const std::string& name, bool infunc = false) :
+        loggerptr_(NULL), name_(name), infunc_(infunc)
+    {}
 
 
     /// \brief Destructor
@@ -158,35 +160,35 @@ public:
     /// are used for more verbose output.
     /// \param ident Message identification.
     /// \param ... Optional arguments for the message.
-    void debug(int dbglevel, MessageID ident, ...);
+    void debug(int dbglevel, const MessageID& ident, ...);
 
 
     /// \brief Output Informational Message
     ///
     /// \param ident Message identification.
     /// \param ... Optional arguments for the message.
-    void info(MessageID ident, ...);
+    void info(const MessageID& ident, ...);
 
 
     /// \brief Output Warning Message
     ///
     /// \param ident Message identification.
     /// \param ... Optional arguments for the message.
-    void warn(MessageID ident, ...);
+    void warn(const MessageID& ident, ...);
 
 
     /// \brief Output Error Message
     ///
     /// \param ident Message identification.
     /// \param ... Optional arguments for the message.
-    void error(MessageID ident, ...);
+    void error(const MessageID& ident, ...);
 
 
     /// \brief Output Fatal Message
     ///
     /// \param ident Message identification.
     /// \param ... Optional arguments for the message.
-    void fatal(MessageID ident, ...);
+    void fatal(const MessageID& ident, ...);
 
     /// \brief Equality
     ///
@@ -194,7 +196,7 @@ public:
     /// (This method is principally for testing.)
     ///
     /// \return true if the logger objects are instances of the same logger.
-    bool operator==(const Logger& other);
+    bool operator==(Logger& other);
 
 protected:
 
@@ -205,7 +207,32 @@ protected:
     static void reset();
 
 private:
-    LoggerImpl*     loggerptr_;     /// Pointer to the underlying logger
+    /// \brief Initialize Implementation
+    ///
+    /// Returns the logger pointer.  If not yet set, the underlying
+    /// implementation class is initialized.\n
+    /// \n
+    /// The reason for this indirection is to avoid the "static initialization
+    /// fiacso", whereby we cannot rely on the order of static initializations.
+    /// The main problem is the root logger name - declared statically - which
+    /// is referenced by various loggers.  By deferring a reference to it until
+    /// after the program starts executing - by which time the root name object
+    /// will be initialized - we avoid this problem.
+    ///
+    /// \return Returns pointer to implementation
+    LoggerImpl* getLoggerptr() {
+        if (!loggerptr_) {
+            initLoggerImpl();
+        }
+        return loggerptr_;
+    }
+
+    /// \brief Initialize Underlying Implementation and Set loggerptr_
+    void initLoggerImpl();
+
+    LoggerImpl*     loggerptr_;     ///< Pointer to the underlying logger
+    std::string     name_;          ///< Copy of the logger name
+    bool            infunc_;        ///< Copy of the infunc argument
 };
 
 } // namespace log

src/lib/log/logger_impl.cc

@@ -19,6 +19,7 @@
 
 #include <stdarg.h>
 #include <stdio.h>
+#include <boost/lexical_cast.hpp>
 
 #include <log/debug_levels.h>
 #include <log/root_logger_name.h>
@@ -223,32 +224,32 @@ LoggerImpl::output(const char* sev_text, const string& message) {
 // Output various levels
 
 void
-LoggerImpl::debug(MessageID ident, const char* text) {
-    string message = ident + ", " + text;
+LoggerImpl::debug(const MessageID& ident, const char* text) {
+    string message = boost::lexical_cast<string>(ident) + ", " + text;
     output("DEBUG", message);
 }
 
 void
-LoggerImpl::info(MessageID ident, const char* text) {
-    string message = ident + ", " + text;
+LoggerImpl::info(const MessageID& ident, const char* text) {
+    string message = boost::lexical_cast<string>(ident) + ", " + text;
     output("INFO ", message);
 }
 
 void
-LoggerImpl::warn(MessageID ident, const char* text) {
-    string message = ident + ", " + text;
+LoggerImpl::warn(const MessageID& ident, const char* text) {
+    string message = boost::lexical_cast<string>(ident) + ", " + text;
     output("WARN ", message);
 }
 
 void
-LoggerImpl::error(MessageID ident, const char* text) {
-    string message = ident + ", " + text;
+LoggerImpl::error(const MessageID& ident, const char* text) {
+    string message = boost::lexical_cast<string>(ident) + ", " + text;
     output("ERROR", message);
 }
 
 void
-LoggerImpl::fatal(MessageID ident, const char* text) {
-    string message = ident + ", " + text;
+LoggerImpl::fatal(const MessageID& ident, const char* text) {
+    string message = boost::lexical_cast<string>(ident) + ", " + text;
     output("FATAL", message);
 }
 

src/lib/log/logger_impl.h

@@ -192,35 +192,35 @@ public:
     ///
     /// \param ident Message identification.
     /// \param text Text to log
-    void debug(MessageID ident, const char* text);
+    void debug(const MessageID& ident, const char* text);
 
 
     /// \brief Output Informational Message
     ///
     /// \param ident Message identification.
     /// \param text Text to log
-    void info(MessageID ident, const char* text);
+    void info(const MessageID& ident, const char* text);
 
 
     /// \brief Output Warning Message
     ///
     /// \param ident Message identification.
     /// \param text Text to log
-    void warn(MessageID ident, const char* text);
+    void warn(const MessageID& ident, const char* text);
 
 
     /// \brief Output Error Message
     ///
     /// \param ident Message identification.
     /// \param text Text to log
-    void error(MessageID ident, const char* text);
+    void error(const MessageID& ident, const char* text);
 
 
     /// \brief Output Fatal Message
     ///
     /// \param ident Message identification.
     /// \param text Text to log
-    void fatal(MessageID ident, const char* text);
+    void fatal(const MessageID& ident, const char* text);
 
 
     /// \brief Equality

src/lib/log/logger_impl_log4cxx.h

@@ -168,7 +168,7 @@ public:
     ///
     /// \param ident Message identification.
     /// \param text Text to log
-    void debug(MessageID ident, const char* text) {
+    void debug(const MessageID& ident, const char* text) {
         LOG4CXX_DEBUG(getLogger(), ident << ", " << text);
     }
 
@@ -177,7 +177,7 @@ public:
     ///
     /// \param ident Message identification.
     /// \param text Text to log
-    void info(MessageID ident, const char* text) {
+    void info(const MessageID& ident, const char* text) {
         LOG4CXX_INFO(getLogger(), ident << ", " << text);
     }
 
@@ -186,7 +186,7 @@ public:
     ///
     /// \param ident Message identification.
     /// \param text Text to log
-    void warn(MessageID ident, const char* text) {
+    void warn(const MessageID& ident, const char* text) {
         LOG4CXX_WARN(getLogger(), ident << ", " << text);
     }
 
@@ -195,7 +195,7 @@ public:
     ///
     /// \param ident Message identification.
     /// \param text Text to log
-    void error(MessageID ident, const char* text) {
+    void error(const MessageID& ident, const char* text) {
         LOG4CXX_ERROR(getLogger(), ident << ", " << text);
     }
 
@@ -204,7 +204,7 @@ public:
     ///
     /// \param ident Message identification.
     /// \param text Text to log
-    void fatal(MessageID ident, const char* text) {
+    void fatal(const MessageID& ident, const char* text) {
         LOG4CXX_FATAL(getLogger(), ident << ", " << text);
     }
 
@@ -220,7 +220,7 @@ public:
     /// (This method is principally for testing.)
     ///
     /// \return true if the logger objects are instances of the same logger.
-    bool operator==(const LoggerImpl& other) {
+    bool operator==(LoggerImpl& other) {
         return (*loggerptr_ == *other.loggerptr_);
     }
 
@@ -302,7 +302,7 @@ private:
     // problems with memory deletion on program exit, explained in the comments
     // for the "exit_delete" parameter in this class's constructor.
 
-    mutable log4cxx::LoggerPtr*  loggerptr_; ///< Pointer to the underlying logger
+    log4cxx::LoggerPtr*  loggerptr_;    ///< Pointer to the underlying logger
     std::string          name_;         ///< Name of this logger]
     bool                 exit_delete_;  ///< Delete loggerptr_ on exit?
 

src/lib/log/logger_support.cc

@@ -29,6 +29,7 @@
 /// the logging parameters from the configuration database.
 
 #include <vector>
+#include <boost/lexical_cast.hpp>
 
 #include <log/logger.h>
 #include <log/logger_support.h>
@@ -67,12 +68,13 @@ readLocalMessageFile(const char* file) {
         MessageReader::MessageIDCollection unknown = reader.getNotAdded();
         for (MessageReader::MessageIDCollection::const_iterator
             i = unknown.begin(); i != unknown.end(); ++i) {
-                logger.warn(MSG_IDNOTFND, (*i).c_str());
+            string message_id = boost::lexical_cast<string>(*i);
+                logger.warn(MSG_IDNOTFND, message_id.c_str());
         }
     }
     catch (MessageException& e) {
         MessageID ident = e.id();
-        vector<MessageID> args = e.arguments();
+        vector<string> args = e.arguments();
         switch (args.size()) {
         case 0:
             logger.error(ident);

src/lib/log/message_dictionary.cc

@@ -14,6 +14,8 @@