[trac901] Code size reduction

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

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());
     }

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);