From: Razvan Becheriu Date: Wed, 3 Jun 2020 09:43:50 +0000 (+0300) Subject: [#1266] fix possible memory leak X-Git-Tag: Kea-1.7.9~82 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=9715a225c506eee9606bf9670d9d6c5c508bf117;p=thirdparty%2Fkea.git [#1266] fix possible memory leak --- diff --git a/src/lib/log/compiler/message.cc b/src/lib/log/compiler/message.cc index 8b7138813a..93e9acd098 100644 --- a/src/lib/log/compiler/message.cc +++ b/src/lib/log/compiler/message.cc @@ -560,7 +560,7 @@ main(int argc, char* argv[]) { vector 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 a794cca45c..eb08d427f7 100644 --- 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(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(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 baac6c3075..58bd2897d0 100644 --- a/src/lib/log/log_formatter.h +++ b/src/lib/log/log_formatter.h @@ -12,9 +12,12 @@ #include #include -#include #include +#include +#include +#include + 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 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 message = boost::make_shared(), 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 8f9e1eaa16..7b94a25be7 100644 --- a/src/lib/log/logger_impl.cc +++ b/src/lib/log/logger_impl.cc @@ -6,14 +6,16 @@ #include -#include -#include -#include +#include +#include +#include +#include #include #include -#include #include + +#include #include #include #include @@ -128,17 +130,16 @@ LoggerImpl::getEffectiveDebugLevel() { // Output a general message -string* +boost::shared_ptr LoggerImpl::lookupMessage(const MessageID& ident) { - return (new string(string(ident) + " " + - MessageDictionary::globalDictionary()->getText(ident))); + return (boost::make_shared(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 39a6f1304b..7f96aa95a1 100644 --- 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 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 21a98faa2a..205050123f 100644 --- 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 s(const char* text) { + return (boost::make_shared(text)); } };