BIND 10 master, updated. 409ca483d117688be552d27d1e06616d44701c82 [1964] updated the non-check mode case of logging to be consistent.

BIND 10 source code commits bind10-changes at lists.isc.org
Wed May 9 05:33:24 UTC 2012


The branch, master has been updated
       via  409ca483d117688be552d27d1e06616d44701c82 (commit)
       via  892f6b266acb98ad005e1c6dad6c0621c04f1d82 (commit)
       via  18675a2d6a237c775ac8bb91c26c94c7b4219095 (commit)
       via  25f0bb96ddbe5adc6538e68c2d6eb2da6bcc7c0c (commit)
       via  6c0da87a7bdbb7be59fbac47b6c5f2ef0043aecb (commit)
      from  36d930c2e66f13fadfff64689bee2547ca2ab8cd (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 409ca483d117688be552d27d1e06616d44701c82
Author: JINMEI Tatuya <jinmei at isc.org>
Date:   Tue May 8 16:32:15 2012 -0700

    [1964] updated the non-check mode case of logging to be consistent.

commit 892f6b266acb98ad005e1c6dad6c0621c04f1d82
Author: JINMEI Tatuya <jinmei at isc.org>
Date:   Tue May 8 16:09:18 2012 -0700

    [1964] added another test case where we can really check the thrown exception.
    
    also clarified the comment: the previous one didn't make perfect sense in
    that if it's really about exception we could have simply used EXCEPT_THROW.

commit 18675a2d6a237c775ac8bb91c26c94c7b4219095
Author: JINMEI Tatuya <jinmei at isc.org>
Date:   Tue May 8 15:57:11 2012 -0700

    [1964] use assert() instead of throwing exception in checkExcessPlaceholders.
    
    unfortunatelly, we cannot throw here because it's called from the Formatter
    destructor.  death tests now work for environments where it previously failed.

commit 25f0bb96ddbe5adc6538e68c2d6eb2da6bcc7c0c
Author: JINMEI Tatuya <jinmei at isc.org>
Date:   Tue May 8 15:23:06 2012 -0700

    [1964] small cleanups for checkExcessPlaceholders.
    
    - style fixes
    - constify (also for replacePlaceholder)
    - make sure all parameters are used regardless of ENABLE_LOGGER_CHECKS

commit 6c0da87a7bdbb7be59fbac47b6c5f2ef0043aecb
Author: JINMEI Tatuya <jinmei at isc.org>
Date:   Tue May 8 15:11:55 2012 -0700

    [1964] moved dontCreateCoreDumps to libutil_unittests.
    
    this solves various build troubles such as leaving it unused, causing
    circular dependcies,  or the use of unnamed namespace in a public header file.

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

Summary of changes:
 src/lib/log/log_formatter.cc                       |   25 ++++++----
 src/lib/log/tests/log_formatter_unittest.cc        |   54 +++++++++++++------
 src/lib/log/tests/logger_unittest.cc               |    6 +--
 .../log/tests/message_initializer_2_unittest.cc    |    6 +--
 src/lib/server_common/tests/portconfig_unittest.cc |    8 +--
 src/lib/testutils/Makefile.am                      |    2 +-
 src/lib/util/tests/buffer_unittest.cc              |    4 +-
 src/lib/util/unittests/Makefile.am                 |    1 +
 .../logger.cc => util/unittests/resource.cc}       |   22 ++++++--
 src/lib/{testutils => util/unittests}/resource.h   |   43 ++++-----------
 10 files changed, 91 insertions(+), 80 deletions(-)
 copy src/lib/{asiodns/logger.cc => util/unittests/resource.cc} (70%)
 rename src/lib/{testutils => util/unittests}/resource.h (60%)

-----------------------------------------------------------------------
diff --git a/src/lib/log/log_formatter.cc b/src/lib/log/log_formatter.cc
index 885f499..c728cb5 100644
--- a/src/lib/log/log_formatter.cc
+++ b/src/lib/log/log_formatter.cc
@@ -15,6 +15,8 @@
 #include "config.h"
 #include <log/log_formatter.h>
 
+#include <cassert>
+
 using namespace std;
 using namespace boost;
 
