BIND 10 trac901, updated. a23491a635d7f74132fc0b91eac832275f8f1f87 [trac901] Replacing in the formatter
BIND 10 source code commits
bind10-changes at lists.isc.org
Thu May 5 09:53:40 UTC 2011
The branch, trac901 has been updated
via a23491a635d7f74132fc0b91eac832275f8f1f87 (commit)
from db0ca2fbc64b390f261b5e36938c736321b171f9 (commit)
Those revisions listed above that are new to this repository have
not appeared on any other notification email; so we list those
revisions in full, below.
- Log -----------------------------------------------------------------
commit a23491a635d7f74132fc0b91eac832275f8f1f87
Author: Michal 'vorner' Vaner <michal.vaner at nic.cz>
Date: Thu May 5 11:53:28 2011 +0200
[trac901] Replacing in the formatter
-----------------------------------------------------------------------
Summary of changes:
src/lib/log/Makefile.am | 2 +-
src/lib/log/log_formatter.h | 49 +++++++++++++++++++++++----
src/lib/log/tests/log_formatter_unittest.cc | 18 ++++++----
3 files changed, 54 insertions(+), 15 deletions(-)
-----------------------------------------------------------------------
diff --git a/src/lib/log/Makefile.am b/src/lib/log/Makefile.am
index 36a508b..53d320e 100644
--- a/src/lib/log/Makefile.am
+++ b/src/lib/log/Makefile.am
@@ -21,7 +21,7 @@ liblog_la_SOURCES += message_initializer.cc message_initializer.h
liblog_la_SOURCES += message_reader.cc message_reader.h
liblog_la_SOURCES += message_types.h
liblog_la_SOURCES += root_logger_name.cc root_logger_name.h
-liblog_la_SOURCES += log_formatter.h
+liblog_la_SOURCES += log_formatter.h log_formatter.cc
EXTRA_DIST = README
EXTRA_DIST += messagedef.mes
diff --git a/src/lib/log/log_formatter.h b/src/lib/log/log_formatter.h
index 6f3af2a..e7f5128 100644
--- a/src/lib/log/log_formatter.h
+++ b/src/lib/log/log_formatter.h
@@ -16,10 +16,19 @@
#define __LOG_FORMMATER_H
#include <string>
+#include <boost/lexical_cast.hpp>
namespace isc {
namespace log {
+/// \brief The internal replacement routine
+///
+/// This is used internally by the Formatter. Replaces a 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,
+ const unsigned placeholder);
+
///
/// \brief The log message formatter
///
@@ -59,7 +68,7 @@ private:
/// \brief Prefix (eg. "ERROR", "DEBUG" or like that)
const char* prefix_;
/// \brief The messages with %1, %2... placeholders
- const std::string message_;
+ std::string* message_;
/// \brief Which will be the next placeholder to replace
const unsigned nextPlaceholder_;
/// \brief Should we do output?
@@ -72,13 +81,17 @@ public:
/// This will create a formatter in active mode -- the one when it
/// will generate output.
///
+ /// It is not expected to be called by user of logging system directly.
+ ///
/// \param prefix The prefix, like "ERROR" or "DEBUG"
- /// \param message The message with placeholders
+ /// \param message The message with placeholders. We take ownership of
+ /// it and we will modify the string. Must not be NULL and it's
+ /// not checked.
/// \param nextPlaceholder It is the number of next placeholder which
/// should be replaced. It should be called with 1, higher numbers
/// are used internally in the chain.
/// \param logger The logger where the final output will go.
- Formatter(const char* prefix, const std::string& message,
+ Formatter(const char* prefix, std::string* message,
const unsigned nextPlaceholder, Logger& logger) :
logger_(&logger), prefix_(prefix), message_(message),
nextPlaceholder_(nextPlaceholder), active_(true)
@@ -86,8 +99,11 @@ public:
}
/// \brief Constructor of "inactive" formatter
///
+ /// It is not expected to be called by user of the logging system directly.
+ ///
/// This will create a formatter that produces no output.
Formatter() :
+ message_(NULL),
nextPlaceholder_(0),
active_(false)
{
@@ -97,8 +113,9 @@ public:
/// This is the place where output happens if the formatter is active.
~ Formatter() {
if (active_) {
- logger_->output(prefix_, message_);
+ logger_->output(prefix_, *message_);
}
+ delete message_;
}
/// \brief Replaces another placeholder
///
@@ -107,11 +124,29 @@ public:
/// only produces another inactive formatter.
///
/// \param arg The argument to place into the placeholder.
- template<class Arg> Formatter arg(const Arg&) {
+ template<class Arg> Formatter arg(const Arg& value) {
+ return (arg(boost::lexical_cast<std::string>(value)));
+ }
+ /// \brief String version of arg.
+ Formatter arg(const std::string& arg) {
if (active_) {
+ // FIXME: This logic has a problem. If we had a message like
+ // "%1 %2" and called .arg("%2").arg(42), we would get "42 %2".
+ // But we consider this to be rare enough not to complicate
+ // matters.
active_ = false;
- return (Formatter<Logger>(prefix_, message_, nextPlaceholder_ + 1,
- *logger_));
+ replacePlaceholder(message_, arg, nextPlaceholder_);
+ std::string *message(message_);
+ message_ = NULL;
+ try {
+ return (Formatter<Logger>(prefix_, message,
+ nextPlaceholder_ + 1, *logger_));
+ }
+ // Make sure the memory is not leaked on stuff like bad_alloc
+ catch (...) {
+ delete message;
+ throw;
+ }
} else {
return (Formatter<Logger>());
}
diff --git a/src/lib/log/tests/log_formatter_unittest.cc b/src/lib/log/tests/log_formatter_unittest.cc
index efc8bea..59157db 100644
--- a/src/lib/log/tests/log_formatter_unittest.cc
+++ b/src/lib/log/tests/log_formatter_unittest.cc
@@ -31,6 +31,10 @@ public:
void output(const char* prefix, const string& message) {
outputs.push_back(Output(prefix, message));
}
+ // Just shortcut for new string
+ string* s(const char* text) {
+ return (new string(text));
+ }
};
// Create an inactive formatter and check it doesn't produce any output
@@ -42,7 +46,7 @@ TEST_F(FormatterTest, inactive) {
// Create an active formatter and check it produces output. Does no arg
// substitution yet
TEST_F(FormatterTest, active) {
- Formatter("TEST", "Text of message", 1, *this);
+ Formatter("TEST", s("Text of message"), 1, *this);
ASSERT_LE(1, outputs.size());
EXPECT_EQ(1, outputs.size());
EXPECT_STREQ("TEST", outputs[0].first);
@@ -59,7 +63,7 @@ TEST_F(FormatterTest, inactiveArg) {
TEST_F(FormatterTest, stringArg) {
{
SCOPED_TRACE("C++ string");
- Formatter("TEST", "Hello %1", 1, *this).arg(string("World"));
+ Formatter("TEST", s("Hello %1"), 1, *this).arg(string("World"));
ASSERT_LE(1, outputs.size());
EXPECT_EQ(1, outputs.size());
EXPECT_STREQ("TEST", outputs[0].first);
@@ -67,7 +71,7 @@ TEST_F(FormatterTest, stringArg) {
}
{
SCOPED_TRACE("C++ string");
- Formatter("TEST", "Hello %1", 1, *this).arg(string("Internet"));
+ Formatter("TEST", s("Hello %1"), 1, *this).arg(string("Internet"));
ASSERT_LE(2, outputs.size());
EXPECT_EQ(2, outputs.size());
EXPECT_STREQ("TEST", outputs[1].first);
@@ -77,7 +81,7 @@ TEST_F(FormatterTest, stringArg) {
// Can convert to string
TEST_F(FormatterTest, intArg) {
- Formatter("TEST", "The answer is %1", 1, *this).arg(42);
+ Formatter("TEST", s("The answer is %1"), 1, *this).arg(42);
ASSERT_LE(1, outputs.size());
EXPECT_EQ(1, outputs.size());
EXPECT_STREQ("TEST", outputs[0].first);
@@ -86,7 +90,7 @@ TEST_F(FormatterTest, intArg) {
// Can use multiple arguments at different places
TEST_F(FormatterTest, multiArg) {
- Formatter("TEST", "The %2 are %1", 1, *this).arg("switched").
+ Formatter("TEST", s("The %2 are %1"), 1, *this).arg("switched").
arg("arguments");
ASSERT_LE(1, outputs.size());
EXPECT_EQ(1, outputs.size());
@@ -96,8 +100,8 @@ TEST_F(FormatterTest, multiArg) {
// Can survive and complains if placeholder is missing
TEST_F(FormatterTest, missingPlace) {
- Formatter("TEST", "Missing the first %2", 1, *this).arg("missing").
- arg("argument");
+ EXPECT_NO_THROW(Formatter("TEST", s("Missing the first %2"), 1, *this).
+ arg("missing").arg("argument"));
ASSERT_LE(1, outputs.size());
EXPECT_EQ(1, outputs.size());
EXPECT_STREQ("TEST", outputs[0].first);
More information about the bind10-changes
mailing list