Browse Source

[trac558] Address more initialization points

* RootLoggerName now correctly avoids static initialization fiasco.
* Message header file now references message definition file.
* Moved message identifiers in logging subsystem into isc::log namespace.
Stephen Morris 14 years ago
parent
commit
2ad85af5e3

+ 85 - 57
src/lib/log/compiler/message.cc

@@ -70,7 +70,8 @@ static const char* VERSION = "1.0-0";
 ///
 /// Prints the program's version number.
 
-static void version() {
+void
+version() {
     cout << VERSION << "\n";
 }
 
@@ -78,7 +79,8 @@ static void version() {
 ///
 /// Prints program usage to stdout.
 
-static void usage() {
+void
+usage() {
     cout <<
         "Usage: message [-h] [-p] [-v] <message-file>\n" <<
         "\n" <<
@@ -98,7 +100,8 @@ static void usage() {
 ///
 /// \return Current time
 
-static string currentTime() {
+string
+currentTime() {
 
     // Get a text representation of the current time.
     time_t curtime;
@@ -123,7 +126,8 @@ static string currentTime() {
 ///
 /// \return Sentinel name
 
-static string sentinel(Filename& file) {
+string
+sentinel(Filename& file) {
 
     string name = file.name();
     string ext = file.extension();
@@ -139,7 +143,8 @@ static string sentinel(Filename& file) {
 /// characters.  This is used to handle the fact that the input file does not
 /// contain quotes, yet the string will be included in a C++ literal string.
 
-string quoteString(const string& instring) {
+string
+quoteString(const string& instring) {
 
     // Create the output string and reserve the space needed to hold the input
     // string. (Most input strings will not contain quotes, so this single
@@ -168,7 +173,8 @@ string quoteString(const string& instring) {
 ///
 /// \return Sorted list of message IDs
 
-vector<string> sortedIdentifiers(MessageDictionary* dictionary) {
+vector<string>
+sortedIdentifiers(MessageDictionary* dictionary) {
     vector<string> ident;
 
     for (MessageDictionary::const_iterator i = dictionary->begin();
@@ -194,7 +200,8 @@ vector<string> sortedIdentifiers(MessageDictionary* dictionary) {
 /// \param ns Argument to $NAMESPACE (passed by valid, as we will be modifying
 /// it.)
 
-vector<string> splitNamespace(string ns) {
+vector<string>
+splitNamespace(string ns) {
 
     // Namespaces components are separated by double colon characters - convert
     // to single colons.
@@ -209,6 +216,34 @@ vector<string> splitNamespace(string ns) {
 }
 
 
+/// \brief Write Opening Namespace(s)
+///
+/// Writes the lines listing the namespaces in use.
+void
+writeOpeningNamespace(ostream& output, vector<string>& ns) {
+    if (!ns.empty()) {
+        for (int i = 0; i < ns.size(); ++i) {
+            output << "namespace " << ns[i] << " {\n";
+        }
+        output << "\n";
+    }
+}
+
+
+/// \brief Write Closing Namespace(s)
+///
+/// Writes the lines listing the namespaces in use.
+void
+writeClosingNamespace(ostream& output, vector<string>& ns) {
+    if (!ns.empty()) {
+        for (int i = ns.size() - 1; i >= 0; --i) {
+            output << "} // namespace " << ns[i] << "\n";
+        }
+        output << "\n";
+    }
+}
+
+
 /// \brief Write Header File
 ///
 /// Writes the C++ header file containing the symbol definitions.
@@ -217,12 +252,12 @@ vector<string> splitNamespace(string ns) {
 /// file of the same name but with a .h suffix.
 /// \param prefix Prefix string to use in symbols
 /// \param ns Namespace in which the definitions are to be placed.  An empty
-/// string indicates no namespace, the special string of "::" the anonymous
-/// namespace, and anything else is the namspace desired.
+/// string indicates no namespace.
 /// \param dictionary Dictionary holding the message definitions.
 
-void writeHeaderFile(const string& file, const string& prefix, const string& ns,
-    MessageDictionary* dictionary)
+void
+writeHeaderFile(const string& file, const string& prefix, const string& ns,
+    string& mi_name, MessageDictionary* dictionary)
 {
     Filename message_file(file);
     Filename header_file(message_file.useAsDefault(".h"));
@@ -250,56 +285,37 @@ void writeHeaderFile(const string& file, const string& prefix, const string& ns,
              "#define "  << sentinel_text << "\n" <<
              "\n" <<
              "#include <log/message_types.h>\n" <<
+             "#include <log/message_initializer.h>\n" <<
              "\n";
 
         // Namespaces
-        vector<string> ns_components;
-        if (ns == "") {
-
-            ; // Nothing given, so no namespace
-
-        } else if (ns == "::") {
-
-            // Use the anonymous namespace.
-            hfile << "namespace {\n" <<
-                "\n";
-        } else {
-
-            // Output one namespace statement for each namespace component.
-            ns_components = splitNamespace(ns);
-            for (int i = 0; i < ns_components.size(); ++i) {
-                hfile << "namespace " << ns_components[i] << " {\n";
-            }
-            hfile << "\n";
-        }
+        vector<string> ns_components = splitNamespace(ns);
+        writeOpeningNamespace(hfile, ns_components);
 
         // Now the m,essage identifications themselves.
         vector<string> idents = sortedIdentifiers(dictionary);
         for (vector<string>::const_iterator j = idents.begin();
             j != idents.end(); ++j) {
-            hfile << "const isc::log::MessageID " << prefix << *j <<
+            hfile << "static const isc::log::MessageID " << prefix << *j <<
                 " = \"" << *j << "\";\n";
         }
         hfile << "\n";
 
         // Close off namespaces if appropriate.
-        if (ns == "") {
-
-            ; // Nothing given, so no namespace
-
-        } else if (ns == "::") {
+        writeClosingNamespace(hfile, ns_components);
 
-            // Use the anonymous namespace.
-            hfile << "}\n" <<
-                "\n";
-        } else {
+        // Now create the reference to the message initializer to ensure that
+        // it gets run at program startup.
 
-            // Output one namespace statement for each namespace component.
-            for (int i = (ns_components.size() - 1); i >= 0; --i) {
-                hfile << "} // namespace " << ns_components[i] << "\n";
-            }
-            hfile << "\n";
-        }
+        hfile << "namespace isc {\n" <<
+            "namespace log {\n" <<
+            "\n" <<
+            "extern MessageInitializer " << mi_name << ";\n" <<
+            "static MessageInstantiator m(&" << mi_name << ");\n" <<
+            "\n" <<
+            "} // namespace log\n" <<
+            "} // namespace isc\n" <<
+            "\n";
 
 
         // ... and finally the postamble
@@ -324,7 +340,8 @@ void writeHeaderFile(const string& file, const string& prefix, const string& ns,
 ///
 /// Simple function for use in a call to transform
 
-char replaceNonAlphaNum(char c) {
+char
+replaceNonAlphaNum(char c) {
     return (isalnum(c) ? c : '_');
 }
 
@@ -335,7 +352,8 @@ char replaceNonAlphaNum(char c) {
 /// constructor is run at initialization time.  The constructor adds the message
 /// definitions to the main global dictionary.
 
-void writeProgramFile(const string& file, MessageDictionary* dictionary)
+string
+writeProgramFile(const string& file, MessageDictionary* dictionary)
 {
     Filename message_file(file);
     Filename program_file(message_file.useAsDefault(".cc"));
@@ -358,8 +376,6 @@ void writeProgramFile(const string& file, MessageDictionary* dictionary)
              "#include <cstddef>\n" <<
              "#include <log/message_initializer.h>\n" <<
              "\n" <<
-             "using namespace isc::log;\n" <<
-             "\n" <<
              "namespace {\n" <<
              "\n" <<
              "const char* values[] = {\n";
@@ -390,7 +406,14 @@ void writeProgramFile(const string& file, MessageDictionary* dictionary)
 
         // ... and write the initialization code
         ccfile <<
-            "MessageInitializer " << unique_name << "(values);\n";
+            "namespace isc {\n" <<
+            "namespace log {\n" <<
+            "\n" <<
+            "MessageInitializer " << unique_name << "(values);\n" <<
+            "\n" <<
+            "} // namespace log\n" <<
+            "} // namespace isc\n" <<
+            "\n";
 
         // Report errors (if any) and exit
         if (ccfile.fail()) {
@@ -399,6 +422,8 @@ void writeProgramFile(const string& file, MessageDictionary* dictionary)
         }
 
         ccfile.close();
+
+        return unique_name;
     }
     catch (MessageException&) {
         ccfile.close();
@@ -414,7 +439,8 @@ void writeProgramFile(const string& file, MessageDictionary* dictionary)
 ///
 /// \param reader Message Reader used to read the file
 
-static void warnDuplicates(MessageReader& reader) {
+void
+warnDuplicates(MessageReader& reader) {
 
     // Get the duplicates (the overflow) and, if present, sort them into some
     // order and remove those which occur more than once (which mean that they
@@ -439,7 +465,8 @@ static void warnDuplicates(MessageReader& reader) {
 /// Parses the options then dispatches to the appropriate function.  See the
 /// main file header for the invocation.
 
-int main(int argc, char** argv) {
+int
+main(int argc, char** argv) {
     
     const struct option loptions[] = {          // Long options
         {"help",    no_argument, NULL, 'h'},
@@ -488,12 +515,13 @@ int main(int argc, char** argv) {
         MessageReader reader(&dictionary);
         reader.readFile(message_file);
 
+        // Write the file that defines the message text
+        std::string mi_name =
+            writeProgramFile(message_file, &dictionary);
+
         // Now write the header file.
         writeHeaderFile(message_file, reader.getPrefix(), reader.getNamespace(),
-            &dictionary);
-
-        // ... and the message text file.
-        writeProgramFile(message_file, &dictionary);
+            mi_name, &dictionary);
 
         // Finally, warn of any duplicates encountered.
         warnDuplicates(reader);

+ 2 - 6
src/lib/log/message_dictionary.cc

@@ -103,12 +103,8 @@ MessageDictionary::getText(const string& ident) const {
 
 MessageDictionary*
 MessageDictionary::globalDictionary() {
-    static MessageDictionary* global = NULL;
-
-    if (global == NULL) {
-        global = new MessageDictionary();
-    }
-    return global;
+    static MessageDictionary global;
+    return &global;
 }
 
 

+ 5 - 0
src/lib/log/message_initializer.cc

@@ -28,5 +28,10 @@ MessageInitializer::MessageInitializer(const char* values[]) {
     global->load(values);
 }
 
+// Dummy constructor for the MessageInstantiator
+
+MessageInstantiator::MessageInstantiator(MessageInitializer*) {
+}
+
 } // namespace log
 } // namespace isc

+ 24 - 0
src/lib/log/message_initializer.h

@@ -57,6 +57,30 @@ public:
     MessageInitializer(const char* values[]);
 };
 
+/// \brief Instantiate Message Initializer
+///
+/// A problem with the MessageInitializer class is that an instance of it is
+/// created in an external file and initialization of a set of messages requires
+/// that that file be included in the link.  Unfortunately, if there is no
+/// reference to the MessageInitializer object, we cannot guarantee that that
+/// will be the case.\n
+/// \n
+/// The MessageInitializer object is created as a global object, so in theory
+/// an "extern" reference to it should work.  However, that reference may well
+/// be optimised away.  To overcome this, the MessageInstantiator class is
+/// used.\n
+/// \n
+/// In the message header file, an instance of MessageInstantiator is created
+/// that takes the extern reference to the MessageInitializer as its constructor
+/// argument.  The constructor - declared in another file - is a no-op.  But as
+/// the linker doesn't know, it must resolve the reference, hence pulling in the
+/// file containing the MessageInitializr.
+
+class MessageInstantiator {
+public:
+    MessageInstantiator(MessageInitializer* dummy);
+};
+
 } // namespace log
 } // namespace isc
 

+ 9 - 4
src/lib/log/messagedef.cc

@@ -1,10 +1,8 @@
-// File created from messagedef.mes on Sat Feb  5 18:08:17 2011
+// File created from messagedef.mes on Mon Feb  7 11:18:04 2011
 
 #include <cstddef>
 #include <log/message_initializer.h>
 
-using namespace isc::log;
-
 namespace {
 
 const char* values[] = {
@@ -28,4 +26,11 @@ const char* values[] = {
 
 } // Anonymous namespace
 
-MessageInitializer messagedef_cc_Sat_Feb__5_18_08_17_2011(values);
+namespace isc {
+namespace log {
+
+MessageInitializer messagedef_cc_Mon_Feb__7_11_18_04_2011(values);
+
+} // namespace log
+} // namespace isc
+

+ 32 - 20
src/lib/log/messagedef.h

@@ -1,28 +1,40 @@
-// File created from messagedef.mes on Sat Feb  5 18:08:17 2011
+// File created from messagedef.mes on Mon Feb  7 11:18:04 2011
 
 #ifndef __MESSAGEDEF_H
 #define __MESSAGEDEF_H
 
 #include <log/message_types.h>
+#include <log/message_initializer.h>
 
-namespace {
-
-const isc::log::MessageID MSG_DUPLNS = "DUPLNS";
-const isc::log::MessageID MSG_DUPLPRFX = "DUPLPRFX";
-const isc::log::MessageID MSG_IDNOTFND = "IDNOTFND";
-const isc::log::MessageID MSG_NSEXTRARG = "NSEXTRARG";
-const isc::log::MessageID MSG_NSINVARG = "NSINVARG";
-const isc::log::MessageID MSG_NSNOARG = "NSNOARG";
-const isc::log::MessageID MSG_ONETOKEN = "ONETOKEN";
-const isc::log::MessageID MSG_OPENIN = "OPENIN";
-const isc::log::MessageID MSG_OPENOUT = "OPENOUT";
-const isc::log::MessageID MSG_PRFEXTRARG = "PRFEXTRARG";
-const isc::log::MessageID MSG_PRFINVARG = "PRFINVARG";
-const isc::log::MessageID MSG_PRFNOARG = "PRFNOARG";
-const isc::log::MessageID MSG_READERR = "READERR";
-const isc::log::MessageID MSG_UNRECDIR = "UNRECDIR";
-const isc::log::MessageID MSG_WRITERR = "WRITERR";
-
-}
+namespace isc {
+namespace log {
+
+static const isc::log::MessageID MSG_DUPLNS = "DUPLNS";
+static const isc::log::MessageID MSG_DUPLPRFX = "DUPLPRFX";
+static const isc::log::MessageID MSG_IDNOTFND = "IDNOTFND";
+static const isc::log::MessageID MSG_NSEXTRARG = "NSEXTRARG";
+static const isc::log::MessageID MSG_NSINVARG = "NSINVARG";
+static const isc::log::MessageID MSG_NSNOARG = "NSNOARG";
+static const isc::log::MessageID MSG_ONETOKEN = "ONETOKEN";
+static const isc::log::MessageID MSG_OPENIN = "OPENIN";
+static const isc::log::MessageID MSG_OPENOUT = "OPENOUT";
+static const isc::log::MessageID MSG_PRFEXTRARG = "PRFEXTRARG";
+static const isc::log::MessageID MSG_PRFINVARG = "PRFINVARG";
+static const isc::log::MessageID MSG_PRFNOARG = "PRFNOARG";
+static const isc::log::MessageID MSG_READERR = "READERR";
+static const isc::log::MessageID MSG_UNRECDIR = "UNRECDIR";
+static const isc::log::MessageID MSG_WRITERR = "WRITERR";
+
+} // namespace log
+} // namespace isc
+
+namespace isc {
+namespace log {
+
+extern MessageInitializer messagedef_cc_Mon_Feb__7_11_18_04_2011;
+static MessageInstantiator m(&messagedef_cc_Mon_Feb__7_11_18_04_2011);
+
+} // namespace log
+} // namespace isc
 
 #endif // __MESSAGEDEF_H

+ 1 - 1
src/lib/log/messagedef.mes

@@ -15,7 +15,7 @@
 # $Id$
 
 $PREFIX MSG_
-$NAMESPACE ::
+$NAMESPACE isc::log
 
 # \brief Message Utility Message File
 #

+ 4 - 1
src/lib/log/root_logger_name.cc

@@ -20,7 +20,10 @@
 namespace isc {
 namespace log {
 
-std::string RootLoggerName::name_("");
+std::string& RootLoggerName::rootName() {
+    static std::string root_name("");
+    return root_name;
+}
 
 }
 }

+ 12 - 3
src/lib/log/root_logger_name.h

@@ -46,18 +46,27 @@ public:
     /// \param name Name of the root logger.  This should be the program
     /// name.
     static void setName(const std::string& name) {
-        name_ = name;
+        rootName() = name;
     }
 
     /// \brief Get Root Logger Name
     ///
     /// \return Name of the root logger.
     static std::string getName() {
-        return name_;
+        return rootName();
     }
     
 private:
-    static std::string name_;      ///< Name of the root logger
+    /// \brief Store Root Logger Name
+    ///
+    /// Access the singleton root logger name.  This is stored as a static
+    /// variable inside a method to avoid the "static initialization fiasco";
+    /// when accessed by another class during initialization, the name will be
+    /// instantiated. (Else it will be only by chance that it is instantiated
+    /// before the cassling class.)
+    ///
+    /// \return Name addisnged to the root logger.
+    static std::string& rootName();
 };
 
 }

+ 1 - 1
src/lib/log/tests/message_types_unittest_2.cc

@@ -18,5 +18,5 @@
 // Return a value for testing in message_types_unittest.cc
 
 isc::log::MessageID MessageTypeTest_MSG_DUPLNS() {
-    return (MSG_DUPLNS);
+    return (isc::log::MSG_DUPLNS);
 }