BIND 10 trac900, updated. 569b2b6e5d84277316bce84c0e118ffd116ffe5e [trac900] Clean up a few loose ends after the merge
BIND 10 source code commits
bind10-changes at lists.isc.org
Mon May 9 13:34:01 UTC 2011
The branch, trac900 has been updated
via 569b2b6e5d84277316bce84c0e118ffd116ffe5e (commit)
via e5687a75c06966ab68ad453093fb6c7b9d8fe761 (commit)
via 119678f768f09b13d6ecb715ad731d1dca8f05b0 (commit)
via 4903410e45670b30d7283f5d69dc28c2069237d6 (commit)
via 9eb7b41d5253c44e59987170a74e345b651c96bc (commit)
via 4ff5a9f61d9ad1c96468556771562f1dbad7ec98 (commit)
via c16b3ab523860ccca5e8e9648b58801c3eefae29 (commit)
via 9a8870c515be763e8be63cdc28231883601867d4 (commit)
via fb1d629419b6c58b9c9f410e6c027c8769e0c9a3 (commit)
via 0709f8b56df68cfe0c79ed98bfe7e9dd25ce0e1e (commit)
via 40e06e655ce78197f0b52d8c5dc9c7516795362f (commit)
via b5646b50e23e6049f233f755d7e143a09d4fb19c (commit)
via d4d910b28b3226d1330b7da1df170c406771b939 (commit)
via 7bb5dc265a830d8993c9cb1da55194631e657587 (commit)
via 1e3c154cd21cede83a8bbd7a559c20752b58ce24 (commit)
via 7c40e60eeaad7f2c1ea79f92e866dee08eafd5ab (commit)
via fcc1b5e13946c5e58e2bd0bf287f551e161f0544 (commit)
via b6d140b6169c130b06896d2b40df58752991a47d (commit)
via eab5008d28b04fdd7c0f4c93051fd3b1c1a416f7 (commit)
via 121d3e844ad6d8ed8aa21dd48c53095e2f770117 (commit)
via c3dad479414b01979693ba71c832aedd681f5044 (commit)
via a23491a635d7f74132fc0b91eac832275f8f1f87 (commit)
via db0ca2fbc64b390f261b5e36938c736321b171f9 (commit)
via 2aca19e705f02400a6b213ef84fa81c86dff0375 (commit)
via cf46f370333b15487f6c02f85749d90c2e0bd710 (commit)
from 27a4c936d87d31952f6a8377e04cae1fb90edffd (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 569b2b6e5d84277316bce84c0e118ffd116ffe5e
Author: Stephen Morris <stephen at isc.org>
Date: Mon May 9 14:33:36 2011 +0100
[trac900] Clean up a few loose ends after the merge
commit e5687a75c06966ab68ad453093fb6c7b9d8fe761
Merge: 27a4c936d87d31952f6a8377e04cae1fb90edffd 119678f768f09b13d6ecb715ad731d1dca8f05b0
Author: Stephen Morris <stephen at isc.org>
Date: Mon May 9 14:09:16 2011 +0100
[trac900] Merge branch 'master' into trac900
Conflicts:
src/lib/log/README
src/lib/log/logger_support.cc
src/lib/log/messagedef.cc
src/lib/log/messagedef.h
src/lib/log/messagedef.mes
src/lib/log/tests/logger_support_test.cc
src/lib/log/tests/run_time_init_test.sh.in
-----------------------------------------------------------------------
Summary of changes:
ChangeLog | 8 +
src/lib/asiodns/Makefile.am | 11 ++-
src/lib/asiodns/asiodef.cc | 39 -----
src/lib/asiodns/asiodef.h | 23 ---
src/lib/asiodns/asiodef.mes | 56 ++++++++
src/lib/asiodns/asiodef.msg | 56 --------
src/lib/asiodns/io_fetch.cc | 54 ++++----
src/lib/dns/tests/message_unittest.cc | 13 +-
src/lib/log/Makefile.am | 2 +
src/lib/log/README | 4 +-
src/lib/log/compiler/message.cc | 7 +-
.../log/{root_logger_name.cc => log_formatter.cc} | 42 +++---
src/lib/log/log_formatter.h | 146 ++++++++++++++++++++
src/lib/log/logger.cc | 63 +++++----
src/lib/log/logger.h | 25 ++--
src/lib/log/logger_impl.cc | 20 ++--
src/lib/log/logger_impl.h | 62 +--------
src/lib/log/logger_support.cc | 12 +-
src/lib/log/macros.h | 50 +++++++
src/lib/log/messagedef.cc | 36 +++---
src/lib/log/messagedef.h | 2 +-
src/lib/log/messagedef.mes | 34 +++---
src/lib/log/tests/Makefile.am | 1 +
src/lib/log/tests/log_formatter_unittest.cc | 125 +++++++++++++++++
src/lib/log/tests/logger_support_test.cc | 18 ++-
src/lib/log/tests/run_time_init_test.sh.in | 4 +-
src/lib/util/unittests/testdata.h | 8 +
27 files changed, 585 insertions(+), 336 deletions(-)
delete mode 100644 src/lib/asiodns/asiodef.cc
delete mode 100644 src/lib/asiodns/asiodef.h
create mode 100644 src/lib/asiodns/asiodef.mes
delete mode 100644 src/lib/asiodns/asiodef.msg
copy src/lib/log/{root_logger_name.cc => log_formatter.cc} (55%)
create mode 100644 src/lib/log/log_formatter.h
create mode 100644 src/lib/log/macros.h
create mode 100644 src/lib/log/tests/log_formatter_unittest.cc
-----------------------------------------------------------------------
diff --git a/ChangeLog b/ChangeLog
index 859a340..d5de903 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,11 @@
+231. [func]*
+ The logging interface changed slightly. We use
+ logger.foo(MESSAGE_ID).arg(bar); instead of logger.foo(MESSAGE_ID, bar);
+ internally. The message definitions use '%1,%2,...' instead of '%s,%d',
+ which allows us to cope better with mismatched placeholders and allows
+ reordering of them in case of translation.
+ (Trac901, git 4903410e45670b30d7283f5d69dc28c2069237d6)
+
230. [bug] naokikambe
Removed too repeated verbose messages in two cases of:
- when auth sends statistics data to stats
diff --git a/src/lib/asiodns/Makefile.am b/src/lib/asiodns/Makefile.am
index 4bcdde6..a632488 100644
--- a/src/lib/asiodns/Makefile.am
+++ b/src/lib/asiodns/Makefile.am
@@ -8,12 +8,17 @@ AM_CPPFLAGS += -I$(top_srcdir)/src/lib/util -I$(top_builddir)/src/lib/util
AM_CXXFLAGS = $(B10_CXXFLAGS)
-CLEANFILES = *.gcno *.gcda
+CLEANFILES = *.gcno *.gcda asiodef.h asiodef.cc
+
+# Define rule to build logging source files from message file
+asiodef.h asiodef.cc: asiodef.mes
+ $(top_builddir)/src/lib/log/compiler/message $(top_srcdir)/src/lib/asiodns/asiodef.mes
+
+BUILT_SOURCES = asiodef.h asiodef.cc
lib_LTLIBRARIES = libasiodns.la
libasiodns_la_SOURCES = dns_answer.h
libasiodns_la_SOURCES += asiodns.h
-libasiodns_la_SOURCES += asiodef.cc asiodef.h
libasiodns_la_SOURCES += dns_lookup.h
libasiodns_la_SOURCES += dns_server.h
libasiodns_la_SOURCES += dns_service.cc dns_service.h
@@ -21,6 +26,8 @@ libasiodns_la_SOURCES += tcp_server.cc tcp_server.h
libasiodns_la_SOURCES += udp_server.cc udp_server.h
libasiodns_la_SOURCES += io_fetch.cc io_fetch.h
+nodist_libasiodns_la_SOURCES = asiodef.cc asiodef.h
+
EXTRA_DIST = asiodef.msg
# Note: the ordering matters: -Wno-... must follow -Wextra (defined in
diff --git a/src/lib/asiodns/asiodef.cc b/src/lib/asiodns/asiodef.cc
deleted file mode 100644
index 127e848..0000000
--- a/src/lib/asiodns/asiodef.cc
+++ /dev/null
@@ -1,39 +0,0 @@
-// File created from asiodef.msg on Mon Feb 28 17:15:30 2011
-
-#include <cstddef>
-#include <log/message_types.h>
-#include <log/message_initializer.h>
-
-namespace isc {
-namespace asiodns {
-
-extern const isc::log::MessageID ASIODNS_FETCHCOMP = "FETCHCOMP";
-extern const isc::log::MessageID ASIODNS_FETCHSTOP = "FETCHSTOP";
-extern const isc::log::MessageID ASIODNS_OPENSOCK = "OPENSOCK";
-extern const isc::log::MessageID ASIODNS_RECVSOCK = "RECVSOCK";
-extern const isc::log::MessageID ASIODNS_RECVTMO = "RECVTMO";
-extern const isc::log::MessageID ASIODNS_SENDSOCK = "SENDSOCK";
-extern const isc::log::MessageID ASIODNS_UNKORIGIN = "UNKORIGIN";
-extern const isc::log::MessageID ASIODNS_UNKRESULT = "UNKRESULT";
-
-} // namespace asiodns
-} // namespace isc
-
-namespace {
-
-const char* values[] = {
- "FETCHCOMP", "upstream fetch to %s(%d) has now completed",
- "FETCHSTOP", "upstream fetch to %s(%d) has been stopped",
- "OPENSOCK", "error %d opening %s socket to %s(%d)",
- "RECVSOCK", "error %d reading %s data from %s(%d)",
- "RECVTMO", "receive timeout while waiting for data from %s(%d)",
- "SENDSOCK", "error %d sending data using %s to %s(%d)",
- "UNKORIGIN", "unknown origin for ASIO error code %d (protocol: %s, address %s)",
- "UNKRESULT", "unknown result (%d) when IOFetch::stop() was executed for I/O to %s(%d)",
- NULL
-};
-
-const isc::log::MessageInitializer initializer(values);
-
-} // Anonymous namespace
-
diff --git a/src/lib/asiodns/asiodef.h b/src/lib/asiodns/asiodef.h
deleted file mode 100644
index 50aa8a9..0000000
--- a/src/lib/asiodns/asiodef.h
+++ /dev/null
@@ -1,23 +0,0 @@
-// File created from asiodef.msg on Mon Feb 28 17:15:30 2011
-
-#ifndef __ASIODEF_H
-#define __ASIODEF_H
-
-#include <log/message_types.h>
-
-namespace isc {
-namespace asiodns {
-
-extern const isc::log::MessageID ASIODNS_FETCHCOMP;
-extern const isc::log::MessageID ASIODNS_FETCHSTOP;
-extern const isc::log::MessageID ASIODNS_OPENSOCK;
-extern const isc::log::MessageID ASIODNS_RECVSOCK;
-extern const isc::log::MessageID ASIODNS_RECVTMO;
-extern const isc::log::MessageID ASIODNS_SENDSOCK;
-extern const isc::log::MessageID ASIODNS_UNKORIGIN;
-extern const isc::log::MessageID ASIODNS_UNKRESULT;
-
-} // namespace asiodns
-} // namespace isc
-
-#endif // __ASIODEF_H
diff --git a/src/lib/asiodns/asiodef.mes b/src/lib/asiodns/asiodef.mes
new file mode 100644
index 0000000..3f2e80c
--- /dev/null
+++ b/src/lib/asiodns/asiodef.mes
@@ -0,0 +1,56 @@
+# Copyright (C) 2011 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.
+
+$PREFIX ASIODNS_
+$NAMESPACE isc::asiodns
+
+% FETCHCOMP upstream fetch to %1(%2) has now completed
+A debug message, this records the the upstream fetch (a query made by the
+resolver on behalf of its client) to the specified address has completed.
+
+% FETCHSTOP upstream fetch to %1(%2) has been stopped
+An external component has requested the halting of an upstream fetch. This
+is an allowed operation, and the message should only appear if debug is
+enabled.
+
+% OPENSOCK error %1 opening %2 socket to %3(%4)
+The asynchronous I/O code encountered an error when trying to open a socket
+of the specified protocol in order to send a message to the target address.
+The the number of the system error that cause the problem is given in the
+message.
+
+% RECVSOCK error %1 reading %2 data from %3(%4)
+The asynchronous I/O code encountered an error when trying read data from
+the specified address on the given protocol. The the number of the system
+error that cause the problem is given in the message.
+
+% SENDSOCK error %1 sending data using %2 to %3(%4)
+The asynchronous I/O code encountered an error when trying send data to
+the specified address on the given protocol. The the number of the system
+error that cause the problem is given in the message.
+
+% RECVTMO receive timeout while waiting for data from %1(%2)
+An upstream fetch from the specified address timed out. This may happen for
+any number of reasons and is most probably a problem at the remote server
+or a problem on the network. The message will only appear if debug is
+enabled.
+
+% UNKORIGIN unknown origin for ASIO error code %1 (protocol: %2, address %3)
+This message should not appear and indicates an internal error if it does.
+Please enter a bug report.
+
+% UNKRESULT unknown result (%1) when IOFetch::stop() was executed for I/O to %2(%3)
+The termination method of the resolver's upstream fetch class was called with
+an unknown result code (which is given in the message). This message should
+not appear and may indicate an internal error. Please enter a bug report.
diff --git a/src/lib/asiodns/asiodef.msg b/src/lib/asiodns/asiodef.msg
deleted file mode 100644
index 7f86acb..0000000
--- a/src/lib/asiodns/asiodef.msg
+++ /dev/null
@@ -1,56 +0,0 @@
-# Copyright (C) 2011 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.
-
-$PREFIX ASIODNS_
-$NAMESPACE isc::asiodns
-
-FETCHCOMP upstream fetch to %s(%d) has now completed
-+ A debug message, this records the the upstream fetch (a query made by the
-+ resolver on behalf of its client) to the specified address has completed.
-
-FETCHSTOP upstream fetch to %s(%d) has been stopped
-+ An external component has requested the halting of an upstream fetch. This
-+ is an allowed operation, and the message should only appear if debug is
-+ enabled.
-
-OPENSOCK error %d opening %s socket to %s(%d)
-+ The asynchronous I/O code encountered an error when trying to open a socket
-+ of the specified protocol in order to send a message to the target address.
-+ The the number of the system error that cause the problem is given in the
-+ message.
-
-RECVSOCK error %d reading %s data from %s(%d)
-+ The asynchronous I/O code encountered an error when trying read data from
-+ the specified address on the given protocol. The the number of the system
-+ error that cause the problem is given in the message.
-
-SENDSOCK error %d sending data using %s to %s(%d)
-+ The asynchronous I/O code encountered an error when trying send data to
-+ the specified address on the given protocol. The the number of the system
-+ error that cause the problem is given in the message.
-
-RECVTMO receive timeout while waiting for data from %s(%d)
-+ An upstream fetch from the specified address timed out. This may happen for
-+ any number of reasons and is most probably a problem at the remote server
-+ or a problem on the network. The message will only appear if debug is
-+ enabled.
-
-UNKORIGIN unknown origin for ASIO error code %d (protocol: %s, address %s)
-+ This message should not appear and indicates an internal error if it does.
-+ Please enter a bug report.
-
-UNKRESULT unknown result (%d) when IOFetch::stop() was executed for I/O to %s(%d)
-+ The termination method of the resolver's upstream fetch class was called with
-+ an unknown result code (which is given in the message). This message should
-+ not appear and may indicate an internal error. Please enter a bug report.
diff --git a/src/lib/asiodns/io_fetch.cc b/src/lib/asiodns/io_fetch.cc
index 8f57d8e..cc8bd11 100644
--- a/src/lib/asiodns/io_fetch.cc
+++ b/src/lib/asiodns/io_fetch.cc
@@ -40,6 +40,7 @@
#include <dns/opcode.h>
#include <dns/rcode.h>
#include <log/logger.h>
+#include <log/macros.h>
#include <asiodns/asiodef.h>
#include <asiodns/io_fetch.h>
@@ -61,7 +62,17 @@ namespace asiodns {
/// Use the ASIO logger
+namespace {
+
isc::log::Logger logger("asiolink");
+// Log debug verbosity
+enum {
+ DBG_IMPORTANT = 1,
+ DBG_COMMON = 20,
+ DBG_ALL = 50
+};
+
+}
/// \brief IOFetch Data
///
@@ -331,42 +342,34 @@ IOFetch::stop(Result result) {
// numbers indicating the most important information. The relative
// values are somewhat arbitrary.
//
- // Although Logger::debug checks the debug flag internally, doing it
- // below before calling Logger::debug avoids the overhead of a string
- // conversion in the common case when debug is not enabled.
- //
// TODO: Update testing of stopped_ if threads are used.
data_->stopped = true;
switch (result) {
case TIME_OUT:
- if (logger.isDebugEnabled(1)) {
- logger.debug(20, ASIODNS_RECVTMO,
- data_->remote_snd->getAddress().toText().c_str(),
- static_cast<int>(data_->remote_snd->getPort()));
- }
+ LOG_DEBUG(logger, DBG_COMMON, ASIODNS_RECVTMO).
+ arg(data_->remote_snd->getAddress().toText()).
+ arg(data_->remote_snd->getPort());
break;
case SUCCESS:
- if (logger.isDebugEnabled(50)) {
- logger.debug(30, ASIODNS_FETCHCOMP,
- data_->remote_rcv->getAddress().toText().c_str(),
- static_cast<int>(data_->remote_rcv->getPort()));
- }
+ LOG_DEBUG(logger, DBG_ALL, ASIODNS_FETCHCOMP).
+ arg(data_->remote_rcv->getAddress().toText()).
+ arg(data_->remote_rcv->getPort());
break;
case STOPPED:
// Fetch has been stopped for some other reason. This is
// allowed but as it is unusual it is logged, but with a lower
// debug level than a timeout (which is totally normal).
- logger.debug(1, ASIODNS_FETCHSTOP,
- data_->remote_snd->getAddress().toText().c_str(),
- static_cast<int>(data_->remote_snd->getPort()));
+ LOG_DEBUG(logger, DBG_IMPORTANT, ASIODNS_FETCHSTOP).
+ arg(data_->remote_snd->getAddress().toText()).
+ arg(data_->remote_snd->getPort());
break;
default:
- logger.error(ASIODNS_UNKRESULT, static_cast<int>(result),
- data_->remote_snd->getAddress().toText().c_str(),
- static_cast<int>(data_->remote_snd->getPort()));
+ LOG_ERROR(logger, ASIODNS_UNKRESULT).
+ arg(data_->remote_snd->getAddress().toText()).
+ arg(data_->remote_snd->getPort());
}
// Stop requested, cancel and I/O's on the socket and shut it down,
@@ -394,12 +397,11 @@ void IOFetch::logIOFailure(asio::error_code ec) {
(data_->origin == ASIODNS_UNKORIGIN));
static const char* PROTOCOL[2] = {"TCP", "UDP"};
- logger.error(data_->origin,
- ec.value(),
- ((data_->remote_snd->getProtocol() == IPPROTO_TCP) ?
- PROTOCOL[0] : PROTOCOL[1]),
- data_->remote_snd->getAddress().toText().c_str(),
- static_cast<int>(data_->remote_snd->getPort()));
+ LOG_ERROR(logger, data_->origin).arg(ec.value()).
+ arg((data_->remote_snd->getProtocol() == IPPROTO_TCP) ?
+ PROTOCOL[0] : PROTOCOL[1]).
+ arg(data_->remote_snd->getAddress().toText()).
+ arg(data_->remote_snd->getPort());
}
} // namespace asiodns
diff --git a/src/lib/dns/tests/message_unittest.cc b/src/lib/dns/tests/message_unittest.cc
index 700e2bd..c79ea2c 100644
--- a/src/lib/dns/tests/message_unittest.cc
+++ b/src/lib/dns/tests/message_unittest.cc
@@ -683,11 +683,12 @@ TEST_F(MessageTest, toWireWithoutRcode) {
TEST_F(MessageTest, toText) {
// Check toText() output for a typical DNS response with records in
// all sections
- ifstream ifs;
- unittests::openTestData("message_toText1.txt", ifs);
+
factoryFromFile(message_parse, "message_toText1.wire");
{
SCOPED_TRACE("Message toText test (basic case)");
+ ifstream ifs;
+ unittests::openTestData("message_toText1.txt", ifs);
unittests::matchTextData(ifs, message_parse.toText());
}
@@ -695,12 +696,12 @@ TEST_F(MessageTest, toText) {
// from the dig output (other than replacing tabs with a space): adding
// a newline after the "OPT PSEUDOSECTION". This is an intentional change
// in our version for better readability.
- ifs.close();
message_parse.clear(Message::PARSE);
- unittests::openTestData("message_toText2.txt", ifs);
factoryFromFile(message_parse, "message_toText2.wire");
{
SCOPED_TRACE("Message toText test with EDNS");
+ ifstream ifs;
+ unittests::openTestData("message_toText2.txt", ifs);
unittests::matchTextData(ifs, message_parse.toText());
}
@@ -708,12 +709,12 @@ TEST_F(MessageTest, toText) {
// from the dig output (other than replacing tabs with a space): removing
// a redundant white space at the end of TSIG RDATA. We'd rather consider
// it a dig's defect than a feature.
- ifs.close();
message_parse.clear(Message::PARSE);
- unittests::openTestData("message_toText3.txt", ifs);
factoryFromFile(message_parse, "message_toText3.wire");
{
SCOPED_TRACE("Message toText test with TSIG");
+ ifstream ifs;
+ unittests::openTestData("message_toText3.txt", ifs);
unittests::matchTextData(ifs, message_parse.toText());
}
}
diff --git a/src/lib/log/Makefile.am b/src/lib/log/Makefile.am
index 0ea8ff6..c27b3e4 100644
--- a/src/lib/log/Makefile.am
+++ b/src/lib/log/Makefile.am
@@ -21,6 +21,8 @@ liblog_la_SOURCES += message_initializer.cc message_initializer.h
liblog_la_SOURCES += message_reader.cc message_reader.h
liblog_la_SOURCES += message_types.h
liblog_la_SOURCES += root_logger_name.cc root_logger_name.h
+liblog_la_SOURCES += log_formatter.h log_formatter.cc
+liblog_la_SOURCES += macros.h
EXTRA_DIST = README
EXTRA_DIST += messagedef.mes
diff --git a/src/lib/log/README b/src/lib/log/README
index 00e4911..529eefc 100644
--- a/src/lib/log/README
+++ b/src/lib/log/README
@@ -94,6 +94,7 @@ An example file could be:
$PREFIX TEST_
$NAMESPACE isc::log
+
% TEST1 message %1 is much too large
This message is a test for the general message code
@@ -141,7 +142,6 @@ Points to note:
are intended to be processed by a separate program and used to generate
an error messages manual. They are ignored by the message compiler.
-
Message Compiler
----------------
The message compiler is a program built in the src/log/compiler directory.
@@ -264,7 +264,7 @@ To use the current version of the logging:
4. Issue logging calls using methods on logger, e.g.
- logger.error(DPS_NSTIMEOUT).arg("isc.org")
+ logger.error(DPS_NSTIMEOUT).arg("isc.org");
(where, in the example above we might have defined the symbol in the message
file with something along the lines of:
diff --git a/src/lib/log/compiler/message.cc b/src/lib/log/compiler/message.cc
index 28e1595..457a62e 100644
--- a/src/lib/log/compiler/message.cc
+++ b/src/lib/log/compiler/message.cc
@@ -527,9 +527,12 @@ main(int argc, char* argv[]) {
string text = e.id();
text += ", ";
text += global.getText(e.id());
-
// Format with arguments
- text = isc::util::str::format(text, e.arguments());
+ vector<string> args(e.arguments());
+ for (size_t i(0); i < args.size(); ++ i) {
+ replacePlaceholder(&text, args[i], i + 1);
+ }
+
cerr << text << "\n";
return 1;
diff --git a/src/lib/log/log_formatter.cc b/src/lib/log/log_formatter.cc
new file mode 100644
index 0000000..18c4741
--- /dev/null
+++ b/src/lib/log/log_formatter.cc
@@ -0,0 +1,42 @@
+// Copyright (C) 2011 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 <log/log_formatter.h>
+
+using namespace std;
+using namespace boost;
+
+namespace isc {
+namespace log {
+
+void
+replacePlaceholder(string* message, const string& arg,
+ const unsigned placeholder)
+{
+ string mark("%" + lexical_cast<string>(placeholder));
+ size_t pos(message->find(mark));
+ if (pos != string::npos) {
+ do {
+ message->replace(pos, mark.size(), arg);
+ pos = message->find(mark, pos + arg.size());
+ } while (pos != string::npos);
+ } else {
+ // We're missing the placeholder, so add some complain
+ message->append(" @@Missing placeholder " + mark + " for '" + arg +
+ "'@@");
+ }
+}
+
+}
+}
diff --git a/src/lib/log/log_formatter.h b/src/lib/log/log_formatter.h
new file mode 100644
index 0000000..07a024c
--- /dev/null
+++ b/src/lib/log/log_formatter.h
@@ -0,0 +1,146 @@
+// Copyright (C) 2011 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 __LOG_FORMATTER_H
+#define __LOG_FORMMATER_H
+
+#include <string>
+#include <boost/lexical_cast.hpp>
+
+namespace isc {
+namespace log {
+
+/// \brief The internal replacement routine
+///
+/// This is used internally by the Formatter. Replaces a placeholder
+/// in the message by replacement. If the placeholder is not found,
+/// it adds a complain at the end.
+void
+replacePlaceholder(std::string* message, const std::string& replacement,
+ const unsigned placeholder);
+
+///
+/// \brief The log message formatter
+///
+/// This class allows us to format logging messages conveniently. We
+/// call something like logger.warn(WARN_MSG).arg(15).arg(dnsMsg). This
+/// outputs some text with placeholders replaced by the arguments, if
+/// the logging verbosity is at WARN level or more.
+///
+/// To make this work, we use the Formatter. The warn (or whatever logging
+/// function) returns a Formatter object. That one holds the string to be
+/// output with the placeholders. It also remembers if there should be any
+/// output at all (eg. if the logging is enabled for this level). When there's
+/// no .arg call on the object, it is destroyed right away and we use the
+/// destructor to output the text (but only in case we should output anything).
+///
+/// If there's an .arg call, we return reference to the same object, so another
+/// .arg can be called on it. After the last .arg call is done, the object is
+/// destroyed and, again, we can produce the output.
+///
+/// Of course, if the logging is turned off, we don't bother with any replacing
+/// and just return.
+///
+/// User of logging code should not really care much about this class, only
+/// call the .arg method to generate the correct output.
+///
+/// The class is a template to allow easy testing. Also, we want everything
+/// here in the header anyway and it doesn't depend on the details of what
+/// Logger really is, so it doesn't hurt anything.
+///
+/// Also, if you are interested in the internals, you might find the copy
+/// constructor a bit strange. It deactivates the original formatter. We don't
+/// really want to support copying of the Formatter by user, but C++ needs a
+/// copy constructor when returning from the logging functions, so we need one.
+/// And if we did not deactivate the original Formatter, that one would get
+/// destroyed before any call to .arg, producing an output, and then the one
+/// the .arg calls are called on would get destroyed as well, producing output
+/// 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.
+ ///
+ /// 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_;
+ Formatter& operator =(const Formatter& other);
+public:
+ /// \brief Constructor of "active" formatter
+ ///
+ /// 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 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 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_)
+ {
+ other.logger_ = false;
+ }
+ /// \brief Destructor.
+ //
+ /// This is the place where output happens if the formatter is active.
+ ~ Formatter() {
+ if (logger_) {
+ logger_->output(prefix_, *message_);
+ delete message_;
+ }
+ }
+ /// \brief Replaces another placeholder
+ ///
+ /// Replaces another placeholder and returns a new formatter with it.
+ /// Deactivates the current formatter. In case the formatter is not active,
+ /// only produces another inactive formatter.
+ ///
+ /// \param arg The argument to place into the placeholder.
+ template<class Arg> Formatter& arg(const Arg& value) {
+ return (arg(boost::lexical_cast<std::string>(value)));
+ }
+ /// \brief String version of arg.
+ Formatter& arg(const std::string& arg) {
+ 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
+ // matters.
+ replacePlaceholder(message_, arg, nextPlaceholder_ ++);
+ }
+ return (*this);
+ }
+};
+
+}
+}
+
+#endif
diff --git a/src/lib/log/logger.cc b/src/lib/log/logger.cc
index 99dc3a1..c340315 100644
--- a/src/lib/log/logger.cc
+++ b/src/lib/log/logger.cc
@@ -112,52 +112,57 @@ Logger::isFatalEnabled() {
// Output methods
void
-Logger::debug(int dbglevel, const isc::log::MessageID& ident, ...) {
+Logger::output(const char* sevText, const string& message) {
+ getLoggerPtr()->outputRaw(sevText, message);
+}
+
+Logger::Formatter
+Logger::debug(int dbglevel, const isc::log::MessageID& ident) {
if (isDebugEnabled(dbglevel)) {
- va_list ap;
- va_start(ap, ident);
- getLoggerPtr()->debug(ident, ap);
- va_end(ap);
+ return (Formatter("DEBUG", getLoggerPtr()->lookupMessage(ident),
+ this));
+ } else {
+ return (Formatter());
}
}
-void
-Logger::info(const isc::log::MessageID& ident, ...) {
+Logger::Formatter
+Logger::info(const isc::log::MessageID& ident) {
if (isInfoEnabled()) {
- va_list ap;
- va_start(ap, ident);
- getLoggerPtr()->info(ident, ap);
- va_end(ap);
+ return (Formatter("INFO ", getLoggerPtr()->lookupMessage(ident),
+ this));
+ } else {
+ return (Formatter());
}
}
-void
-Logger::warn(const isc::log::MessageID& ident, ...) {
+Logger::Formatter
+Logger::warn(const isc::log::MessageID& ident) {
if (isWarnEnabled()) {
- va_list ap;
- va_start(ap, ident);
- getLoggerPtr()->warn(ident, ap);
- va_end(ap);
+ return (Formatter("WARN ", getLoggerPtr()->lookupMessage(ident),
+ this));
+ } else {
+ return (Formatter());
}
}
-void
-Logger::error(const isc::log::MessageID& ident, ...) {
+Logger::Formatter
+Logger::error(const isc::log::MessageID& ident) {
if (isErrorEnabled()) {
- va_list ap;
- va_start(ap, ident);
- getLoggerPtr()->error(ident, ap);
- va_end(ap);
+ return (Formatter("ERROR", getLoggerPtr()->lookupMessage(ident),
+ this));
+ } else {
+ return (Formatter());
}
}
-void
-Logger::fatal(const isc::log::MessageID& ident, ...) {
+Logger::Formatter
+Logger::fatal(const isc::log::MessageID& ident) {
if (isFatalEnabled()) {
- va_list ap;
- va_start(ap, ident);
- getLoggerPtr()->fatal(ident, ap);
- va_end(ap);
+ return (Formatter("FATAL", getLoggerPtr()->lookupMessage(ident),
+ this));
+ } else {
+ return (Formatter());
}
}
diff --git a/src/lib/log/logger.h b/src/lib/log/logger.h
index 88e88e2..6bd8924 100644
--- a/src/lib/log/logger.h
+++ b/src/lib/log/logger.h
@@ -21,6 +21,7 @@
#include <log/debug_levels.h>
#include <log/logger_levels.h>
#include <log/message_types.h>
+#include <log/log_formatter.h>
namespace isc {
namespace log {
@@ -83,10 +84,11 @@ public:
loggerptr_(NULL), name_(name), infunc_(infunc)
{}
-
/// \brief Destructor
virtual ~Logger();
+ /// \brief The formatter used to replace placeholders
+ typedef isc::log::Formatter<Logger> Formatter;
/// \brief Get Name of Logger
///
@@ -157,36 +159,31 @@ public:
/// \param dbglevel Debug level, ranging between 0 and 99. Higher numbers
/// are used for more verbose output.
/// \param ident Message identification.
- /// \param ... Optional arguments for the message.
- void debug(int dbglevel, const MessageID& ident, ...);
+ Formatter debug(int dbglevel, const MessageID& ident);
/// \brief Output Informational Message
///
/// \param ident Message identification.
- /// \param ... Optional arguments for the message.
- void info(const MessageID& ident, ...);
+ Formatter info(const MessageID& ident);
/// \brief Output Warning Message
///
/// \param ident Message identification.
- /// \param ... Optional arguments for the message.
- void warn(const MessageID& ident, ...);
+ Formatter warn(const MessageID& ident);
/// \brief Output Error Message
///
/// \param ident Message identification.
- /// \param ... Optional arguments for the message.
- void error(const MessageID& ident, ...);
+ Formatter error(const MessageID& ident);
/// \brief Output Fatal Message
///
/// \param ident Message identification.
- /// \param ... Optional arguments for the message.
- void fatal(const MessageID& ident, ...);
+ Formatter fatal(const MessageID& ident);
/// \brief Equality
///
@@ -205,6 +202,12 @@ protected:
static void reset();
private:
+ friend class isc::log::Formatter<Logger>;
+ /// \brief Raw output function
+ ///
+ /// This is used by the formatter to output formatted output.
+ void output(const char* sevText, const std::string& message);
+
/// \brief Copy Constructor
///
/// Disabled (marked private) as it makes no sense to copy the logger -
diff --git a/src/lib/log/logger_impl.cc b/src/lib/log/logger_impl.cc
index 41153e9..b30f835 100644
--- a/src/lib/log/logger_impl.cc
+++ b/src/lib/log/logger_impl.cc
@@ -13,6 +13,7 @@
// PERFORMANCE OF THIS SOFTWARE
#include <iostream>
+#include <iomanip>
#include <algorithm>
#include <stdarg.h>
@@ -194,17 +195,14 @@ LoggerImpl::isDebugEnabled(int dbglevel) {
}
// Output a general message
+string*
+LoggerImpl::lookupMessage(const MessageID& ident) {
+ return (new string(string(ident) + ", " +
+ MessageDictionary::globalDictionary().getText(ident)));
+}
void
-LoggerImpl::output(const char* sev_text, const MessageID& ident,
- va_list ap)
-{
- char message[512]; // Should be large enough for any message
-
- // Obtain text of the message and substitute arguments.
- const string format = MessageDictionary::globalDictionary().getText(ident);
- vsnprintf(message, sizeof(message), format.c_str(), ap);
-
+LoggerImpl::outputRaw(const char* sevText, const string& message) {
// Get the time in a struct tm format, and convert to text
time_t t_time;
time(&t_time);
@@ -214,8 +212,8 @@ LoggerImpl::output(const char* sev_text, const MessageID& ident,
(void) strftime(chr_time, sizeof(chr_time), "%Y-%m-%d %H:%M:%S", tm_time);
// Now output.
- std::cout << chr_time << " " << sev_text << " [" << getName() << "] " <<
- ident << ", " << message << "\n";
+ cout << chr_time << " " << sevText << " [" << getName() << "] " <<
+ message << endl;
}
} // namespace log
diff --git a/src/lib/log/logger_impl.h b/src/lib/log/logger_impl.h
index 9fc9cf9..187e478 100644
--- a/src/lib/log/logger_impl.h
+++ b/src/lib/log/logger_impl.h
@@ -167,64 +167,16 @@ public:
}
}
-
- /// \brief Output General Message
- ///
- /// The message is formatted to include the date and time, the severity
- /// and the logger generating the message.
+ /// \brief Raw output
///
- /// \param sev_text Severity level as a text string
- /// \param ident Message identification
- /// \param ap Variable argument list holding message arguments
- void output(const char* sev_text, const MessageID& ident,
- va_list ap);
-
+ /// Writes the message with time into the log. Used by the Formatter
+ /// to produce output.
+ void outputRaw(const char* sev_text, const std::string& message);
- /// \brief Output Debug Message
+ /// \brief Look up message text in dictionary
///
- /// \param ident Message identification.
- /// \param text Text to log
- /// \param ap Variable argument list holding message arguments
- void debug(const MessageID& ident, va_list ap) {
- output("DEBUG", ident, ap);
- }
-
-
- /// \brief Output Informational Message
- ///
- /// \param ident Message identification.
- /// \param text Text to log
- /// \param ap Variable argument list holding message arguments
- void info(const MessageID& ident, va_list ap) {
- output("INFO ", ident, ap);
- }
-
- /// \brief Output Warning Message
- ///
- /// \param ident Message identification.
- /// \param text Text to log
- /// \param ap Variable argument list holding message arguments
- void warn(const MessageID& ident, va_list ap) {
- output("WARN ", ident, ap);
- }
-
- /// \brief Output Error Message
- ///
- /// \param ident Message identification.
- /// \param text Text to log
- /// \param ap Variable argument list holding message arguments
- void error(const MessageID& ident, va_list ap) {
- output("ERROR", ident, ap);
- }
-
- /// \brief Output Fatal Message
- ///
- /// \param ident Message identification.
- /// \param text Text to log
- /// \param ap Variable argument list holding message arguments
- void fatal(const MessageID& ident, va_list ap) {
- output("FATAL", ident, ap);
- }
+ /// This gets you the unformatted text of message for given ID.
+ std::string* lookupMessage(const MessageID& id);
/// \brief Equality
///
diff --git a/src/lib/log/logger_support.cc b/src/lib/log/logger_support.cc
index 372b43d..b744440 100644
--- a/src/lib/log/logger_support.cc
+++ b/src/lib/log/logger_support.cc
@@ -63,7 +63,7 @@ readLocalMessageFile(const char* file) {
MessageDictionary& dictionary = MessageDictionary::globalDictionary();
MessageReader reader(&dictionary);
try {
- logger.info(MSG_RDLOCMES, file);
+ logger.info(MSG_RDLOCMES).arg(file);
reader.readFile(file, MessageReader::REPLACE);
// File successfully read, list the duplicates
@@ -71,7 +71,7 @@ readLocalMessageFile(const char* file) {
for (MessageReader::MessageIDCollection::const_iterator
i = unknown.begin(); i != unknown.end(); ++i) {
string message_id = boost::lexical_cast<string>(*i);
- logger.warn(MSG_IDNOTFND, message_id.c_str());
+ logger.warn(MSG_IDNOTFND).arg(message_id);
}
}
catch (MessageException& e) {
@@ -83,15 +83,15 @@ readLocalMessageFile(const char* file) {
break;
case 1:
- logger.error(ident, args[0].c_str());
+ logger.error(ident).arg(args[0]);
break;
case 2:
- logger.error(ident, args[0].c_str(), args[1].c_str());
+ logger.error(ident).arg(args[0]).arg(args[1]);
break;
default: // 3 or more (3 should be the maximum)
- logger.error(ident, args[0].c_str(), args[1].c_str(), args[2].c_str());
+ logger.error(ident).arg(args[0]).arg(args[1]).arg(args[2]);
}
}
}
@@ -122,7 +122,7 @@ initLogger(const string& root, isc::log::Severity severity, int dbglevel,
vector<string>::iterator new_end =
unique(duplicates.begin(), duplicates.end());
for (vector<string>::iterator i = duplicates.begin(); i != new_end; ++i) {
- logger.warn(MSG_DUPMSGID, (*i).c_str());
+ logger.warn(MSG_DUPMSGID).arg(*i);
}
}
diff --git a/src/lib/log/macros.h b/src/lib/log/macros.h
new file mode 100644
index 0000000..3128131
--- /dev/null
+++ b/src/lib/log/macros.h
@@ -0,0 +1,50 @@
+// Copyright (C) 2011 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 __LOG_MACROS_H
+#define __LOG_MACROS_H
+
+#include <log/logger.h>
+
+/// \brief Macro to conveniently test debug output and log it
+#define LOG_DEBUG(LOGGER, LEVEL, MESSAGE) \
+ if (!(LOGGER).isDebugEnabled((LEVEL))) { \
+ } else \
+ (LOGGER).debug((LEVEL), (MESSAGE))
+
+/// \brief Macro to conveniently test info output and log it
+#define LOG_INFO(LOGGER, MESSAGE) \
+ if (!(LOGGER).isInfoEnabled()) { \
+ } else \
+ (LOGGER).info((MESSAGE))
+
+/// \brief Macro to conveniently test warn output and log it
+#define LOG_WARN(LOGGER, MESSAGE) \
+ if (!(LOGGER).isWarnEnabled()) { \
+ } else \
+ (LOGGER).warn((MESSAGE))
+
+/// \brief Macro to conveniently test error output and log it
+#define LOG_ERROR(LOGGER, MESSAGE) \
+ if (!(LOGGER).isErrorEnabled()) { \
+ } else \
+ (LOGGER).error((MESSAGE))
+
+/// \brief Macro to conveniently test fatal output and log it
+#define LOG_FATAL(LOGGER, MESSAGE) \
+ if (!(LOGGER).isFatalEnabled()) { \
+ } else \
+ (LOGGER).fatal((MESSAGE))
+
+#endif
diff --git a/src/lib/log/messagedef.cc b/src/lib/log/messagedef.cc
index 3082a97..5cc89b3 100644
--- a/src/lib/log/messagedef.cc
+++ b/src/lib/log/messagedef.cc
@@ -1,4 +1,4 @@
-// File created from messagedef.mes on Mon May 9 12:54:57 2011
+// File created from messagedef.mes on Mon May 9 13:52:54 2011
#include <cstddef>
#include <log/message_types.h>
@@ -31,23 +31,23 @@ extern const isc::log::MessageID MSG_WRITERR = "MSG_WRITERR";
namespace {
const char* values[] = {
- "MSG_DUPLNS", "line %s: duplicate $NAMESPACE directive found",
- "MSG_DUPMSGID", "duplicate message ID (%s) in compiled code",
- "MSG_IDNOTFND", "could not replace message text for '%s': no such message",
- "MSG_INVMSGID", "line %s: invalid message identification '%s'",
- "MSG_NOMSGID", "line %s: message definition line found without a message ID",
- "MSG_NOMSGTXT", "line %s: line found containing a message ID ('%s') and no text",
- "MSG_NSEXTRARG", "line %s: $NAMESPACE directive has too many arguments",
- "MSG_NSINVARG", "line %s: $NAMESPACE directive has an invalid argument ('%s')",
- "MSG_NSNOARG", "line %s: no arguments were given to the $NAMESPACE directive",
- "MSG_OPENIN", "unable to open message file %s for input: %s",
- "MSG_OPENOUT", "unable to open %s for output: %s",
- "MSG_PRFEXTRARG", "line %s: $PREFIX directive has too many arguments",
- "MSG_PRFINVARG", "line %s: $PREFIX directive has an invalid argument ('%s')",
- "MSG_RDLOCMES", "reading local message file %s",
- "MSG_READERR", "error reading from message file %s: %s",
- "MSG_UNRECDIR", "line %s: unrecognised directive '%s'",
- "MSG_WRITERR", "error writing to %s: %s",
+ "MSG_DUPLNS", "line %1: duplicate $NAMESPACE directive found",
+ "MSG_DUPMSGID", "duplicate message ID (%1) in compiled code",
+ "MSG_IDNOTFND", "could not replace message text for '%1': no such message",
+ "MSG_INVMSGID", "line %1: invalid message identification '%2'",
+ "MSG_NOMSGID", "line %1: message definition line found without a message ID",
+ "MSG_NOMSGTXT", "line %1: line found containing a message ID ('%2') and no text",
+ "MSG_NSEXTRARG", "line %1: $NAMESPACE directive has too many arguments",
+ "MSG_NSINVARG", "line %1: $NAMESPACE directive has an invalid argument ('%2')",
+ "MSG_NSNOARG", "line %1: no arguments were given to the $NAMESPACE directive",
+ "MSG_OPENIN", "unable to open message file %1 for input: %2",
+ "MSG_OPENOUT", "unable to open %1 for output: %2",
+ "MSG_PRFEXTRARG", "line %1: $PREFIX directive has too many arguments",
+ "MSG_PRFINVARG", "line %1: $PREFIX directive has an invalid argument ('%2')",
+ "MSG_RDLOCMES", "reading local message file %1",
+ "MSG_READERR", "error reading from message file %1: %2",
+ "MSG_UNRECDIR", "line %1: unrecognised directive '%2'",
+ "MSG_WRITERR", "error writing to %1: %2",
NULL
};
diff --git a/src/lib/log/messagedef.h b/src/lib/log/messagedef.h
index f73618c..79c8bab 100644
--- a/src/lib/log/messagedef.h
+++ b/src/lib/log/messagedef.h
@@ -1,4 +1,4 @@
-// File created from messagedef.mes on Mon May 9 12:54:57 2011
+// File created from messagedef.mes on Mon May 9 13:52:54 2011
#ifndef __MESSAGEDEF_H
#define __MESSAGEDEF_H
diff --git a/src/lib/log/messagedef.mes b/src/lib/log/messagedef.mes
index 68df947..51c04fa 100644
--- a/src/lib/log/messagedef.mes
+++ b/src/lib/log/messagedef.mes
@@ -23,7 +23,7 @@
$PREFIX MSG_
$NAMESPACE isc::log
-% DUPMSGID duplicate message ID (%s) in compiled code
+% DUPMSGID duplicate message ID (%1) in compiled code
Indicative of a programming error, when it started up, BIND10 detected that
the given message ID had been registered by one or more modules. (All message
IDs should be unique throughout BIND10.) This has no impact on the operation
@@ -34,12 +34,12 @@ particular message, the text supplied by the module that added the original
ID will be output - something that may bear no relation to the condition being
logged.
-% DUPLNS line %s: duplicate $NAMESPACE directive found
+% DUPLNS line %1: duplicate $NAMESPACE directive found
When reading a message file, more than one $NAMESPACE directive was found. In
this version of the code, such a condition is regarded as an error and the
read will be abandoned.
-% IDNOTFND could not replace message text for '%s': no such message
+% IDNOTFND could not replace message text for '%1': no such message
During start-up a local message file was read. A line with the listed
message identification was found in the file, but the identification is not
one contained in the compiled-in message dictionary. Either the message
@@ -50,70 +50,70 @@ identification has been removed.
This message may appear a number of times in the file, once for every such
unknown message identification.
-% INVMSGID line %s: invalid message identification '%s'
+% INVMSGID line %1: invalid message identification '%2'
The concatenation of the prefix and the message identification is used as
a symbol in the C++ module; as such it may only contain
-% NOMSGID line %s: message definition line found without a message ID
+% NOMSGID line %1: message definition line found without a message ID
Message definition lines are lines starting with a "%". The rest of the line
should comprise the message ID and text describing the message. This error
indicates the message compiler found a line in the message file comprising
just the "%" and nothing else.
-% NOMSGTXT line %s: line found containing a message ID ('%s') and no text
+% NOMSGTXT line %1: line found containing a message ID ('%2') and no text
Message definition lines are lines starting with a "%". The rest of the line
should comprise the message ID and text describing the message. This error
is generated when a line is found in the message file that contains the
leading "%" and the message identification but no text.
-% NSEXTRARG line %s: $NAMESPACE directive has too many arguments
+% NSEXTRARG line %1: $NAMESPACE directive has too many arguments
The $NAMESPACE directive takes a single argument, a namespace in which all the
generated symbol names are placed. This error is generated when the
compiler finds a $NAMESPACE directive with more than one argument.
-% NSINVARG line %s: $NAMESPACE directive has an invalid argument ('%s')
+% NSINVARG line %1: $NAMESPACE directive has an invalid argument ('%2')
The $NAMESPACE argument should be a valid C++ namespace. The reader does a
cursory check on its validity, checking that the characters in the namespace
are correct. The error is generated when the reader finds an invalid
character. (Valid are alphanumeric characters, underscores and colons.)
-% NSNOARG line %s: no arguments were given to the $NAMESPACE directive
+% NSNOARG line %1: no arguments were given to the $NAMESPACE directive
The $NAMESPACE directive takes a single argument, a namespace in which all the
generated symbol names are placed. This error is generated when the
compiler finds a $NAMESPACE directive with no arguments.
-% OPENIN unable to open message file %s for input: %s
+% OPENIN unable to open message file %1 for input: %2
The program was not able to open the specified input message file for the
reason given.
-% OPENOUT unable to open %s for output: %s
+% OPENOUT unable to open %1 for output: %2
The program was not able to open the specified output file for the reason
given.
-% PRFEXTRARG line %s: $PREFIX directive has too many arguments
+% PRFEXTRARG line %1: $PREFIX directive has too many arguments
The $PREFIX directive takes a single argument, a prefix to be added to the
symbol names when a C++ .h file is created. This error is generated when the
compiler finds a $PREFIX directive with more than one argument.
-% PRFINVARG line %s: $PREFIX directive has an invalid argument ('%s')
+% PRFINVARG line %1: $PREFIX directive has an invalid argument ('%2')
The $PREFIX argument is used in a symbol name in a C++ header file. As such,
it must adhere to restrictions on C++ symbol names (e.g. may only contain
alphanumeric characters or underscores, and may nor start with a digit).
A $PREFIX directive was found with an argument (given in the message) that
violates those restictions.
-% RDLOCMES reading local message file %s
+% RDLOCMES reading local message file %1
This is an informational message output by BIND10 when it starts to read a
local message file. (A local message file may replace the text of one of more
messages; the ID of the message will not be changed though.)
-% READERR error reading from message file %s: %s
+% READERR error reading from message file %1: %2
The specified error was encountered reading from the named message file.
-% WRITERR error writing to %s: %s
+% WRITERR error writing to %1: %2
The specified error was encountered by the message compiler when writing to
the named output file.
-% UNRECDIR line %s: unrecognised directive '%s'
+% UNRECDIR line %1: unrecognised directive '%2'
A line starting with a dollar symbol was found, but the first word on the line
(shown in the message) was not a recognised message compiler directive.
diff --git a/src/lib/log/tests/Makefile.am b/src/lib/log/tests/Makefile.am
index b9fe150..46065e8 100644
--- a/src/lib/log/tests/Makefile.am
+++ b/src/lib/log/tests/Makefile.am
@@ -22,6 +22,7 @@ run_unittests_SOURCES += message_reader_unittest.cc
run_unittests_SOURCES += message_initializer_unittest.cc
run_unittests_SOURCES += message_initializer_unittest_2.cc
run_unittests_SOURCES += run_unittests.cc
+run_unittests_SOURCES += log_formatter_unittest.cc
run_unittests_CPPFLAGS = $(AM_CPPFLAGS) $(GTEST_INCLUDES)
run_unittests_LDFLAGS = $(AM_LDFLAGS) $(GTEST_LDFLAGS)
diff --git a/src/lib/log/tests/log_formatter_unittest.cc b/src/lib/log/tests/log_formatter_unittest.cc
new file mode 100644
index 0000000..b67831a
--- /dev/null
+++ b/src/lib/log/tests/log_formatter_unittest.cc
@@ -0,0 +1,125 @@
+// Copyright (C) 2011 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 <gtest/gtest.h>
+#include <log/log_formatter.h>
+
+#include <vector>
+#include <string>
+
+using namespace std;
+
+namespace {
+
+class FormatterTest : public ::testing::Test {
+protected:
+ typedef pair<const char*, string> Output;
+ typedef isc::log::Formatter<FormatterTest> Formatter;
+ vector<Output> outputs;
+public:
+ void output(const char* prefix, const string& message) {
+ outputs.push_back(Output(prefix, message));
+ }
+ // Just shortcut for new string
+ string* s(const char* text) {
+ return (new string(text));
+ }
+};
+
+// Create an inactive formatter and check it doesn't produce any output
+TEST_F(FormatterTest, inactive) {
+ Formatter();
+ EXPECT_EQ(0, outputs.size());
+}
+
+// Create an active formatter and check it produces output. Does no arg
+// substitution yet
+TEST_F(FormatterTest, active) {
+ 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);
+}
+
+// No output even when we have an arg on the inactive formatter
+TEST_F(FormatterTest, inactiveArg) {
+ Formatter().arg("Hello");
+ EXPECT_EQ(0, outputs.size());
+}
+
+// Create an active formatter and replace a placeholder with string
+TEST_F(FormatterTest, stringArg) {
+ {
+ SCOPED_TRACE("C++ string");
+ 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"), this).arg(string("Internet"));
+ ASSERT_EQ(2, outputs.size());
+ EXPECT_STREQ("TEST", outputs[1].first);
+ EXPECT_EQ("Hello Internet", outputs[1].second);
+ }
+}
+
+// Can convert to string
+TEST_F(FormatterTest, intArg) {
+ 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);
+}
+
+// Can use multiple arguments at different places
+TEST_F(FormatterTest, multiArg) {
+ Formatter("TEST", s("The %2 are %1"), this).arg("switched").
+ arg("arguments");
+ ASSERT_EQ(1, outputs.size());
+ EXPECT_STREQ("TEST", outputs[0].first);
+ EXPECT_EQ("The arguments are switched", outputs[0].second);
+}
+
+// Can survive and complains if placeholder is missing
+TEST_F(FormatterTest, missingPlace) {
+ 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);
+ EXPECT_EQ("Missing the first argument "
+ "@@Missing placeholder %1 for 'missing'@@", outputs[0].second);
+}
+
+// Can replace multiple placeholders
+TEST_F(FormatterTest, multiPlaceholder) {
+ 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);
+ EXPECT_EQ("The first rule of tautology club is "
+ "the first rule of tautology club", outputs[0].second);
+}
+
+// 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"), this).arg("%1 %1");
+ ASSERT_EQ(1, outputs.size());
+ EXPECT_STREQ("TEST", outputs[0].first);
+ EXPECT_EQ("%1 %1", outputs[0].second);
+}
+
+}
diff --git a/src/lib/log/tests/logger_support_test.cc b/src/lib/log/tests/logger_support_test.cc
index d1a10fb..0a2338b 100644
--- a/src/lib/log/tests/logger_support_test.cc
+++ b/src/lib/log/tests/logger_support_test.cc
@@ -23,6 +23,7 @@
#include <iostream>
#include <log/logger.h>
+#include <log/macros.h>
#include <log/logger_support.h>
#include <log/root_logger_name.h>
@@ -92,13 +93,14 @@ int main(int argc, char** argv) {
initLogger("alpha", severity, dbglevel, localfile);
// Log a few messages
- logger_ex.fatal(MSG_WRITERR, "test1", "42");
- logger_ex.error(MSG_RDLOCMES, "dummy/file");
- logger_dlm.warn(MSG_READERR, "a.txt", "dummy reason");
- logger_dlm.info(MSG_OPENIN, "example.msg", "dummy reason");
- logger_ex.debug( 0, MSG_RDLOCMES, "dummy/0");
- logger_ex.debug(24, MSG_RDLOCMES, "dummy/24");
- logger_ex.debug(25, MSG_RDLOCMES, "dummy/25");
- logger_ex.debug(26, MSG_RDLOCMES, "dummy/26");
+ LOG_FATAL(logger_ex, MSG_WRITERR).arg("test1").arg("42");
+ LOG_ERROR(logger_ex, MSG_RDLOCMES).arg("dummy/file");
+ LOG_WARN(logger_dlm, MSG_READERR).arg("a.txt").arg("dummy reason");
+ LOG_INFO(logger_dlm, MSG_OPENIN).arg("example.msg").arg("dummy reason");
+ LOG_DEBUG(logger_ex, 0, MSG_RDLOCMES).arg("dummy/0");
+ LOG_DEBUG(logger_ex, 24, MSG_RDLOCMES).arg("dummy/24");
+ LOG_DEBUG(logger_ex, 25, MSG_RDLOCMES).arg("dummy/25");
+ LOG_DEBUG(logger_ex, 26, MSG_RDLOCMES).arg("dummy/26");
+
return (0);
}
diff --git a/src/lib/log/tests/run_time_init_test.sh.in b/src/lib/log/tests/run_time_init_test.sh.in
index fc7c463..e48a781 100755
--- a/src/lib/log/tests/run_time_init_test.sh.in
+++ b/src/lib/log/tests/run_time_init_test.sh.in
@@ -31,8 +31,8 @@ passfail() {
cat > $localmes << .
\$PREFIX MSG_
% NOTHERE this message is not in the global dictionary
-% READERR replacement read error, parameters: '%s' and '%s'
-% RDLOCMES replacement read local message file, parameter is '%s'
+% READERR replacement read error, parameters: '%1' and '%2'
+% RDLOCMES replacement read local message file, parameter is '%1'
.
echo -n "1. runInitTest default parameters: "
diff --git a/src/lib/util/unittests/testdata.h b/src/lib/util/unittests/testdata.h
index ed2722e..03bd83a 100644
--- a/src/lib/util/unittests/testdata.h
+++ b/src/lib/util/unittests/testdata.h
@@ -34,6 +34,14 @@ void addTestDataPath(const std::string& path);
/// addTestDataPath(). On success, ifs will be ready for reading the data
/// stored in 'datafile'. If the data file cannot be open with any of the
/// registered paths, a runtime_error exception will be thrown.
+///
+/// \note Care should be taken if you want to reuse the same single \c ifs
+/// for multiple input data. Some standard C++ library implementations retain
+/// the failure bit if the first stream reaches the end of the first file,
+/// and make the second call to \c ifstream::open() fail. The safest way
+/// is to use a different \c ifstream object for a new call to this function;
+/// alternatively make sure you explicitly clear the error bit by calling
+/// \c ifstream::clear() on \c ifs.
void openTestData(const char* const datafile, std::ifstream& ifs);
}
}
More information about the bind10-changes
mailing list