BIND 10 trac1003, updated. 07e015d587c487ce1934144abe59010b8f588c81 [trac1003] Modifications as a result of review.

BIND 10 source code commits bind10-changes at lists.isc.org
Fri Jul 22 12:24:29 UTC 2011


The branch, trac1003 has been updated
       via  07e015d587c487ce1934144abe59010b8f588c81 (commit)
      from  253a3fad875abba510e13a3112b6176b9e272e84 (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 07e015d587c487ce1934144abe59010b8f588c81
Author: Stephen Morris <stephen at isc.org>
Date:   Fri Jul 22 13:24:06 2011 +0100

    [trac1003] Modifications as a result of review.

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

Summary of changes:
 src/lib/config/ccsession.cc                 |   61 ++++++++++++++-------------
 src/lib/config/config_messages.mes          |    4 +-
 src/lib/config/tests/ccsession_unittests.cc |   11 +++--
 3 files changed, 39 insertions(+), 37 deletions(-)

-----------------------------------------------------------------------
diff --git a/src/lib/config/ccsession.cc b/src/lib/config/ccsession.cc
index 2890e22..ac85077 100644
--- a/src/lib/config/ccsession.cc
+++ b/src/lib/config/ccsession.cc
@@ -180,31 +180,30 @@ ConstElementPtr getValueOrDefault(ConstElementPtr config_part,
 
 // Prefix name with "b10-".
 //
-// Root logger names are based on the name of the binary they're from (e.g.
-// b10-resolver). This, however, is not how they appear internally (in for
-// instance bindctl, where a module name is based on what is specified in
-// the .spec file (e.g. Resolver)).
+// In BIND 10, modules have names taken from the .spec file, which are typically
+// names starting with a capital letter (e.g. "Resolver", "Auth" etc.).  The
+// names of the associated binaries are derived from the module names, being
+// prefixed "b10-" and having the first letter of the module name lower-cased
+// (e.g. "b10-resolver", "b10-auth").  (It is a required convention that there
+// be this relationship between the names.)
 //
-// This function prefixes the name read in the configuration with 'b10-" and
-// leaves the module code as it is. (It is now a required convention that the
-// name from the specfile and the actual binary name should match).  To take
-// account of the use of capital letters in module names in bindctl, the first
-// letter of the name read in is lower-cased.
+// Within the binaries the root loggers are named after the binaries themselves.
+// (The reason for this is that the name of the logger is included in the
+// message logged, so making it clear which message comes from which BIND 10
+// process.) As logging is configured using module names, the configuration code
+// has to match these with the corresponding logger names. This function
+// converts a module name to a root logger name by lowercasing the first letter
+// of the module name and prepending "b10-".
 //
-// In this way, you configure resolver logging with the name "resolver" and in
-// the printed output it becomes "b10-resolver".
+// \param instring String to convert.  (This may be empty, in which case
+//        "b10-" will be returned.)
 //
-// To allow for (a) people using b10-resolver in the configuration instead of
-// "resolver" and (b) that fact that during the resolution of wildcards in
-
-//
-// \param instring String to prefix.  Lowercase the first character and apply
-//        the prefix.  If empty, "b10-" is returned.
+// \return Converted string.
 std::string
 b10Prefix(const std::string& instring) {
     std::string result = instring;
     if (!result.empty()) {
-        result[0] = static_cast<char>(tolower(result[0]));
+        result[0] = tolower(result[0]);
     }
     return (std::string("b10-") + result);
 }
@@ -282,18 +281,20 @@ readLoggersConf(std::vector<isc::log::LoggerSpecification>& specs,
     specs.push_back(logger_spec);
 }
 
-// Copies the map for a logger, changing the of the logger.  This is
-// used because the logger being copied is "const", but we want to
-// change a top-level name, so need to create a new one.
-
-ElementPtr
+// Copies the map for a logger, changing the name of the logger in the process.
+// This is used because the map being copied is "const", so in order to
+// change the name we need to create a new one.
+//
+// \param cur_logger Logger being copied.
+// \param new_name New value of the "name" element at the top level.
+//
+// \return Pointer to the map with the updated element.
+ConstElementPtr
 copyLogger(ConstElementPtr& cur_logger, const std::string& new_name) {
 
+    // Since we'll only be updating one first-level element and subsequent
+    // use won't change the contents of the map, a shallow map copy is enough.
     ElementPtr new_logger(Element::createMap());
-
-    // since we'll only be updating one first-level element,
-    // and we return as const again, a shallow map copy is
-    // enough
     new_logger->setValue(cur_logger->mapValue());
     new_logger->set("name", Element::create(new_name));
 
@@ -394,7 +395,7 @@ ModuleSpec
 ModuleCCSession::readModuleSpecification(const std::string& filename) {
     std::ifstream file;
     ModuleSpec module_spec;
-    
+
     // this file should be declared in a @something@ directive
     file.open(filename.c_str());
     if (!file) {
@@ -461,7 +462,7 @@ ModuleCCSession::ModuleCCSession(
         LOG_ERROR(config_logger, CONFIG_MOD_SPEC_REJECT).arg(answer->str());
         isc_throw(CCSessionInitError, answer->str());
     }
-    
+
     setLocalConfig(Element::fromJSON("{}"));
     // get any stored configuration from the manager
     if (config_handler_) {
@@ -587,7 +588,7 @@ int
 ModuleCCSession::checkCommand() {
     ConstElementPtr cmd, routing, data;
     if (session_.group_recvmsg(routing, data, true)) {
-        
+
         /* ignore result messages (in case we're out of sync, to prevent
          * pingpongs */
         if (data->getType() != Element::map || data->contains("result")) {
diff --git a/src/lib/config/config_messages.mes b/src/lib/config/config_messages.mes
index 53cb410..c439edd 100644
--- a/src/lib/config/config_messages.mes
+++ b/src/lib/config/config_messages.mes
@@ -41,7 +41,7 @@ running configuration manager.
 This is a debug message.  When processing the "loggers" part of the
 configuration file, the configuration library found an entry for the named
 logger that matches the logger specification for the program.  The logging
-configuration for the program will updated with the information.
+configuration for the program will be updated with the information.
 
 % CONFIG_LOG_IGNORE_EXPLICIT ignoring logging configuration for explicitly-named logger %1
 This is a debug message.  When processing the "loggers" part of the
@@ -60,7 +60,7 @@ This is a debug message.  When processing the "loggers" part of
 the configuration file, the configuration library found the named
 wildcard entry (one containing the "*" character) that matches a logger
 specification in the program. The logging configuration for the program
-will updated with the information.
+will be updated with the information.
 
 % CONFIG_JSON_PARSE JSON parse error in %1: %2
 There was an error parsing the JSON file. The given file does not appear
diff --git a/src/lib/config/tests/ccsession_unittests.cc b/src/lib/config/tests/ccsession_unittests.cc
index bf8cc3d..5ea4f32 100644
--- a/src/lib/config/tests/ccsession_unittests.cc
+++ b/src/lib/config/tests/ccsession_unittests.cc
@@ -21,7 +21,6 @@
 #include <config/ccsession.h>
 
 #include <fstream>
-#include <iostream>
 
 #include <config/tests/data_def_unittests_config.h>
 
@@ -45,20 +44,21 @@ el(const std::string& str) {
 
 class CCSessionTest : public ::testing::Test {
 protected:
-    CCSessionTest() : session(el("[]"), el("[]"), el("[]")) {
+    CCSessionTest() : session(el("[]"), el("[]"), el("[]")),
+                      root_name(isc::log::getRootLoggerName())
+    {
         // upon creation of a ModuleCCSession, the class
         // sends its specification to the config manager.
         // it expects an ok answer back, so everytime we
         // create a ModuleCCSession, we must set an initial
         // ok answer.
         session.getMessages()->add(createAnswer());
-        root_name = isc::log::getRootLoggerName();
     }
     ~CCSessionTest() {
         isc::log::setRootLoggerName(root_name);
     }
     FakeSession session;
-    std::string root_name;
+    const std::string root_name;
 };
 
 TEST_F(CCSessionTest, createAnswer) {
@@ -693,7 +693,8 @@ TEST(LogConfigTest, relatedLoggersTest) {
     doRelatedLoggersTest("[ { \"name\": \"*\", \"severity\": \"DEBUG\" },"
                          "  { \"name\": \"some_module\", \"severity\": \"WARN\"}]",
                          "[ { \"name\": \"b10-test\", \"severity\": \"DEBUG\"} ]");
-
+    doRelatedLoggersTest("[ { \"name\": \"b10-test\" }]",
+                         "[]");
     // make sure 'bad' things like '*foo.x' or '*lib' are ignored
     // (cfgmgr should have already caught it in the logconfig plugin
     // check, and is responsible for reporting the error)




More information about the bind10-changes mailing list