BIND 10 trac901, updated. 9eb7b41d5253c44e59987170a74e345b651c96bc [trac901] Code size reduction

BIND 10 source code commits bind10-changes at lists.isc.org
Mon May 9 09:37:26 UTC 2011


The branch, trac901 has been updated
       via  9eb7b41d5253c44e59987170a74e345b651c96bc (commit)
      from  4ff5a9f61d9ad1c96468556771562f1dbad7ec98 (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 9eb7b41d5253c44e59987170a74e345b651c96bc
Author: Michal 'vorner' Vaner <michal.vaner at nic.cz>
Date:   Mon May 9 11:36:26 2011 +0200

    [trac901] Code size reduction

-----------------------------------------------------------------------

Summary of changes:
 src/lib/log/log_formatter.h                 |   53 ++++++++++-----------------
 src/lib/log/logger.cc                       |   20 +++++-----
 src/lib/log/tests/log_formatter_unittest.cc |   16 ++++----
 3 files changed, 38 insertions(+), 51 deletions(-)

-----------------------------------------------------------------------
diff --git a/src/lib/log/log_formatter.h b/src/lib/log/log_formatter.h
index bc4340c..07a024c 100644
--- a/src/lib/log/log_formatter.h
+++ b/src/lib/log/log_formatter.h
@@ -69,63 +69,50 @@ replacePlaceholder(std::string* message, const std::string& replacement,
 /// again. So, think of this behaviour as soul moving from one to another.
 template<class Logger> class Formatter {
 private:
-    /// \brief The logger we will use to output the final message
-    Logger* logger_;
+    /// \brief The logger we will use to output the final message.
+    ///
+    /// If NULL, we are not active and should not produce anything.
+    mutable Logger* logger_;
     /// \brief Prefix (eg. "ERROR", "DEBUG" or like that)
     const char* prefix_;
     /// \brief The messages with %1, %2... placeholders
     std::string* message_;
     /// \brief Which will be the next placeholder to replace
     unsigned nextPlaceholder_;
-    /// \brief Should we do output?
-    mutable bool active_;
     Formatter& operator =(const Formatter& other);
 public:
     /// \brief Constructor of "active" formatter
     ///
-    /// This will create a formatter in active mode -- the one when it
-    /// will generate output.
+    /// This will create a formatter. If the arguments are set, it
+    /// will be active (will produce output). If you leave them all as NULL,
+    /// it will create an inactive Formatter -- one that'll produce no output.
     ///
     /// It is not expected to be called by user of logging system directly.
     ///
-    /// \param prefix The prefix, like "ERROR" or "DEBUG"
+    /// \param prefix The severity prefix, like "ERROR" or "DEBUG"
     /// \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, std::string* message,
-              const unsigned nextPlaceholder, Logger& logger) :
-        logger_(&logger), prefix_(prefix), message_(message),
-        nextPlaceholder_(nextPlaceholder), active_(true)
-    {
-    }
-    /// \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)
+    ///     it and we will modify the string. Must not be NULL unless
+    ///     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 char* prefix = NULL, std::string* message = NULL,
+              Logger* logger = NULL) :
+        logger_(logger), prefix_(prefix), message_(message),
+        nextPlaceholder_(1)
     {
     }
 
     Formatter(const Formatter& other) :
         logger_(other.logger_), prefix_(other.prefix_),
-        message_(other.message_), nextPlaceholder_(other.nextPlaceholder_),
-        active_(other.active_)
+        message_(other.message_), nextPlaceholder_(other.nextPlaceholder_)
     {
-        other.active_ = false;
+        other.logger_ = false;
     }
     /// \brief Destructor.
     //
     /// This is the place where output happens if the formatter is active.
     ~ Formatter() {
-        if (active_) {
+        if (logger_) {
             logger_->output(prefix_, *message_);
             delete message_;
         }
@@ -142,7 +129,7 @@ public:
     }
     /// \brief String version of arg.
     Formatter& arg(const std::string& arg) {
-        if (active_) {
+        if (logger_) {
             // 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
diff --git a/src/lib/log/logger.cc b/src/lib/log/logger.cc
index 3131c0b..c340315 100644
--- a/src/lib/log/logger.cc
+++ b/src/lib/log/logger.cc
@@ -119,8 +119,8 @@ Logger::output(const char* sevText, const string& message) {
 Logger::Formatter
 Logger::debug(int dbglevel, const isc::log::MessageID& ident) {
     if (isDebugEnabled(dbglevel)) {
-        return (Formatter("DEBUG", getLoggerPtr()->lookupMessage(ident), 1,
-                          *this));
+        return (Formatter("DEBUG", getLoggerPtr()->lookupMessage(ident),
+                          this));
     } else {
         return (Formatter());
     }
@@ -129,8 +129,8 @@ Logger::debug(int dbglevel, const isc::log::MessageID& ident) {
 Logger::Formatter
 Logger::info(const isc::log::MessageID& ident) {
     if (isInfoEnabled()) {
-        return (Formatter("INFO ", getLoggerPtr()->lookupMessage(ident), 1,
-                          *this));
+        return (Formatter("INFO ", getLoggerPtr()->lookupMessage(ident),
+                          this));
     } else {
         return (Formatter());
     }
@@ -139,8 +139,8 @@ Logger::info(const isc::log::MessageID& ident) {
 Logger::Formatter
 Logger::warn(const isc::log::MessageID& ident) {
     if (isWarnEnabled()) {
-        return (Formatter("WARN ", getLoggerPtr()->lookupMessage(ident), 1,
-                          *this));
+        return (Formatter("WARN ", getLoggerPtr()->lookupMessage(ident),
+                          this));
     } else {
         return (Formatter());
     }
@@ -149,8 +149,8 @@ Logger::warn(const isc::log::MessageID& ident) {
 Logger::Formatter
 Logger::error(const isc::log::MessageID& ident) {
     if (isErrorEnabled()) {
-        return (Formatter("ERROR", getLoggerPtr()->lookupMessage(ident), 1,
-                          *this));
+        return (Formatter("ERROR", getLoggerPtr()->lookupMessage(ident),
+                          this));
     } else {
         return (Formatter());
     }
@@ -159,8 +159,8 @@ Logger::error(const isc::log::MessageID& ident) {
 Logger::Formatter
 Logger::fatal(const isc::log::MessageID& ident) {
     if (isFatalEnabled()) {
-        return (Formatter("FATAL", getLoggerPtr()->lookupMessage(ident), 1,
-                          *this));
+        return (Formatter("FATAL", getLoggerPtr()->lookupMessage(ident),
+                          this));
     } else {
         return (Formatter());
     }
diff --git a/src/lib/log/tests/log_formatter_unittest.cc b/src/lib/log/tests/log_formatter_unittest.cc
index 351bf24..b67831a 100644
--- a/src/lib/log/tests/log_formatter_unittest.cc
+++ b/src/lib/log/tests/log_formatter_unittest.cc
@@ -46,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", s("Text of message"), 1, *this);
+    Formatter("TEST", s("Text of message"), this);
     ASSERT_EQ(1, outputs.size());
     EXPECT_STREQ("TEST", outputs[0].first);
     EXPECT_EQ("Text of message", outputs[0].second);
@@ -62,14 +62,14 @@ TEST_F(FormatterTest, inactiveArg) {
 TEST_F(FormatterTest, stringArg) {
     {
         SCOPED_TRACE("C++ string");
-        Formatter("TEST", s("Hello %1"), 1, *this).arg(string("World"));
+        Formatter("TEST", s("Hello %1"), this).arg(string("World"));
         ASSERT_EQ(1, outputs.size());
         EXPECT_STREQ("TEST", outputs[0].first);
         EXPECT_EQ("Hello World", outputs[0].second);
     }
     {
         SCOPED_TRACE("C++ string");
-        Formatter("TEST", s("Hello %1"), 1, *this).arg(string("Internet"));
+        Formatter("TEST", s("Hello %1"), this).arg(string("Internet"));
         ASSERT_EQ(2, outputs.size());
         EXPECT_STREQ("TEST", outputs[1].first);
         EXPECT_EQ("Hello Internet", outputs[1].second);
@@ -78,7 +78,7 @@ TEST_F(FormatterTest, stringArg) {
 
 // Can convert to string
 TEST_F(FormatterTest, intArg) {
-    Formatter("TEST", s("The answer is %1"), 1, *this).arg(42);
+    Formatter("TEST", s("The answer is %1"), this).arg(42);
     ASSERT_EQ(1, outputs.size());
     EXPECT_STREQ("TEST", outputs[0].first);
     EXPECT_EQ("The answer is 42", outputs[0].second);
@@ -86,7 +86,7 @@ TEST_F(FormatterTest, intArg) {
 
 // Can use multiple arguments at different places
 TEST_F(FormatterTest, multiArg) {
-    Formatter("TEST", s("The %2 are %1"), 1, *this).arg("switched").
+    Formatter("TEST", s("The %2 are %1"), this).arg("switched").
         arg("arguments");
     ASSERT_EQ(1, outputs.size());
     EXPECT_STREQ("TEST", outputs[0].first);
@@ -95,7 +95,7 @@ TEST_F(FormatterTest, multiArg) {
 
 // Can survive and complains if placeholder is missing
 TEST_F(FormatterTest, missingPlace) {
-    EXPECT_NO_THROW(Formatter("TEST", s("Missing the first %2"), 1, *this).
+    EXPECT_NO_THROW(Formatter("TEST", s("Missing the first %2"), this).
                     arg("missing").arg("argument"));
     ASSERT_EQ(1, outputs.size());
     EXPECT_STREQ("TEST", outputs[0].first);
@@ -105,7 +105,7 @@ TEST_F(FormatterTest, missingPlace) {
 
 // Can replace multiple placeholders
 TEST_F(FormatterTest, multiPlaceholder) {
-    Formatter("TEST", s("The %1 is the %1"), 1, *this).
+    Formatter("TEST", s("The %1 is the %1"), this).
         arg("first rule of tautology club");
     ASSERT_EQ(1, outputs.size());
     EXPECT_STREQ("TEST", outputs[0].first);
@@ -116,7 +116,7 @@ TEST_F(FormatterTest, multiPlaceholder) {
 // Test we can cope with replacement containing the placeholder
 TEST_F(FormatterTest, noRecurse) {
     // If we recurse, this will probably eat all the memory and crash
-    Formatter("TEST", s("%1"), 1, *this).arg("%1 %1");
+    Formatter("TEST", s("%1"), this).arg("%1 %1");
     ASSERT_EQ(1, outputs.size());
     EXPECT_STREQ("TEST", outputs[0].first);
     EXPECT_EQ("%1 %1", outputs[0].second);




More information about the bind10-changes mailing list