BIND 10 trac1944, updated. 00472db8fd05a7837e468e9354710cc8be7d0534 TEMPORARY! REMOVE BEFORE MERGE!

BIND 10 source code commits bind10-changes at lists.isc.org
Fri May 25 11:35:23 UTC 2012


The branch, trac1944 has been updated
  discards  d4b343d81021e598e47ae862ef37c7beb61c258d (commit)
  discards  64446bcfae9ef04b9a62ea27ddeb75959fd20cf4 (commit)
  discards  585f86e070a013708fc36169d4b2f10d195eae6e (commit)
       via  00472db8fd05a7837e468e9354710cc8be7d0534 (commit)
       via  b94f49bf264e058dac14287eca360c82edb0071d (commit)
       via  f77a1e4539afd8dea0a1db15d46d85b810c5c4ee (commit)
       via  eacdcb4322f7fc89f35af526768bf42403ad09a6 (commit)

This update added new revisions after undoing existing revisions.  That is
to say, the old revision is not a strict subset of the new revision.  This
situation occurs when you --force push a change and generate a repository
containing something like this:

 * -- * -- B -- O -- O -- O (d4b343d81021e598e47ae862ef37c7beb61c258d)
            \
             N -- N -- N (00472db8fd05a7837e468e9354710cc8be7d0534)

When this happens we assume that you've already had alert emails for all
of the O revisions, and so we here report only the revisions in the N
branch from the common base, B.

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 00472db8fd05a7837e468e9354710cc8be7d0534
Author: Michal 'vorner' Vaner <michal.vaner at nic.cz>
Date:   Thu May 24 14:04:08 2012 +0200

    TEMPORARY! REMOVE BEFORE MERGE!
    
    This commit is here to suppress some false positives in the check (by
    completely disabling the logging there). There are several messages that
    ignore some arguments on purpose. While this produces wrong messages and
    is overall questionable, I'm not going to solve it right now.
    
    This commit will be removed before merging. Ignore during review,
    please (but you can see it passes the test with it).

commit b94f49bf264e058dac14287eca360c82edb0071d
Author: Michal 'vorner' Vaner <michal.vaner at nic.cz>
Date:   Fri May 25 12:48:34 2012 +0200

    [1944] Deactivate logger on exception
    
    We want to disable the logger if there was an exception while putting
    the parameters in. While we can't detect all possible ones, we can
    detect some and reduce some false positives of the strict log checker
    and not output mangled messages.

commit f77a1e4539afd8dea0a1db15d46d85b810c5c4ee
Author: Michal 'vorner' Vaner <michal.vaner at nic.cz>
Date:   Thu May 24 14:30:54 2012 +0200

    [1944] Make sure we print the message ID on abort
    
    When we enable the strict log checking and the excess placeholders check
    triggers, we need to print the message to easily identify the message ID
    with the problem.

commit eacdcb4322f7fc89f35af526768bf42403ad09a6
Author: Michal 'vorner' Vaner <michal.vaner at nic.cz>
Date:   Thu May 24 14:07:48 2012 +0200

    [1944] Add a missing placeholder for number of labels
    
    The DATASRC_DATABASE_FINDNSEC3_COVER logging message was missing a
    placeholder. Added.

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

Summary of changes:
 src/lib/log/log_formatter.h                 |   31 +++++++++++++++++++++++++--
 src/lib/log/tests/log_formatter_unittest.cc |   21 +++++++++++-------
 src/lib/python/isc/log/log.cc               |   10 +++++++--
 src/lib/python/isc/log/tests/log_test.py    |    2 +-
 4 files changed, 51 insertions(+), 13 deletions(-)

-----------------------------------------------------------------------
diff --git a/src/lib/log/log_formatter.h b/src/lib/log/log_formatter.h
index fc60203..eebdb1a 100644
--- a/src/lib/log/log_formatter.h
+++ b/src/lib/log/log_formatter.h
@@ -197,7 +197,9 @@ public:
             try {
                 return (arg(boost::lexical_cast<std::string>(value)));
             } catch (const boost::bad_lexical_cast& ex) {
-
+                // The formatting of the log message got wrong, we don't want
+                // to output it.
+                deactivate();
                 // A bad_lexical_cast during a conversion to a string is
                 // *extremely* unlikely to fail.  However, there is nothing
                 // in the documentation that rules it out, so we need to handle
@@ -229,10 +231,35 @@ public:
             // occurrences of "%2" with 42. (Conversely, the sequence
             // .arg(42).arg("%1") would return "42 %1" - there are no recursive
             // replacements).
-            replacePlaceholder(message_, arg, ++nextPlaceholder_ );
+            try {
+                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).
+                deactivate();
+                throw;
+            }
         }
         return (*this);
     }
