[trac900] Updated tests

Also updated some message IDs - since the message logged now includes the
prefix, there is no need to suggest the prefix in the ID itself (e.g.
MSGRDERR now becomes READERR as it will be output as MSG_READERR).

src/lib/log/README

@@ -44,7 +44,7 @@ stored definitions; this updated text is logged.  However, to aid support,
 the message identifier so in the example above, the message finally logged
 would be something like:
 
-    OPENIN, unable to open a.txt for input
+    FAC_OPENIN, unable to open a.txt for input
 
 
 Using The System
@@ -55,17 +55,15 @@ The steps in using the system are:
    mnemonic for the message, typically 6-12 characters long - and a message.
    The file is described in more detail below.
 
-   Ideally the file should have a file type of ".msg".
+   Ideally the file should have a file type of ".mes".
 
-2. Run it through the message compiler to produce the .h and .cc files.  It
-   is intended that this step be included in the build process.  However,
-   for now run the compiler (found in the "compiler" subdirectory) manually.
-   The only argument is the name of the message file: it will produce as
-   output two files, having the same name as the input file but with file
-   types of ".h" and ".cc".
-
-   The compiler is built in the "compiler" subdirectory of the "src/lib/log"
-   directory.
+2. Run it through the message compiler to produce the .h and .cc files.  This
+   step should be included in the build process. (For an example of how to
+   do this, see src/lib/nsas/Makefile.am.) During development though, the
+   message compiler (found in the "src/lib/log/compiler" directory) will need
+   to be run manually.  The only argument is the name of the message file: it
+   will produce as output two files in the default directory, having the same
+   name as the input file but with file types of ".h" and ".cc".
 
 3. Include the .h file in your source code to define message symbols, and
    make sure that the .cc file is compiled and linked into your program -
@@ -96,11 +94,11 @@ An example file could be:
 
 $PREFIX TEST_
 $NAMESPACE isc::log
-TEST1       message %s is much too large
-+ This message is a test for the general message code
+% TEST1       message %1 is much too large
+This message is a test for the general message code
 
-UNKNOWN     unknown message
-+ Issued when the message is unknown.
+% UNKNOWN     unknown message
+Issued when the message is unknown.
 
 -- END --
 
@@ -117,22 +115,31 @@ Points to note:
 
 * Lines starting $ are directives.  At present, two directives are recognised:
 
-  * $PREFIX, which has one argument: the string used to prefix symbols.  If
-    absent, there is no prefix to the symbols. (Prefixes are explained below.)
+  * $PREFIX, which has one optional argument: the string used to prefix symbols.
+    If absent, there is no prefix to the symbols. (Prefixes are explained below.)
+
   * $NAMESPACE, which has one argument: the namespace in which the symbols are
-    created.  In the absence of a $NAMESPACE directive, symbols will be put
-    in the global namespace.
+    created.  In the absence of a $NAMESPACE directive, symbols will be put in
+    the anonymous namespace.
 
-* Lines starting + indicate an explanation for the preceding message.  These
-  are intended to be processed by a separate program and used to generate
-  an error messages manual.  However they are treated like comments by the
-  message compiler.  As with comments, these must be on a line by themselves;
-  if inline, the text (including the leading "+") will be interpreted as
-  part of the line.
+* Message lines.  These start with a "%" and are followed by the message
+  identification and the message text, the latter including zero or more
+  replacement tokens, e.g.
+
+     % TEST  message %1 is larger than the permitted length of %2
 
-* Message lines.  These comprise a symbol name and a message, which may
-  include zero or more printf-style tokens.  Symbol names will be upper-cased
-  by the compiler.
+  * There may be zero or more spaces between the leading "%" and the message
+    identification (which, in the example above, is the word "TEST").
+
+  * The replacement tokens are the strings "%1", "%2" etc.  When a message
+    is logged, these are replaced with the arguments passed to the logging
+    call: %1 refers to the first argument, %2 to the second etc.  Within the
+    message text, the placeholders can appear in any order, and placeholders
+    can be repeated.
+     
+* Remaining lines indicate an explanation for the preceding message.  These
+  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
@@ -153,9 +160,8 @@ the form:
       :
    }
 