@@ -25,7 +27,7 @@ void
 replacePlaceholder(string* message, const string& arg,
                    const unsigned placeholder)
 {
-    string mark("%" + lexical_cast<string>(placeholder));
+    const string mark("%" + lexical_cast<string>(placeholder));
     size_t pos(message->find(mark));
     if (pos != string::npos) {
         do {
@@ -48,17 +50,20 @@ replacePlaceholder(string* message, const string& arg,
 }
 
 void
-checkExcessPlaceholders(string* message, unsigned int placeholder)
-{
-#ifdef ENABLE_LOGGER_CHECKS
-    string mark("%" + lexical_cast<string>(placeholder));
-    size_t pos(message->find(mark));
+checkExcessPlaceholders(string* message, unsigned int placeholder) {
+    const string mark("%" + lexical_cast<string>(placeholder));
+    const size_t pos(message->find(mark));
     if (pos != string::npos) {
-        // Excess placeholders were found, so throw an exception
-        isc_throw(MismatchedPlaceholders,
-		  "Excess logger placeholders still exist in message: " << *message);
-    }
+        // Excess placeholders were found.  If we enable the harsh check,
+        // abort it.  Note: ideally we'd like to throw MismatchedPlaceholders,
+        // but we can't at least for now because this function is called from
+        // the Formatter's destructor.
+#ifdef ENABLE_LOGGER_CHECKS
+        assert("Excess logger placeholders still exist in message" == NULL);
+#else
+        message->append(" @@Excess logger placeholders still exist@@");
 #endif /* ENABLE_LOGGER_CHECKS */
+    }
 }
 
 }
diff --git a/src/lib/log/tests/log_formatter_unittest.cc b/src/lib/log/tests/log_formatter_unittest.cc
index 1aa7e4e..83fc062 100644
--- a/src/lib/log/tests/log_formatter_unittest.cc
+++ b/src/lib/log/tests/log_formatter_unittest.cc
@@ -15,9 +15,7 @@
 #include "config.h"
 #include <gtest/gtest.h>
 
-#ifdef EXPECT_DEATH
-#include <testutils/resource.h>
-#endif /* EXPECT_DEATH */
+#include <util/unittests/resource.h>
 
 #include <log/log_formatter.h>
 #include <log/logger_level.h>
@@ -102,38 +100,60 @@ TEST_F(FormatterTest, multiArg) {
 
 #ifdef ENABLE_LOGGER_CHECKS
 
-#ifdef EXPECT_DEATH
-// Throws MismatchedPlaceholders exception if number of placeholders
-// don't match number of arguments. This causes it to abort.
 TEST_F(FormatterTest, mismatchedPlaceholders) {
+    // Throws MismatchedPlaceholders exception if the placeholder is missing
+    // for a supplied argument.
+    EXPECT_THROW(Formatter(isc::log::INFO, s("Missing the second %1"), this).
+                 arg("argument").arg("missing"),
+                 isc::log::MismatchedPlaceholders);
+
+#ifdef EXPECT_DEATH
+    // Likewise, if there's a redundant placeholder (or missing argument), the
+    // check detects it and aborts the program.  Due to the restriction of the
+    // current implementation, it doesn't throw.
     EXPECT_DEATH({
-        isc::testutils::dontCreateCoreDumps();
-        Formatter(isc::log::INFO, s("Missing the first %2"), this).arg("missing").arg("argument");
+        isc::util::unittests::dontCreateCoreDumps();
+        Formatter(isc::log::INFO, s("Too many arguments in %1 %2"), this).
+            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::testutils::dontCreateCoreDumps();
-        Formatter(isc::log::INFO, s("Too many arguments in %1 %2"), this).arg("only one");
+        isc::util::unittests::dontCreateCoreDumps();
+        Formatter(isc::log::INFO, s("Missing the first %2"), this).
+            arg("missing").arg("argument");
     }, ".*");
-}
 #endif /* EXPECT_DEATH */
+}
 
 #else
 
 // If logger checks are not enabled, nothing is thrown
 TEST_F(FormatterTest, mismatchedPlaceholders) {
-    EXPECT_NO_THROW(Formatter(isc::log::INFO, s("Missing the first %2"), this).
-                    arg("missing").arg("argument"));
+    Formatter(isc::log::INFO, s("Missing the second %1"), this).
+        arg("argument").arg("missing");
     ASSERT_EQ(1, outputs.size());
     EXPECT_EQ(isc::log::INFO, outputs[0].first);
-    EXPECT_EQ("Missing the first argument "
-              "@@Missing placeholder %1 for 'missing'@@", outputs[0].second);
+    EXPECT_EQ("Missing the second argument "
+              "@@Missing placeholder %2 for 'missing'@@", outputs[0].second);
 
-    EXPECT_NO_THROW(Formatter(isc::log::INFO, s("Too many arguments in %1 %2"), this).
+    EXPECT_NO_THROW(Formatter(isc::log::INFO,
+                              s("Too many arguments in %1 %2"), this).
                     arg("only one"));
     ASSERT_EQ(2, outputs.size());
     EXPECT_EQ(isc::log::INFO, outputs[1].first);
-    EXPECT_EQ("Too many arguments in only one %2", outputs[1].second);
+    EXPECT_EQ("Too many arguments in only one %2 "
+              "@@Excess logger placeholders still exist@@",
+              outputs[1].second);
+
+    EXPECT_NO_THROW(Formatter(isc::log::INFO, s("Missing the first %2"), this).
+                    arg("missing").arg("argument"));
+    ASSERT_EQ(3, outputs.size());
+    EXPECT_EQ(isc::log::INFO, outputs[2].first);
+    EXPECT_EQ("Missing the first argument "
+              "@@Missing placeholder %1 for 'missing'@@", outputs[2].second);
 }
 
 #endif /* ENABLE_LOGGER_CHECKS */
diff --git a/src/lib/log/tests/logger_unittest.cc b/src/lib/log/tests/logger_unittest.cc
index 5dddf32..069205e 100644
--- a/src/lib/log/tests/logger_unittest.cc
+++ b/src/lib/log/tests/logger_unittest.cc
@@ -17,9 +17,7 @@
 
 #include <gtest/gtest.h>
 
-#ifdef EXPECT_DEATH
-#include <testutils/resource.h>
-#endif /* EXPECT_DEATH */
+#include <util/unittests/resource.h>
 
 #include <log/logger.h>
 #include <log/logger_manager.h>
@@ -374,7 +372,7 @@ TEST_F(LoggerTest, LoggerNameLength) {
     // Note that we just check that it dies - we don't check what message is
     // output.
     EXPECT_DEATH({
-        isc::testutils::dontCreateCoreDumps();
+        isc::util::unittests::dontCreateCoreDumps();
 
         string ok3(Logger::MAX_LOGGER_NAME_SIZE + 1, 'x');
         Logger l3(ok3.c_str());
diff --git a/src/lib/log/tests/message_initializer_2_unittest.cc b/src/lib/log/tests/message_initializer_2_unittest.cc
index e848a6c..b479eee 100644
--- a/src/lib/log/tests/message_initializer_2_unittest.cc
+++ b/src/lib/log/tests/message_initializer_2_unittest.cc
@@ -15,9 +15,7 @@
 #include <log/message_initializer.h>
 #include <gtest/gtest.h>
 
-#ifdef EXPECT_DEATH
-#include <testutils/resource.h>
-#endif /* EXPECT_DEATH */
+#include <util/unittests/resource.h>
 
 using namespace isc::log;
 
@@ -46,7 +44,7 @@ TEST(MessageInitializerTest2, MessageLoadTest) {
 #ifdef EXPECT_DEATH
     // Adding one more should take us over the limit.
     EXPECT_DEATH({
-        isc::testutils::dontCreateCoreDumps();
+        isc::util::unittests::dontCreateCoreDumps();
 
         MessageInitializer initializer2(values);
       }, ".*");
diff --git a/src/lib/server_common/tests/portconfig_unittest.cc b/src/lib/server_common/tests/portconfig_unittest.cc
index ad8d1e9..ac880c0 100644
--- a/src/lib/server_common/tests/portconfig_unittest.cc
+++ b/src/lib/server_common/tests/portconfig_unittest.cc
@@ -18,9 +18,7 @@
 #include <testutils/socket_request.h>
 #include <testutils/mockups.h>
 
-#ifdef EXPECT_DEATH
-#include <testutils/resource.h>
-#endif /* EXPECT_DEATH */
+#include <util/unittests/resource.h>
 
 #include <cc/data.h>
 #include <exceptions/exceptions.h>
@@ -318,7 +316,7 @@ TEST_F(InstallListenAddressesDeathTest, inconsistent) {
     // Make sure it actually kills the application (there should be an abort
     // in this case)
     EXPECT_DEATH({
-        isc::testutils::dontCreateCoreDumps();
+        isc::util::unittests::dontCreateCoreDumps();
 
         try {
           installListenAddresses(deathAddresses, store_, dnss_);
@@ -337,7 +335,7 @@ TEST_F(InstallListenAddressesDeathTest, cantClose) {
     // Instruct it to fail on close
     sock_requestor_.break_release_ = true;
     EXPECT_DEATH({
-        isc::testutils::dontCreateCoreDumps();
+        isc::util::unittests::dontCreateCoreDumps();
 
         try {
           // Setting to empty will close all current sockets.
diff --git a/src/lib/testutils/Makefile.am b/src/lib/testutils/Makefile.am
index 77f5a96..7a4c8d7 100644
--- a/src/lib/testutils/Makefile.am
+++ b/src/lib/testutils/Makefile.am
@@ -14,4 +14,4 @@ libtestutils_la_CPPFLAGS = $(AM_CPPFLAGS) $(GTEST_INCLUDES)
 libtestutils_la_LIBADD = $(top_builddir)/src/lib/asiolink/libasiolink.la
 endif
 
-EXTRA_DIST = portconfig.h resource.h socket_request.h
+EXTRA_DIST = portconfig.h socket_request.h
diff --git a/src/lib/testutils/resource.h b/src/lib/testutils/resource.h
deleted file mode 100644
index 192d4a6..0000000
--- a/src/lib/testutils/resource.h
+++ /dev/null
@@ -1,58 +0,0 @@
-// Copyright (C) 2012  Internet Systems Consortium, Inc. ("ISC")
-//
-// Permission to use, copy, modify, and/or distribute this software for any
-// purpose with or without fee is hereby granted, provided that the above
-// copyright notice and this permission notice appear in all copies.
-//
-// THE SOFTWARE IS PROVIDED "AS IS" AND ISC DISCLAIMS ALL WARRANTIES WITH
-// REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF MERCHANTABILITY
-// AND FITNESS.  IN NO EVENT SHALL ISC BE LIABLE FOR ANY SPECIAL, DIRECT,
-// INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES WHATSOEVER RESULTING FROM
-// LOSS OF USE, DATA OR PROFITS, WHETHER IN AN ACTION OF CONTRACT, NEGLIGENCE
-// OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR
-// PERFORMANCE OF THIS SOFTWARE.
-
-#ifndef __ISC_TESTUTILS_RESOURCE_H
-#define __ISC_TESTUTILS_RESOURCE_H
-
-#include <sys/time.h>
-#include <sys/resource.h>
-
-#include <gtest/gtest.h>
-
-namespace isc {
-namespace testutils {
-
-/// Don't create core dumps.
-///
-/// This function sets the core size to 0, inhibiting the creation of
-/// core dumps. It is meant to be used in testcases where EXPECT_DEATH
-/// is used, where processes abort (and create cores in the process).
-/// As a new process is forked to run EXPECT_DEATH tests, the rlimits of
-/// the parent process that runs the other tests should be unaffected.
-///
-/// This function definition is in the header file as otherwise there'd
-/// be a circular dependency from
-/// testutils->asiolink->log->testutils. See bug #1880.
-
-namespace {
-
-inline void
-dontCreateCoreDumps(void)
-{
-    /* Set rlimits so that no coredumps are created. As a new
-       process is forked to run this EXPECT_DEATH test, the rlimits
-       of the parent process that runs the other tests should be
-       unaffected. */
-
-    rlimit core_limit = {0, 0};
-
-    EXPECT_EQ(setrlimit(RLIMIT_CORE, &core_limit), 0);
-}
-
-} // end of anonymous namespace
-
-} // end of namespace testutils
-} // end of namespace isc
-
-#endif /* __ISC_TESTUTILS_RESOURCE_H */
diff --git a/src/lib/util/tests/buffer_unittest.cc b/src/lib/util/tests/buffer_unittest.cc
index ae164ff..ccd6989 100644
--- a/src/lib/util/tests/buffer_unittest.cc
+++ b/src/lib/util/tests/buffer_unittest.cc
@@ -17,7 +17,7 @@
 #include <exceptions/exceptions.h>
 
 #ifdef EXPECT_DEATH
-#include <testutils/resource.h>
+#include <util/unittests/resource.h>
 #endif /* EXPECT_DEATH */
 
 #include <util/buffer.h>
@@ -189,7 +189,7 @@ TEST_F(BufferTest, outputBufferReadat) {
 #ifdef EXPECT_DEATH
     // We use assert now, so we check it dies
     EXPECT_DEATH({
-        isc::testutils::dontCreateCoreDumps();
+        isc::util::unittests::dontCreateCoreDumps();
 
         try {
             obuffer[sizeof(testdata)];
diff --git a/src/lib/util/unittests/Makefile.am b/src/lib/util/unittests/Makefile.am
index bbb0d49..2827471 100644
--- a/src/lib/util/unittests/Makefile.am
+++ b/src/lib/util/unittests/Makefile.am
@@ -6,6 +6,7 @@ libutil_unittests_la_SOURCES = fork.h fork.cc resolver.h
 libutil_unittests_la_SOURCES += newhook.h newhook.cc
 libutil_unittests_la_SOURCES += testdata.h testdata.cc
 if HAVE_GTEST
+libutil_unittests_la_SOURCES += resource.h resource.cc
 libutil_unittests_la_SOURCES += run_all.h run_all.cc
 libutil_unittests_la_SOURCES += textdata.h
 endif
diff --git a/src/lib/util/unittests/resource.cc b/src/lib/util/unittests/resource.cc
new file mode 100644
index 0000000..3e77e0d
--- /dev/null
+++ b/src/lib/util/unittests/resource.cc
@@ -0,0 +1,35 @@
+// Copyright (C) 2012  Internet Systems Consortium, Inc. ("ISC")
+//
+// Permission to use, copy, modify, and/or distribute this software for any
+// purpose with or without fee is hereby granted, provided that the above
+// copyright notice and this permission notice appear in all copies.
+//
+// THE SOFTWARE IS PROVIDED "AS IS" AND ISC DISCLAIMS ALL WARRANTIES WITH
+// REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF MERCHANTABILITY
+// AND FITNESS.  IN NO EVENT SHALL ISC BE LIABLE FOR ANY SPECIAL, DIRECT,
+// INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES WHATSOEVER RESULTING FROM
+// LOSS OF USE, DATA OR PROFITS, WHETHER IN AN ACTION OF CONTRACT, NEGLIGENCE
+// OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR
+// PERFORMANCE OF THIS SOFTWARE.
+
+#include "resource.h"
+
+#include <gtest/gtest.h>
+
+#include <sys/time.h>
+#include <sys/resource.h>
+
+namespace isc {
+namespace util {
+namespace unittests {
+
+void
+dontCreateCoreDumps() {
+    const rlimit core_limit = {0, 0};
+
+    EXPECT_EQ(setrlimit(RLIMIT_CORE, &core_limit), 0);
+}
+
+} // end of namespace unittests
+} // end of namespace util
+} // end of namespace isc
diff --git a/src/lib/util/unittests/resource.h b/src/lib/util/unittests/resource.h
new file mode 100644
index 0000000..6430ab2
--- /dev/null
+++ b/src/lib/util/unittests/resource.h
@@ -0,0 +1,39 @@
+// Copyright (C) 2012  Internet Systems Consortium, Inc. ("ISC")
+//
+// Permission to use, copy, modify, and/or distribute this software for any
+// purpose with or without fee is hereby granted, provided that the above
+// copyright notice and this permission notice appear in all copies.
+//
+// THE SOFTWARE IS PROVIDED "AS IS" AND ISC DISCLAIMS ALL WARRANTIES WITH
+// REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF MERCHANTABILITY
+// AND FITNESS.  IN NO EVENT SHALL ISC BE LIABLE FOR ANY SPECIAL, DIRECT,
+// INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES WHATSOEVER RESULTING FROM
+// LOSS OF USE, DATA OR PROFITS, WHETHER IN AN ACTION OF CONTRACT, NEGLIGENCE
+// OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR
+// PERFORMANCE OF THIS SOFTWARE.
+
+#ifndef __UTIL_UNITTESTS_RESOURCE_H
+#define __UTIL_UNITTESTS_RESOURCE_H 1
+
+namespace isc {
+namespace util {
+namespace unittests {
+
+/// Don't create core dumps.
+///
+/// This function sets the core size to 0, inhibiting the creation of
+/// core dumps. It is meant to be used in testcases where EXPECT_DEATH
+/// is used, where processes abort (and create cores in the process).
+/// As a new process is forked to run EXPECT_DEATH tests, the rlimits of
+/// the parent process that runs the other tests should be unaffected.
+void dontCreateCoreDumps();
+
+} // end of namespace unittests
+} // end of namespace util
+} // end of namespace isc
+
+#endif /* __UTIL_UNITTESTS_RESOURCE_H */
+
+// Local Variables:
+// mode: c++
+// End:



More information about the bind10-changes mailing list