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()
{
diff --git a/tests/unit-tests/util/logging.t.cpp b/tests/unit-tests/util/logging.t.cpp
index 172d58c..b431a05 100644
--- a/tests/unit-tests/util/logging.t.cpp
+++ b/tests/unit-tests/util/logging.t.cpp
@@ -31,8 +31,6 @@
NDN_LOG_INIT(ndn.util.tests.Logging);
-using boost::test_tools::output_test_stream;
-
void
logFromModule1();
@@ -43,7 +41,7 @@
logFromFilterModule();
static void
-logFromNewLogger(const std::string& moduleName)
+logFromNewLogger(const char* moduleName)
{
Logger logger(moduleName);
auto ndn_cxx_getLogger = [&logger] () -> Logger& { return logger; };
@@ -98,9 +96,11 @@
}
private:
- NDN_LOG_MEMBER_INIT(ndn.util.tests.ClassWithLogger);
+ NDN_LOG_MEMBER_DECL();
};
+NDN_LOG_MEMBER_INIT(ClassWithLogger, ndn.util.tests.ClassWithLogger);
+
template<class T, class U>
class ClassTemplateWithLogger
{
@@ -150,7 +150,7 @@
}
protected:
- output_test_stream os;
+ boost::test_tools::output_test_stream os;
private:
std::unordered_map<std::string, LogLevel> m_oldEnabledLevel;
@@ -162,9 +162,12 @@
BOOST_AUTO_TEST_CASE(GetLoggerNames)
{
- NDN_LOG_TRACE("GetLoggerNames"); // to avoid unused function warning
+ // check that all names are registered from the start even if
+ // logger instances are lazily created on first use
+ auto n = Logging::getLoggerNames().size();
+ NDN_LOG_TRACE("GetLoggerNames");
auto names = Logging::getLoggerNames();
- BOOST_CHECK(!names.empty());
+ BOOST_CHECK_EQUAL(names.size(), n);
BOOST_CHECK_EQUAL(names.count("ndn.util.tests.Logging"), 1);
}
@@ -279,6 +282,10 @@
BOOST_CHECK_EQUAL(levels.at("ndn.util.tests.ns1"), LogLevel::INFO);
BOOST_CHECK_EQUAL(levels.at("ndn.util.tests.ns2"), LogLevel::DEBUG);
+ const auto& names = Logging::getLoggerNames();
+ BOOST_CHECK_EQUAL(names.count("ndn.util.tests.ns1"), 1);
+ BOOST_CHECK_EQUAL(names.count("ndn.util.tests.ns2"), 1);
+
ns1::logFromNamespace1();
ns2::logFromNamespace2();
@@ -295,6 +302,11 @@
Logging::setLevel("ndn.util.tests.Specialized1", LogLevel::INFO);
// ndn.util.tests.Specialized2 is not enabled
+ const auto& names = Logging::getLoggerNames();
+ BOOST_CHECK_EQUAL(names.count("ndn.util.tests.ClassWithLogger"), 1);
+ BOOST_CHECK_EQUAL(names.count("ndn.util.tests.Specialized1"), 1);
+ BOOST_CHECK_EQUAL(names.count("ndn.util.tests.Specialized2"), 1);
+
ClassWithLogger::logFromStaticMemberFunction();
ClassWithLogger{}.logFromConstMemberFunction();
@@ -322,6 +334,8 @@
logFromModule1();
logFromNewLogger("Module1");
+ BOOST_CHECK_EQUAL(Logging::getLoggerNames().count("Module1"), 1);
+
Logging::flush();
BOOST_CHECK(os.is_equal(
LOG_SYSTIME_STR + " WARNING: [Module1] warn1\n" +
@@ -603,6 +617,8 @@
BOOST_AUTO_TEST_CASE(ChangeDestination)
{
+ using boost::test_tools::output_test_stream;
+
logFromModule1();
auto os2 = make_shared<output_test_stream>();