Browse Source

[1698-test] Avoid use of heap storage for logger name

Logger now stores its name in a pre-defined array instead of
using heap storage (through a std::string).  This should avoid
static initialization fiasco problems if the Logger is statically
initialized.
Stephen Morris 13 years ago
parent
commit
21a0d06de0

+ 22 - 4
src/lib/log/logger.h

@@ -15,14 +15,17 @@
 #ifndef __LOGGER_H
 #define __LOGGER_H
 
+#include <cassert>
 #include <cstdlib>
 #include <string>
+#include <cstring>
 
 #include <exceptions/exceptions.h>
 #include <log/logger_level.h>
 #include <log/message_types.h>
 #include <log/log_formatter.h>
 
+
 namespace isc {
 namespace log {
 
@@ -99,6 +102,9 @@ public:
 
 class Logger {
 public:
+    enum {
+        MAX_LOGGER_NAME_SIZE = 32   ///< Maximum size of logger name
+    };
 
     /// \brief Constructor
     ///
@@ -107,8 +113,20 @@ public:
     /// \param name Name of the logger.  If the name is that of the root name,
     /// this creates an instance of the root logger; otherwise it creates a
     /// child of the root logger.
-    Logger(const std::string& name) : loggerptr_(NULL), name_(name)
-    {}
+    ///
+    /// \note The name of the logger may be no longer than MAX_LOGGER_NAME_SIZE
+    /// else the program will halt with an assertion failure.  This restriction
+    /// allows loggers to be declared statically: the name is stored in a fixed-
+    /// size array to avoid the need to allocate heap storage during program
+    /// initialization (which causes problems on some operating systems).
+    ///
+    /// \note Note also that there is no constructor taking a std::string. This
+    /// minimises the possibility of initializing a static logger with a string,
+    /// so leading to problems mentioned above.
+    Logger(const char* name) : loggerptr_(NULL) {
+        assert(std::strlen(name) < sizeof(name_));
+        std::strcpy(name_, name);
+    }
 
     /// \brief Destructor
     virtual ~Logger();
@@ -256,8 +274,8 @@ private:
     /// \brief Initialize Underlying Implementation and Set loggerptr_
     void initLoggerImpl();
 
-    LoggerImpl*     loggerptr_;     ///< Pointer to the underlying logger
-    std::string     name_;          ///< Copy of the logger name
+    LoggerImpl* loggerptr_;                  ///< Pointer to underlying logger
+    char        name_[MAX_LOGGER_NAME_SIZE]; ///< Copy of the logger name
 };
 
 } // namespace log

+ 1 - 1
src/lib/log/tests/logger_example.cc

@@ -105,7 +105,7 @@ void usage() {
 // messages.  Looking at the output determines whether the program worked.
 
 int main(int argc, char** argv) {
-    const string ROOT_NAME = "example";
+    const char* ROOT_NAME = "example";
 
     bool                sw_found = false;   // Set true if switch found
     bool                c_found = false;    // Set true if "-c" found

+ 4 - 4
src/lib/log/tests/logger_manager_unittest.cc

@@ -201,7 +201,7 @@ TEST_F(LoggerManagerTest, FileLogger) {
         // 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.
-        Logger logger(file_spec.getLoggerName());
+        Logger logger(file_spec.getLoggerName().c_str());
 
         LOG_FATAL(logger, LOG_DUPLICATE_MESSAGE_ID).arg("test");
         ids.push_back(LOG_DUPLICATE_MESSAGE_ID);
@@ -223,7 +223,7 @@ TEST_F(LoggerManagerTest, FileLogger) {
     manager.process(spec.begin(), spec.end());
 
     // Create a new instance of the logger and log three more messages.
-    Logger logger(file_spec.getLoggerName());
+    Logger logger(file_spec.getLoggerName().c_str());
 
     LOG_FATAL(logger, LOG_NO_SUCH_MESSAGE).arg("test");
     ids.push_back(LOG_NO_SUCH_MESSAGE);
@@ -275,7 +275,7 @@ TEST_F(LoggerManagerTest, FileSizeRollover) {
     // three files as for the log4cplus implementation, the files appear to
     // be rolled after the message is logged.
     {
-        Logger logger(file_spec.getLoggerName());
+        Logger logger(file_spec.getLoggerName().c_str());
         LOG_FATAL(logger, LOG_NO_SUCH_MESSAGE).arg(big_arg);
         LOG_FATAL(logger, LOG_DUPLICATE_NAMESPACE).arg(big_arg);
     }
@@ -295,7 +295,7 @@ TEST_F(LoggerManagerTest, FileSizeRollover) {
     // a .3 version does not exist.
     manager.process(spec);
     {
-        Logger logger(file_spec.getLoggerName());
+        Logger logger(file_spec.getLoggerName().c_str());
         LOG_FATAL(logger, LOG_NO_MESSAGE_TEXT).arg(big_arg);
     }
 

+ 2 - 2
src/lib/log/tests/logger_unittest.cc

@@ -58,8 +58,8 @@ TEST_F(LoggerTest, Name) {
 
 TEST_F(LoggerTest, GetLogger) {
 
-    const string name1 = "alpha";
-    const string name2 = "beta";
+    const char* name1 = "alpha";
+    const char* name2 = "beta";
 
     // Instantiate two loggers that should be the same
     Logger logger1(name1);