[trac558] Addressed comments made in review

src/lib/log/compiler/message.cc

@@ -335,16 +335,25 @@ writeHeaderFile(const string& file, const string& prefix, const string& ns,
         // might be included in the file, and identical multiple static names
         // would clash.
 
-        hfile << "namespace isc {\n" <<
+        hfile <<
+            "namespace isc {\n" <<
             "namespace log {\n" <<
             "\n" <<
+            "// The next two objects are needed to bring the default message\n" <<
+            "// definitions into the program.  They make sure that the file\n" <<
+            "// containing the message text is included in the link process.\n" <<
+            "//\n" <<
+            "// The objects are uniquely named (with file name and date and\n" <<
+            "// time of compilation) to avoid clashes with other objects of\n" <<
+            "// the same type, either by another #include or as a global\n" <<
+            "// symbol in another module.\n" <<
+            "\n" <<
             "extern MessageInitializer " << mi_name << ";\n" <<
             "static MessageInstantiator instantiate_" << mi_name << "(\n" <<
             "   &" << mi_name << ");\n" <<
             "\n" <<
             "} // namespace log\n" <<
-            "} // namespace isc\n" <<
-            "\n";
+            "} // namespace isc\n";
 
 
         // ... and finally the postamble

src/lib/log/logger.cc

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

src/lib/log/logger.h

@@ -207,6 +207,18 @@ protected:
     static void reset();
 
 private:
+    /// \brief Copy Constructor
+    ///
+    /// Disabled (marked private) as it makes no sense to copy the logger -
+    /// just create another one of the same name.
+    Logger(const Logger&);
+
+    /// \brief Assignment Operator
+    ///
+    /// Disabled (marked private) as it makes no sense to copy the logger -
+    /// just create another one of the same name.
+    Logger& operator=(const Logger&);
+
     /// \brief Initialize Implementation
     ///
     /// Returns the logger pointer.  If not yet set, the underlying
@@ -220,7 +232,7 @@ private:
     /// will be initialized - we avoid this problem.
     ///
     /// \return Returns pointer to implementation
-    LoggerImpl* getLoggerptr() {
+    LoggerImpl* getLoggerPtr() {
         if (!loggerptr_) {
             initLoggerImpl();
         }

src/lib/log/logger_impl.cc

@@ -205,7 +205,9 @@ LoggerImpl::isDebugEnabled(int dbglevel) {
 // Output a general message
 
 void
-LoggerImpl::output(const char* sev_text, const string& message) {
+LoggerImpl::output(const char* sev_text, const MessageID& ident,
+    const char* text)
+{
     
     // Get the time in a struct tm format, and convert to text
     time_t t_time;
@@ -218,39 +220,7 @@ LoggerImpl::output(const char* sev_text, const string& message) {
 
     // Now output.
     std::cout << chr_time << " " << sev_text << " [" << getName() << "] " <<
-        message << "\n";
-}
-
-// Output various levels
-
-void
-LoggerImpl::debug(const MessageID& ident, const char* text) {
-    string message = boost::lexical_cast<string>(ident) + ", " + text;
-    output("DEBUG", message);
-}
-
-void
-LoggerImpl::info(const MessageID& ident, const char* text) {
-    string message = boost::lexical_cast<string>(ident) + ", " + text;
-    output("INFO ", message);
-}
-
-void
-LoggerImpl::warn(const MessageID& ident, const char* text) {
-    string message = boost::lexical_cast<string>(ident) + ", " + text;
-    output("WARN ", message);
-}
-
-void
-LoggerImpl::error(const MessageID& ident, const char* text) {
-    string message = boost::lexical_cast<string>(ident) + ", " + text;
-    output("ERROR", message);
-}
-
-void
-LoggerImpl::fatal(const MessageID& ident, const char* text) {
-    string message = boost::lexical_cast<string>(ident) + ", " + text;
-    output("FATAL", message);
+        ident << ", " << text << "\n";
 }
 
 } // namespace log

src/lib/log/logger_impl.h

@@ -183,45 +183,53 @@ public:
     /// The message is formatted to include the date and time, the severity
     /// and the logger generating the message.
     ///
-    /// \param sev_text Severity lovel as a text string
+    /// \param sev_text Severity level as a text string
+    /// \param ident Message identification
     /// \param text Text to log
-    void output(const char* sev_text, const std::string& message);
+    void output(const char* sev_text, const MessageID& ident,
+        const char* text);
 
 
     /// \brief Output Debug Message
     ///
     /// \param ident Message identification.
     /// \param text Text to log
-    void debug(const MessageID& ident, const char* text);
+    void debug(const MessageID& ident, const char* text) {
+        output("DEBUG", ident, text);
+    }
 
 
     /// \brief Output Informational Message
     ///
     /// \param ident Message identification.
     /// \param text Text to log
-    void info(const MessageID& ident, const char* text);
-
+    void info(const MessageID& ident, const char* text) {
+        output("INFO ", ident, text);
+    }
 
     /// \brief Output Warning Message
     ///
     /// \param ident Message identification.
     /// \param text Text to log
-    void warn(const MessageID& ident, const char* text);
-
+    void warn(const MessageID& ident, const char* text) {
+        output("WARN ", ident, text);
+    }
 
     /// \brief Output Error Message
     ///
     /// \param ident Message identification.
     /// \param text Text to log
-    void error(const MessageID& ident, const char* text);
-
+    void error(const MessageID& ident, const char* text) {
+        output("ERROR", ident, text);
+    }
 
     /// \brief Output Fatal Message
     ///
     /// \param ident Message identification.
     /// \param text Text to log
-    void fatal(const MessageID& ident, const char* text);
-
+    void fatal(const MessageID& ident, const char* text) {
+        output("FATAL", ident, text);
+    }
 
     /// \brief Equality
     ///

src/lib/log/message_initializer.h

@@ -74,7 +74,14 @@ public:
 /// that takes the extern reference to the MessageInitializer as its constructor
 /// argument.  The constructor - declared in another file - is a no-op.  But as
 /// the linker doesn't know, it must resolve the reference, hence pulling in the
-/// file containing the MessageInitializr.
+/// file containing the MessageInitializer.
+///\n
+/// Note that there is no problem about the static initialization fiasco here -
+/// a pointer to the MessageInitializer is passed to the MessageInstantiator,
+/// not the object itself; at the MessageInstantiator does nothing with the
+/// pointer and does not touch the MessageInitializer.  So it doesn't matter
+/// whether or not the MessageInitializer's constructor has been called when the
+/// MessageInstantiator runs.
 
 class MessageInstantiator {
 public:

src/lib/log/messagedef.cc

@@ -1,4 +1,4 @@
-// File created from messagedef.mes on Mon Feb  7 11:47:45 2011
+// File created from messagedef.mes on Tue Feb  8 18:01:54 2011
 
 #include <cstddef>
 #include <log/message_initializer.h>
@@ -29,7 +29,7 @@ const char* values[] = {
 namespace isc {
 namespace log {
 
-MessageInitializer messagedef_cc_Mon_Feb__7_11_47_45_2011(values);
+MessageInitializer messagedef_cc_Tue_Feb__8_18_01_54_2011(values);
 
 } // namespace log
 } // namespace isc

src/lib/log/messagedef.h

@@ -1,4 +1,4 @@
-// File created from messagedef.mes on Mon Feb  7 11:47:45 2011
+// File created from messagedef.mes on Tue Feb  8 18:01:54 2011
 
 #ifndef __MESSAGEDEF_H
 #define __MESSAGEDEF_H
@@ -31,11 +31,19 @@ static const isc::log::MessageID MSG_WRITERR = "WRITERR";
 namespace isc {
 namespace log {
 
-extern MessageInitializer messagedef_cc_Mon_Feb__7_11_47_45_2011;
-static MessageInstantiator instantiate_messagedef_cc_Mon_Feb__7_11_47_45_2011(
-   &messagedef_cc_Mon_Feb__7_11_47_45_2011);
+// The next two objects are needed to bring the default message
+// definitions into the program.  They make sure that the file
+// containing the message text is included in the link process.
+//
+// The objects are uniquely named (with file name and date and
+// time of compilation) to avoid clashes with other objects of
+// the same type, either by another #include or as a global
+// symbol in another module.
+
+extern MessageInitializer messagedef_cc_Tue_Feb__8_18_01_54_2011;
+static MessageInstantiator instantiate_messagedef_cc_Tue_Feb__8_18_01_54_2011(
+   &messagedef_cc_Tue_Feb__8_18_01_54_2011);
 
 } // namespace log
 } // namespace isc
-
 #endif // __MESSAGEDEF_H