util: fix Logging::getLoggerNames()

Change-Id: Ia243b7eeb7aae69427526e23e7eff4896506871a
diff --git a/src/util/logger.cpp b/src/util/logger.cpp
index 92a1039..8c752d3 100644
--- a/src/util/logger.cpp
+++ b/src/util/logger.cpp
@@ -101,14 +101,24 @@
   return true;
 }
 
-Logger::Logger(const std::string& name)
+Logger::Logger(const char* name)
   : m_moduleName(name)
 {
-  if (!isValidLoggerName(name)) {
-    BOOST_THROW_EXCEPTION(std::invalid_argument("Logger name '" + name + "' is invalid"));
+  if (!isValidLoggerName(m_moduleName)) {
+    BOOST_THROW_EXCEPTION(std::invalid_argument("Logger name '" + m_moduleName + "' is invalid"));
   }
   this->setLevel(LogLevel::NONE);
-  Logging::addLogger(*this);
+  Logging::get().addLoggerImpl(*this);
+}
+
+void
+Logger::registerModuleName(const char* name)
+{
+  std::string moduleName(name);
+  if (!isValidLoggerName(moduleName)) {
+    BOOST_THROW_EXCEPTION(std::invalid_argument("Logger name '" + moduleName + "' is invalid"));
+  }
+  Logging::get().registerLoggerNameImpl(std::move(moduleName));
 }
 
 namespace detail {
diff --git a/src/util/logger.hpp b/src/util/logger.hpp
index 87e317b..75d86b4 100644
--- a/src/util/logger.hpp
+++ b/src/util/logger.hpp
@@ -61,13 +61,18 @@
 parseLogLevel(const std::string& s);
 
 /** \brief Represents a log module in the logging facility.
- *  \note New loggers should be defined using #NDN_LOG_INIT or #NDN_LOG_MEMBER_INIT.
+ *
+ *  \note Normally, loggers should be defined using #NDN_LOG_INIT, #NDN_LOG_MEMBER_INIT,
+ *        or #NDN_LOG_MEMBER_INIT_SPECIALIZED.
  */
 class Logger : public boost::log::sources::logger_mt
 {
 public:
   explicit
-  Logger(const std::string& name);
+  Logger(const char* name);
+
+  static void
+  registerModuleName(const char* name);
 
   const std::string&
   getModuleName() const
@@ -126,6 +131,14 @@
 } // namespace detail
 
 /** \cond */
+// implementation detail
+#define NDN_LOG_REGISTER_NAME(name) \
+  []() -> bool { \
+    ::ndn::util::Logger::registerModuleName(BOOST_STRINGIZE(name)); \
+    return true; \
+  }()
+
+// implementation detail
 #define NDN_LOG_INIT_FUNCTION_BODY(name) \
   { \
     static ::ndn::util::Logger logger(BOOST_STRINGIZE(name)); \
@@ -149,47 +162,61 @@
  */
 #define NDN_LOG_INIT(name) \
   namespace { \
+    const bool ndn_cxx_loggerRegistration __attribute__((used)) = NDN_LOG_REGISTER_NAME(name); \
     ::ndn::util::Logger& ndn_cxx_getLogger() \
     NDN_LOG_INIT_FUNCTION_BODY(name) \
   } \
   struct ndn_cxx_allow_trailing_semicolon
 
-/** \brief Define a member log module.
+/** \brief Declare a member log module, without initializing it.
  *
- *  This macro should only be used to define a log module as a class or struct member.
+ *  This macro should only be used to declare a log module as a class or struct member.
  *  It is recommended to place this macro in the private or protected section of the
- *  class or struct definition. Use #NDN_LOG_INIT to define a non-member log module.
+ *  class or struct definition. Use #NDN_LOG_INIT to declare a non-member log module.
  *
+ *  If the enclosing class is a template, this macro can be used in conjunction with
+ *  #NDN_LOG_MEMBER_DECL_SPECIALIZED and #NDN_LOG_MEMBER_INIT_SPECIALIZED to provide
+ *  different loggers for different template specializations.
+ */
+#define NDN_LOG_MEMBER_DECL() \
+  static ::ndn::util::Logger& ndn_cxx_getLogger(); \
+  private: \
+  static const bool ndn_cxx_loggerRegistration
+
+/** \brief Initialize a member log module.
+ *
+ *  This macro should only be used to initialize a previously declared member log module.
+ *  It must be placed in a .cpp file (NOT in a header file), in the same namespace as
+ *  the class or struct that contains the log module.
+ *
+ *  \param cls class name; wrap in parentheses if it contains commas
  *  \param name the logger name
  *  \note The logger name is restricted to alphanumeric characters and a select set of
  *        symbols: `~`, `#`, `%`, `_`, `<`, `>`, `.`, `-`. It must not start or end with
  *        a dot (`.`), or contain multiple consecutive dots.
  */
-#define NDN_LOG_MEMBER_INIT(name) \
-  static ::ndn::util::Logger& ndn_cxx_getLogger() \
+#define NDN_LOG_MEMBER_INIT(cls, name) \
+  const bool ::ndn::util::detail::ArgumentType<void(cls)>::ndn_cxx_loggerRegistration = \
+  NDN_LOG_REGISTER_NAME(name); \
+  ::ndn::util::Logger& ::ndn::util::detail::ArgumentType<void(cls)>::ndn_cxx_getLogger() \
   NDN_LOG_INIT_FUNCTION_BODY(name) \
   struct ndn_cxx_allow_trailing_semicolon
 
-/** \brief Forward-declare a member log module, without fully defining it.
- *
- *  This macro can be used to declare a log module as a member of a class template.
- *  Use this macro in conjunction with #NDN_LOG_MEMBER_DECL_SPECIALIZED and
- *  #NDN_LOG_MEMBER_INIT_SPECIALIZED to provide different loggers for different
- *  template specializations.
- */
-#define NDN_LOG_MEMBER_DECL() \
-  static ::ndn::util::Logger& ndn_cxx_getLogger()
-
 /** \brief Declare an explicit specialization of a member log module of a class template.
  *
  *  \param cls fully specialized class name; wrap in parentheses if it contains commas
  */
 #define NDN_LOG_MEMBER_DECL_SPECIALIZED(cls) \
   template<> \
+  const bool ::ndn::util::detail::ArgumentType<void(cls)>::ndn_cxx_loggerRegistration; \
+  template<> \
   ::ndn::util::Logger& ::ndn::util::detail::ArgumentType<void(cls)>::ndn_cxx_getLogger()
 
 /** \brief Define an explicit specialization of a member log module of a class template.
  *
+ *  This macro must be placed in a .cpp file (NOT in a header file), in the same namespace
+ *  as the class template that contains the log module.
+ *
  *  \param cls fully specialized class name; wrap in parentheses if it contains commas
  *  \param name the logger name
  *  \note The logger name is restricted to alphanumeric characters and a select set of
@@ -197,7 +224,10 @@
  *        a dot (`.`), or contain multiple consecutive dots.
  */
 #define NDN_LOG_MEMBER_INIT_SPECIALIZED(cls, name) \
-  template<> inline \
+  template<> \
+  const bool ::ndn::util::detail::ArgumentType<void(cls)>::ndn_cxx_loggerRegistration = \
+  NDN_LOG_REGISTER_NAME(name); \
+  template<> \
   ::ndn::util::Logger& ::ndn::util::detail::ArgumentType<void(cls)>::ndn_cxx_getLogger() \
   NDN_LOG_INIT_FUNCTION_BODY(name) \
   struct ndn_cxx_allow_trailing_semicolon
@@ -210,6 +240,7 @@
 #define NDN_BOOST_LOG(x) BOOST_LOG(x)
 #endif
 
+// implementation detail
 #define NDN_LOG_INTERNAL(lvl, lvlstr, expression) \
   do { \
     if (ndn_cxx_getLogger().isLevelEnabled(::ndn::util::LogLevel::lvl)) { \
diff --git a/src/util/logging.cpp b/src/util/logging.cpp
index 21a573c..0e68092 100644
--- a/src/util/logging.cpp
+++ b/src/util/logging.cpp
@@ -1,6 +1,6 @@
 /* -*- Mode:C++; c-file-style:"gnu"; indent-tabs-mode:nil; -*- */
 /*
- * Copyright (c) 2013-2017 Regents of the University of California.
+ * Copyright (c) 2013-2018 Regents of the University of California.
  *
  * This file is part of ndn-cxx library (NDN C++ library with eXperimental eXtensions).
  *
@@ -68,8 +68,14 @@
   const std::string& moduleName = logger.getModuleName();
   m_loggers.emplace(moduleName, &logger);
 
-  LogLevel level = findLevel(moduleName);
-  logger.setLevel(level);
+  logger.setLevel(findLevel(moduleName));
+}
+
+void
+Logging::registerLoggerNameImpl(std::string name)
+{
+  std::lock_guard<std::mutex> lock(m_mutex);
+  m_loggers.emplace(std::move(name), nullptr);
 }
 
 std::set<std::string>
@@ -83,9 +89,8 @@
 }
 
 LogLevel
-Logging::findLevel(const std::string& moduleName) const
+Logging::findLevel(std::string mn) const
 {
-  std::string mn = moduleName;
   while (!mn.empty()) {
     auto it = m_enabledLevel.find(mn);
     if (it != m_enabledLevel.end()) {
@@ -109,13 +114,9 @@
       mn = "";
     }
   }
+
   auto it = m_enabledLevel.find(mn);
-  if (it != m_enabledLevel.end()) {
-    return it->second;
-  }
-  else {
-    return INITIAL_DEFAULT_LEVEL;
-  }
+  return it != m_enabledLevel.end() ? it->second : INITIAL_DEFAULT_LEVEL;
 }
 
 #ifdef NDN_CXX_HAVE_TESTS
@@ -155,17 +156,19 @@
     }
     m_enabledLevel[p] = level;
 
-    for (auto&& it : m_loggers) {
-      if (it.first.compare(0, p.size(), p) == 0) {
-        it.second->setLevel(level);
+    for (const auto& pair : m_loggers) {
+      if (pair.first.compare(0, p.size(), p) == 0 && pair.second != nullptr) {
+        pair.second->setLevel(level);
       }
     }
   }
   else {
     m_enabledLevel[prefix] = level;
     auto range = boost::make_iterator_range(m_loggers.equal_range(prefix));
-    for (auto&& it : range) {
-      it.second->setLevel(level);
+    for (const auto& pair : range) {
+      if (pair.second != nullptr) {
+        pair.second->setLevel(level);
+      }
     }
   }
 }
diff --git a/src/util/logging.hpp b/src/util/logging.hpp
index d85b524..8fc6342 100644
--- a/src/util/logging.hpp
+++ b/src/util/logging.hpp
@@ -1,6 +1,6 @@
 /* -*- Mode:C++; c-file-style:"gnu"; indent-tabs-mode:nil; -*- */
 /*
- * Copyright (c) 2013-2017 Regents of the University of California.
+ * Copyright (c) 2013-2018 Regents of the University of California.
  *
  * This file is part of ndn-cxx library (NDN C++ library with eXperimental eXtensions).
  *
@@ -38,7 +38,7 @@
 enum class LogLevel;
 class Logger;
 
-/** \brief controls the logging facility
+/** \brief Controls the logging facility.
  *
  *  \note Public static methods are thread safe.
  *        Non-public methods are not guaranteed to be thread safe.
@@ -46,65 +46,60 @@
 class Logging : noncopyable
 {
 public:
-  /** \brief register a new logger
-   *  \note App should declare a new logger with \p NDN_LOG_INIT macro.
-   */
-  static void
-  addLogger(Logger& logger);
-
-  /** \brief get list of names of all registered loggers
+  /** \brief Get list of all registered logger names.
    */
   static std::set<std::string>
   getLoggerNames();
 
-  /** \brief set severity level
-   *  \param prefix logger prefix; this can be a specific logger name, a general prefix like ndn.a.*
-   *   to apply a setting for all modules that contain this prefix, or "*" for all modules
+  /** \brief Set severity level.
+   *  \param prefix logger prefix; this can be a specific logger name, a general prefix like
+   *                `"ndn.a.*"` to apply a setting for all modules that contain that prefix,
+   *                or `"*"` for all modules
    *  \param level minimum severity level
    *
-   *  Log messages are output only if its severity is greater than the set minimum severity level.
-   *  Initial default severity level is \p LogLevel::NONE which enables FATAL only.
+   *  Log messages are output only if their severity is greater than the current minimum severity
+   *  level. The initial severity level is \c LogLevel::NONE, which enables FATAL messages only.
    */
   static void
   setLevel(const std::string& prefix, LogLevel level);
 
-  /** \brief set severity levels with a config string
-   *  \param config colon-separate key=value pairs
+  /** \brief Set severity levels with a config string.
+   *  \param config colon-separated `key=value` pairs
    *  \throw std::invalid_argument config string is malformed
    *
    *  \code
-   *  Logging::setSeverityLevels("*=INFO:Face=DEBUG:NfdController=WARN");
+   *  Logging::setLevel("*=INFO:Face=DEBUG:NfdController=WARN");
    *  \endcode
-   *  is equivalent to
+   *  is equivalent to:
    *  \code
-   *  Logging::setSeverityLevel("*", LogLevel::INFO);
-   *  Logging::setSeverityLevel("Face", LogLevel::DEBUG);
-   *  Logging::setSeverityLevel("NfdController", LogLevel::WARN);
+   *  Logging::setLevel("*", LogLevel::INFO);
+   *  Logging::setLevel("Face", LogLevel::DEBUG);
+   *  Logging::setLevel("NfdController", LogLevel::WARN);
    *  \endcode
    */
   static void
   setLevel(const std::string& config);
 
-  /** \brief set log destination
+  /** \brief Set log destination.
    *  \param os a stream for log output
    *
-   *  Initial destination is \p std::clog .
+   *  The initial destination is `std::clog`.
    */
   static void
   setDestination(shared_ptr<std::ostream> os);
 
-  /** \brief set log destination
-   *  \param os a stream for log output; caller must ensure this is valid
-   *            until setDestination is invoked again or program exits
+  /** \brief Set log destination.
+   *  \param os a stream for log output; caller must ensure it remains valid
+   *            until setDestination() is invoked again or program exits
    *
-   *  This is equivalent to setDestination(shared_ptr<std::ostream>(&os, nullDeleter))
+   *  This is equivalent to `setDestination(shared_ptr<std::ostream>(&os, nullDeleter))`.
    */
   static void
   setDestination(std::ostream& os);
 
-  /** \brief flush log backend
+  /** \brief Flush log backend.
    *
-   *  This ensures log messages are written to the destination stream.
+   *  This ensures all log messages are written to the destination stream.
    */
   static void
   flush();
@@ -115,11 +110,14 @@
   void
   addLoggerImpl(Logger& logger);
 
+  void
+  registerLoggerNameImpl(std::string name);
+
   std::set<std::string>
   getLoggerNamesImpl() const;
 
   /**
-   * \brief finds the appropriate LogLevel for a logger
+   * \brief Finds the appropriate LogLevel for a logger.
    * \param moduleName name of logger
    *
    * This searches m_enabledLevel map to determine which LogLevel is appropriate for
@@ -131,7 +129,7 @@
    * found.
    */
   LogLevel
-  findLevel(const std::string& moduleName) const;
+  findLevel(std::string moduleName) const;
 
   void
   setLevelImpl(const std::string& prefix, LogLevel level);
@@ -167,21 +165,17 @@
 #endif // NDN_CXX_HAVE_TESTS
 
 private:
+  friend Logger;
+
   mutable std::mutex m_mutex;
   std::unordered_map<std::string, LogLevel> m_enabledLevel; ///< module prefix => minimum level
-  std::unordered_multimap<std::string, Logger*> m_loggers; ///< moduleName => logger
+  std::unordered_multimap<std::string, Logger*> m_loggers; ///< module name => logger instance
 
-  shared_ptr<std::ostream> m_destination;
-  typedef boost::log::sinks::asynchronous_sink<boost::log::sinks::text_ostream_backend> Sink;
+  using Sink = boost::log::sinks::asynchronous_sink<boost::log::sinks::text_ostream_backend>;
   boost::shared_ptr<Sink> m_sink;
+  shared_ptr<std::ostream> m_destination;
 };
 
-inline void
-Logging::addLogger(Logger& logger)
-{
-  get().addLoggerImpl(logger);
-}
-
 inline std::set<std::string>
 Logging::getLoggerNames()
 {