-The symbols define the keys in the global message dictionary.
-
-The namespace enclosing the symbols is set by the $NAMESPACE directive.
+The symbols define the keys in the global message dictionary, with the
+namespace enclosing the symbols set by the $NAMESPACE directive.
 
 The "PREFIX_" part of the symbol name is the string defined in the $PREFIX
 the argument to the directive.  So "$PREFIX MSG_" would prefix the identifier
@@ -163,12 +169,20 @@ ABC with "MSG_" to give the symbol MSG_ABC.  Similarly "$PREFIX E" would
 prefix it with "E" to give the symbol EABC.  If no $PREFIX is given, no
 prefix appears (so the symbol in this example would be ABC).
 
+The prefix is "syntactic sugar".  Generally all symbols in a given message file
+will be prefixed with the same set of letters.  By extracting these into
+a separate prefix, it becomes easier to disambiguate the different symbols.
+
+There may be multiple $PREFIX directives in a file.  A $PREFIX directive applies
+to all message definitions between it an the next $PREFIX directive.  A $PREFIX
+directive with no arguments clears the current prefix.
+
 2) A C++ source file (called <message-file-name>.cc) that holds the definitions
 of the global symbols and code to insert the symbols and messages into the map.
 
 Symbols are defined to be equal to strings holding the identifier, e.g.
 
-   extern const isc::log::MessageID MSG_DUPLNS = "DUPLNS";
+   extern const isc::log::MessageID MSG_DUPLNS = "MSG_DUPLNS";
 
 (The implementation allows symbols to be compared.  However, use of strings
 should not be assumed - a future implementation may change this.)
@@ -243,27 +257,49 @@ To use the current version of the logging:
       highest-level debug messages and 99 for the lowest-level (and typically
       more verbose) messages.
 
-   c) Name of an external message file.  This is the same as a standard message
-      file, although it should not include any directives. (A single directive
-      of a particular type will be ignored; multiple directives will cause the
-      read of the file to fail with an error.)  If a message is replaced, the
-      message should include the same printf-format directives in the same order
-      as the original message.
+   c) The external message file.  If present, this is the same as a standard
+      message file, although it should not include any directives. (A single
+      directive of a particular type will be ignored; multiple directives will
+      cause the read of the file to fail with an error.)
 
 4. Issue logging calls using methods on logger, e.g.
 
