BIND 10 master, updated. 5fb18596288d2eae586cd01da0b27f329fb4e155 [1892] Return the placeholders to tests too
BIND 10 source code commits
bind10-changes at lists.isc.org
Fri May 4 04:03:30 UTC 2012
The branch, master has been updated
via 5fb18596288d2eae586cd01da0b27f329fb4e155 (commit)
via dfaf9712d15392caef04df6c2149744f8abace87 (commit)
via f894ef742226f6c9d438a21be01f6a5d0cbcc5fb (commit)
from 3d876bb262c8eea4b3cdc60f293249068adde2c2 (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 5fb18596288d2eae586cd01da0b27f329fb4e155
Author: Michal 'vorner' Vaner <michal.vaner at nic.cz>
Date: Thu May 3 12:37:28 2012 +0200
[1892] Return the placeholders to tests too
The test code now reflects the code change.
commit dfaf9712d15392caef04df6c2149744f8abace87
Author: Mukund Sivaraman <muks at isc.org>
Date: Thu May 3 09:26:35 2012 +0530
[1892] Put back the @@Missing placeholder warning when logger checks are off
commit f894ef742226f6c9d438a21be01f6a5d0cbcc5fb
Author: Mukund Sivaraman <muks at isc.org>
Date: Tue May 1 15:18:42 2012 +0530
[1892] Conditionally throw an exception if logger placeholders are not matched
-----------------------------------------------------------------------
Summary of changes:
configure.ac | 5 ++++
src/lib/log/log_formatter.cc | 29 +++++++++++++++++++++--
src/lib/log/log_formatter.h | 22 ++++++++++++++++++
src/lib/log/tests/log_formatter_unittest.cc | 31 ++++++++++++++++++++++++-
src/lib/log/tests/logger_manager_unittest.cc | 2 +-
src/lib/log/tests/logger_support_unittest.cc | 2 +-
6 files changed, 84 insertions(+), 7 deletions(-)
-----------------------------------------------------------------------
diff --git a/configure.ac b/configure.ac
index 56d2618..d6eec68 100644
--- a/configure.ac
+++ b/configure.ac
@@ -977,6 +977,11 @@ AC_ARG_ENABLE(install-configurations,
AM_CONDITIONAL(INSTALL_CONFIGURATIONS, test x$install_configurations = xyes || test x$install_configurations = xtrue)
+AC_ARG_ENABLE(logger-checks, [AC_HELP_STRING([--enable-logger-checks],
+ [check logger messages [default=no]])], enable_logger_checks=$enableval, enable_logger_checks=no)
+AM_CONDITIONAL(ENABLE_LOGGER_CHECKS, test x$enable_logger_checks != xno)
+AM_COND_IF([ENABLE_LOGGER_CHECKS], [AC_DEFINE([ENABLE_LOGGER_CHECKS], [1], [Check logger messages?])])
+
AC_CONFIG_FILES([Makefile
doc/Makefile
doc/guide/Makefile
diff --git a/src/lib/log/log_formatter.cc b/src/lib/log/log_formatter.cc
index 18c4741..3d140bc 100644
--- a/src/lib/log/log_formatter.cc
+++ b/src/lib/log/log_formatter.cc
@@ -12,6 +12,7 @@
// OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR
// PERFORMANCE OF THIS SOFTWARE.
+#include "config.h"
#include <log/log_formatter.h>
using namespace std;
@@ -31,11 +32,33 @@ replacePlaceholder(string* message, const string& arg,
message->replace(pos, mark.size(), arg);
pos = message->find(mark, pos + arg.size());
} while (pos != string::npos);
- } else {
+ }
+#ifdef ENABLE_LOGGER_CHECKS
+ else {
+ // We're missing the placeholder, so throw an exception
+ isc_throw(MismatchedPlaceholders,
+ "Missing logger placeholder in message: " << message);
+ }
+#else
+ else {
// We're missing the placeholder, so add some complain
- message->append(" @@Missing placeholder " + mark + " for '" + arg +
- "'@@");
+ message->append(" @@Missing placeholder " + mark + " for '" + arg + "'@@");
+ }
+#endif /* ENABLE_LOGGER_CHECKS */
+}
+
+void
+checkExcessPlaceholders(string* message, unsigned int placeholder)
+{
+#ifdef ENABLE_LOGGER_CHECKS
+ string mark("%" + lexical_cast<string>(placeholder));
+ 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);
}
+#endif /* ENABLE_LOGGER_CHECKS */
}
}
diff --git a/src/lib/log/log_formatter.h b/src/lib/log/log_formatter.h
index 7a9e5fa..fc60203 100644
--- a/src/lib/log/log_formatter.h
+++ b/src/lib/log/log_formatter.h
@@ -39,6 +39,27 @@ public:
};
+/// \brief Mismatched Placeholders
+///
+/// This exception is used when the number of placeholders do not match
+/// the number of arguments passed to the formatter.
+
+class MismatchedPlaceholders : public isc::Exception {
+public:
+ MismatchedPlaceholders(const char* file, size_t line, const char* what) :
+ isc::Exception(file, line, what)
+ {}
+};
+
+
+///
+/// \brief Internal excess placeholder checker
+///
+/// This is used internally by the Formatter to check for excess
+/// placeholders (and fewer arguments).
+void
+checkExcessPlaceholders(std::string* message, unsigned int placeholder);
+
///
/// \brief The internal replacement routine
///
@@ -142,6 +163,7 @@ public:
/// This is the place where output happens if the formatter is active.
~ Formatter() {
if (logger_) {
+ checkExcessPlaceholders(message_, ++nextPlaceholder_);
logger_->output(severity_, *message_);
delete message_;
}
diff --git a/src/lib/log/tests/log_formatter_unittest.cc b/src/lib/log/tests/log_formatter_unittest.cc
index b91665d..3fb7689 100644
--- a/src/lib/log/tests/log_formatter_unittest.cc
+++ b/src/lib/log/tests/log_formatter_unittest.cc
@@ -12,6 +12,7 @@
// OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR
// PERFORMANCE OF THIS SOFTWARE.
+#include "config.h"
#include <gtest/gtest.h>
#include <log/log_formatter.h>
#include <log/logger_level.h>
@@ -94,16 +95,42 @@ TEST_F(FormatterTest, multiArg) {
EXPECT_EQ("The arguments are switched", outputs[0].second);
}
-// Can survive and complains if placeholder is missing
-TEST_F(FormatterTest, missingPlace) {
+#ifdef ENABLE_LOGGER_CHECKS
+
+#ifdef EXPECT_ABORT
+// Throws MismatchedPlaceholders exception if number of placeholders
+// don't match number of arguments. This causes it to abort.
+TEST_F(FormatterTest, mismatchedPlaceholders) {
+ EXPECT_ABORT({
+ Formatter(isc::log::INFO, s("Missing the first %2"), this).arg("missing").arg("argument");
+ }, ".*");
+
+ EXPECT_ABORT({
+ Formatter(isc::log::INFO, s("Too many arguments in %1 %2"), this).arg("only one");
+ }, ".*");
+}
+#endif /* EXPECT_ABORT */
+
+#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"));
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_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);
}
+#endif /* ENABLE_LOGGER_CHECKS */
+
// Can replace multiple placeholders
TEST_F(FormatterTest, multiPlaceholder) {
Formatter(isc::log::INFO, s("The %1 is the %1"), this).
diff --git a/src/lib/log/tests/logger_manager_unittest.cc b/src/lib/log/tests/logger_manager_unittest.cc
index f2713b0..584d0f5 100644
--- a/src/lib/log/tests/logger_manager_unittest.cc
+++ b/src/lib/log/tests/logger_manager_unittest.cc
@@ -296,7 +296,7 @@ TEST_F(LoggerManagerTest, FileSizeRollover) {
manager.process(spec);
{
Logger logger(file_spec.getLoggerName().c_str());
- LOG_FATAL(logger, LOG_NO_MESSAGE_TEXT).arg(big_arg);
+ LOG_FATAL(logger, LOG_NO_MESSAGE_TEXT).arg("42").arg(big_arg);
}
LoggerManager::reset(); // Ensure files are closed
diff --git a/src/lib/log/tests/logger_support_unittest.cc b/src/lib/log/tests/logger_support_unittest.cc
index b418906..c626c6d 100644
--- a/src/lib/log/tests/logger_support_unittest.cc
+++ b/src/lib/log/tests/logger_support_unittest.cc
@@ -79,5 +79,5 @@ TEST_F(LoggerSupportTest, LoggingInitializationCheck) {
// ... and check that they work when logging is initialized.
setLoggingInitialized(true);
EXPECT_NO_THROW(test_logger.isDebugEnabled());
- EXPECT_NO_THROW(test_logger.info(LOG_INPUT_OPEN_FAIL));
+ EXPECT_NO_THROW(test_logger.info(LOG_INPUT_OPEN_FAIL).arg("foo").arg("bar"));
}
More information about the bind10-changes
mailing list