[trac558] Make Set/Get Root Logger Name a pair of functions

Adopted suggestion by jinmei to remove the RootLoggerName class and
make them a pair of functions.  Also optimised use of the root name
in the logger implementation class.

src/lib/log/logger.cc

@@ -110,7 +110,9 @@ Logger::isFatalEnabled() {
 // definition of the macro).  Also note that it expects that the message buffer
 // "message" is declared in the compilation unit.
 
-#define MESSAGE_SIZE (512)
+namespace {
+const size_t MESSAGE_SIZE = 512;
+}
 
 #define FORMAT_MESSAGE(message) \
     { \

src/lib/log/logger_impl.cc

@@ -38,22 +38,22 @@ namespace log {
 LoggerImpl::LoggerInfoMap LoggerImpl::logger_info_;
 LoggerImpl::LoggerInfo LoggerImpl::root_logger_info_(isc::log::INFO, 0);
 
-// Destructor. (Here because of virtual declaration.)
-
-LoggerImpl::~LoggerImpl() {
+// Constructor
+LoggerImpl::LoggerImpl(const std::string& name, bool)
+{
+    // Are we the root logger?
+    if (name == getRootLoggerName()) { 
+        is_root_ = true;
+        name_ = name;
+    } else {
+        is_root_ = false;
+        name_ = getRootLoggerName() + "." + name;
+    }
 }
 
-// Return the name of the logger (fully-qualified with the root name if
-// possible).
+// Destructor. (Here because of virtual declaration.)
 
-string LoggerImpl::getName() {
-    string root_name = RootLoggerName::getName();
-    if (root_name.empty() || (name_ == root_name)) {
-        return (name_);
-    }
-    else {
-        return (root_name + "." + name_);
-    }
+LoggerImpl::~LoggerImpl() {
 }
 
 // Set the severity for logging.
@@ -64,7 +64,7 @@ LoggerImpl::setSeverity(isc::log::Severity severity, int dbglevel) {
     // Silently coerce the debug level into the valid range of 0 to 99
 
     int debug_level = max(MIN_DEBUG_LEVEL, min(MAX_DEBUG_LEVEL, dbglevel));
-    if (isRootLogger()) {
+    if (is_root_) {
 
         // Can only set severity for the root logger, you can't disable it.
         // Any attempt to do so is silently ignored.
@@ -93,7 +93,7 @@ LoggerImpl::setSeverity(isc::log::Severity severity, int dbglevel) {
 isc::log::Severity
 LoggerImpl::getSeverity() {
 
-    if (isRootLogger()) {
+    if (is_root_) {
         return (root_logger_info_.severity);
     }
     else {
@@ -112,9 +112,11 @@ LoggerImpl::getSeverity() {
 
 isc::log::Severity
 LoggerImpl::getEffectiveSeverity() {
-    if (! isRootLogger()) {
 
-        // Not root logger, look this logger up in the map.
+    if (!is_root_ && !logger_info_.empty()) {
+
+        // Not root logger and there is at least one item in the info map for a
+        // logger.
         LoggerInfoMap::iterator i = logger_info_.find(name_);
         if (i != logger_info_.end()) {
 
@@ -133,9 +135,10 @@ LoggerImpl::getEffectiveSeverity() {
 int
 LoggerImpl::getDebugLevel() {
 
-    if (! isRootLogger()) {
+    if (!is_root_ && !logger_info_.empty()) {
 
-        // Not root logger, look this logger up in the map.
+        // Not root logger and there is something in the map, check if there
+        // is a setting for this one.
         LoggerInfoMap::iterator i = logger_info_.find(name_);
         if (i != logger_info_.end()) {
 
@@ -164,21 +167,10 @@ LoggerImpl::getDebugLevel() {
 bool
 LoggerImpl::isDebugEnabled(int dbglevel) {
 
-    if (logger_info_.empty()) {
-
-        // Nothing set, return information from the root logger.
-        return ((root_logger_info_.severity <= isc::log::DEBUG) &&
-            (root_logger_info_.dbglevel >= dbglevel));
-    }
-
-    // Something is in the general logger map, so we need to look up the
-    // information.  We don't use getSeverity() and getDebugLevel() separately
-    // as each involves a lookup in the map, something that we can optimise
-    // here.
-
-    if (! isRootLogger()) {
+    if (!is_root_ && !logger_info_.empty()) {
 
-        // Not root logger, look this logger up in the map.
+        // Not root logger and there is something in the map, check if there
+        // is a setting for this one.
         LoggerInfoMap::iterator i = logger_info_.find(name_);
         if (i != logger_info_.end()) {
 
@@ -194,9 +186,9 @@ LoggerImpl::isDebugEnabled(int dbglevel) {
     // Must be the root logger, or this logger is defaulting to the root logger
     // settings.
     if (root_logger_info_.severity <= isc::log::DEBUG) {
-        return (root_logger_info_.dbglevel > dbglevel);
+        return (root_logger_info_.dbglevel >= dbglevel);
     } else {
-        return (false);
+       return (false);
     }
 }
 

src/lib/log/logger_impl.h

@@ -74,8 +74,7 @@ public:
     ///
     /// \param exit_delete This argument is present to get round a bug in
     /// the log4cxx implementation.  It is unused here.
-    LoggerImpl(const std::string& name, bool) : name_(name)
-    {}
+    LoggerImpl(const std::string& name, bool);
 
 
     /// \brief Destructor
@@ -83,15 +82,8 @@ public:
 
 
     /// \brief Get the full name of the logger (including the root name)
-    virtual std::string getName();
-
-
-    /// \brief Check if this is the root logger
-    ///
-    /// \return true if the name of this logger is the same as that of the
-    /// root logger.
-    virtual bool isRootLogger() const {
-        return (name_ == RootLoggerName::getName());
+    virtual std::string getName() {
+        return (name_);
     }
 
 
@@ -251,7 +243,8 @@ public:
 
 
 private:
-    std::string          name_;                 ///< Name of this logger
+    bool                is_root_;           ///< true if a root logger
+    std::string         name_;              ///< Name of this logger
 
     // Split the status of the root logger from this logger.  If - is will
     // probably be the usual case - no per-logger setting is enabled, a

src/lib/log/logger_support.cc

@@ -98,7 +98,7 @@ runTimeInit(isc::log::Severity severity, int dbglevel, const char* file) {
     //
     // 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());
+    Logger logger(isc::log::getRootLoggerName());
 
     // Set the severity associated with it.  If no other logger has a severity,
     // this will be the default.

src/lib/log/root_logger_name.cc

@@ -18,10 +18,17 @@
 namespace isc {
 namespace log {
 
-std::string& RootLoggerName::rootName() {
-    static std::string root_name("");
-    return (root_name);
+static std::string root_name_;
+
+void
+setRootLoggerName(const std::string& name) {
+    root_name_ = name;
+}
+
+const std::string& getRootLoggerName() {
+    return root_name_;
 }
 
+
 }
 }

src/lib/log/root_logger_name.h

@@ -27,45 +27,13 @@
 namespace isc {
 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
-    /// name.
-    static void setName(const std::string& name) {
-        rootName() = name;
-    }
+/// \brief Set Root Logger Name
+///
+/// \param name Name of the root logger.  This should be the program
+/// name.
 
-    /// \brief Get Root Logger Name
-    ///
-    /// \return Name of the root logger.
-    static std::string getName() {
-        return (rootName());
-    }
-    
-private:
-    /// \brief Store Root Logger Name
-    ///
-    /// Access the singleton root logger name.  This is stored as a static
-    /// variable inside a method to avoid the "static initialization fiasco";
-    /// when accessed by another class during initialization, the name will be
-    /// instantiated. (Else it will be only by chance that it is instantiated
-    /// before the cassling class.)
-    ///
-    /// \return Name addisnged to the root logger.
-    static std::string& rootName();
-};
+void setRootLoggerName(const std::string& name);
+const std::string& getRootLoggerName();
 
 }
 }

src/lib/log/tests/logger_support_test.cc

@@ -32,7 +32,6 @@
 using namespace isc::log;
 
 // Declare root logger and a loggers to use an example.
-RootLoggerName root("alpha");
 Logger logger_ex("example");
 Logger logger_dlm("dlm");
 
@@ -54,6 +53,8 @@ int main(int argc, char** argv) {
     const char*         localfile = NULL;
     int                 option;
 
+    isc::log::setRootLoggerName("alpha");
+
     // Parse options
     while ((option = getopt(argc, argv, "s:d:")) != -1) {
         switch (option) {

src/lib/log/tests/logger_unittest.cc

@@ -65,7 +65,7 @@ protected:
 TEST_F(LoggerTest, Name) {
 
     // Create a logger
-    RootLoggerName::setName("test1");
+    setRootLoggerName("test1");
     Logger logger("alpha");
 
     // ... and check the name
@@ -79,7 +79,7 @@ TEST_F(LoggerTest, GetLogger) {
 
     // Set the root logger name (not strictly needed, but this will be the
     // case in the program(.
-    RootLoggerName::setName("test2");
+    setRootLoggerName("test2");
 
     const string name1 = "alpha";
     const string name2 = "beta";
@@ -101,7 +101,7 @@ TEST_F(LoggerTest, GetLogger) {
 TEST_F(LoggerTest, Severity) {
 
     // Create a logger
-    RootLoggerName::setName("test3");
+    setRootLoggerName("test3");
     TestLogger logger("alpha");
 
     // Now check the levels
@@ -132,7 +132,7 @@ TEST_F(LoggerTest, Severity) {
 TEST_F(LoggerTest, DebugLevels) {
 
     // Create a logger
-    RootLoggerName::setName("test4");
+    setRootLoggerName("test4");
     TestLogger logger("alpha");
 
     // Debug level should be 0 if not at debug severity
@@ -178,7 +178,7 @@ TEST_F(LoggerTest, SeverityInheritance) {
     // implementation (in this case log4cxx) will set a parent-child
     // relationship if the loggers are named <parent> and <parent>.<child>.
 
-    RootLoggerName::setName("test5");
+    setRootLoggerName("test5");
     TestLogger parent("alpha");
     TestLogger child("alpha.beta");
 
@@ -210,7 +210,7 @@ TEST_F(LoggerTest, EffectiveSeverityInheritance) {
     // implementation (in this case log4cxx) will set a parent-child
     // relationship if the loggers are named <parent> and <parent>.<child>.
 
-    RootLoggerName::setName("test6");
+    setRootLoggerName("test6");
     Logger parent("test6");
     Logger child("test6.beta");
 
@@ -245,7 +245,7 @@ TEST_F(LoggerTest, EffectiveSeverityInheritance) {
 
 TEST_F(LoggerTest, IsXxxEnabled) {
 
-    RootLoggerName::setName("test7");
+    setRootLoggerName("test7");
     Logger logger("test7");
 
     logger.setSeverity(isc::log::INFO);
@@ -316,7 +316,7 @@ TEST_F(LoggerTest, IsXxxEnabled) {
 
 TEST_F(LoggerTest, IsDebugEnabledLevel) {
 
-    RootLoggerName::setName("test8");
+    setRootLoggerName("test8");
     Logger logger("test8");
 
     int MID_LEVEL = (MIN_DEBUG_LEVEL + MAX_DEBUG_LEVEL) / 2;

src/lib/log/tests/root_logger_name_unittest.cc

@@ -35,8 +35,8 @@ TEST_F(RootLoggerNameTest, SetGet) {
     const std::string name2 = "test2";
 
     // Check that Set/Get works
-    RootLoggerName::setName(name1);
-    EXPECT_EQ(name1, RootLoggerName::getName());
+    setRootLoggerName(name1);
+    EXPECT_EQ(name1, getRootLoggerName());
 
     // We could not test that the root logger name is initialised
     // correctly (as there is one instance of it and we don't know
@@ -45,6 +45,6 @@ TEST_F(RootLoggerNameTest, SetGet) {
     //
     // (There was always the outside chance that the root logger name
     // was initialised with name1 and that setName() has no effect.)
-    RootLoggerName::setName(name2);
-    EXPECT_EQ(name2, RootLoggerName::getName());
+    setRootLoggerName(name2);
+    EXPECT_EQ(name2, getRootLoggerName());
 }