-       logger.error(DPS_NSTIMEOUT, "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:
 
        $PREFIX DPS_
            :
-       NSTIMEOUT  queries to all nameservers for %s have timed out
+       NSTIMEOUT  queries to all nameservers for %1 have timed out
 
    At present, the only logging is to the console.
 
 
+Efficiency Considerations
+-------------------------
+A common pattern in logging is a debug call of the form:
+
+   logger.debug(dbglevel, MSGID).arg(expensive_call()).arg(...
+
+... where "expensive_call()" is a function call to obtain logging information
+that may be computationally intensive.  Although the cost may be justified
+when debugging is enabled, the cost is still incurred even if debugging is
+disabled and the debug() method returns without outputting anything.  (The
+same may be true of other logging levels, although there are likely to be
+fewer calls to logger.info(), logger.error() etc. throughout the code and
+they are less likely to be disabled.)
+
+For this reason, a set of macros is provided and are called using the
+construct:
+
+   LOG_DEBUG(logger, dbglevel, MSGID).arg(expensive_call()).arg(...
+   LOG_INFO(logger, MSGID).arg(expensive_call()...)
+
+If these are used, the arguments passed to the arg() method are not evaluated
+if the relevant logging level is disabled.
+
+
 Severity Guidelines
 ===================
 When using logging, the question arises, what severity should a message be
@@ -361,9 +397,6 @@ is only one piece of code that does this functionality.
 Outstanding Issues
 ==================
 * Ability to configure system according to configuration database.
-* Update the build procedure to create .cc and .h files from the .msg file
-  during the build process. (Requires that the message compiler is built
-  first.)
 
 
 log4cxx Issues

src/lib/log/compiler/message.cc

@@ -266,7 +266,7 @@ writeHeaderFile(const string& file, const vector<string>& ns_components,
 
     try {
         if (hfile.fail()) {
-            throw MessageException(MSG_OPNMSGOUT, header_file.fullName(),
+            throw MessageException(MSG_OPENOUT, header_file.fullName(),
                 strerror(errno));
         }
 
@@ -300,7 +300,7 @@ writeHeaderFile(const string& file, const vector<string>& ns_components,
 
         // Report errors (if any) and exit
         if (hfile.fail()) {
-            throw MessageException(MSG_MSGWRTERR, header_file.fullName(),
+            throw MessageException(MSG_WRITERR, header_file.fullName(),
                 strerror(errno));
         }
 
@@ -359,7 +359,7 @@ writeProgramFile(const string& file, const vector<string>& ns_components,
     ofstream ccfile(program_file.fullName().c_str());
     try {
         if (ccfile.fail()) {
-            throw MessageException(MSG_OPNMSGOUT, program_file.fullName(),
+            throw MessageException(MSG_OPENOUT, program_file.fullName(),
                 strerror(errno));
         }
 
@@ -417,7 +417,7 @@ writeProgramFile(const string& file, const vector<string>& ns_components,
 
         // Report errors (if any) and exit
         if (ccfile.fail()) {
-            throw MessageException(MSG_MSGWRTERR, program_file.fullName(),
+            throw MessageException(MSG_WRITERR, program_file.fullName(),
                 strerror(errno));
         }
 

src/lib/log/logger_support.cc

@@ -24,6 +24,7 @@
 /// These functions will be replaced once the code has been written to obtain
 /// the logging parameters from the configuration database.
 
+#include <iostream>
 #include <algorithm>
 #include <string>
 #include <vector>
@@ -85,8 +86,12 @@ readLocalMessageFile(const char* file) {
             logger.error(ident, args[0].c_str());
             break;
 
-        default:    // 2 or more (2 should be the maximum)
+        case 2:
             logger.error(ident, args[0].c_str(), args[1].c_str());
+            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());
         }
     }
 }
@@ -117,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, (*i).c_str());
         }
 
     }

src/lib/log/message_reader.cc

@@ -15,6 +15,7 @@
 #include <cassert>
 #include <errno.h>
 #include <string.h>
+#include <iostream>
 
 #include <iostream>
 #include <fstream>
@@ -27,51 +28,49 @@
 using namespace std;
 
 namespace {
-
 const char DIRECTIVE_FLAG = '$';    // Starts each directive
 const char MESSAGE_FLAG = '%';      // Starts each message
-
 }
 
 
 namespace isc {
 namespace log {
 
-
 // Read the file.
 
 void
 MessageReader::readFile(const string& file, MessageReader::Mode mode) {
 
-    // Ensure the non-added collection is empty: this object might be
-    // being reused.
+    // Ensure the non-added collection is empty: we could be re-using this
+    // object.
     not_added_.clear();
 
-    // Open the file and store the name (in case we need it elsewhere).
+    // Open the file.
     ifstream infile(file.c_str());
     if (infile.fail()) {
-        throw MessageException(MSG_OPNMSGIN, file, strerror(errno));
+        throw MessageException(MSG_OPENIN, file, strerror(errno));
     }
 
-    // Loop round reading it.  Keep a track of line number of aid diagnosis
-    // of problems.
+    // Loop round reading it.  As we process the file one line at a time,
+    // keep a track of line number of aid diagnosis of problems.
     string line;
     getline(infile, line);
-    lineno_ = 1;
+    lineno_ = 0;
+
     while (infile.good()) {
+        ++lineno_;
         processLine(line, mode);
         getline(infile, line);
-        ++lineno_;
     }
 
     // Why did the loop terminate?
     if (!infile.eof()) {
-        throw MessageException(MSG_MSGRDERR, file, strerror(errno));
+        throw MessageException(MSG_READERR, file, strerror(errno));
     }
     infile.close();
 }
 
-// Parse a line of the file
+// Parse a line of the file.
 
 void
 MessageReader::processLine(const string& line, MessageReader::Mode mode) {
@@ -91,7 +90,7 @@ MessageReader::processLine(const string& line, MessageReader::Mode mode) {
 
     } else {
         ;                           // Other lines are extended message
-                                    // description and are ignored
+                                    // description so are ignored
     }
 }
 
@@ -113,8 +112,9 @@ MessageReader::parseDirective(const std::string& text) {
         parseNamespace(tokens);
 
     } else {
-        throw MessageException(MSG_UNRECDIR, tokens[0], lineno_);
 
+        // Unrecognised directive
+        throw MessageException(MSG_UNRECDIR, tokens[0], lineno_);
     }
 }
 
@@ -126,8 +126,8 @@ MessageReader::parsePrefix(const vector<string>& tokens) {
     assert(tokens.size() > 0);
 
     // Process $PREFIX.  With no arguments, the prefix is set to the empty
-    // string.  One argument sets the prefix to the (upper-case) value of
-    // it, and more than one argument is invalid.
+    // string.  One argument sets the prefix to the to its value and more than
+    // one argument is invalid.
     if (tokens.size() == 1) {
         prefix_ = "";
 

src/lib/log/messagedef.cc

@@ -1,4 +1,4 @@
-// File created from messagedef.mes on Fri May  6 19:06:38 2011
+// File created from messagedef.mes on Mon May  9 12:54:57 2011
 
 #include <cstddef>
 #include <log/message_types.h>
@@ -11,19 +11,19 @@ extern const isc::log::MessageID MSG_DUPLNS = "MSG_DUPLNS";
 extern const isc::log::MessageID MSG_DUPMSGID = "MSG_DUPMSGID";
 extern const isc::log::MessageID MSG_IDNOTFND = "MSG_IDNOTFND";
 extern const isc::log::MessageID MSG_INVMSGID = "MSG_INVMSGID";
-extern const isc::log::MessageID MSG_MSGRDERR = "MSG_MSGRDERR";
-extern const isc::log::MessageID MSG_MSGWRTERR = "MSG_MSGWRTERR";
 extern const isc::log::MessageID MSG_NOMSGID = "MSG_NOMSGID";
 extern const isc::log::MessageID MSG_NOMSGTXT = "MSG_NOMSGTXT";
 extern const isc::log::MessageID MSG_NSEXTRARG = "MSG_NSEXTRARG";
 extern const isc::log::MessageID MSG_NSINVARG = "MSG_NSINVARG";
 extern const isc::log::MessageID MSG_NSNOARG = "MSG_NSNOARG";
-extern const isc::log::MessageID MSG_OPNMSGIN = "MSG_OPNMSGIN";
-extern const isc::log::MessageID MSG_OPNMSGOUT = "MSG_OPNMSGOUT";
+extern const isc::log::MessageID MSG_OPENIN = "MSG_OPENIN";
+extern const isc::log::MessageID MSG_OPENOUT = "MSG_OPENOUT";
 extern const isc::log::MessageID MSG_PRFEXTRARG = "MSG_PRFEXTRARG";
 extern const isc::log::MessageID MSG_PRFINVARG = "MSG_PRFINVARG";
 extern const isc::log::MessageID MSG_RDLOCMES = "MSG_RDLOCMES";
+extern const isc::log::MessageID MSG_READERR = "MSG_READERR";
 extern const isc::log::MessageID MSG_UNRECDIR = "MSG_UNRECDIR";
+extern const isc::log::MessageID MSG_WRITERR = "MSG_WRITERR";
 
 } // namespace log
 } // namespace isc
@@ -33,21 +33,21 @@ 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 for '%s': no such message identification",
+    "MSG_IDNOTFND", "could not replace message text for '%s': no such message",
     "MSG_INVMSGID", "line %s: invalid message identification '%s'",
-    "MSG_MSGRDERR", "error reading from message file %s: %s",
-    "MSG_MSGWRTERR", "error writing to %s: %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 nothing else",
+    "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_OPNMSGIN", "unable to open message file %s for input: %s",
-    "MSG_OPNMSGOUT", "unable to open %s for output: %s",
+    "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",
     NULL
 };
 

src/lib/log/messagedef.h

@@ -1,4 +1,4 @@
-// File created from messagedef.mes on Fri May  6 19:06:38 2011
+// File created from messagedef.mes on Mon May  9 12:54:57 2011
 
 #ifndef __MESSAGEDEF_H
 #define __MESSAGEDEF_H
@@ -12,19 +12,19 @@ extern const isc::log::MessageID MSG_DUPLNS;
 extern const isc::log::MessageID MSG_DUPMSGID;
 extern const isc::log::MessageID MSG_IDNOTFND;
 extern const isc::log::MessageID MSG_INVMSGID;
-extern const isc::log::MessageID MSG_MSGRDERR;
-extern const isc::log::MessageID MSG_MSGWRTERR;
 extern const isc::log::MessageID MSG_NOMSGID;
 extern const isc::log::MessageID MSG_NOMSGTXT;
 extern const isc::log::MessageID MSG_NSEXTRARG;
 extern const isc::log::MessageID MSG_NSINVARG;
 extern const isc::log::MessageID MSG_NSNOARG;
-extern const isc::log::MessageID MSG_OPNMSGIN;
-extern const isc::log::MessageID MSG_OPNMSGOUT;
+extern const isc::log::MessageID MSG_OPENIN;
+extern const isc::log::MessageID MSG_OPENOUT;
 extern const isc::log::MessageID MSG_PRFEXTRARG;
 extern const isc::log::MessageID MSG_PRFINVARG;
 extern const isc::log::MessageID MSG_RDLOCMES;
+extern const isc::log::MessageID MSG_READERR;
 extern const isc::log::MessageID MSG_UNRECDIR;
+extern const isc::log::MessageID MSG_WRITERR;
 
 } // namespace log
 } // namespace isc

src/lib/log/messagedef.mes

@@ -12,18 +12,18 @@
 # OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR
 # PERFORMANCE OF THIS SOFTWARE.
 
-$PREFIX MSG_
-$NAMESPACE isc::log
-
 # \brief Message Utility Message File
 #
-# This is the source of the set of messages generated by the message and logging
-# components.  The associated .h and .cc files are created by hand from this
-# file though and are not built during the build process; this is to avoid the
-# chicken-and-egg situation where we need the files to build the message
+# This is the source of the set of messages generated by the message and
+# logging components.  The associated .h and .cc files are created by hand from
+# this file though and are not built during the build process; this is to avoid
+# the chicken-and-egg situation where we need the files to build the message
 # compiler, yet we need the compiler to build the files.
 
-% DUPMSGID  duplicate message ID (%s) in compiled code
+$PREFIX MSG_
+$NAMESPACE isc::log
+
+% DUPMSGID      duplicate message ID (%s) 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 %s: 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 for '%s': no such message identification
+% IDNOTFND      could not replace message text for '%s': 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,50 +50,43 @@ 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 %s: invalid message identification '%s'
 The concatenation of the prefix and the message identification is used as
 a symbol in the C++ module; as such it may only contain 
 
-% MSGRDERR  error reading from message file %s: %s
-The specified error was encountered reading from the named message file.
+% NOMSGID       line %s: 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.
 
-% MSGWRTERR error writing to %s: %s
-The specified error was encountered by the message compiler when writing to
-the named output file.
+% NOMSGTXT      line %s: line found containing a message ID ('%s') 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 %s: $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 %s: $NAMESPACE directive has an invalid argument ('%s')
 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.)
 
-% NOMSGID   line %s: 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 nothing else
-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.
-
-% NSNOARG   line %s: no arguments were given to the $NAMESPACE directive
+% NSNOARG       line %s: 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.
 
-% OPNMSGIN  unable to open message file %s for input: %s
+% OPENIN        unable to open message file %s for input: %s
 The program was not able to open the specified input message file for the
 reason given.
 
-% OPNMSGOUT unable to open %s for output: %s
+% OPENOUT       unable to open %s for output: %s
 The program was not able to open the specified output file for the reason
 given.
 
@@ -102,18 +95,25 @@ 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 %s: $PREFIX directive has an invalid argument ('%s')
 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 %s
 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.)
 
-% UNRECDIR  line %s: unrecognised directive '%s'
+% READERR       error reading from message file %s: %s
+The specified error was encountered reading from the named message file.
+
+% WRITERR       error writing to %s: %s
+The specified error was encountered by the message compiler when writing to
+the named output file.
+
+% UNRECDIR      line %s: unrecognised directive '%s'
 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.

src/lib/log/tests/logger_support_test.cc

@@ -92,13 +92,13 @@ int main(int argc, char** argv) {
     initLogger("alpha", severity, dbglevel, localfile);
 
     // Log a few messages
-    logger_ex.fatal(MSG_MSGWRTERR, "test1", "42");
-    logger_ex.error(MSG_UNRECDIR, "false");
-    logger_dlm.warn(MSG_MSGRDERR, "a.txt", "dummy test");
-    logger_dlm.info(MSG_OPNMSGIN, "example.msg", "dummy test");
-    logger_ex.debug(0, MSG_UNRECDIR, "[abc]");
-    logger_ex.debug(24, MSG_UNRECDIR, "[24]");
-    logger_ex.debug(25, MSG_UNRECDIR, "[25]");
-    logger_ex.debug(26, MSG_UNRECDIR, "[26]");
+    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");
     return (0);
 }

src/lib/log/tests/message_dictionary_unittest.cc

@@ -29,7 +29,7 @@ using namespace std;
 // and the latter should be present.
 
 static const char* values[] = {
-    "DUPLNS", "duplicate $NAMESPACE directive found",
+    "MSG_DUPLNS", "duplicate $NAMESPACE directive found",
     "NEWSYM", "new symbol added",
     NULL
 };
@@ -190,7 +190,7 @@ TEST_F(MessageDictionaryTest, GlobalTest) {
 TEST_F(MessageDictionaryTest, GlobalLoadTest) {
     vector<string>& duplicates = MessageInitializer::getDuplicates();
     ASSERT_EQ(1, duplicates.size());
-    EXPECT_EQ(string("DUPLNS"), duplicates[0]);
+    EXPECT_EQ(string("MSG_DUPLNS"), duplicates[0]);
 
     string text = MessageDictionary::globalDictionary().getText("NEWSYM");
     EXPECT_EQ(string("new symbol added"), text);

src/lib/log/tests/message_reader_unittest.cc

@@ -68,8 +68,8 @@ TEST_F(MessageReaderTest, BlanksAndComments) {
     EXPECT_NO_THROW(reader_.processLine(" \n "));
     EXPECT_NO_THROW(reader_.processLine("# This is a comment"));
     EXPECT_NO_THROW(reader_.processLine("\t\t # Another comment"));
-    EXPECT_NO_THROW(reader_.processLine("  + A description line"));
-    EXPECT_NO_THROW(reader_.processLine("#+ A comment"));
+    EXPECT_NO_THROW(reader_.processLine("  A description line"));
+    EXPECT_NO_THROW(reader_.processLine("# A comment"));
     EXPECT_NO_THROW(reader_.processLine("  +# A description line"));
 
     // ... and (b) nothing gets added to either the map or the not-added section.
@@ -97,6 +97,15 @@ processLineException(MessageReader& reader, const char* what,
     }
 }
 
+// Check that it recognises invalid directives
+
+TEST_F(MessageReaderTest, InvalidDirectives) {
+
+    // Check that a "$" with nothing else generates an error
+    processLineException(reader_, "$", MSG_UNRECDIR);
+    processLineException(reader_, "$xyz", MSG_UNRECDIR);
+}
+
 // Check that it can parse a prefix
 
 TEST_F(MessageReaderTest, Prefix) {
@@ -104,8 +113,8 @@ TEST_F(MessageReaderTest, Prefix) {
     // Check that no $PREFIX is present
     EXPECT_EQ(string(""), reader_.getPrefix());
 
-    // Check that a $PREFIX directive with no argument generates an error.
-    processLineException(reader_, "$PREFIX", MSG_PRFNOARG);
+    // Check that a $PREFIX directive with no argument is OK
+    EXPECT_NO_THROW(reader_.processLine("$PREFIX"));
 
     // Check a $PREFIX with multiple arguments is invalid
     processLineException(reader_, "$prefix A B", MSG_PRFEXTRARG);
@@ -118,17 +127,19 @@ TEST_F(MessageReaderTest, Prefix) {
 
     // A valid prefix should be accepted
     EXPECT_NO_THROW(reader_.processLine("$PREFIX   dlm__"));
-    EXPECT_EQ(string("DLM__"), reader_.getPrefix());
+    EXPECT_EQ(string("dlm__"), reader_.getPrefix());
 
     // And check that the parser fails on invalid prefixes...
     processLineException(reader_, "$prefix 1ABC", MSG_PRFINVARG);
 
-    // ... and rejects another valid one
-    processLineException(reader_, "$PREFIX ABC", MSG_DUPLPRFX);
-
     // Check that we can clear the prefix as well
     reader_.clearPrefix();
     EXPECT_EQ(string(""), reader_.getPrefix());
+
+    EXPECT_NO_THROW(reader_.processLine("$PREFIX   dlm__"));
+    EXPECT_EQ(string("dlm__"), reader_.getPrefix());
+    EXPECT_NO_THROW(reader_.processLine("$PREFIX"));
+    EXPECT_EQ(string(""), reader_.getPrefix());
 }
 
 // Check that it can parse a namespace
@@ -173,8 +184,8 @@ TEST_F(MessageReaderTest, Namespace) {
 TEST_F(MessageReaderTest, ValidMessageAddDefault) {
 
     // Add a couple of valid messages
-    reader_.processLine("GLOBAL1\t\tthis is message global one\n");
-    reader_.processLine("GLOBAL2 this is message global two");
+    reader_.processLine("% GLOBAL1\t\tthis is message global one\n");
+    reader_.processLine("%GLOBAL2 this is message global two");
 
     // ... and check them
     EXPECT_EQ(string("this is message global one"),
@@ -191,9 +202,9 @@ TEST_F(MessageReaderTest, ValidMessageAddDefault) {
 TEST_F(MessageReaderTest, ValidMessageAdd) {
 
     // Add a couple of valid messages
-    reader_.processLine("GLOBAL1\t\tthis is message global one\n",
+    reader_.processLine("%GLOBAL1\t\tthis is message global one\n",
         MessageReader::ADD);
-    reader_.processLine("GLOBAL2 this is message global two",
+    reader_.processLine("% GLOBAL2 this is message global two",
         MessageReader::ADD);
 
     // ... and check them
@@ -214,9 +225,9 @@ TEST_F(MessageReaderTest, ValidMessageReplace) {
     dictionary_->add("GLOBAL2", "original global2 message");
 
     // Replace a couple of valid messages
-    reader_.processLine("GLOBAL1\t\tthis is message global one\n",
+    reader_.processLine("% GLOBAL1\t\tthis is message global one\n",
         MessageReader::REPLACE);
-    reader_.processLine("GLOBAL2 this is message global two",
+    reader_.processLine("% GLOBAL2 this is message global two",
         MessageReader::REPLACE);
 
     // ... and check them
@@ -237,14 +248,14 @@ TEST_F(MessageReaderTest, ValidMessageReplace) {
 TEST_F(MessageReaderTest, Overflows) {
 
     // Add a couple of valid messages
-    reader_.processLine("GLOBAL1\t\tthis is message global one\n");
-    reader_.processLine("GLOBAL2 this is message global two");
+    reader_.processLine("% GLOBAL1\t\tthis is message global one\n");
+    reader_.processLine("% GLOBAL2 this is message global two");
 
     // Add a duplicate in ADD mode.
-    reader_.processLine("GLOBAL1\t\tthis is a replacement for global one");
+    reader_.processLine("% GLOBAL1\t\tthis is a replacement for global one");
 
     // Replace a non-existent one in REPLACE mode
-    reader_.processLine("LOCAL\t\tthis is a new message",
+    reader_.processLine("% LOCAL\t\tthis is a new message",
         MessageReader::REPLACE);
 
     // Check what is in the dictionary.

src/lib/log/tests/run_time_init_test.sh.in

@@ -29,48 +29,49 @@ passfail() {
 # Create the local message file for testing
 
 cat > $localmes << .
-NOTHERE     this message is not in the global dictionary
-MSGRDERR    replacement read error, parameters: '%s' and '%s'
-UNRECDIR    replacement unrecognised directive message, parameter is '%s'
+\$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'
 .
 
 echo -n "1. runInitTest default parameters: "
 cat > $tempfile << .
-FATAL [alpha.example] MSGWRTERR, error writing to test1: 42
-ERROR [alpha.example] UNRECDIR, unrecognised directive 'false'
-WARN  [alpha.dlm] MSGRDERR, error reading from message file a.txt: dummy test
-INFO  [alpha.dlm] OPNMSGIN, unable to open message file example.msg for input: dummy test
+FATAL [alpha.example] MSG_WRITERR, error writing to test1: 42
+ERROR [alpha.example] MSG_RDLOCMES, reading local message file dummy/file
+WARN  [alpha.dlm] MSG_READERR, error reading from message file a.txt: dummy reason
+INFO  [alpha.dlm] MSG_OPENIN, unable to open message file example.msg for input: dummy reason
 .
 ./logger_support_test | cut -d' ' -f3- | diff $tempfile -
 passfail $?
 
 echo -n "2. Severity filter: "
 cat > $tempfile << .
-FATAL [alpha.example] MSGWRTERR, error writing to test1: 42
-ERROR [alpha.example] UNRECDIR, unrecognised directive 'false'
+FATAL [alpha.example] MSG_WRITERR, error writing to test1: 42
+ERROR [alpha.example] MSG_RDLOCMES, reading local message file dummy/file
 .
 ./logger_support_test -s error | cut -d' ' -f3- | diff $tempfile -
 passfail $?
 
 echo -n "3. Debug level: "
 cat > $tempfile << .
-FATAL [alpha.example] MSGWRTERR, error writing to test1: 42
-ERROR [alpha.example] UNRECDIR, unrecognised directive 'false'
-WARN  [alpha.dlm] MSGRDERR, error reading from message file a.txt: dummy test
-INFO  [alpha.dlm] OPNMSGIN, unable to open message file example.msg for input: dummy test
-DEBUG [alpha.example] UNRECDIR, unrecognised directive '[abc]'
-DEBUG [alpha.example] UNRECDIR, unrecognised directive '[24]'
-DEBUG [alpha.example] UNRECDIR, unrecognised directive '[25]'
+FATAL [alpha.example] MSG_WRITERR, error writing to test1: 42
+ERROR [alpha.example] MSG_RDLOCMES, reading local message file dummy/file
+WARN  [alpha.dlm] MSG_READERR, error reading from message file a.txt: dummy reason
+INFO  [alpha.dlm] MSG_OPENIN, unable to open message file example.msg for input: dummy reason
+DEBUG [alpha.example] MSG_RDLOCMES, reading local message file dummy/0
+DEBUG [alpha.example] MSG_RDLOCMES, reading local message file dummy/24
+DEBUG [alpha.example] MSG_RDLOCMES, reading local message file dummy/25
 .
 ./logger_support_test -s debug -d 25 | cut -d' ' -f3- | diff $tempfile -
 passfail $?
 
 echo -n "4. Local message replacement: "
 cat > $tempfile << .
-WARN  [alpha.log] IDNOTFND, could not replace message for 'NOTHERE': no such message identification
-FATAL [alpha.example] MSGWRTERR, error writing to test1: 42
-ERROR [alpha.example] UNRECDIR, replacement unrecognised directive message, parameter is 'false'
-WARN  [alpha.dlm] MSGRDERR, replacement read error, parameters: 'a.txt' and 'dummy test'
+WARN  [alpha.log] MSG_IDNOTFND, could not replace message text for 'MSG_NOTHERE': no such message
+FATAL [alpha.example] MSG_WRITERR, error writing to test1: 42
+ERROR [alpha.example] MSG_RDLOCMES, replacement read local message file, parameter is 'dummy/file'
+WARN  [alpha.dlm] MSG_READERR, replacement read error, parameters: 'a.txt' and 'dummy reason'
 .
 ./logger_support_test -s warn $localmes | cut -d' ' -f3- | diff $tempfile -
 passfail $?