[#1266] addressed review

src/lib/log/logger.h

@@ -81,13 +81,14 @@ class InterprocessSync;
 /// the string passed to the Logger constructor) to a maximum of 31 characters.
 /// There is no reason for this particular value other than limiting the amount
 /// of memory used.  It is defined by the constant Logger::MAX_LOGGER_NAME_SIZE,
-/// and can be made larger (or smaller) if so desired.
+/// and can be made larger (or smaller) if so desired.  Note however, using a
+/// logger name larger than this limit will cause an assertion failure.
 
 class LoggerImpl;   // Forward declaration of the implementation class
 
 /// \brief Bad Interprocess Sync
 ///
-/// Exception thrown if a bad InterprocessSync object (such as NULL) is
+/// Exception thrown if a bad InterprocessSync object (such as null) is
 /// used.
 class BadInterprocessSync : public isc::Exception {
 public:
@@ -106,7 +107,7 @@ public:
     {}
 };
 
-/// \brief Logger Name is Null
+/// \brief Logger Name is null
 ///
 /// Exception thrown if a logger name is null
 class LoggerNameNull : public isc::Exception {
@@ -164,10 +165,7 @@ public:
     Logger(const char* name) : loggerptr_(), initialized_(false) {
 
         // Validate the name of the logger.
-        if (name == NULL) {
-            isc_throw(LoggerNameNull, "logger names may not be null");
-
-        } else {
+        if (name) {
             // Name not null, is it too short or too long?
             size_t namelen = std::strlen(name);
             if ((namelen == 0) || (namelen > MAX_LOGGER_NAME_SIZE)) {
@@ -176,9 +174,17 @@ public:
                           << "and " << MAX_LOGGER_NAME_SIZE << " characters in "
                           << "length");
             }
+        } else {
+            isc_throw(LoggerNameNull, "logger names may not be null");
         }
 
-        // Do the copy, ensuring a trailing NULL in all cases.
+        // The checks above and the assertion below ensure that the contents of
+        // "name" plus a trailing null will fit into the space allocated for
+        // "name_".
+        static_assert(MAX_LOGGER_NAME_SIZE < sizeof(name_));
+
+
+        // Do the copy, ensuring a trailing null in all cases.
         std::strncpy(name_, name, MAX_LOGGER_NAME_SIZE);
         name_[MAX_LOGGER_NAME_SIZE] = '\0';
     }
@@ -282,7 +288,7 @@ public:
 
     /// \brief Replace the interprocess synchronization object
     ///
-    /// If this method is called with NULL as the argument, it throws a
+    /// If this method is called with null as the argument, it throws a
     /// BadInterprocessSync exception.
     ///
     /// \note This method is intended to be used only within this log library
@@ -292,7 +298,7 @@ public:
     ///
     /// \param sync The logger uses this synchronization object for
     /// synchronizing output of log messages. It should be deletable and
-    /// the ownership is transferred to the logger. If NULL is passed,
+    /// the ownership is transferred to the logger. If null is passed,
     /// a BadInterprocessSync exception is thrown.
     void setInterprocessSync(isc::log::interprocess::InterprocessSync* sync);
 

src/lib/log/logger_impl.h

@@ -194,6 +194,7 @@ private:
     isc::log::interprocess::InterprocessSync* sync_;
 };
 
+/// \brief Pointer to the Logger implementation.
 typedef boost::shared_ptr<LoggerImpl> LoggerImplPtr;
 
 } // namespace log