[trac736] address review comments (the trivial ones)

src/bin/cfgmgr/plugins/b10logging.py

@@ -46,19 +46,29 @@ def check(config):
     # The 'layout' is ok, now check for specific values
     if 'loggers' in config:
         for logger in config['loggers']:
+            # name should always be present
+            name = logger['name']
+
             if 'severity' in logger and\
                logger['severity'].lower() not in ALLOWED_SEVERITIES:
-                errors.append("bad severity value " + logger['severity'])
+                errors.append("bad severity value for logger " + name +
+                              ": " + logger['severity'])
             if 'output_options' in logger:
                 for output_option in logger['output_options']:
                     if 'destination' in output_option:
                         if output_option['destination'].lower() not in ALLOWED_DESTINATIONS:
-                            errors.append("bad destination: " + output_option['destination'])
-                        if 'filename' not in output_option:
-                            errors.append("destination set to file but no filename specified")
+                            errors.append("bad destination for logger " +
+                            name + ": " + output_option['destination'])
+                        if output_option['destination'].lower() == "file" and\
+                           ('filename' not in output_option or
+                            output_option('filename') == ""):
+                            errors.append("destination set to file but "
+                                          "no filename specified for"
+                                          "logger " + name)
                     if 'stream' in output_option and\
                        output_option['stream'].lower() not in ALLOWED_STREAMS:
-                        errors.append("bad stream: " + output_option['stream'])
+                        errors.append("bad stream for logger " + name +
+                                      ": " + output_option['stream'])
 
     if errors:
         return ', '.join(errors)

src/lib/config/ccsession.h

@@ -161,9 +161,6 @@ public:
      * configuration of the local module needs to be updated.
      * This must refer to a valid object of a concrete derived class of
      * AbstractSession without establishing the session.
-     * @param handle_logging If true, the ModuleCCSession will automatically
-     * take care of logging configuration through the virtual Logging config
-     * module.
      *
      * Note: the design decision on who is responsible for establishing the
      * session is in flux, and may change in near future.
@@ -175,11 +172,14 @@ public:
      * @param command_handler A callback function pointer to be called when
      * a control command from a remote agent needs to be performed on the
      * local module.
-     * @start_immediately If true (default), start listening to new commands
+     * @param start_immediately If true (default), start listening to new commands
      * and configuration changes asynchronously at the end of the constructor;
      * if false, it will be delayed until the start() method is explicitly
      * called. (This is a short term workaround for an initialization trouble.
      * We'll need to develop a cleaner solution, and then remove this knob)
+     * @param handle_logging If true, the ModuleCCSession will automatically
+     * take care of logging configuration through the virtual Logging config
+     * module.
      */
     ModuleCCSession(const std::string& spec_file_name,
                     isc::cc::AbstractSession& session,

src/lib/config/config_data.cc

@@ -23,7 +23,9 @@ using namespace isc::data;
 
 namespace {
 
-// Returns the '_spec' part of a list or map specification
+// Returns the '_spec' part of a list or map specification (recursively,
+// i.e. if it is a list of lists or maps, will return the spec of the
+// inner-most list or map).
 //
 // \param spec_part the list or map specification (part)
 // \return the value of spec_part's "list_item_spec" or "map_item_spec",