+
+    /// \brief Turn off the output of this logger.
+    ///
+    /// If the logger would output anything at the end, now it won't.
+    /// Also, this turns off the strict checking of placeholders, if
+    /// it is compiled in.
+    ///
+    /// The expected use is when there was an exception processing
+    /// the arguments for the message.
+    void deactivate() {
+        if (logger_) {
+            delete message_;
+            message_ = NULL;
+            logger_ = NULL;
+        }
+    }
 };
 
 }
diff --git a/src/lib/log/tests/log_formatter_unittest.cc b/src/lib/log/tests/log_formatter_unittest.cc
index 83fc062..435b200 100644
--- a/src/lib/log/tests/log_formatter_unittest.cc
+++ b/src/lib/log/tests/log_formatter_unittest.cc
@@ -81,6 +81,14 @@ TEST_F(FormatterTest, stringArg) {
     }
 }
 
+// Test the .deactivate() method
+TEST_F(FormatterTest, deactivate) {
+    Formatter(isc::log::INFO, s("Text of message"), this).deactivate();
+    // If there was no .deactivate, it should have output it.
+    // But not now.
+    ASSERT_EQ(0, outputs.size());
+}
+
 // Can convert to string
 TEST_F(FormatterTest, intArg) {
     Formatter(isc::log::INFO, s("The answer is %1"), this).arg(42);
@@ -117,15 +125,12 @@ TEST_F(FormatterTest, mismatchedPlaceholders) {
             arg("only one");
     }, ".*");
 
-    // Mixed case of above two: the exception will be thrown due to the missing
-    // placeholder, but before even it's caught the program will be aborted
-    // due to the unused placeholder as a result of the exception.
-    EXPECT_DEATH({
-        isc::util::unittests::dontCreateCoreDumps();
-        Formatter(isc::log::INFO, s("Missing the first %2"), this).
-            arg("missing").arg("argument");
-    }, ".*");
 #endif /* EXPECT_DEATH */
+    // Mixed case of above two: the exception will be thrown due to the missing
+    // placeholder. The other check is disabled due to that.
+    EXPECT_THROW(Formatter(isc::log::INFO, s("Missing the first %2"), this).
+                 arg("missing").arg("argument"),
+                 isc::log::MismatchedPlaceholders);
 }
 
 #else
diff --git a/src/lib/python/isc/log/log.cc b/src/lib/python/isc/log/log.cc
index ed05398..69e70b7 100644
--- a/src/lib/python/isc/log/log.cc
+++ b/src/lib/python/isc/log/log.cc
@@ -541,8 +541,14 @@ Logger_performOutput(Function function, PyObject* args, bool dbgLevel) {
         // into the formatter. It will print itself in the end.
         for (size_t i(start); i < number; ++ i) {
             PyObjectContainer param_container(PySequence_GetItem(args, i));
-            formatter = formatter.arg(objectToStr(param_container.get(),
-                                                  true));
+            try {
+                formatter = formatter.arg(objectToStr(param_container.get(),
+                                                      true));
+            }
+            catch (...) {
+                formatter.deactivate();
+                throw;
+            }
         }
         Py_RETURN_NONE;
     }
diff --git a/src/lib/python/isc/log/tests/log_test.py b/src/lib/python/isc/log/tests/log_test.py
index f627b26..1337654 100644
--- a/src/lib/python/isc/log/tests/log_test.py
+++ b/src/lib/python/isc/log/tests/log_test.py
@@ -192,7 +192,7 @@ class Logger(unittest.TestCase):
         self.assertRaises(TypeError, logger.debug, param, self.TEST_MSG, param)
         self.assertEqual(sys.getrefcount(param), orig_msgrefcnt)
 
-    def disable_test_bad_parameter(self):
+    def test_bad_parameter(self):
         # a log parameter cannot be converted to a string object.
         class LogParam:
             def __str__(self):



More information about the bind10-changes mailing list