[#1266] fix possible memory leak

diff --git a/src/lib/log/compiler/message.cc b/src/lib/log/compiler/message.cc
index 8b7138813ac430e7dbe1dbe0a9e34eefc2de5d5b..93e9acd09893db57f6305a9ef5994997d8cd1448 100644 (file)
--- a/src/lib/log/compiler/message.cc
+++ b/src/lib/log/compiler/message.cc
@@ -560,7 +560,7 @@ main(int argc, char* argv[]) {
         vector<string> args(e.arguments());
         for (size_t i(0); i < args.size(); ++ i) {
             try {
-                replacePlaceholder(&text, args[i], i + 1);
+                replacePlaceholder(text, args[i], i + 1);
             } catch (...) {
                 // Error in error handling: nothing right to do...
             }

diff --git a/src/lib/log/log_formatter.cc b/src/lib/log/log_formatter.cc
index a794cca45c1249679039b7b63376c4951d4e9a9c..eb08d427f758127cee40e017bd16baa9fbc26d24 100644 (file)
--- a/src/lib/log/log_formatter.cc
+++ b/src/lib/log/log_formatter.cc
@@ -20,35 +20,35 @@ namespace isc {
 namespace log {
 
 void
-replacePlaceholder(string* message, const string& arg,
-                   const unsigned placeholder)
-{
+replacePlaceholder(std::string& message, const string& arg,
+                   const unsigned placeholder) {
     const string mark("%" + lexical_cast<string>(placeholder));
-    size_t pos(message->find(mark));
+    size_t pos(message.find(mark));
     if (pos != string::npos) {
         do {
-            message->replace(pos, mark.size(), arg);
-            pos = message->find(mark, pos + arg.size());
+            message.replace(pos, mark.size(), arg);
+            pos = message.find(mark, pos + arg.size());
         } while (pos != string::npos);
     }
 #ifdef ENABLE_LOGGER_CHECKS
     else {
         // We're missing the placeholder, so throw an exception
         isc_throw(MismatchedPlaceholders,
-                  "Missing logger placeholder in message: " << *message);
+                  "Missing logger placeholder in message: " << message);
     }
 #else
     else {
         // We're missing the placeholder, so add some complain
-        message->append(" @@Missing placeholder " + mark + " for '" + arg + "'@@");
+        message.append(" @@Missing placeholder " + mark + " for '" + arg + "'@@");
     }
 #endif /* ENABLE_LOGGER_CHECKS */
 }
 
 void
-checkExcessPlaceholders(string* message, unsigned int placeholder) {
+checkExcessPlaceholders(std::string& message,
+                        unsigned int placeholder) {
     const string mark("%" + lexical_cast<string>(placeholder));
-    const size_t pos(message->find(mark));
+    const size_t pos(message.find(mark));
     if (pos != string::npos) {
         // Excess placeholders were found.  If we enable the harsh check,
         // abort it.  Note: ideally we'd like to throw MismatchedPlaceholders,
@@ -57,13 +57,13 @@ checkExcessPlaceholders(string* message, unsigned int placeholder) {
 #ifdef ENABLE_LOGGER_CHECKS
         // Also, make sure we print the message so we can identify which
         // identifier has the problem.
-        cerr << "Message " << *message << endl;
+        cerr << "Message " << message << endl;
         assert("Excess logger placeholders still exist in message" == NULL);
 #else
-        message->append(" @@Excess logger placeholders still exist@@");
+        message.append(" @@Excess logger placeholders still exist@@");
 #endif /* ENABLE_LOGGER_CHECKS */
     }
 }
 
-}
-}
+} // namespace log
+} // namespace isc

diff --git a/src/lib/log/log_formatter.h b/src/lib/log/log_formatter.h
index baac6c3075a3edb615292c9d782e82382b5613eb..58bd2897d02d865cfa9c536087a70d5cc38a7c47 100644 (file)
--- a/src/lib/log/log_formatter.h
+++ b/src/lib/log/log_formatter.h
@@ -12,9 +12,12 @@
 #include <iostream>
 
 #include <exceptions/exceptions.h>
-#include <boost/lexical_cast.hpp>
 #include <log/logger_level.h>
 
+#include <boost/make_shared.hpp>
+#include <boost/shared_ptr.hpp>
+#include <boost/lexical_cast.hpp>
+
 namespace isc {
 namespace log {
 
@@ -50,7 +53,7 @@ public:
 /// This is used internally by the Formatter to check for excess
 /// placeholders (and fewer arguments).
 void
-checkExcessPlaceholders(std::string* message, unsigned int placeholder);
+checkExcessPlaceholders(std::string& message, unsigned int placeholder);
 
 ///
 /// \brief The internal replacement routine
@@ -59,7 +62,7 @@ checkExcessPlaceholders(std::string* message, unsigned int placeholder);
 /// in the message by replacement. If the placeholder is not found,
 /// it adds a complain at the end.
 void
-replacePlaceholder(std::string* message, const std::string& replacement,
+replacePlaceholder(std::string& message, const std::string& replacement,
                    const unsigned placeholder);
 
 ///
@@ -110,7 +113,7 @@ private:
     Severity severity_;
 
     /// \brief The messages with %1, %2... placeholders
-    std::string* message_;
+    boost::shared_ptr<std::string> message_;
 
     /// \brief Which will be the next placeholder to replace
     unsigned nextPlaceholder_;
@@ -131,11 +134,11 @@ public:
     ///     logger is also NULL, but it's not checked.
     /// \param logger The logger where the final output will go, or NULL
     ///     if no output is wanted.
-    Formatter(const Severity& severity = NONE, std::string* message = NULL,
+    Formatter(const Severity& severity = NONE,
+              boost::shared_ptr<std::string> message = boost::make_shared<std::string>(),
               Logger* logger = NULL) :
         logger_(logger), severity_(severity), message_(message),
-        nextPlaceholder_(0)
-    {
+        nextPlaceholder_(0) {
     }
 
     /// \brief Copy constructor
@@ -145,23 +148,21 @@ public:
     /// object being copied relinquishes that responsibility.
     Formatter(const Formatter& other) :
         logger_(other.logger_), severity_(other.severity_),
-        message_(other.message_), nextPlaceholder_(other.nextPlaceholder_)
-    {
+        message_(other.message_), nextPlaceholder_(other.nextPlaceholder_) {
         other.logger_ = NULL;
     }
 
     /// \brief Destructor.
     //
     /// This is the place where output happens if the formatter is active.
-    ~ Formatter() {
+    ~Formatter() {
         if (logger_) {
             try {
-                checkExcessPlaceholders(message_, ++nextPlaceholder_);
+                checkExcessPlaceholders(*message_, ++nextPlaceholder_);
                 logger_->output(severity_, *message_);
             } catch (...) {
                 // Catch and ignore all exceptions here.
             }
-            delete message_;
         }
     }
 
@@ -228,9 +229,8 @@ public:
             // .arg(42).arg("%1") would return "42 %1" - there are no recursive
             // replacements).
             try {
-                replacePlaceholder(message_, arg, ++nextPlaceholder_ );
-            }
-            catch (...) {
+                replacePlaceholder(*message_, arg, ++nextPlaceholder_ );
+            } catch (...) {
                 // Something went wrong here, the log message is broken, so
                 // we don't want to output it, nor we want to check all the
                 // placeholders were used (because they won't be).
@@ -251,14 +251,13 @@ public:
     /// the arguments for the message.
     void deactivate() {
         if (logger_) {
-            delete message_;
-            message_ = NULL;
+            message_.reset();
             logger_ = NULL;
         }
     }
 };
 
-}
-}
+} // namespace log
+} // namespace isc
 
 #endif

diff --git a/src/lib/log/logger_impl.cc b/src/lib/log/logger_impl.cc
index 8f9e1eaa165ae636de33f15513eb44e8764087cf..7b94a25be7252877d629b591f4577631942d155a 100644 (file)
--- a/src/lib/log/logger_impl.cc
+++ b/src/lib/log/logger_impl.cc
@@ -6,14 +6,16 @@
 
 #include <config.h>
 
-#include <iostream>
-#include <iomanip>
-#include <algorithm>
 
+#include <algorithm>
+#include <cstring>
+#include <iomanip>
+#include <iostream>
 #include <stdarg.h>
 #include <stdio.h>
-#include <cstring>
 #include <sstream>
+
+#include <boost/make_shared.hpp>
 #include <boost/lexical_cast.hpp>
 #include <boost/static_assert.hpp>
 #include <boost/algorithm/string.hpp>
@@ -128,17 +130,16 @@ LoggerImpl::getEffectiveDebugLevel() {
 
 
 // Output a general message
-string*
+boost::shared_ptr<string>
 LoggerImpl::lookupMessage(const MessageID& ident) {
-    return (new string(string(ident) + " " +
-                       MessageDictionary::globalDictionary()->getText(ident)));
+    return (boost::make_shared<string>(string(ident) + " " +
+        MessageDictionary::globalDictionary()->getText(ident)));
 }
 
 // Replace the interprocess synchronization object
 
 void
-LoggerImpl::setInterprocessSync(isc::log::interprocess::InterprocessSync* sync)
-{
+LoggerImpl::setInterprocessSync(interprocess::InterprocessSync* sync) {
     if (sync == NULL) {
         isc_throw(BadInterprocessSync,
                   "NULL was passed to setInterprocessSync()");

diff --git a/src/lib/log/logger_impl.h b/src/lib/log/logger_impl.h
index 39a6f1304b339444448d2a715be40d7df3f25679..7f96aa95a154d6bc0fb45122a31a01be53460b64 100644 (file)
--- a/src/lib/log/logger_impl.h
+++ b/src/lib/log/logger_impl.h
@@ -165,7 +165,7 @@ public:
     /// \brief Look up message text in dictionary
     ///
     /// This gets you the unformatted text of message for given ID.
-    std::string* lookupMessage(const MessageID& id);
+    boost::shared_ptr<std::string> lookupMessage(const MessageID& id);
 
     /// \brief Replace the interprocess synchronization object
     ///

diff --git a/src/lib/log/tests/log_formatter_unittest.cc b/src/lib/log/tests/log_formatter_unittest.cc
index 21a98faa2aae62ac2092f115a2326c57f8ca3f24..205050123f0e685a94f63b3ea42707c10477c21b 100644 (file)
--- a/src/lib/log/tests/log_formatter_unittest.cc
+++ b/src/lib/log/tests/log_formatter_unittest.cc
@@ -30,8 +30,8 @@ public:
         outputs.push_back(Output(prefix, message));
     }
     // Just shortcut for new string
-    string* s(const char* text) {
-        return (new string(text));
+    boost::shared_ptr<string> s(const char* text) {
+        return (boost::make_shared<string>(text));
     }
 };