From 02931157ec65d010f0149ac01a81e4a7defdd9e3 Mon Sep 17 00:00:00 2001 From: Dimitar Krastev Date: Fri, 20 Sep 2024 21:40:10 +0300 Subject: [PATCH 01/52] Pulled ostream operators for IPAddress and MacAddress into the namespace to satisfy ADL requirements. --- Common++/header/IpAddress.h | 62 ++++++++++++++++++------------------ Common++/header/MacAddress.h | 12 +++---- 2 files changed, 37 insertions(+), 37 deletions(-) diff --git a/Common++/header/IpAddress.h b/Common++/header/IpAddress.h index 243dae0ef9..44d0f98735 100644 --- a/Common++/header/IpAddress.h +++ b/Common++/header/IpAddress.h @@ -1082,40 +1082,40 @@ namespace pcpp std::unique_ptr m_IPv4Network; std::unique_ptr m_IPv6Network; }; -} // namespace pcpp -inline std::ostream& operator<<(std::ostream& os, const pcpp::IPv4Address& ipv4Address) -{ - os << ipv4Address.toString(); - return os; -} + inline std::ostream& operator<<(std::ostream& os, const pcpp::IPv4Address& ipv4Address) + { + os << ipv4Address.toString(); + return os; + } -inline std::ostream& operator<<(std::ostream& os, const pcpp::IPv6Address& ipv6Address) -{ - os << ipv6Address.toString(); - return os; -} + inline std::ostream& operator<<(std::ostream& os, const pcpp::IPv6Address& ipv6Address) + { + os << ipv6Address.toString(); + return os; + } -inline std::ostream& operator<<(std::ostream& os, const pcpp::IPAddress& ipAddress) -{ - os << ipAddress.toString(); - return os; -} + inline std::ostream& operator<<(std::ostream& os, const pcpp::IPAddress& ipAddress) + { + os << ipAddress.toString(); + return os; + } -inline std::ostream& operator<<(std::ostream& os, const pcpp::IPv4Network& network) -{ - os << network.toString(); - return os; -} + inline std::ostream& operator<<(std::ostream& os, const pcpp::IPv4Network& network) + { + os << network.toString(); + return os; + } -inline std::ostream& operator<<(std::ostream& os, const pcpp::IPv6Network& network) -{ - os << network.toString(); - return os; -} + inline std::ostream& operator<<(std::ostream& os, const pcpp::IPv6Network& network) + { + os << network.toString(); + return os; + } -inline std::ostream& operator<<(std::ostream& os, const pcpp::IPNetwork& network) -{ - os << network.toString(); - return os; -} + inline std::ostream& operator<<(std::ostream& os, const pcpp::IPNetwork& network) + { + os << network.toString(); + return os; + } +} // namespace pcpp diff --git a/Common++/header/MacAddress.h b/Common++/header/MacAddress.h index 29333aa02d..8b59f7d53c 100644 --- a/Common++/header/MacAddress.h +++ b/Common++/header/MacAddress.h @@ -173,10 +173,10 @@ namespace pcpp private: uint8_t m_Address[6] = { 0 }; }; -} // namespace pcpp -inline std::ostream& operator<<(std::ostream& os, const pcpp::MacAddress& macAddress) -{ - os << macAddress.toString(); - return os; -} + inline std::ostream& operator<<(std::ostream& os, const pcpp::MacAddress& macAddress) + { + os << macAddress.toString(); + return os; + } +} // namespace pcpp From 45115e327f06ea8fd3aa142847d0a49c6097db81 Mon Sep 17 00:00:00 2001 From: Dimitar Krastev Date: Mon, 30 Sep 2024 16:52:23 +0300 Subject: [PATCH 02/52] Refactor of Logger. - LogLevel is now a top level enum class. - Added a new log level Off to disable output from a specific module. - Logger::LogLevel is a deprecated alias to LogLevel. - Logger::Info, Debug, Error are deprecated aliases to LogLevel::... - Removed public "internal*" functions from Logger.Logger - Added LogSource struct to encapsulate source information. - Added shouldLog method to check if a log should be emitted for a given level and module. - Removed nonfunctional artifacts "m_LogStream" and "Logger::operator<<" - Added templated "log" functions that are friends to Logger. - Reworked PCPP_LOG macros to no longer utilize the now removed internal functions. - Added PCPP_LOG_INFO macro level. - Changed PCPP_LOG_ERROR to now check if the log should be emitted. - Fixed NetworkUtils log module name overlapping with NetworkUtils class. - Fixed missing enum value for PacketLogModuleSll2Layer. --- Common++/header/Logger.h | 179 ++++++++++++++++++------- Common++/src/Logger.cpp | 39 +++--- Pcap++/src/NetworkUtils.cpp | 2 +- Tests/Pcap++Test/Tests/LoggerTests.cpp | 2 +- 4 files changed, 150 insertions(+), 72 deletions(-) diff --git a/Common++/header/Logger.h b/Common++/header/Logger.h index 6973cf530d..962754066d 100644 --- a/Common++/header/Logger.h +++ b/Common++/header/Logger.h @@ -5,6 +5,7 @@ #include #include #include +#include "DeprecationUtils.h" #ifndef LOG_MODULE # define LOG_MODULE UndefinedLogModule @@ -17,29 +18,6 @@ # define PCAPPP_FILENAME __FILE__ #endif -#define PCPP_LOG(level, message) \ - do \ - { \ - std::ostringstream* sstream = pcpp::Logger::getInstance().internalCreateLogStream(); \ - (*sstream) << message; \ - pcpp::Logger::getInstance().internalPrintLogMessage(sstream, level, PCAPPP_FILENAME, __FUNCTION__, __LINE__); \ - } while (0) - -#define PCPP_LOG_DEBUG(message) \ - do \ - { \ - if (pcpp::Logger::getInstance().logsEnabled() && pcpp::Logger::getInstance().isDebugEnabled(LOG_MODULE)) \ - { \ - PCPP_LOG(pcpp::Logger::Debug, message); \ - } \ - } while (0) - -#define PCPP_LOG_ERROR(message) \ - do \ - { \ - PCPP_LOG(pcpp::Logger::Error, message); \ - } while (0) - /// @file /** @@ -79,6 +57,7 @@ namespace pcpp PacketLogModuleGreLayer, ///< GreLayer module (Packet++) PacketLogModuleSSLLayer, ///< SSLLayer module (Packet++) PacketLogModuleSllLayer, ///< SllLayer module (Packet++) + PacketLogModuleSll2Layer, ///< Sll2Layer module (Packet++) PacketLogModuleNflogLayer, ///< NflogLayer module (Packet++) PacketLogModuleDhcpLayer, ///< DhcpLayer module (Packet++) PacketLogModuleDhcpV6Layer, ///< DhcpV6Layer module (Packet++) @@ -112,10 +91,47 @@ namespace pcpp PcapLogModuleDpdkDevice, ///< DpdkDevice module (Pcap++) PcapLogModuleKniDevice, ///< KniDevice module (Pcap++) PcapLogModuleXdpDevice, ///< XdpDevice module (Pcap++) - NetworkUtils, ///< NetworkUtils module (Pcap++) + PcapLogModuleNetworkUtils, ///< Network Utils module (Pcap++) NumOfLogModules }; + struct LogSource + { + constexpr LogSource() = default; + constexpr LogSource(LogModule logModule) : logModule(logModule) + {} + constexpr LogSource(LogModule logModule, const char* file, const char* function, int line) + : file(file), function(function), line(line), logModule(logModule) + {} + + const char* file = nullptr; + const char* function = nullptr; + int line = 0; + LogModule logModule = UndefinedLogModule; + }; + + /** + * An enum representing the log level. Currently 3 log levels are supported: Error, Info and Debug. Info is the + * default log level + */ + enum class LogLevel + { + Off, ///< No log messages are emitted. + Error, ///< Error level logs are emitted. + Info, ///< Info level logs and above are emitted. + Debug ///< Debug level logs and above are emitted. + }; + + inline std::ostream& operator<<(std::ostream& s, LogLevel v) + { + return s << static_cast::type>(v); + } + + // Forward declaration + template void log(LogSource source, LogLevel level, T const& message); + template <> void log(LogSource source, LogLevel level, std::string const& message); + template <> void log(LogSource source, LogLevel level, const char* const& message); + /** * @class Logger * PcapPlusPlus logger manager. @@ -142,12 +158,21 @@ namespace pcpp * An enum representing the log level. Currently 3 log levels are supported: Error, Info and Debug. Info is the * default log level */ - enum LogLevel + /* enum LogLevel { - Error, ///< Error log level - Info, ///< Info log level - Debug ///< Debug log level - }; + Error, ///< Error log level + Info, ///< Info log level + Debug ///< Debug log level + };*/ + + // Deprecated, Use the LogLevel in the pcpp namespace instead. + using LogLevel = pcpp::LogLevel; + PCPP_DEPRECATED("Use the LogLevel in the pcpp namespace instead.") + static const LogLevel Error = LogLevel::Error; + PCPP_DEPRECATED("Use the LogLevel in the pcpp namespace instead.") + static const LogLevel Info = LogLevel::Info; + PCPP_DEPRECATED("Use the LogLevel in the pcpp namespace instead.") + static const LogLevel Debug = LogLevel::Debug; /** * @typedef LogPrinter @@ -195,7 +220,18 @@ namespace pcpp */ bool isDebugEnabled(LogModule module) const { - return m_LogModulesArray[module] == Debug; + return m_LogModulesArray[module] == LogLevel::Debug; + } + + /** + * @brief Check whether a log level should be emitted by the logger. + * @param level The level of the log message. + * @param module PcapPlusPlus module + * @return True if the message should be emitted. False otherwise. + */ + bool shouldLog(LogLevel level, LogModule module) const + { + return level != LogLevel::Off && m_LogModulesArray[module] >= level; } /** @@ -258,20 +294,6 @@ namespace pcpp return m_LogsEnabled; } - template Logger& operator<<(const T& msg) - { - (*m_LogStream) << msg; - return *this; - } - - std::ostringstream* internalCreateLogStream(); - - /** - * An internal method to print log messages. Shouldn't be used externally. - */ - void internalPrintLogMessage(std::ostringstream* logStream, Logger::LogLevel logLevel, const char* file, - const char* method, int line); - /** * Get access to Logger singleton * @todo: make this singleton thread-safe/ @@ -283,17 +305,82 @@ namespace pcpp return instance; } + template friend void pcpp::log(LogSource source, LogLevel level, T const& message); + private: bool m_LogsEnabled; - Logger::LogLevel m_LogModulesArray[NumOfLogModules]; + LogLevel m_LogModulesArray[NumOfLogModules]; LogPrinter m_LogPrinter; std::string m_LastError; - std::ostringstream* m_LogStream; // private c'tor - this class is a singleton Logger(); + void printLogMessage(LogSource source, LogLevel logLevel, std::string const& message); static void defaultLogPrinter(LogLevel logLevel, const std::string& logMessage, const std::string& file, const std::string& method, const int line); }; + + template inline void log(LogSource source, LogLevel level, T const& message) + { + auto& logger = Logger::getInstance(); + if (logger.shouldLog(level, source.module)) + { + std::ostringstream sstream; + sstream << message; + logger.printLogMessage(source, level, sstream); + } + }; + + // Specialization for string to skip the stringstream + template <> inline void log(LogSource source, LogLevel level, std::string const& message) + { + auto& logger = Logger::getInstance(); + if (logger.shouldLog(level, source.logModule)) + { + logger.printLogMessage(source, level, message); + } + }; + + // Specialization for const char* to skip the stringstream + template <> inline void log(LogSource source, LogLevel level, const char* const& message) + { + auto& logger = Logger::getInstance(); + if (logger.shouldLog(level, source.logModule)) + { + logger.printLogMessage(source, level, message); + } + }; + + template inline void logError(LogSource source, T const& message) + { + log(source, LogLevel::Error, message); + }; + + template inline void logInfo(LogSource source, T const& message) + { + log(source, LogLevel::Info, message); + }; + + template inline void logDebug(LogSource source, T const& message) + { + log(source, LogLevel::Debug, message); + }; } // namespace pcpp + +#define PCPP_LOG(level, message) \ + do \ + { \ + if (pcpp::Logger::getInstance().shouldLog(level, LOG_MODULE)) \ + { \ + std::ostringstream sstream; \ + sstream << message; \ + pcpp::log(pcpp::LogSource(LOG_MODULE, PCAPPP_FILENAME, __FUNCTION__, __LINE__), level, sstream.str()); \ + } \ + } while (0) + +#define PCPP_LOG_DEBUG(message) PCPP_LOG(pcpp::LogLevel::Debug, message) + +#define PCPP_LOG_INFO(message) PCPP_LOG(pcpp::LogLevel::Info, message) + +#define PCPP_LOG_ERROR(message) PCPP_LOG(pcpp::LogLevel::Error, message) diff --git a/Common++/src/Logger.cpp b/Common++/src/Logger.cpp index 7608911b6b..ec52604a74 100644 --- a/Common++/src/Logger.cpp +++ b/Common++/src/Logger.cpp @@ -8,49 +8,40 @@ namespace pcpp { m_LastError.reserve(200); for (int i = 0; i < NumOfLogModules; i++) - m_LogModulesArray[i] = Info; + m_LogModulesArray[i] = LogLevel::Info; } std::string Logger::logLevelAsString(LogLevel logLevel) { switch (logLevel) { - case Logger::Error: + case LogLevel::Error: return "ERROR"; - case Logger::Info: + case LogLevel::Info: return "INFO"; default: return "DEBUG"; } } - void Logger::defaultLogPrinter(LogLevel logLevel, const std::string& logMessage, const std::string& file, - const std::string& method, const int line) - { - std::ostringstream sstream; - sstream << file << ": " << method << ":" << line; - std::cerr << std::left << "[" << std::setw(5) << Logger::logLevelAsString(logLevel) << ": " << std::setw(45) - << sstream.str() << "] " << logMessage << std::endl; - } - - std::ostringstream* Logger::internalCreateLogStream() - { - return new std::ostringstream(); - } - - void Logger::internalPrintLogMessage(std::ostringstream* logStream, Logger::LogLevel logLevel, const char* file, - const char* method, int line) + void Logger::printLogMessage(LogSource source, LogLevel logLevel, std::string const& message) { - std::string logMessage = logStream->str(); - delete logStream; - if (logLevel == Logger::Error) + if (logLevel == LogLevel::Error) { - m_LastError = logMessage; + m_LastError = message; } if (m_LogsEnabled) { - m_LogPrinter(logLevel, logMessage, file, method, line); + m_LogPrinter(logLevel, message, source.file, source.function, source.line); } } + void Logger::defaultLogPrinter(LogLevel logLevel, const std::string& logMessage, const std::string& file, + const std::string& method, const int line) + { + std::ostringstream sstream; + sstream << file << ": " << method << ":" << line; + std::cerr << std::left << "[" << std::setw(5) << Logger::logLevelAsString(logLevel) << ": " << std::setw(45) + << sstream.str() << "] " << logMessage << std::endl; + } } // namespace pcpp diff --git a/Pcap++/src/NetworkUtils.cpp b/Pcap++/src/NetworkUtils.cpp index 06e26f5315..a9eddf5e49 100644 --- a/Pcap++/src/NetworkUtils.cpp +++ b/Pcap++/src/NetworkUtils.cpp @@ -1,4 +1,4 @@ -#define LOG_MODULE NetworkUtils +#define LOG_MODULE PcapLogModuleNetworkUtils #include #include diff --git a/Tests/Pcap++Test/Tests/LoggerTests.cpp b/Tests/Pcap++Test/Tests/LoggerTests.cpp index 00f06d332a..e40b235ea8 100644 --- a/Tests/Pcap++Test/Tests/LoggerTests.cpp +++ b/Tests/Pcap++Test/Tests/LoggerTests.cpp @@ -136,7 +136,7 @@ class LoggerCleaner ~LoggerCleaner() { pcpp::Logger::getInstance().enableLogs(); - pcpp::Logger::getInstance().setAllModulesToLogLevel(pcpp::Logger::Info); + pcpp::Logger::getInstance().setAllModulesToLogLevel(pcpp::LogLevel::Info); pcpp::Logger::getInstance().resetLogPrinter(); std::cout.clear(); LogPrinter::clean(); From 9062c483d746d901bdd9e1efb70f7a1b614114d2 Mon Sep 17 00:00:00 2001 From: Dimitar Krastev Date: Mon, 30 Sep 2024 16:59:50 +0300 Subject: [PATCH 03/52] Cleanup and fixes. --- Common++/header/Logger.h | 13 +------------ 1 file changed, 1 insertion(+), 12 deletions(-) diff --git a/Common++/header/Logger.h b/Common++/header/Logger.h index 1428d4d240..6b7cd6da4a 100644 --- a/Common++/header/Logger.h +++ b/Common++/header/Logger.h @@ -155,17 +155,6 @@ namespace pcpp class Logger { public: - /** - * An enum representing the log level. Currently 3 log levels are supported: Error, Info and Debug. Info is the - * default log level - */ - /* enum LogLevel - { - Error, ///< Error log level - Info, ///< Info log level - Debug ///< Debug log level - };*/ - // Deprecated, Use the LogLevel in the pcpp namespace instead. using LogLevel = pcpp::LogLevel; PCPP_DEPRECATED("Use the LogLevel in the pcpp namespace instead.") @@ -329,7 +318,7 @@ namespace pcpp { std::ostringstream sstream; sstream << message; - logger.printLogMessage(source, level, sstream); + logger.printLogMessage(source, level, sstream.str()); } }; From 1e856f00346e4037585e9732bd6817c1a30ba9fb Mon Sep 17 00:00:00 2001 From: Dimitar Krastev Date: Mon, 30 Sep 2024 17:39:14 +0300 Subject: [PATCH 04/52] Added the new Off log level to the string conversion. --- Common++/src/Logger.cpp | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/Common++/src/Logger.cpp b/Common++/src/Logger.cpp index ec52604a74..8b5e22125c 100644 --- a/Common++/src/Logger.cpp +++ b/Common++/src/Logger.cpp @@ -15,12 +15,16 @@ namespace pcpp { switch (logLevel) { + case LogLevel::Off: + return "OFF"; case LogLevel::Error: return "ERROR"; case LogLevel::Info: return "INFO"; - default: + case LogLevel::Debug: return "DEBUG"; + default: + return "UNKNOWN"; } } From 02618fc35a961a89b7db2ffea82519244297989f Mon Sep 17 00:00:00 2001 From: Dimitar Krastev Date: Mon, 30 Sep 2024 17:56:02 +0300 Subject: [PATCH 05/52] Fixed wrong variable name. --- Common++/header/Logger.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Common++/header/Logger.h b/Common++/header/Logger.h index 6b7cd6da4a..8e9e2f477d 100644 --- a/Common++/header/Logger.h +++ b/Common++/header/Logger.h @@ -314,7 +314,7 @@ namespace pcpp template inline void log(LogSource source, LogLevel level, T const& message) { auto& logger = Logger::getInstance(); - if (logger.shouldLog(level, source.module)) + if (logger.shouldLog(level, source.logModule)) { std::ostringstream sstream; sstream << message; From 32ff8f936b244fe485b2a540874b6a0eb0eb81aa Mon Sep 17 00:00:00 2001 From: Dimitar Krastev Date: Mon, 30 Sep 2024 17:58:40 +0300 Subject: [PATCH 06/52] Added documentation to log source. --- Common++/header/Logger.h | 27 +++++++++++++++++++++++---- 1 file changed, 23 insertions(+), 4 deletions(-) diff --git a/Common++/header/Logger.h b/Common++/header/Logger.h index 8e9e2f477d..47bc7928a1 100644 --- a/Common++/header/Logger.h +++ b/Common++/header/Logger.h @@ -98,17 +98,36 @@ namespace pcpp struct LogSource { + /** + * @struct LogSource + * Represents the source of a log message. + * Contains information about the source file, function, line number, and the log module. + */ + constexpr LogSource() = default; + + /** + * Constructor for LogSource with only the log module. + * @param logModule The log module. + */ constexpr LogSource(LogModule logModule) : logModule(logModule) {} + + /** + * Constructor for LogSource with all parameters. + * @param logModule The log module. + * @param file The source file. + * @param function The source function. + * @param line The line number. + */ constexpr LogSource(LogModule logModule, const char* file, const char* function, int line) : file(file), function(function), line(line), logModule(logModule) {} - const char* file = nullptr; - const char* function = nullptr; - int line = 0; - LogModule logModule = UndefinedLogModule; + const char* file = nullptr; /**< The source file. */ + const char* function = nullptr; /**< The source function. */ + int line = 0; /**< The line number. */ + LogModule logModule = UndefinedLogModule; /**< The log module. */ }; /** From 12492d2aca78616ac2a0f72c939678af9bed90f9 Mon Sep 17 00:00:00 2001 From: Dimitar Krastev Date: Mon, 30 Sep 2024 17:59:14 +0300 Subject: [PATCH 07/52] Lint. --- Common++/header/Logger.h | 1 - 1 file changed, 1 deletion(-) diff --git a/Common++/header/Logger.h b/Common++/header/Logger.h index 47bc7928a1..6a9d8560a8 100644 --- a/Common++/header/Logger.h +++ b/Common++/header/Logger.h @@ -103,7 +103,6 @@ namespace pcpp * Represents the source of a log message. * Contains information about the source file, function, line number, and the log module. */ - constexpr LogSource() = default; /** From f21f85d9f53b57f06a70923788b3044b11414151 Mon Sep 17 00:00:00 2001 From: Dimitar Krastev Date: Mon, 30 Sep 2024 18:01:05 +0300 Subject: [PATCH 08/52] Fixed docstring for LogSource. --- Common++/header/Logger.h | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/Common++/header/Logger.h b/Common++/header/Logger.h index 6a9d8560a8..c0c1fd2fbc 100644 --- a/Common++/header/Logger.h +++ b/Common++/header/Logger.h @@ -96,13 +96,16 @@ namespace pcpp NumOfLogModules }; + /** + * @struct LogSource + * Represents the source of a log message. + * Contains information about the source file, function, line number, and the log module. + */ struct LogSource { /** - * @struct LogSource - * Represents the source of a log message. - * Contains information about the source file, function, line number, and the log module. - */ + * Default constructor for LogSource. + */ / constexpr LogSource() = default; /** From 0faf2e83422abc70fec01415b130c3c37c003228 Mon Sep 17 00:00:00 2001 From: Dimitar Krastev Date: Mon, 30 Sep 2024 18:09:53 +0300 Subject: [PATCH 09/52] Fixed extra / --- Common++/header/Logger.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Common++/header/Logger.h b/Common++/header/Logger.h index c0c1fd2fbc..a6ec97e7af 100644 --- a/Common++/header/Logger.h +++ b/Common++/header/Logger.h @@ -105,7 +105,7 @@ namespace pcpp { /** * Default constructor for LogSource. - */ / + */ constexpr LogSource() = default; /** From 43d7b1c527fd7f3e55a211dd4be0fb42780bb0f5 Mon Sep 17 00:00:00 2001 From: Dimitar Krastev Date: Mon, 7 Oct 2024 02:07:54 +0300 Subject: [PATCH 10/52] Fixed explicit warning. --- Common++/header/Logger.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Common++/header/Logger.h b/Common++/header/Logger.h index a6ec97e7af..5533aed7d9 100644 --- a/Common++/header/Logger.h +++ b/Common++/header/Logger.h @@ -112,7 +112,7 @@ namespace pcpp * Constructor for LogSource with only the log module. * @param logModule The log module. */ - constexpr LogSource(LogModule logModule) : logModule(logModule) + explicit constexpr LogSource(LogModule logModule) : logModule(logModule) {} /** From be95ab890d242fdfa3d1ad89bfc720aac06b6cab Mon Sep 17 00:00:00 2001 From: Dimitar Krastev Date: Mon, 14 Oct 2024 17:44:14 +0300 Subject: [PATCH 11/52] Moved log functions inside logger. --- Common++/header/Logger.h | 99 ++++++++++++++++++---------------------- 1 file changed, 45 insertions(+), 54 deletions(-) diff --git a/Common++/header/Logger.h b/Common++/header/Logger.h index 5533aed7d9..d3f303b7e1 100644 --- a/Common++/header/Logger.h +++ b/Common++/header/Logger.h @@ -149,11 +149,6 @@ namespace pcpp return s << static_cast::type>(v); } - // Forward declaration - template void log(LogSource source, LogLevel level, T const& message); - template <> void log(LogSource source, LogLevel level, std::string const& message); - template <> void log(LogSource source, LogLevel level, const char* const& message); - /** * @class Logger * PcapPlusPlus logger manager. @@ -316,7 +311,48 @@ namespace pcpp return instance; } - template friend void pcpp::log(LogSource source, LogLevel level, T const& message); + template void log(LogSource source, LogLevel level, T const& message) + { + if (shouldLog(level, source.logModule)) + { + std::ostringstream sstream; + sstream << message; + printLogMessage(source, level, sstream.str()); + } + }; + + // Specialization for string to skip the stringstream + template <> void log(LogSource source, LogLevel level, std::string const& message) + { + if (shouldLog(level, source.logModule)) + { + printLogMessage(source, level, message); + } + }; + + // Specialization for const char* to skip the stringstream + template <> void log(LogSource source, LogLevel level, const char* const& message) + { + if (shouldLog(level, source.logModule)) + { + printLogMessage(source, level, message); + } + }; + + template void logError(LogSource source, T const& message) + { + log(source, LogLevel::Error, message); + }; + + template void logInfo(LogSource source, T const& message) + { + log(source, LogLevel::Info, message); + }; + + template void logDebug(LogSource source, T const& message) + { + log(source, LogLevel::Debug, message); + }; private: bool m_LogsEnabled; @@ -331,62 +367,17 @@ namespace pcpp static void defaultLogPrinter(LogLevel logLevel, const std::string& logMessage, const std::string& file, const std::string& method, const int line); }; - - template inline void log(LogSource source, LogLevel level, T const& message) - { - auto& logger = Logger::getInstance(); - if (logger.shouldLog(level, source.logModule)) - { - std::ostringstream sstream; - sstream << message; - logger.printLogMessage(source, level, sstream.str()); - } - }; - - // Specialization for string to skip the stringstream - template <> inline void log(LogSource source, LogLevel level, std::string const& message) - { - auto& logger = Logger::getInstance(); - if (logger.shouldLog(level, source.logModule)) - { - logger.printLogMessage(source, level, message); - } - }; - - // Specialization for const char* to skip the stringstream - template <> inline void log(LogSource source, LogLevel level, const char* const& message) - { - auto& logger = Logger::getInstance(); - if (logger.shouldLog(level, source.logModule)) - { - logger.printLogMessage(source, level, message); - } - }; - - template inline void logError(LogSource source, T const& message) - { - log(source, LogLevel::Error, message); - }; - - template inline void logInfo(LogSource source, T const& message) - { - log(source, LogLevel::Info, message); - }; - - template inline void logDebug(LogSource source, T const& message) - { - log(source, LogLevel::Debug, message); - }; } // namespace pcpp #define PCPP_LOG(level, message) \ do \ { \ - if (pcpp::Logger::getInstance().shouldLog(level, LOG_MODULE)) \ + auto& logger = Logger::getInstance(); \ + if (logger.shouldLog(level, LOG_MODULE)) \ { \ std::ostringstream sstream; \ sstream << message; \ - pcpp::log(pcpp::LogSource(LOG_MODULE, PCAPPP_FILENAME, __FUNCTION__, __LINE__), level, sstream.str()); \ + logger.log(pcpp::LogSource(LOG_MODULE, PCAPPP_FILENAME, __FUNCTION__, __LINE__), level, sstream.str()); \ } \ } while (0) From 4b21a99a0b846126b5f0d44c246bb1decd99762f Mon Sep 17 00:00:00 2001 From: Dimitar Krastev Date: Mon, 14 Oct 2024 23:21:39 +0300 Subject: [PATCH 12/52] Revert "Moved log functions inside logger." This reverts commit be95ab890d242fdfa3d1ad89bfc720aac06b6cab. --- Common++/header/Logger.h | 99 ++++++++++++++++++++++------------------ 1 file changed, 54 insertions(+), 45 deletions(-) diff --git a/Common++/header/Logger.h b/Common++/header/Logger.h index d3f303b7e1..5533aed7d9 100644 --- a/Common++/header/Logger.h +++ b/Common++/header/Logger.h @@ -149,6 +149,11 @@ namespace pcpp return s << static_cast::type>(v); } + // Forward declaration + template void log(LogSource source, LogLevel level, T const& message); + template <> void log(LogSource source, LogLevel level, std::string const& message); + template <> void log(LogSource source, LogLevel level, const char* const& message); + /** * @class Logger * PcapPlusPlus logger manager. @@ -311,48 +316,7 @@ namespace pcpp return instance; } - template void log(LogSource source, LogLevel level, T const& message) - { - if (shouldLog(level, source.logModule)) - { - std::ostringstream sstream; - sstream << message; - printLogMessage(source, level, sstream.str()); - } - }; - - // Specialization for string to skip the stringstream - template <> void log(LogSource source, LogLevel level, std::string const& message) - { - if (shouldLog(level, source.logModule)) - { - printLogMessage(source, level, message); - } - }; - - // Specialization for const char* to skip the stringstream - template <> void log(LogSource source, LogLevel level, const char* const& message) - { - if (shouldLog(level, source.logModule)) - { - printLogMessage(source, level, message); - } - }; - - template void logError(LogSource source, T const& message) - { - log(source, LogLevel::Error, message); - }; - - template void logInfo(LogSource source, T const& message) - { - log(source, LogLevel::Info, message); - }; - - template void logDebug(LogSource source, T const& message) - { - log(source, LogLevel::Debug, message); - }; + template friend void pcpp::log(LogSource source, LogLevel level, T const& message); private: bool m_LogsEnabled; @@ -367,17 +331,62 @@ namespace pcpp static void defaultLogPrinter(LogLevel logLevel, const std::string& logMessage, const std::string& file, const std::string& method, const int line); }; + + template inline void log(LogSource source, LogLevel level, T const& message) + { + auto& logger = Logger::getInstance(); + if (logger.shouldLog(level, source.logModule)) + { + std::ostringstream sstream; + sstream << message; + logger.printLogMessage(source, level, sstream.str()); + } + }; + + // Specialization for string to skip the stringstream + template <> inline void log(LogSource source, LogLevel level, std::string const& message) + { + auto& logger = Logger::getInstance(); + if (logger.shouldLog(level, source.logModule)) + { + logger.printLogMessage(source, level, message); + } + }; + + // Specialization for const char* to skip the stringstream + template <> inline void log(LogSource source, LogLevel level, const char* const& message) + { + auto& logger = Logger::getInstance(); + if (logger.shouldLog(level, source.logModule)) + { + logger.printLogMessage(source, level, message); + } + }; + + template inline void logError(LogSource source, T const& message) + { + log(source, LogLevel::Error, message); + }; + + template inline void logInfo(LogSource source, T const& message) + { + log(source, LogLevel::Info, message); + }; + + template inline void logDebug(LogSource source, T const& message) + { + log(source, LogLevel::Debug, message); + }; } // namespace pcpp #define PCPP_LOG(level, message) \ do \ { \ - auto& logger = Logger::getInstance(); \ - if (logger.shouldLog(level, LOG_MODULE)) \ + if (pcpp::Logger::getInstance().shouldLog(level, LOG_MODULE)) \ { \ std::ostringstream sstream; \ sstream << message; \ - logger.log(pcpp::LogSource(LOG_MODULE, PCAPPP_FILENAME, __FUNCTION__, __LINE__), level, sstream.str()); \ + pcpp::log(pcpp::LogSource(LOG_MODULE, PCAPPP_FILENAME, __FUNCTION__, __LINE__), level, sstream.str()); \ } \ } while (0) From b2055c0bed16117ff724be5aa19e4d801e711dd2 Mon Sep 17 00:00:00 2001 From: Dimitar Krastev Date: Tue, 5 Nov 2024 22:11:50 +0200 Subject: [PATCH 13/52] Moved the log functions to the Logger class. - Added optional compile time elimination of log calls below set level. --- Common++/header/Logger.h | 115 ++++++++++++++++++++++----------------- 1 file changed, 66 insertions(+), 49 deletions(-) diff --git a/Common++/header/Logger.h b/Common++/header/Logger.h index 5533aed7d9..b59b056eb8 100644 --- a/Common++/header/Logger.h +++ b/Common++/header/Logger.h @@ -20,6 +20,19 @@ /// @file +// Compile time log levels. +// Allows for conditional removal of unwanted log calls at compile time. +#define PCAP_LOG_LEVEL_OFF 0 +#define PCAP_LOG_LEVEL_ERROR 1 +#define PCAP_LOG_LEVEL_INFO 2 +#define PCAP_LOG_LEVEL_DEBUG 3 + +// All log messages built via a PCAP_LOG_* macro below the PCAP_ACTIVE_LOG_LEVEL will be removed at compile time. +// Uses the PCAP_ACTIVE_LOG_LEVEL if it is defined, otherwise defaults to PCAP_LOG_LEVEL_DEBUG +#ifndef PCAP_ACTIVE_LOG_LEVEL +# define PCAP_ACTIVE_LOG_LEVEL PCAP_LOG_LEVEL_DEBUG +#endif // !PCAP_ACTIVE_LOG_LEVEL + /** * \namespace pcpp * \brief The main namespace for the PcapPlusPlus lib @@ -133,15 +146,15 @@ namespace pcpp }; /** - * An enum representing the log level. Currently 3 log levels are supported: Error, Info and Debug. Info is the + * An enum representing the log level. Currently 4 log levels are supported: Off, Error, Info and Debug. Info is the * default log level */ enum class LogLevel { - Off, ///< No log messages are emitted. - Error, ///< Error level logs are emitted. - Info, ///< Info level logs and above are emitted. - Debug ///< Debug level logs and above are emitted. + Off = PCAP_LOG_LEVEL_OFF, ///< No log messages are emitted. + Error = PCAP_LOG_LEVEL_ERROR, ///< Error level logs are emitted. + Info = PCAP_LOG_LEVEL_INFO, ///< Info level logs and above are emitted. + Debug = PCAP_LOG_LEVEL_DEBUG ///< Debug level logs and above are emitted. }; inline std::ostream& operator<<(std::ostream& s, LogLevel v) @@ -149,11 +162,6 @@ namespace pcpp return s << static_cast::type>(v); } - // Forward declaration - template void log(LogSource source, LogLevel level, T const& message); - template <> void log(LogSource source, LogLevel level, std::string const& message); - template <> void log(LogSource source, LogLevel level, const char* const& message); - /** * @class Logger * PcapPlusPlus logger manager. @@ -316,7 +324,30 @@ namespace pcpp return instance; } - template friend void pcpp::log(LogSource source, LogLevel level, T const& message); + template void log(LogSource source, LogLevel level, T const& message) + { + if (shouldLog(level, source.logModule)) + { + std::ostringstream sstream; + sstream << message; + printLogMessage(source, level, sstream.str()); + } + }; + + template void logError(LogSource source, T const& message) + { + log(source, LogLevel::Error, message); + }; + + template void logInfo(LogSource source, T const& message) + { + log(source, LogLevel::Info, message); + }; + + template void logDebug(LogSource source, T const& message) + { + log(source, LogLevel::Debug, message); + }; private: bool m_LogsEnabled; @@ -332,66 +363,52 @@ namespace pcpp const std::string& method, const int line); }; - template inline void log(LogSource source, LogLevel level, T const& message) - { - auto& logger = Logger::getInstance(); - if (logger.shouldLog(level, source.logModule)) - { - std::ostringstream sstream; - sstream << message; - logger.printLogMessage(source, level, sstream.str()); - } - }; - // Specialization for string to skip the stringstream - template <> inline void log(LogSource source, LogLevel level, std::string const& message) + template <> inline void Logger::log(LogSource source, LogLevel level, std::string const& message) { - auto& logger = Logger::getInstance(); - if (logger.shouldLog(level, source.logModule)) + if (shouldLog(level, source.logModule)) { - logger.printLogMessage(source, level, message); + printLogMessage(source, level, message); } }; // Specialization for const char* to skip the stringstream - template <> inline void log(LogSource source, LogLevel level, const char* const& message) + template <> inline void Logger::log(LogSource source, LogLevel level, const char* const& message) { - auto& logger = Logger::getInstance(); - if (logger.shouldLog(level, source.logModule)) + if (shouldLog(level, source.logModule)) { - logger.printLogMessage(source, level, message); + printLogMessage(source, level, message); } }; - template inline void logError(LogSource source, T const& message) - { - log(source, LogLevel::Error, message); - }; - - template inline void logInfo(LogSource source, T const& message) - { - log(source, LogLevel::Info, message); - }; - - template inline void logDebug(LogSource source, T const& message) - { - log(source, LogLevel::Debug, message); - }; } // namespace pcpp #define PCPP_LOG(level, message) \ do \ { \ - if (pcpp::Logger::getInstance().shouldLog(level, LOG_MODULE)) \ + auto& logger = Logger::getInstance(); \ + if (logger.shouldLog(level, LOG_MODULE)) \ { \ std::ostringstream sstream; \ sstream << message; \ - pcpp::log(pcpp::LogSource(LOG_MODULE, PCAPPP_FILENAME, __FUNCTION__, __LINE__), level, sstream.str()); \ + logger.log(pcpp::LogSource(LOG_MODULE, PCAPPP_FILENAME, __FUNCTION__, __LINE__), level, sstream.str()); \ } \ } while (0) -#define PCPP_LOG_DEBUG(message) PCPP_LOG(pcpp::LogLevel::Debug, message) +#if PCAP_ACTIVE_LOG_LEVEL >= PCAP_LOG_LEVEL_DEBUG +# define PCPP_LOG_DEBUG(message) PCPP_LOG(pcpp::LogLevel::Debug, message) +#else +# define PCPP_LOG_DEBUG(message) (void)0 +#endif -#define PCPP_LOG_INFO(message) PCPP_LOG(pcpp::LogLevel::Info, message) +#if PCAP_ACTIVE_LOG_LEVEL >= PCAP_LOG_LEVEL_INFO +# define PCPP_LOG_INFO(message) PCPP_LOG(pcpp::LogLevel::Info, message) +#else +# define PCPP_LOG_INFO(message) (void)0 +#endif -#define PCPP_LOG_ERROR(message) PCPP_LOG(pcpp::LogLevel::Error, message) +#if PCAP_ACTIVE_LOG_LEVEL >= PCAP_LOG_LEVEL_ERROR +# define PCPP_LOG_ERROR(message) PCPP_LOG(pcpp::LogLevel::Error, message) +#else +# define PCPP_LOG_ERROR(message) (void)0 +#endif From ac2a38334a15bc1ae8a0e25aa18b11b7c536989d Mon Sep 17 00:00:00 2001 From: Dimitar Krastev Date: Thu, 14 Nov 2024 17:22:04 +0200 Subject: [PATCH 14/52] Fixed typo in macro names. --- Common++/header/Logger.h | 26 +++++++++++++------------- 1 file changed, 13 insertions(+), 13 deletions(-) diff --git a/Common++/header/Logger.h b/Common++/header/Logger.h index b59b056eb8..b360f24802 100644 --- a/Common++/header/Logger.h +++ b/Common++/header/Logger.h @@ -22,15 +22,15 @@ // Compile time log levels. // Allows for conditional removal of unwanted log calls at compile time. -#define PCAP_LOG_LEVEL_OFF 0 -#define PCAP_LOG_LEVEL_ERROR 1 -#define PCAP_LOG_LEVEL_INFO 2 -#define PCAP_LOG_LEVEL_DEBUG 3 +#define PCPP_LOG_LEVEL_OFF 0 +#define PCPP_LOG_LEVEL_ERROR 1 +#define PCPP_LOG_LEVEL_INFO 2 +#define PCPP_LOG_LEVEL_DEBUG 3 // All log messages built via a PCAP_LOG_* macro below the PCAP_ACTIVE_LOG_LEVEL will be removed at compile time. // Uses the PCAP_ACTIVE_LOG_LEVEL if it is defined, otherwise defaults to PCAP_LOG_LEVEL_DEBUG -#ifndef PCAP_ACTIVE_LOG_LEVEL -# define PCAP_ACTIVE_LOG_LEVEL PCAP_LOG_LEVEL_DEBUG +#ifndef PCPP_ACTIVE_LOG_LEVEL +# define PCPP_ACTIVE_LOG_LEVEL PCPP_LOG_LEVEL_DEBUG #endif // !PCAP_ACTIVE_LOG_LEVEL /** @@ -151,10 +151,10 @@ namespace pcpp */ enum class LogLevel { - Off = PCAP_LOG_LEVEL_OFF, ///< No log messages are emitted. - Error = PCAP_LOG_LEVEL_ERROR, ///< Error level logs are emitted. - Info = PCAP_LOG_LEVEL_INFO, ///< Info level logs and above are emitted. - Debug = PCAP_LOG_LEVEL_DEBUG ///< Debug level logs and above are emitted. + Off = PCPP_LOG_LEVEL_OFF, ///< No log messages are emitted. + Error = PCPP_LOG_LEVEL_ERROR, ///< Error level logs are emitted. + Info = PCPP_LOG_LEVEL_INFO, ///< Info level logs and above are emitted. + Debug = PCPP_LOG_LEVEL_DEBUG ///< Debug level logs and above are emitted. }; inline std::ostream& operator<<(std::ostream& s, LogLevel v) @@ -395,19 +395,19 @@ namespace pcpp } \ } while (0) -#if PCAP_ACTIVE_LOG_LEVEL >= PCAP_LOG_LEVEL_DEBUG +#if PCPP_ACTIVE_LOG_LEVEL >= PCPP_LOG_LEVEL_DEBUG # define PCPP_LOG_DEBUG(message) PCPP_LOG(pcpp::LogLevel::Debug, message) #else # define PCPP_LOG_DEBUG(message) (void)0 #endif -#if PCAP_ACTIVE_LOG_LEVEL >= PCAP_LOG_LEVEL_INFO +#if PCPP_ACTIVE_LOG_LEVEL >= PCPP_LOG_LEVEL_INFO # define PCPP_LOG_INFO(message) PCPP_LOG(pcpp::LogLevel::Info, message) #else # define PCPP_LOG_INFO(message) (void)0 #endif -#if PCAP_ACTIVE_LOG_LEVEL >= PCAP_LOG_LEVEL_ERROR +#if PCPP_ACTIVE_LOG_LEVEL >= PCPP_LOG_LEVEL_ERROR # define PCPP_LOG_ERROR(message) PCPP_LOG(pcpp::LogLevel::Error, message) #else # define PCPP_LOG_ERROR(message) (void)0 From 37766532cb85cb0881fd16de3e00d44dead27c22 Mon Sep 17 00:00:00 2001 From: Dimitar Krastev Date: Thu, 14 Nov 2024 18:49:26 +0200 Subject: [PATCH 15/52] Changed value param to const-ref. --- Common++/header/Logger.h | 14 +++++++------- Common++/src/Logger.cpp | 2 +- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/Common++/header/Logger.h b/Common++/header/Logger.h index b360f24802..4f01299bbc 100644 --- a/Common++/header/Logger.h +++ b/Common++/header/Logger.h @@ -324,7 +324,7 @@ namespace pcpp return instance; } - template void log(LogSource source, LogLevel level, T const& message) + template void log(LogSource const& source, LogLevel level, T const& message) { if (shouldLog(level, source.logModule)) { @@ -334,17 +334,17 @@ namespace pcpp } }; - template void logError(LogSource source, T const& message) + template void logError(LogSource const& source, T const& message) { log(source, LogLevel::Error, message); }; - template void logInfo(LogSource source, T const& message) + template void logInfo(LogSource const& source, T const& message) { log(source, LogLevel::Info, message); }; - template void logDebug(LogSource source, T const& message) + template void logDebug(LogSource const& source, T const& message) { log(source, LogLevel::Debug, message); }; @@ -358,13 +358,13 @@ namespace pcpp // private c'tor - this class is a singleton Logger(); - void printLogMessage(LogSource source, LogLevel logLevel, std::string const& message); + void printLogMessage(LogSource const& source, LogLevel logLevel, std::string const& message); static void defaultLogPrinter(LogLevel logLevel, const std::string& logMessage, const std::string& file, const std::string& method, const int line); }; // Specialization for string to skip the stringstream - template <> inline void Logger::log(LogSource source, LogLevel level, std::string const& message) + template <> inline void Logger::log(LogSource const& source, LogLevel level, std::string const& message) { if (shouldLog(level, source.logModule)) { @@ -373,7 +373,7 @@ namespace pcpp }; // Specialization for const char* to skip the stringstream - template <> inline void Logger::log(LogSource source, LogLevel level, const char* const& message) + template <> inline void Logger::log(LogSource const& source, LogLevel level, const char* const& message) { if (shouldLog(level, source.logModule)) { diff --git a/Common++/src/Logger.cpp b/Common++/src/Logger.cpp index 8b5e22125c..78bdc8458a 100644 --- a/Common++/src/Logger.cpp +++ b/Common++/src/Logger.cpp @@ -28,7 +28,7 @@ namespace pcpp } } - void Logger::printLogMessage(LogSource source, LogLevel logLevel, std::string const& message) + void Logger::printLogMessage(LogSource const& source, LogLevel logLevel, std::string const& message) { if (logLevel == LogLevel::Error) { From 76079a76e473df6d0a4b7781a0f4819acaed1ea7 Mon Sep 17 00:00:00 2001 From: Dimitar Krastev Date: Thu, 14 Nov 2024 18:50:33 +0200 Subject: [PATCH 16/52] Added "venv" and "./out" to ignored directories by codespell. --- .codespellrc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.codespellrc b/.codespellrc index c7019c766a..7506fd03df 100644 --- a/.codespellrc +++ b/.codespellrc @@ -1,4 +1,4 @@ [codespell] -skip = *.dat,typos-config.toml,.git,.venv,./ci,./Dist,./mk,./Tests/ExamplesTest/expected_output,./Tests/ExamplesTest/pcap_examples,./Tests/Packet++Test/PacketExamples,./Tests/Pcap++Test/PcapExamples,./3rdParty,./Examples/PcapSearch/dirent-for-Visual-Studio +skip = *.dat,typos-config.toml,.git,.venv,venv,./out,./ci,./Dist,./mk,./Tests/ExamplesTest/expected_output,./Tests/ExamplesTest/pcap_examples,./Tests/Packet++Test/PacketExamples,./Tests/Pcap++Test/PcapExamples,./3rdParty,./Examples/PcapSearch/dirent-for-Visual-Studio ignore-words = codespell-ignore-list.txt count = From 52b90075c7c9a22cea18071d57b7308fdeae2395 Mon Sep 17 00:00:00 2001 From: Dimitar Krastev Date: Thu, 14 Nov 2024 22:19:30 +0200 Subject: [PATCH 17/52] Reverted to previous optimizations to keep executable binary size low. - Renamed printLogMessage to emit and changed visibility to public. - Added new class LogContext to encapsulate a single emittable log message. - Added new methods createLogContext which is practically a rework of internalCreateLogStream but returns a LogContext. - Added optional use of object pooling optimization for reusing log contexts. (Enabled via preprocessor flag PCPP_LOG_USE_OBJECT_POOL) --- Common++/CMakeLists.txt | 1 + Common++/header/Logger.h | 94 ++++++++++++++++++++++--- Common++/header/ObjectPool.h | 130 +++++++++++++++++++++++++++++++++++ Common++/src/Logger.cpp | 35 +++++++++- 4 files changed, 249 insertions(+), 11 deletions(-) create mode 100644 Common++/header/ObjectPool.h diff --git a/Common++/CMakeLists.txt b/Common++/CMakeLists.txt index 5b75bf9f77..01cdbd671e 100644 --- a/Common++/CMakeLists.txt +++ b/Common++/CMakeLists.txt @@ -20,6 +20,7 @@ set(public_headers header/Logger.h header/LRUList.h header/MacAddress.h + header/ObjectPool.h header/OUILookup.h header/PcapPlusPlusVersion.h header/PointerVector.h diff --git a/Common++/header/Logger.h b/Common++/header/Logger.h index 4f01299bbc..7a67c419b2 100644 --- a/Common++/header/Logger.h +++ b/Common++/header/Logger.h @@ -1,11 +1,15 @@ #pragma once #include +#include #include #include #include #include #include "DeprecationUtils.h" +#ifdef PCPP_LOG_USE_OBJECT_POOL +# include "ObjectPool.h" +#endif // PCPP_LOG_USE_OBJECT_POOL #ifndef LOG_MODULE # define LOG_MODULE UndefinedLogModule @@ -162,6 +166,41 @@ namespace pcpp return s << static_cast::type>(v); } + // Forward declaration + class Logger; + + namespace internal + { + class LogContext + { + public: + friend class Logger; + + LogContext() = default; + LogContext(LogLevel level, LogSource source = {}) : level(level), m_Source(source) + {} + + void init(LogLevel level, LogSource source) + { + m_Source = source; + level = level; + m_Stream.clear(); + m_Stream.str({}); + } + + template inline LogContext& operator<<(T const& value) + { + m_Stream << value; + return *this; + } + + LogLevel level = LogLevel::Info; // Public to be able to be directly set for now. + private: + LogSource m_Source; + std::ostringstream m_Stream; + }; + } // namespace internal + /** * @class Logger * PcapPlusPlus logger manager. @@ -324,13 +363,45 @@ namespace pcpp return instance; } + /** + * @brief Creates a new LogContext with Info level and no source. + * @return A new LogContext. + */ + std::unique_ptr createLogContext(); + /** + * @brief Creates a new LogContext with the given level and source. + * @param level The log level for this message. + * @param source The log source. + * @return A new LogContext. + */ + std::unique_ptr createLogContext(LogLevel level, LogSource const& source = {}); + + /** + * @brief Directly emits a log message bypassing all level checks. + * @param source The log source. + * @param level The log level for this message. This is only used for the log printer. + * @param message The log message. + */ + void emit(LogSource const& source, LogLevel level, std::string const& message); + /** + * @brief Directly emits a log message bypassing all level checks. + * @param message The log message. + */ + void emit(std::unique_ptr message); + + /** + * @brief Logs a message with the given source, level, and message. + * @param message The log message. + */ + void log(std::unique_ptr message); + template void log(LogSource const& source, LogLevel level, T const& message) { if (shouldLog(level, source.logModule)) { - std::ostringstream sstream; - sstream << message; - printLogMessage(source, level, sstream.str()); + auto ctx = createLogContext(); + ctx << message; + emit(std::move(ctx)); } }; @@ -355,10 +426,14 @@ namespace pcpp LogPrinter m_LogPrinter; std::string m_LastError; +#ifdef PCPP_LOG_USE_OBJECT_POOL + ObjectPool m_LogContextPool = { 10 }; ///< Keep a maximum of 10 LogContext objects. + +#endif // PCPP_LOG_USE_OBJECT_POOL + // private c'tor - this class is a singleton Logger(); - void printLogMessage(LogSource const& source, LogLevel logLevel, std::string const& message); static void defaultLogPrinter(LogLevel logLevel, const std::string& logMessage, const std::string& file, const std::string& method, const int line); }; @@ -368,7 +443,7 @@ namespace pcpp { if (shouldLog(level, source.logModule)) { - printLogMessage(source, level, message); + emit(source, level, message); } }; @@ -377,7 +452,7 @@ namespace pcpp { if (shouldLog(level, source.logModule)) { - printLogMessage(source, level, message); + emit(source, level, message); } }; @@ -389,9 +464,10 @@ namespace pcpp auto& logger = Logger::getInstance(); \ if (logger.shouldLog(level, LOG_MODULE)) \ { \ - std::ostringstream sstream; \ - sstream << message; \ - logger.log(pcpp::LogSource(LOG_MODULE, PCAPPP_FILENAME, __FUNCTION__, __LINE__), level, sstream.str()); \ + auto ctx = \ + logger.createLogContext(level, pcpp::LogSource(LOG_MODULE, PCAPPP_FILENAME, __FUNCTION__, __LINE__)); \ + (*ctx) << message; \ + logger.emit(std::move(ctx)); \ } \ } while (0) diff --git a/Common++/header/ObjectPool.h b/Common++/header/ObjectPool.h new file mode 100644 index 0000000000..65fc464ca8 --- /dev/null +++ b/Common++/header/ObjectPool.h @@ -0,0 +1,130 @@ +#pragma once + +#include +#include + +namespace pcpp +{ + /** + * @brief A generic object pool implementation. + * + * This class provides a generic object pool that can be used to efficiently manage and reuse objects of any type. + * Objects can be acquired from the pool using the `acquireObject` method, and released back to the pool using the + * `releaseObject` method. If the pool is empty when acquiring an object, a new object will be created. If the pool + * is full when releasing an object, the object will be deleted. + * + * @tparam T The type of objects managed by the pool. + */ + template class ObjectPool + { + public: + constexpr static std::size_t DEFAULT_POOL_SIZE = 100; + constexpr static std::size_t INFINITE_POOL_SIZE = 0; + + /** + * A constructor for this class that creates a pool of objects + * @param[in] maxPoolSize The maximum number of objects in the pool + */ + ObjectPool(std::size_t maxPoolSize = DEFAULT_POOL_SIZE) : m_maxPoolSize(maxPoolSize) + {} + + // These don't strictly need to be deleted, but don't need to be implemented for now either. + ObjectPool(const ObjectPool&) = delete; + ObjectPool(ObjectPool&&) = delete; + ObjectPool& operator=(const ObjectPool&) = delete; + ObjectPool& operator=(ObjectPool&&) = delete; + + /** + * A destructor for this class that deletes all objects in the pool + */ + ~ObjectPool() + { + std::unique_lock lock(m_mutex); + while (!m_pool.empty()) + { + delete m_pool.top(); + m_pool.pop(); + } + } + + /** + * @brief Acquires a unique pointer to an object from the pool. + * + * This method acquires a unique pointer to an object from the pool. + * If the pool is empty, a new object will be created. + * + * @return A unique pointer to an object from the pool. + */ + std::unique_ptr acquireObject() + { + return std::unique_ptr(acquireObjectRaw()); + } + + /** + * @brief Acquires a raw pointer to an object from the pool. + * + * This method acquires a raw pointer to an object from the pool. + * If the pool is empty, a new object will be created. + * + * @return A raw pointer to an object from the pool. + */ + T* acquireObjectRaw() + { + std::unique_lock lock(m_mutex); + + if (m_pool.empty()) + { + // We don't need the lock anymore, so release it. + lock.unlock(); + return new T(); + } + + T* obj = m_pool.top(); + m_pool.pop(); + return obj; + } + + /** + * @brief Releases a unique pointer to an object back to the pool. + * + * This method releases a unique pointer to an object back to the pool. + * If the pool is full, the object will be deleted. + * + * @param[in] obj The unique pointer to the object to release. + */ + void releaseObject(std::unique_ptr obj) + { + releaseObjectRaw(obj.release()); + } + + /** + * @brief Releases a raw pointer to an object back to the pool. + * + * This method releases a raw pointer to an object back to the pool. + * If the pool is full, the object will be deleted. + * + * @param[in] obj The raw pointer to the object to release. + */ + void releaseObjectRaw(T* obj) + { + std::unique_lock lock(m_mutex); + + if (m_maxPoolSize == INFINITE_POOL_SIZE || m_pool.size() < m_maxPoolSize) + { + m_pool.push(obj); + int x = 1; + } + else + { + // We don't need the lock anymore, so release it. + lock.unlock(); + delete obj; + } + } + + private: + std::size_t m_maxPoolSize; /**< The maximum number of objects in the pool */ + std::mutex m_mutex; /**< Mutex for thread safety */ + std::stack m_pool; /**< The pool of objects */ + }; +} // namespace pcpp diff --git a/Common++/src/Logger.cpp b/Common++/src/Logger.cpp index 78bdc8458a..8eae324d5b 100644 --- a/Common++/src/Logger.cpp +++ b/Common++/src/Logger.cpp @@ -1,9 +1,9 @@ +#include #include #include "Logger.h" namespace pcpp { - Logger::Logger() : m_LogsEnabled(true), m_LogPrinter(&defaultLogPrinter) { m_LastError.reserve(200); @@ -28,7 +28,30 @@ namespace pcpp } } - void Logger::printLogMessage(LogSource const& source, LogLevel logLevel, std::string const& message) + std::unique_ptr Logger::createLogContext() + { + return createLogContext(LogLevel::Info, {}); // call the other createLogContext method + } + std::unique_ptr Logger::createLogContext(LogLevel level, LogSource const& source) + { +#ifdef PCPP_LOG_USE_OBJECT_POOL + auto ctx = m_LogContextPool.acquireObject(); + ctx->init(level, source); + return ctx; +#else + return std::unique_ptr(new internal::LogContext(level, source)); +#endif // PCPP_LOG_USE_OBJECT_POOL + } + + void Logger::emit(std::unique_ptr message) + { + emit(message->m_Source, message->level, message->m_Stream.str()); +#ifdef PCPP_LOG_USE_OBJECT_POOL + m_LogContextPool.releaseObject(std::move(message)); +#endif // PCPP_LOG_USE_OBJECT_POOL + } + + void Logger::emit(LogSource const& source, LogLevel logLevel, std::string const& message) { if (logLevel == LogLevel::Error) { @@ -40,6 +63,14 @@ namespace pcpp } } + void Logger::log(std::unique_ptr message) + { + if (shouldLog(message->level, message->m_Source.logModule)) + { + emit(std::move(message)); + } + } + void Logger::defaultLogPrinter(LogLevel logLevel, const std::string& logMessage, const std::string& file, const std::string& method, const int line) { From c71dd2763da8739403a625bc3843d315e01f7cfb Mon Sep 17 00:00:00 2001 From: Dimitar Krastev Date: Thu, 14 Nov 2024 22:20:49 +0200 Subject: [PATCH 18/52] Fixed warnings about unreferenced local variables if the compile time minimum log level set to too high severity.. --- Common++/src/IpAddress.cpp | 2 ++ Common++/src/IpUtils.cpp | 2 ++ 2 files changed, 4 insertions(+) diff --git a/Common++/src/IpAddress.cpp b/Common++/src/IpAddress.cpp index dd1c1a3d5f..781a053188 100644 --- a/Common++/src/IpAddress.cpp +++ b/Common++/src/IpAddress.cpp @@ -66,6 +66,7 @@ namespace pcpp } catch (const std::invalid_argument& e) { + (void)e; // Suppress the unreferenced local variable warning when PCPP_LOG_ERROR is disabled PCPP_LOG_ERROR(e.what()); return false; } @@ -126,6 +127,7 @@ namespace pcpp } catch (const std::invalid_argument& e) { + (void)e; // Suppress the unreferenced local variable warning when PCPP_LOG_ERROR is disabled PCPP_LOG_ERROR(e.what()); return false; } diff --git a/Common++/src/IpUtils.cpp b/Common++/src/IpUtils.cpp index 3c812d5bfe..b4b21689a8 100644 --- a/Common++/src/IpUtils.cpp +++ b/Common++/src/IpUtils.cpp @@ -37,6 +37,7 @@ namespace pcpp } catch (const std::invalid_argument& e) { + (void)e; // Suppress the unreferenced local variable warning when PCPP_LOG_DEBUG is disabled PCPP_LOG_DEBUG("Extraction failed: " << e.what() << " Returning nullptr."); return nullptr; } @@ -61,6 +62,7 @@ namespace pcpp } catch (const std::invalid_argument& e) { + (void)e; // Suppress the unreferenced local variable warning when PCPP_LOG_DEBUG is disabled PCPP_LOG_DEBUG("Extraction failed: " << e.what() << " Returning nullptr."); return nullptr; } From 803c98a21974f22af5b60b61851122f495e309a3 Mon Sep 17 00:00:00 2001 From: Dimitar Krastev Date: Thu, 14 Nov 2024 22:37:39 +0200 Subject: [PATCH 19/52] Removed useless variable. --- Common++/header/ObjectPool.h | 1 - 1 file changed, 1 deletion(-) diff --git a/Common++/header/ObjectPool.h b/Common++/header/ObjectPool.h index 65fc464ca8..358cac93fe 100644 --- a/Common++/header/ObjectPool.h +++ b/Common++/header/ObjectPool.h @@ -112,7 +112,6 @@ namespace pcpp if (m_maxPoolSize == INFINITE_POOL_SIZE || m_pool.size() < m_maxPoolSize) { m_pool.push(obj); - int x = 1; } else { From 9d44f762dcb75cfe0e9f0da75564207c2ee0c8b3 Mon Sep 17 00:00:00 2001 From: Dimitar Krastev Date: Thu, 14 Nov 2024 22:38:21 +0200 Subject: [PATCH 20/52] Fixed friend class definition. --- Common++/header/Logger.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Common++/header/Logger.h b/Common++/header/Logger.h index 7a67c419b2..6a07eb5540 100644 --- a/Common++/header/Logger.h +++ b/Common++/header/Logger.h @@ -174,7 +174,7 @@ namespace pcpp class LogContext { public: - friend class Logger; + friend class pcpp::Logger; LogContext() = default; LogContext(LogLevel level, LogSource source = {}) : level(level), m_Source(source) From af63a9b9db4a9b07ad67f822dafea77505cf78bc Mon Sep 17 00:00:00 2001 From: Dimitar Krastev Date: Thu, 14 Nov 2024 22:39:57 +0200 Subject: [PATCH 21/52] Fixed variable assignment. --- Common++/header/Logger.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Common++/header/Logger.h b/Common++/header/Logger.h index 6a07eb5540..cbc7ed1e54 100644 --- a/Common++/header/Logger.h +++ b/Common++/header/Logger.h @@ -183,7 +183,7 @@ namespace pcpp void init(LogLevel level, LogSource source) { m_Source = source; - level = level; + this->level = level; m_Stream.clear(); m_Stream.str({}); } From 02e717ab9062839576b542ff76abd734bd2e330d Mon Sep 17 00:00:00 2001 From: Dimitar Krastev Date: Sat, 16 Nov 2024 17:46:19 +0200 Subject: [PATCH 22/52] Added method useContextPooling to control if the logger should use context pooling. - Removed preprocessor variable PCPP_LOG_USE_OBJECT_POOL. - Disabled context pooling for the unit tests as it interferes with the memory leak checker. --- Common++/header/Logger.h | 25 ++++++++++++++++++------- Common++/header/ObjectPool.h | 26 ++++++++++++++++++-------- Common++/src/Logger.cpp | 20 +++++++++++--------- Tests/Pcap++Test/Common/TestUtils.cpp | 4 ++++ 4 files changed, 51 insertions(+), 24 deletions(-) diff --git a/Common++/header/Logger.h b/Common++/header/Logger.h index cbc7ed1e54..15e3229528 100644 --- a/Common++/header/Logger.h +++ b/Common++/header/Logger.h @@ -7,9 +7,7 @@ #include #include #include "DeprecationUtils.h" -#ifdef PCPP_LOG_USE_OBJECT_POOL -# include "ObjectPool.h" -#endif // PCPP_LOG_USE_OBJECT_POOL +#include "ObjectPool.h" #ifndef LOG_MODULE # define LOG_MODULE UndefinedLogModule @@ -352,6 +350,20 @@ namespace pcpp return m_LogsEnabled; } + /** + * @brief Controls if the logger should use a pool of LogContext objects. + * @param enabled True to enable context pooling, false to disable. + * @remarks Disabling the pooling clears the pool. + */ + void useContextPooling(bool enabled) + { + m_UseContextPooling = enabled; + + // Clear the pool if we're disabling pooling. + if (!m_UseContextPooling) + m_LogContextPool.clear(); + } + /** * Get access to Logger singleton * @todo: make this singleton thread-safe/ @@ -426,10 +438,9 @@ namespace pcpp LogPrinter m_LogPrinter; std::string m_LastError; -#ifdef PCPP_LOG_USE_OBJECT_POOL - ObjectPool m_LogContextPool = { 10 }; ///< Keep a maximum of 10 LogContext objects. - -#endif // PCPP_LOG_USE_OBJECT_POOL + bool m_UseContextPooling = true; + // Keep a maximum of 10 LogContext objects in the pool. + ObjectPool m_LogContextPool = { 10 }; // private c'tor - this class is a singleton Logger(); diff --git a/Common++/header/ObjectPool.h b/Common++/header/ObjectPool.h index 358cac93fe..999e198cf9 100644 --- a/Common++/header/ObjectPool.h +++ b/Common++/header/ObjectPool.h @@ -2,6 +2,7 @@ #include #include +#include namespace pcpp { @@ -13,9 +14,10 @@ namespace pcpp * `releaseObject` method. If the pool is empty when acquiring an object, a new object will be created. If the pool * is full when releasing an object, the object will be deleted. * - * @tparam T The type of objects managed by the pool. + * @tparam T The type of objects managed by the pool. Must be default constructable. */ - template class ObjectPool + template ::value, bool>::type = true> + class ObjectPool { public: constexpr static std::size_t DEFAULT_POOL_SIZE = 100; @@ -39,12 +41,7 @@ namespace pcpp */ ~ObjectPool() { - std::unique_lock lock(m_mutex); - while (!m_pool.empty()) - { - delete m_pool.top(); - m_pool.pop(); - } + clear(); } /** @@ -121,6 +118,19 @@ namespace pcpp } } + /** + * @brief Deallocates and releases all objects currently held by the pool. + */ + void clear() + { + std::unique_lock lock(m_mutex); + while (!m_pool.empty()) + { + delete m_pool.top(); + m_pool.pop(); + } + } + private: std::size_t m_maxPoolSize; /**< The maximum number of objects in the pool */ std::mutex m_mutex; /**< Mutex for thread safety */ diff --git a/Common++/src/Logger.cpp b/Common++/src/Logger.cpp index 8eae324d5b..bc049623ae 100644 --- a/Common++/src/Logger.cpp +++ b/Common++/src/Logger.cpp @@ -34,21 +34,23 @@ namespace pcpp } std::unique_ptr Logger::createLogContext(LogLevel level, LogSource const& source) { -#ifdef PCPP_LOG_USE_OBJECT_POOL - auto ctx = m_LogContextPool.acquireObject(); - ctx->init(level, source); - return ctx; -#else + if (m_UseContextPooling) + { + auto ctx = m_LogContextPool.acquireObject(); + ctx->init(level, source); + return ctx; + } return std::unique_ptr(new internal::LogContext(level, source)); -#endif // PCPP_LOG_USE_OBJECT_POOL } void Logger::emit(std::unique_ptr message) { emit(message->m_Source, message->level, message->m_Stream.str()); -#ifdef PCPP_LOG_USE_OBJECT_POOL - m_LogContextPool.releaseObject(std::move(message)); -#endif // PCPP_LOG_USE_OBJECT_POOL + // Pushes the message back to the pool if pooling is enabled. Otherwise, the message is deleted. + if (m_UseContextPooling) + { + m_LogContextPool.releaseObject(std::move(message)); + } } void Logger::emit(LogSource const& source, LogLevel logLevel, std::string const& message) diff --git a/Tests/Pcap++Test/Common/TestUtils.cpp b/Tests/Pcap++Test/Common/TestUtils.cpp index c31dd43577..5b171584a2 100644 --- a/Tests/Pcap++Test/Common/TestUtils.cpp +++ b/Tests/Pcap++Test/Common/TestUtils.cpp @@ -1,6 +1,7 @@ #include "TestUtils.h" #include #include "GlobalTestArgs.h" +#include "Logger.h" #include "PcapFileDevice.h" #include "PcapLiveDeviceList.h" // clang-format off @@ -91,6 +92,9 @@ uint8_t* readFileIntoBuffer(const std::string& filename, int& bufferLength) void testSetUp() { + // Disables context pooling for the logger as it causes issues when used with the memory leak detection tool. + pcpp::Logger::getInstance().useContextPooling(false); + pcpp::PcapLiveDeviceList::getInstance(); #ifdef USE_PF_RING From 341023150d20879c357a1be5cc0ed5ca886591f1 Mon Sep 17 00:00:00 2001 From: Dimitar Krastev Date: Sat, 16 Nov 2024 17:47:21 +0200 Subject: [PATCH 23/52] Fixed more warnings about unreferenced local variables if the compile time minimum log level set to too high severity.. --- Pcap++/src/PcapLiveDevice.cpp | 1 + Pcap++/src/PcapLiveDeviceList.cpp | 1 + Pcap++/src/PcapRemoteDeviceList.cpp | 2 ++ 3 files changed, 4 insertions(+) diff --git a/Pcap++/src/PcapLiveDevice.cpp b/Pcap++/src/PcapLiveDevice.cpp index 6eef2111d2..c30b3c47e0 100644 --- a/Pcap++/src/PcapLiveDevice.cpp +++ b/Pcap++/src/PcapLiveDevice.cpp @@ -1044,6 +1044,7 @@ namespace pcpp } catch (const std::exception& e) { + (void)e; // Suppress the unreferenced local variable warning when PCPP_LOG_ERROR is disabled PCPP_LOG_ERROR("Error retrieving default gateway address: " << e.what()); } break; diff --git a/Pcap++/src/PcapLiveDeviceList.cpp b/Pcap++/src/PcapLiveDeviceList.cpp index 9fb46b4ce4..a68e65a406 100644 --- a/Pcap++/src/PcapLiveDeviceList.cpp +++ b/Pcap++/src/PcapLiveDeviceList.cpp @@ -51,6 +51,7 @@ namespace pcpp } catch (const std::exception& e) { + (void)e; // Suppress the unreferenced local variable warning when PCPP_LOG_ERROR is disabled PCPP_LOG_ERROR(e.what()); } diff --git a/Pcap++/src/PcapRemoteDeviceList.cpp b/Pcap++/src/PcapRemoteDeviceList.cpp index b725f3b817..5eea5fe0e5 100644 --- a/Pcap++/src/PcapRemoteDeviceList.cpp +++ b/Pcap++/src/PcapRemoteDeviceList.cpp @@ -96,6 +96,7 @@ namespace pcpp } catch (const std::exception& e) { + (void)e; // Suppress the unreferenced local variable warning when PCPP_LOG_ERROR is disabled PCPP_LOG_ERROR(e.what()); return nullptr; } @@ -120,6 +121,7 @@ namespace pcpp { delete device; } + (void)e; // Suppress the unreferenced local variable warning when PCPP_LOG_ERROR is disabled PCPP_LOG_ERROR("Error creating remote devices: " << e.what()); return nullptr; } From 379c0742943069609cefca26e2b42d0b22f6c774 Mon Sep 17 00:00:00 2001 From: Dimitar Krastev Date: Sat, 16 Nov 2024 18:14:21 +0200 Subject: [PATCH 24/52] Addressed warnings and documentation. - Added documentation to new methods and classes. - Addressed explicit constructor warnings. - Moved disable of context pooling for unit tests to the main.cpp files of the respective tests. --- Common++/header/Logger.h | 67 +++++++++++++++++++++++++-- Common++/header/ObjectPool.h | 2 +- Tests/Packet++Test/main.cpp | 3 +- Tests/Pcap++Test/Common/TestUtils.cpp | 4 -- Tests/Pcap++Test/main.cpp | 3 +- 5 files changed, 68 insertions(+), 11 deletions(-) diff --git a/Common++/header/Logger.h b/Common++/header/Logger.h index 15e3229528..9f509ff93c 100644 --- a/Common++/header/Logger.h +++ b/Common++/header/Logger.h @@ -169,16 +169,33 @@ namespace pcpp namespace internal { + /** + * @class LogContext + * @brief A context encapsulating the details of a single log message to be passed to the Logger. + */ class LogContext { public: friend class pcpp::Logger; + /** + * @brief Creates a context with an empty message with Info level and no source. + */ LogContext() = default; - LogContext(LogLevel level, LogSource source = {}) : level(level), m_Source(source) + /** + * @brief Creates a context with an empty message with the given level and source. + * @param level The log level for this message. + * @param source The log source. + */ + explicit LogContext(LogLevel level, LogSource const& source = {}) : level(level), m_Source(source) {} - void init(LogLevel level, LogSource source) + /** + * @brief Initializes the context with an empty message and the given level and source. + * @param level The log level for this message. + * @param source The log source. + */ + void init(LogLevel level, LogSource const& source) { m_Source = source; this->level = level; @@ -186,13 +203,22 @@ namespace pcpp m_Stream.str({}); } + /** + * @brief Appends to the message. + * @param value The value to append. + * @return A reference to this context. + */ template inline LogContext& operator<<(T const& value) { m_Stream << value; return *this; } - LogLevel level = LogLevel::Info; // Public to be able to be directly set for now. + /** + * @brief The log level at which the message will be emitted. + */ + LogLevel level = LogLevel::Info; + private: LogSource m_Source; std::ostringstream m_Stream; @@ -407,6 +433,15 @@ namespace pcpp */ void log(std::unique_ptr message); + /** + * @brief Logs an object with the given source, level. + * + * The object is converted to a string via the std::ostream << operator. + * @tparam T The type of object to be logged. + * @param source The log source. + * @param level The log level for this message. + * @param message The object to be logged. + */ template void log(LogSource const& source, LogLevel level, T const& message) { if (shouldLog(level, source.logModule)) @@ -417,16 +452,40 @@ namespace pcpp } }; + /** + * @brief Logs an object with the given source at the Error level. + * + * The object is converted to a string via the std::ostream << operator. + * @tparam T The type of object to be logged. + * @param source The log source. + * @param message The object to be logged. + */ template void logError(LogSource const& source, T const& message) { log(source, LogLevel::Error, message); }; + /** + * @brief Logs an object with the given source at the Info level. + * + * The object is converted to a string via the std::ostream << operator. + * @tparam T The type of object to be logged. + * @param source The log source. + * @param message The object to be logged. + */ template void logInfo(LogSource const& source, T const& message) { log(source, LogLevel::Info, message); }; + /** + * @brief Logs an object with the given source at the Debug level. + * + * The object is converted to a string via the std::ostream << operator. + * @tparam T The type of object to be logged. + * @param source The log source. + * @param message The object to be logged. + */ template void logDebug(LogSource const& source, T const& message) { log(source, LogLevel::Debug, message); @@ -440,7 +499,7 @@ namespace pcpp bool m_UseContextPooling = true; // Keep a maximum of 10 LogContext objects in the pool. - ObjectPool m_LogContextPool = { 10 }; + ObjectPool m_LogContextPool{ 10 }; // private c'tor - this class is a singleton Logger(); diff --git a/Common++/header/ObjectPool.h b/Common++/header/ObjectPool.h index 999e198cf9..81304265ab 100644 --- a/Common++/header/ObjectPool.h +++ b/Common++/header/ObjectPool.h @@ -27,7 +27,7 @@ namespace pcpp * A constructor for this class that creates a pool of objects * @param[in] maxPoolSize The maximum number of objects in the pool */ - ObjectPool(std::size_t maxPoolSize = DEFAULT_POOL_SIZE) : m_maxPoolSize(maxPoolSize) + explicit ObjectPool(std::size_t maxPoolSize = DEFAULT_POOL_SIZE) : m_maxPoolSize(maxPoolSize) {} // These don't strictly need to be deleted, but don't need to be implemented for now either. diff --git a/Tests/Packet++Test/main.cpp b/Tests/Packet++Test/main.cpp index 4163780110..a2423dabb0 100644 --- a/Tests/Packet++Test/main.cpp +++ b/Tests/Packet++Test/main.cpp @@ -85,7 +85,8 @@ int main(int argc, char* argv[]) #endif // The logger singleton looks like a memory leak. Invoke it before starting the memory check - pcpp::Logger::getInstance(); + // Also disables context pooling for the logger as it causes issues when used with the memory leak detection tool. + pcpp::Logger::getInstance().useContextPooling(false); // cppcheck-suppress knownConditionTrueFalse if (skipMemLeakCheck) diff --git a/Tests/Pcap++Test/Common/TestUtils.cpp b/Tests/Pcap++Test/Common/TestUtils.cpp index 5b171584a2..c31dd43577 100644 --- a/Tests/Pcap++Test/Common/TestUtils.cpp +++ b/Tests/Pcap++Test/Common/TestUtils.cpp @@ -1,7 +1,6 @@ #include "TestUtils.h" #include #include "GlobalTestArgs.h" -#include "Logger.h" #include "PcapFileDevice.h" #include "PcapLiveDeviceList.h" // clang-format off @@ -92,9 +91,6 @@ uint8_t* readFileIntoBuffer(const std::string& filename, int& bufferLength) void testSetUp() { - // Disables context pooling for the logger as it causes issues when used with the memory leak detection tool. - pcpp::Logger::getInstance().useContextPooling(false); - pcpp::PcapLiveDeviceList::getInstance(); #ifdef USE_PF_RING diff --git a/Tests/Pcap++Test/main.cpp b/Tests/Pcap++Test/main.cpp index 784a9e0d6f..df8950edce 100644 --- a/Tests/Pcap++Test/main.cpp +++ b/Tests/Pcap++Test/main.cpp @@ -144,7 +144,8 @@ int main(int argc, char* argv[]) #endif // The logger singleton looks like a memory leak. Invoke it before starting the memory check - pcpp::Logger::getInstance(); + // Also disables context pooling for the logger as it causes issues when used with the memory leak detection tool. + pcpp::Logger::getInstance().useContextPooling(false); // cppcheck-suppress knownConditionTrueFalse if (skipMemLeakCheck) From bc82cafdea063f7b5c189e3efebeeaa49202b526 Mon Sep 17 00:00:00 2001 From: Dimitar Krastev Date: Sat, 16 Nov 2024 18:19:05 +0200 Subject: [PATCH 25/52] Fixed include. --- Common++/header/ObjectPool.h | 1 + 1 file changed, 1 insertion(+) diff --git a/Common++/header/ObjectPool.h b/Common++/header/ObjectPool.h index 81304265ab..62f35d8900 100644 --- a/Common++/header/ObjectPool.h +++ b/Common++/header/ObjectPool.h @@ -2,6 +2,7 @@ #include #include +#include #include namespace pcpp From 69aa6151558d9a806d9754515466da9079c976de Mon Sep 17 00:00:00 2001 From: Dimitar Krastev Date: Wed, 20 Nov 2024 10:22:39 +0200 Subject: [PATCH 26/52] Fixed pointer dereference. --- Common++/header/Logger.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Common++/header/Logger.h b/Common++/header/Logger.h index 9f509ff93c..57f8fd6066 100644 --- a/Common++/header/Logger.h +++ b/Common++/header/Logger.h @@ -447,7 +447,7 @@ namespace pcpp if (shouldLog(level, source.logModule)) { auto ctx = createLogContext(); - ctx << message; + (*ctx) << message; emit(std::move(ctx)); } }; From 17ec9a2b037c38af7306c520e2457017e3c25f63 Mon Sep 17 00:00:00 2001 From: Dimitar Krastev Date: Wed, 20 Nov 2024 11:41:13 +0200 Subject: [PATCH 27/52] Fixed memory checker issues with logger. - Added 2 preallocated log contexts to the object pool. --- Common++/header/Logger.h | 18 ++++++++++++++---- Common++/header/ObjectPool.h | 20 ++++++++++++++++++-- Tests/Packet++Test/main.cpp | 4 ++-- Tests/Pcap++Test/main.cpp | 4 ++-- 4 files changed, 36 insertions(+), 10 deletions(-) diff --git a/Common++/header/Logger.h b/Common++/header/Logger.h index 57f8fd6066..1d4376a5f5 100644 --- a/Common++/header/Logger.h +++ b/Common++/header/Logger.h @@ -379,15 +379,25 @@ namespace pcpp /** * @brief Controls if the logger should use a pool of LogContext objects. * @param enabled True to enable context pooling, false to disable. + * @param preallocate The number of LogContext objects to preallocate in the pool. * @remarks Disabling the pooling clears the pool. */ - void useContextPooling(bool enabled) + void useContextPooling(bool enabled, std::size_t preallocate = 2) { m_UseContextPooling = enabled; - // Clear the pool if we're disabling pooling. - if (!m_UseContextPooling) + if (m_UseContextPooling) + { + if (preallocate > 0) + { + m_LogContextPool.preallocate(preallocate); + } + } + else + { + // Clear the pool if we're disabling pooling. m_LogContextPool.clear(); + } } /** @@ -499,7 +509,7 @@ namespace pcpp bool m_UseContextPooling = true; // Keep a maximum of 10 LogContext objects in the pool. - ObjectPool m_LogContextPool{ 10 }; + ObjectPool m_LogContextPool{ 10, 2 }; // private c'tor - this class is a singleton Logger(); diff --git a/Common++/header/ObjectPool.h b/Common++/header/ObjectPool.h index 62f35d8900..acdcfa994a 100644 --- a/Common++/header/ObjectPool.h +++ b/Common++/header/ObjectPool.h @@ -27,9 +27,12 @@ namespace pcpp /** * A constructor for this class that creates a pool of objects * @param[in] maxPoolSize The maximum number of objects in the pool + * @param[in] preallocate The number of objects to preallocate in the pool */ - explicit ObjectPool(std::size_t maxPoolSize = DEFAULT_POOL_SIZE) : m_maxPoolSize(maxPoolSize) - {} + explicit ObjectPool(std::size_t maxPoolSize = DEFAULT_POOL_SIZE, std::size_t preallocate = 0) : m_maxPoolSize(maxPoolSize) + { + this->preallocate(preallocate); + } // These don't strictly need to be deleted, but don't need to be implemented for now either. ObjectPool(const ObjectPool&) = delete; @@ -119,6 +122,19 @@ namespace pcpp } } + /** + * @brief Pre-allocates up to a minimum number of objects in the pool. + * @param count The number of objects to pre-allocate. + */ + void preallocate(std::size_t count) + { + std::unique_lock lock(m_mutex); + for (std::size_t i = m_pool.size(); i < count; i++) + { + m_pool.push(new T()); + } + } + /** * @brief Deallocates and releases all objects currently held by the pool. */ diff --git a/Tests/Packet++Test/main.cpp b/Tests/Packet++Test/main.cpp index a2423dabb0..aec896dae7 100644 --- a/Tests/Packet++Test/main.cpp +++ b/Tests/Packet++Test/main.cpp @@ -85,8 +85,8 @@ int main(int argc, char* argv[]) #endif // The logger singleton looks like a memory leak. Invoke it before starting the memory check - // Also disables context pooling for the logger as it causes issues when used with the memory leak detection tool. - pcpp::Logger::getInstance().useContextPooling(false); + // Context pooling can cause issues if the logger's built-in context pool allocates new LogContext instances. + pcpp::Logger::getInstance(); // cppcheck-suppress knownConditionTrueFalse if (skipMemLeakCheck) diff --git a/Tests/Pcap++Test/main.cpp b/Tests/Pcap++Test/main.cpp index df8950edce..7445c3fe7f 100644 --- a/Tests/Pcap++Test/main.cpp +++ b/Tests/Pcap++Test/main.cpp @@ -144,8 +144,8 @@ int main(int argc, char* argv[]) #endif // The logger singleton looks like a memory leak. Invoke it before starting the memory check - // Also disables context pooling for the logger as it causes issues when used with the memory leak detection tool. - pcpp::Logger::getInstance().useContextPooling(false); + // Context pooling can cause issues if the logger's built-in context pool allocates new LogContext instances. + pcpp::Logger::getInstance(); // cppcheck-suppress knownConditionTrueFalse if (skipMemLeakCheck) From 2029b4b702a0a3b33de5876b2f2c8f3e158d0394 Mon Sep 17 00:00:00 2001 From: Dimitar Krastev Date: Thu, 21 Nov 2024 01:30:28 +0200 Subject: [PATCH 28/52] Lint --- Common++/header/ObjectPool.h | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/Common++/header/ObjectPool.h b/Common++/header/ObjectPool.h index acdcfa994a..ec73f1e820 100644 --- a/Common++/header/ObjectPool.h +++ b/Common++/header/ObjectPool.h @@ -29,7 +29,8 @@ namespace pcpp * @param[in] maxPoolSize The maximum number of objects in the pool * @param[in] preallocate The number of objects to preallocate in the pool */ - explicit ObjectPool(std::size_t maxPoolSize = DEFAULT_POOL_SIZE, std::size_t preallocate = 0) : m_maxPoolSize(maxPoolSize) + explicit ObjectPool(std::size_t maxPoolSize = DEFAULT_POOL_SIZE, std::size_t preallocate = 0) + : m_maxPoolSize(maxPoolSize) { this->preallocate(preallocate); } From c09a11abcb0d039cff87b376b6546934fe0e6377 Mon Sep 17 00:00:00 2001 From: Dimitar Krastev Date: Thu, 21 Nov 2024 01:49:44 +0200 Subject: [PATCH 29/52] Added mutex lock on the default log printer to support proper multi-threading and eliminate possibility of data races during log emission. --- Common++/header/Logger.h | 1 + Common++/src/Logger.cpp | 10 ++++++++-- 2 files changed, 9 insertions(+), 2 deletions(-) diff --git a/Common++/header/Logger.h b/Common++/header/Logger.h index 1d4376a5f5..c206cf76d9 100644 --- a/Common++/header/Logger.h +++ b/Common++/header/Logger.h @@ -264,6 +264,7 @@ namespace pcpp * @param[in] file The source file in PcapPlusPlus code the log message is coming from * @param[in] method The method in PcapPlusPlus code the log message is coming from * @param[in] line The line in PcapPlusPlus code the log message is coming from + * @remarks The printer callback should support being called from multiple threads simultaneously. */ typedef void (*LogPrinter)(LogLevel logLevel, const std::string& logMessage, const std::string& file, const std::string& method, const int line); diff --git a/Common++/src/Logger.cpp b/Common++/src/Logger.cpp index bc049623ae..4468f37a13 100644 --- a/Common++/src/Logger.cpp +++ b/Common++/src/Logger.cpp @@ -1,7 +1,8 @@ -#include -#include #include "Logger.h" +#include +#include + namespace pcpp { Logger::Logger() : m_LogsEnabled(true), m_LogPrinter(&defaultLogPrinter) @@ -76,8 +77,13 @@ namespace pcpp void Logger::defaultLogPrinter(LogLevel logLevel, const std::string& logMessage, const std::string& file, const std::string& method, const int line) { + // This mutex is used to prevent multiple threads from writing to the console at the same time. + static std::mutex logMutex; + std::ostringstream sstream; sstream << file << ": " << method << ":" << line; + + std::unique_lock lock(logMutex); std::cerr << std::left << "[" << std::setw(5) << Logger::logLevelAsString(logLevel) << ": " << std::setw(45) << sstream.str() << "] " << logMessage << std::endl; } From 6b8c1048d39b79c4c02b551d52a098fbc8143ee2 Mon Sep 17 00:00:00 2001 From: Dimitar Krastev Date: Wed, 25 Dec 2024 20:30:26 +0200 Subject: [PATCH 30/52] Fixed typos in documentation. --- Common++/header/Logger.h | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/Common++/header/Logger.h b/Common++/header/Logger.h index 6f9c1bf46f..074380d820 100644 --- a/Common++/header/Logger.h +++ b/Common++/header/Logger.h @@ -29,11 +29,11 @@ #define PCPP_LOG_LEVEL_INFO 2 #define PCPP_LOG_LEVEL_DEBUG 3 -// All log messages built via a PCAP_LOG_* macro below the PCAP_ACTIVE_LOG_LEVEL will be removed at compile time. -// Uses the PCAP_ACTIVE_LOG_LEVEL if it is defined, otherwise defaults to PCAP_LOG_LEVEL_DEBUG +// All log messages built via a PCPP_LOG_* macro below the PCPP_ACTIVE_LOG_LEVEL will be removed at compile time. +// Uses the PCPP_ACTIVE_LOG_LEVEL if it is defined, otherwise defaults to PCAP_LOG_LEVEL_DEBUG #ifndef PCPP_ACTIVE_LOG_LEVEL # define PCPP_ACTIVE_LOG_LEVEL PCPP_LOG_LEVEL_DEBUG -#endif // !PCAP_ACTIVE_LOG_LEVEL +#endif // !PCPP_ACTIVE_LOG_LEVEL /// @namespace pcpp /// @brief The main namespace for the PcapPlusPlus lib From e972e6756dbb298756ad0aa44875c1f5762384ee Mon Sep 17 00:00:00 2001 From: Dimitar Krastev Date: Wed, 25 Dec 2024 20:40:28 +0200 Subject: [PATCH 31/52] Changed level variable to private. --- Common++/header/Logger.h | 10 ++++------ Common++/src/Logger.cpp | 4 ++-- 2 files changed, 6 insertions(+), 8 deletions(-) diff --git a/Common++/header/Logger.h b/Common++/header/Logger.h index 074380d820..1d1880fc8f 100644 --- a/Common++/header/Logger.h +++ b/Common++/header/Logger.h @@ -168,7 +168,7 @@ namespace pcpp /// @brief Creates a context with an empty message with the given level and source. /// @param level The log level for this message. /// @param source The log source. - explicit LogContext(LogLevel level, LogSource const& source = {}) : level(level), m_Source(source) + explicit LogContext(LogLevel level, LogSource const& source = {}) : m_Source(source), m_Level(level) {} /// @brief Initializes the context with an empty message and the given level and source. @@ -177,7 +177,7 @@ namespace pcpp void init(LogLevel level, LogSource const& source) { m_Source = source; - this->level = level; + m_Level = level; m_Stream.clear(); m_Stream.str({}); } @@ -191,12 +191,10 @@ namespace pcpp return *this; } - /// @brief The log level at which the message will be emitted. - LogLevel level = LogLevel::Info; - private: - LogSource m_Source; std::ostringstream m_Stream; + LogSource m_Source; + LogLevel m_Level = LogLevel::Info; }; } // namespace internal diff --git a/Common++/src/Logger.cpp b/Common++/src/Logger.cpp index 4468f37a13..875c8c6ff0 100644 --- a/Common++/src/Logger.cpp +++ b/Common++/src/Logger.cpp @@ -46,7 +46,7 @@ namespace pcpp void Logger::emit(std::unique_ptr message) { - emit(message->m_Source, message->level, message->m_Stream.str()); + emit(message->m_Source, message->m_Level, message->m_Stream.str()); // Pushes the message back to the pool if pooling is enabled. Otherwise, the message is deleted. if (m_UseContextPooling) { @@ -68,7 +68,7 @@ namespace pcpp void Logger::log(std::unique_ptr message) { - if (shouldLog(message->level, message->m_Source.logModule)) + if (shouldLog(message->m_Level, message->m_Source.logModule)) { emit(std::move(message)); } From 85ae097317fbee801c8067535350079385745733 Mon Sep 17 00:00:00 2001 From: Dimitar Krastev Date: Wed, 25 Dec 2024 20:42:30 +0200 Subject: [PATCH 32/52] Changed LogPrinter definition to use the metaprogramming construct std::add_pointer. --- Common++/header/Logger.h | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/Common++/header/Logger.h b/Common++/header/Logger.h index 1d1880fc8f..f7ffc041fc 100644 --- a/Common++/header/Logger.h +++ b/Common++/header/Logger.h @@ -235,8 +235,9 @@ namespace pcpp /// @param[in] method The method in PcapPlusPlus code the log message is coming from /// @param[in] line The line in PcapPlusPlus code the log message is coming from /// @remarks The printer callback should support being called from multiple threads simultaneously. - typedef void (*LogPrinter)(LogLevel logLevel, const std::string& logMessage, const std::string& file, - const std::string& method, const int line); + using LogPrinter = + std::add_pointer::type; /// A static method for converting the log level enum to a string. /// @param[in] logLevel A log level enum From a1e7d563183debfe413b1a5f97b360973dfec145 Mon Sep 17 00:00:00 2001 From: Dimitar Krastev Date: Wed, 25 Dec 2024 21:06:52 +0200 Subject: [PATCH 33/52] Replaced C library includes with C++ equivalents. --- Common++/header/Logger.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Common++/header/Logger.h b/Common++/header/Logger.h index f7ffc041fc..315a12a02b 100644 --- a/Common++/header/Logger.h +++ b/Common++/header/Logger.h @@ -1,11 +1,11 @@ #pragma once -#include +#include +#include #include #include #include #include -#include #include "DeprecationUtils.h" #include "ObjectPool.h" From 445197bc6595fd34ef39756f5d79bac1a5f38b9c Mon Sep 17 00:00:00 2001 From: Dimitar Krastev Date: Wed, 25 Dec 2024 21:10:37 +0200 Subject: [PATCH 34/52] Updated documentation format. --- Common++/header/ObjectPool.h | 102 +++++++++++++++-------------------- 1 file changed, 42 insertions(+), 60 deletions(-) diff --git a/Common++/header/ObjectPool.h b/Common++/header/ObjectPool.h index ec73f1e820..e8ad50f758 100644 --- a/Common++/header/ObjectPool.h +++ b/Common++/header/ObjectPool.h @@ -7,16 +7,14 @@ namespace pcpp { - /** - * @brief A generic object pool implementation. - * - * This class provides a generic object pool that can be used to efficiently manage and reuse objects of any type. - * Objects can be acquired from the pool using the `acquireObject` method, and released back to the pool using the - * `releaseObject` method. If the pool is empty when acquiring an object, a new object will be created. If the pool - * is full when releasing an object, the object will be deleted. - * - * @tparam T The type of objects managed by the pool. Must be default constructable. - */ + /// @brief A generic object pool implementation. + /// + /// This class provides a generic object pool that can be used to efficiently manage and reuse objects of any type. + /// Objects can be acquired from the pool using the `acquireObject` method, and released back to the pool using the + /// `releaseObject` method. If the pool is empty when acquiring an object, a new object will be created. If the pool + /// is full when releasing an object, the object will be deleted. + /// + /// @tparam T The type of objects managed by the pool. Must be default constructable. template ::value, bool>::type = true> class ObjectPool { @@ -24,11 +22,9 @@ namespace pcpp constexpr static std::size_t DEFAULT_POOL_SIZE = 100; constexpr static std::size_t INFINITE_POOL_SIZE = 0; - /** - * A constructor for this class that creates a pool of objects - * @param[in] maxPoolSize The maximum number of objects in the pool - * @param[in] preallocate The number of objects to preallocate in the pool - */ + /// A constructor for this class that creates a pool of objects + /// @param[in] maxPoolSize The maximum number of objects in the pool + /// @param[in] preallocate The number of objects to preallocate in the pool explicit ObjectPool(std::size_t maxPoolSize = DEFAULT_POOL_SIZE, std::size_t preallocate = 0) : m_maxPoolSize(maxPoolSize) { @@ -41,35 +37,29 @@ namespace pcpp ObjectPool& operator=(const ObjectPool&) = delete; ObjectPool& operator=(ObjectPool&&) = delete; - /** - * A destructor for this class that deletes all objects in the pool - */ + /// A destructor for this class that deletes all objects in the pool ~ObjectPool() { clear(); } - /** - * @brief Acquires a unique pointer to an object from the pool. - * - * This method acquires a unique pointer to an object from the pool. - * If the pool is empty, a new object will be created. - * - * @return A unique pointer to an object from the pool. - */ + /// @brief Acquires a unique pointer to an object from the pool. + /// + /// This method acquires a unique pointer to an object from the pool. + /// If the pool is empty, a new object will be created. + /// + /// @return A unique pointer to an object from the pool. std::unique_ptr acquireObject() { return std::unique_ptr(acquireObjectRaw()); } - /** - * @brief Acquires a raw pointer to an object from the pool. - * - * This method acquires a raw pointer to an object from the pool. - * If the pool is empty, a new object will be created. - * - * @return A raw pointer to an object from the pool. - */ + /// @brief Acquires a raw pointer to an object from the pool. + /// + /// This method acquires a raw pointer to an object from the pool. + /// If the pool is empty, a new object will be created. + /// + /// @return A raw pointer to an object from the pool. T* acquireObjectRaw() { std::unique_lock lock(m_mutex); @@ -86,27 +76,23 @@ namespace pcpp return obj; } - /** - * @brief Releases a unique pointer to an object back to the pool. - * - * This method releases a unique pointer to an object back to the pool. - * If the pool is full, the object will be deleted. - * - * @param[in] obj The unique pointer to the object to release. - */ + /// @brief Releases a unique pointer to an object back to the pool. + /// + /// This method releases a unique pointer to an object back to the pool. + /// If the pool is full, the object will be deleted. + /// + /// @param[in] obj The unique pointer to the object to release. void releaseObject(std::unique_ptr obj) { releaseObjectRaw(obj.release()); } - /** - * @brief Releases a raw pointer to an object back to the pool. - * - * This method releases a raw pointer to an object back to the pool. - * If the pool is full, the object will be deleted. - * - * @param[in] obj The raw pointer to the object to release. - */ + /// @brief Releases a raw pointer to an object back to the pool. + /// + /// This method releases a raw pointer to an object back to the pool. + /// If the pool is full, the object will be deleted. + /// + /// @param[in] obj The raw pointer to the object to release. void releaseObjectRaw(T* obj) { std::unique_lock lock(m_mutex); @@ -123,10 +109,8 @@ namespace pcpp } } - /** - * @brief Pre-allocates up to a minimum number of objects in the pool. - * @param count The number of objects to pre-allocate. - */ + /// @brief Pre-allocates up to a minimum number of objects in the pool. + /// @param count The number of objects to pre-allocate. void preallocate(std::size_t count) { std::unique_lock lock(m_mutex); @@ -136,9 +120,7 @@ namespace pcpp } } - /** - * @brief Deallocates and releases all objects currently held by the pool. - */ + /// @brief Deallocates and releases all objects currently held by the pool. void clear() { std::unique_lock lock(m_mutex); @@ -150,8 +132,8 @@ namespace pcpp } private: - std::size_t m_maxPoolSize; /**< The maximum number of objects in the pool */ - std::mutex m_mutex; /**< Mutex for thread safety */ - std::stack m_pool; /**< The pool of objects */ + std::size_t m_maxPoolSize; ///< The maximum number of objects in the pool + std::mutex m_mutex; ///< Mutex for thread safety + std::stack m_pool; ///< The pool of objects }; } // namespace pcpp From 4c2e0cbb8c8cd85d7896b0bfa80a5466381237bf Mon Sep 17 00:00:00 2001 From: Dimitar Krastev Date: Wed, 1 Jan 2025 13:58:10 +0200 Subject: [PATCH 35/52] Added full namespace qualifier for marco code. --- Common++/header/Logger.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Common++/header/Logger.h b/Common++/header/Logger.h index 315a12a02b..7b3100c352 100644 --- a/Common++/header/Logger.h +++ b/Common++/header/Logger.h @@ -468,7 +468,7 @@ namespace pcpp #define PCPP_LOG(level, message) \ do \ { \ - auto& logger = Logger::getInstance(); \ + auto& logger = pcpp::Logger::getInstance(); \ if (logger.shouldLog(level, LOG_MODULE)) \ { \ auto ctx = \ From d63b4dae26d3ed0c3c4432554adf54cda2e13f40 Mon Sep 17 00:00:00 2001 From: Dimitar Krastev Date: Wed, 1 Jan 2025 14:07:18 +0200 Subject: [PATCH 36/52] Changed C-array to std::array. --- Common++/header/Logger.h | 3 ++- Common++/src/Logger.cpp | 3 +-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/Common++/header/Logger.h b/Common++/header/Logger.h index 7b3100c352..b86a4fb367 100644 --- a/Common++/header/Logger.h +++ b/Common++/header/Logger.h @@ -3,6 +3,7 @@ #include #include #include +#include #include #include #include @@ -430,7 +431,7 @@ namespace pcpp private: bool m_LogsEnabled; - LogLevel m_LogModulesArray[NumOfLogModules]; + std::array m_LogModulesArray; LogPrinter m_LogPrinter; std::string m_LastError; diff --git a/Common++/src/Logger.cpp b/Common++/src/Logger.cpp index 875c8c6ff0..fd286ac4cb 100644 --- a/Common++/src/Logger.cpp +++ b/Common++/src/Logger.cpp @@ -8,8 +8,7 @@ namespace pcpp Logger::Logger() : m_LogsEnabled(true), m_LogPrinter(&defaultLogPrinter) { m_LastError.reserve(200); - for (int i = 0; i < NumOfLogModules; i++) - m_LogModulesArray[i] = LogLevel::Info; + m_LogModulesArray.fill(LogLevel::Info); } std::string Logger::logLevelAsString(LogLevel logLevel) From ced2eb917c429ba9029ac54459e7c8e680f4462a Mon Sep 17 00:00:00 2001 From: Dimitar Krastev Date: Wed, 1 Jan 2025 14:34:46 +0200 Subject: [PATCH 37/52] Added a mutex lock when writing or reading to last error string to prevent tearing. --- Common++/header/Logger.h | 6 +++++- Common++/src/Logger.cpp | 2 ++ 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/Common++/header/Logger.h b/Common++/header/Logger.h index b86a4fb367..8d9c7db303 100644 --- a/Common++/header/Logger.h +++ b/Common++/header/Logger.h @@ -4,6 +4,7 @@ #include #include #include +#include #include #include #include @@ -300,8 +301,9 @@ namespace pcpp } /// @return Get the last error message - std::string getLastError() + std::string getLastError() const { + std::lock_guard lock(m_LastErrorMtx); return m_LastError; } @@ -433,6 +435,8 @@ namespace pcpp bool m_LogsEnabled; std::array m_LogModulesArray; LogPrinter m_LogPrinter; + + mutable std::mutex m_LastErrorMtx; std::string m_LastError; bool m_UseContextPooling = true; diff --git a/Common++/src/Logger.cpp b/Common++/src/Logger.cpp index fd286ac4cb..cb49f1160a 100644 --- a/Common++/src/Logger.cpp +++ b/Common++/src/Logger.cpp @@ -55,8 +55,10 @@ namespace pcpp void Logger::emit(LogSource const& source, LogLevel logLevel, std::string const& message) { + // If the log level is an error, save the error to the last error message variable. if (logLevel == LogLevel::Error) { + std::lock_guard lock(m_LastErrorMtx); m_LastError = message; } if (m_LogsEnabled) From 8124585962620feefe32a5faf273ba8193366f90 Mon Sep 17 00:00:00 2001 From: Dimitar Krastev Date: Wed, 1 Jan 2025 15:08:17 +0200 Subject: [PATCH 38/52] Fixed object pool member variables. --- Common++/header/ObjectPool.h | 36 ++++++++++++++++++------------------ 1 file changed, 18 insertions(+), 18 deletions(-) diff --git a/Common++/header/ObjectPool.h b/Common++/header/ObjectPool.h index e8ad50f758..2369fd8cab 100644 --- a/Common++/header/ObjectPool.h +++ b/Common++/header/ObjectPool.h @@ -26,7 +26,7 @@ namespace pcpp /// @param[in] maxPoolSize The maximum number of objects in the pool /// @param[in] preallocate The number of objects to preallocate in the pool explicit ObjectPool(std::size_t maxPoolSize = DEFAULT_POOL_SIZE, std::size_t preallocate = 0) - : m_maxPoolSize(maxPoolSize) + : m_MaxPoolSize(maxPoolSize) { this->preallocate(preallocate); } @@ -62,17 +62,17 @@ namespace pcpp /// @return A raw pointer to an object from the pool. T* acquireObjectRaw() { - std::unique_lock lock(m_mutex); + std::unique_lock lock(m_Mutex); - if (m_pool.empty()) + if (m_Pool.empty()) { // We don't need the lock anymore, so release it. lock.unlock(); return new T(); } - T* obj = m_pool.top(); - m_pool.pop(); + T* obj = m_Pool.top(); + m_Pool.pop(); return obj; } @@ -95,11 +95,11 @@ namespace pcpp /// @param[in] obj The raw pointer to the object to release. void releaseObjectRaw(T* obj) { - std::unique_lock lock(m_mutex); + std::unique_lock lock(m_Mutex); - if (m_maxPoolSize == INFINITE_POOL_SIZE || m_pool.size() < m_maxPoolSize) + if (m_MaxPoolSize == INFINITE_POOL_SIZE || m_Pool.size() < m_MaxPoolSize) { - m_pool.push(obj); + m_Pool.push(obj); } else { @@ -113,27 +113,27 @@ namespace pcpp /// @param count The number of objects to pre-allocate. void preallocate(std::size_t count) { - std::unique_lock lock(m_mutex); - for (std::size_t i = m_pool.size(); i < count; i++) + std::unique_lock lock(m_Mutex); + for (std::size_t i = m_Pool.size(); i < count; i++) { - m_pool.push(new T()); + m_Pool.push(new T()); } } /// @brief Deallocates and releases all objects currently held by the pool. void clear() { - std::unique_lock lock(m_mutex); - while (!m_pool.empty()) + std::unique_lock lock(m_Mutex); + while (!m_Pool.empty()) { - delete m_pool.top(); - m_pool.pop(); + delete m_Pool.top(); + m_Pool.pop(); } } private: - std::size_t m_maxPoolSize; ///< The maximum number of objects in the pool - std::mutex m_mutex; ///< Mutex for thread safety - std::stack m_pool; ///< The pool of objects + std::size_t m_MaxPoolSize; ///< The maximum number of objects in the pool + std::mutex m_Mutex; ///< Mutex for thread safety + std::stack m_Pool; ///< The pool of objects }; } // namespace pcpp From 0831bcfa37a6f3ebac381f8a3ee50ee653b8247d Mon Sep 17 00:00:00 2001 From: Dimitar Krastev Date: Wed, 1 Jan 2025 15:09:09 +0200 Subject: [PATCH 39/52] Added mutators to change the max size of an object pool at runtime. --- Common++/header/ObjectPool.h | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/Common++/header/ObjectPool.h b/Common++/header/ObjectPool.h index 2369fd8cab..7415b02758 100644 --- a/Common++/header/ObjectPool.h +++ b/Common++/header/ObjectPool.h @@ -109,6 +109,27 @@ namespace pcpp } } + /// @brief Gets the maximum number of objects in the pool. + std::size_t maxSize() const + { + std::lock_guard lock(m_Mutex); + return m_MaxPoolSize; + } + + /// @brief Sets the maximum number of objects in the pool. + void setMaxSize(std::size_t maxSize) + { + std::lock_guard lock(m_Mutex); + m_MaxPoolSize = maxSize; + + // If the new max size is less than the current size, we need to remove some objects from the pool. + while (m_Pool.size() > m_MaxPoolSize) + { + delete m_Pool.top(); + m_Pool.pop(); + } + } + /// @brief Pre-allocates up to a minimum number of objects in the pool. /// @param count The number of objects to pre-allocate. void preallocate(std::size_t count) From 715f4d0bc71dea73084bc1fd329ac4bb7d0d7581 Mon Sep 17 00:00:00 2001 From: Dimitar Krastev Date: Wed, 1 Jan 2025 15:09:52 +0200 Subject: [PATCH 40/52] Added exception if pool preallocation size is larger then the maximum allowed pool size. --- Common++/header/ObjectPool.h | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/Common++/header/ObjectPool.h b/Common++/header/ObjectPool.h index 7415b02758..c7d2473917 100644 --- a/Common++/header/ObjectPool.h +++ b/Common++/header/ObjectPool.h @@ -28,7 +28,11 @@ namespace pcpp explicit ObjectPool(std::size_t maxPoolSize = DEFAULT_POOL_SIZE, std::size_t preallocate = 0) : m_MaxPoolSize(maxPoolSize) { - this->preallocate(preallocate); + if (preallocate > maxPoolSize) + throw std::invalid_argument("Preallocated objects cannot exceed the maximum pool size"); + + if (preallocate > 0) + this->preallocate(preallocate); } // These don't strictly need to be deleted, but don't need to be implemented for now either. From b8ee9eda8dffda7f16acfa9aa98b14a7df76a5d8 Mon Sep 17 00:00:00 2001 From: Dimitar Krastev Date: Fri, 3 Jan 2025 14:12:59 +0200 Subject: [PATCH 41/52] Added tests for ObjectPool. - Changed infinite pool size to be maximum value of size_t instead of 0, to fix an issue if max size is set to 0. - Added size getter to the pool. - Marked the pool mutex as mutable. --- Common++/header/ObjectPool.h | 19 ++++-- Tests/Pcap++Test/CMakeLists.txt | 1 + Tests/Pcap++Test/TestDefinition.h | 3 + Tests/Pcap++Test/Tests/ObjectPoolTests.cpp | 76 ++++++++++++++++++++++ Tests/Pcap++Test/main.cpp | 2 + 5 files changed, 97 insertions(+), 4 deletions(-) create mode 100644 Tests/Pcap++Test/Tests/ObjectPoolTests.cpp diff --git a/Common++/header/ObjectPool.h b/Common++/header/ObjectPool.h index c7d2473917..da2931ce11 100644 --- a/Common++/header/ObjectPool.h +++ b/Common++/header/ObjectPool.h @@ -3,6 +3,7 @@ #include #include #include +#include #include namespace pcpp @@ -20,7 +21,10 @@ namespace pcpp { public: constexpr static std::size_t DEFAULT_POOL_SIZE = 100; - constexpr static std::size_t INFINITE_POOL_SIZE = 0; +#pragma push_macro("max") // Undefine max to avoid conflict with std::numeric_limits::max() +#undef max + constexpr static std::size_t INFINITE_POOL_SIZE = std::numeric_limits::max(); +#pragma pop_macro("max") /// A constructor for this class that creates a pool of objects /// @param[in] maxPoolSize The maximum number of objects in the pool @@ -113,6 +117,13 @@ namespace pcpp } } + /// @brief Gets the current number of objects in the pool. + std::size_t size() const + { + std::lock_guard lock(m_Mutex); + return m_Pool.size(); + } + /// @brief Gets the maximum number of objects in the pool. std::size_t maxSize() const { @@ -157,8 +168,8 @@ namespace pcpp } private: - std::size_t m_MaxPoolSize; ///< The maximum number of objects in the pool - std::mutex m_Mutex; ///< Mutex for thread safety - std::stack m_Pool; ///< The pool of objects + std::size_t m_MaxPoolSize; ///< The maximum number of objects in the pool + mutable std::mutex m_Mutex; ///< Mutex for thread safety + std::stack m_Pool; ///< The pool of objects }; } // namespace pcpp diff --git a/Tests/Pcap++Test/CMakeLists.txt b/Tests/Pcap++Test/CMakeLists.txt index 73c6fdcc06..4436b7c655 100644 --- a/Tests/Pcap++Test/CMakeLists.txt +++ b/Tests/Pcap++Test/CMakeLists.txt @@ -10,6 +10,7 @@ add_executable( Tests/KniTests.cpp Tests/LiveDeviceTests.cpp Tests/LoggerTests.cpp + Tests/ObjectPoolTests.cpp Tests/PacketParsingTests.cpp Tests/PfRingTests.cpp Tests/RawSocketTests.cpp diff --git a/Tests/Pcap++Test/TestDefinition.h b/Tests/Pcap++Test/TestDefinition.h index ccaaec4756..74188ff885 100644 --- a/Tests/Pcap++Test/TestDefinition.h +++ b/Tests/Pcap++Test/TestDefinition.h @@ -12,6 +12,9 @@ PTF_TEST_CASE(TestIPv4Network); PTF_TEST_CASE(TestIPv6Network); PTF_TEST_CASE(TestIPNetwork); +// Implemented in ObjectPoolTests.cpp +PTF_TEST_CASE(TestObjectPool); + // Implemented in LoggerTests.cpp PTF_TEST_CASE(TestLogger); PTF_TEST_CASE(TestLoggerMultiThread); diff --git a/Tests/Pcap++Test/Tests/ObjectPoolTests.cpp b/Tests/Pcap++Test/Tests/ObjectPoolTests.cpp new file mode 100644 index 0000000000..9ef3bad258 --- /dev/null +++ b/Tests/Pcap++Test/Tests/ObjectPoolTests.cpp @@ -0,0 +1,76 @@ + +#include "../TestDefinition.h" + +#include "ObjectPool.h" + +PTF_TEST_CASE(TestObjectPool) +{ + using pcpp::ObjectPool; + + { + ObjectPool pool; + PTF_ASSERT_EQUAL(pool.size(), 0); + PTF_ASSERT_EQUAL(pool.maxSize(), 100); + + pool.preallocate(2); + PTF_ASSERT_EQUAL(pool.size(), 2); + + pool.setMaxSize(1); + PTF_ASSERT_EQUAL(pool.size(), 1); + PTF_ASSERT_EQUAL(pool.maxSize(), 1); + + pool.clear(); + PTF_ASSERT_EQUAL(pool.size(), 0); + PTF_ASSERT_EQUAL(pool.maxSize(), 1); + } + + { + ObjectPool pool(10, 2); + PTF_ASSERT_EQUAL(pool.size(), 2); + PTF_ASSERT_EQUAL(pool.maxSize(), 10); + + PTF_ASSERT_RAISES(ObjectPool(0, 2), std::invalid_argument, + "Preallocated objects cannot exceed the maximum pool size"); + } + + { + ObjectPool pool; + PTF_ASSERT_EQUAL(pool.size(), 0); + + // Acquire an object, since the pool is empty, a new object will be created. + auto obj1 = pool.acquireObject(); + PTF_ASSERT_NOT_NULL(obj1); + + // Acquire a second object, since the pool is still empty, a new object will be created. + auto obj2 = pool.acquireObject(); + + // For the purposes of this test a value will be assigned to track the object. + *obj1 = 55; + *obj2 = 66; + + // Release the objects back to the pool. + pool.releaseObject(std::move(obj1)); + pool.releaseObject(std::move(obj2)); + + PTF_ASSERT_EQUAL(pool.size(), 2); + + // Acquire an object again, this time the object should be reused. + // Since the pool is a LIFO stack the object that was released last should be acquired first. + obj1 = pool.acquireObject(); + + // The value should be the same as the one assigned before releasing the object. + PTF_ASSERT_EQUAL(*obj1, 66); + + // Acquire the second object, this time the object that was released first should be acquired. + obj2 = pool.acquireObject(); + PTF_ASSERT_EQUAL(*obj2, 55); + + // Set the max size of the pool to zero to test the deletion of objects. + pool.setMaxSize(0); + + // Release the objects back to the pool, this time the objects should be deleted. + pool.releaseObject(std::move(obj1)); + pool.releaseObject(std::move(obj2)); + PTF_ASSERT_EQUAL(pool.size(), 0); + } +} diff --git a/Tests/Pcap++Test/main.cpp b/Tests/Pcap++Test/main.cpp index 7445c3fe7f..4904cd7fff 100644 --- a/Tests/Pcap++Test/main.cpp +++ b/Tests/Pcap++Test/main.cpp @@ -210,6 +210,8 @@ int main(int argc, char* argv[]) PTF_RUN_TEST(TestIPv6Network, "no_network;ip"); PTF_RUN_TEST(TestIPNetwork, "no_network;ip"); + PTF_RUN_TEST(TestObjectPool, "no_network"); + PTF_RUN_TEST(TestLogger, "no_network;logger"); PTF_RUN_TEST(TestLoggerMultiThread, "no_network;logger;skip_mem_leak_check"); From c42f637624d21e907a07046a42e42b9fbc91763f Mon Sep 17 00:00:00 2001 From: Dimitar Krastev Date: Fri, 3 Jan 2025 14:14:07 +0200 Subject: [PATCH 42/52] Lint --- Common++/header/Logger.h | 4 ++-- Tests/Pcap++Test/Tests/ObjectPoolTests.cpp | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/Common++/header/Logger.h b/Common++/header/Logger.h index 8d9c7db303..ea10a481c9 100644 --- a/Common++/header/Logger.h +++ b/Common++/header/Logger.h @@ -137,8 +137,8 @@ namespace pcpp LogModule logModule = UndefinedLogModule; /**< The log module. */ }; - /// An enum representing the log level. Currently 4 log levels are supported: Off, Error, Info and Debug. Info is the - /// default log level + /// An enum representing the log level. Currently 4 log levels are supported: Off, Error, Info and Debug. Info is + /// the default log level enum class LogLevel { Off = PCPP_LOG_LEVEL_OFF, ///< No log messages are emitted. diff --git a/Tests/Pcap++Test/Tests/ObjectPoolTests.cpp b/Tests/Pcap++Test/Tests/ObjectPoolTests.cpp index 9ef3bad258..e3c06ab4df 100644 --- a/Tests/Pcap++Test/Tests/ObjectPoolTests.cpp +++ b/Tests/Pcap++Test/Tests/ObjectPoolTests.cpp @@ -40,14 +40,14 @@ PTF_TEST_CASE(TestObjectPool) // Acquire an object, since the pool is empty, a new object will be created. auto obj1 = pool.acquireObject(); PTF_ASSERT_NOT_NULL(obj1); - + // Acquire a second object, since the pool is still empty, a new object will be created. auto obj2 = pool.acquireObject(); // For the purposes of this test a value will be assigned to track the object. *obj1 = 55; *obj2 = 66; - + // Release the objects back to the pool. pool.releaseObject(std::move(obj1)); pool.releaseObject(std::move(obj2)); From c63042317fc714b3f1a82b771c5767085137e02c Mon Sep 17 00:00:00 2001 From: Dimitar Krastev Date: Fri, 3 Jan 2025 14:20:23 +0200 Subject: [PATCH 43/52] Added option to set the max pool size for the Logger context pool. --- Common++/header/Logger.h | 7 ++++++- Common++/header/ObjectPool.h | 7 +++++++ Tests/Pcap++Test/Tests/ObjectPoolTests.cpp | 3 +++ 3 files changed, 16 insertions(+), 1 deletion(-) diff --git a/Common++/header/Logger.h b/Common++/header/Logger.h index ea10a481c9..81f7387fa1 100644 --- a/Common++/header/Logger.h +++ b/Common++/header/Logger.h @@ -327,15 +327,20 @@ namespace pcpp } /// @brief Controls if the logger should use a pool of LogContext objects. + /// + /// If enabled is set to false, preallocate and maxPoolSize are ignored. /// @param enabled True to enable context pooling, false to disable. /// @param preallocate The number of LogContext objects to preallocate in the pool. + /// @param maxPoolSize The maximum number of LogContext objects to keep in the pool. /// @remarks Disabling the pooling clears the pool. - void useContextPooling(bool enabled, std::size_t preallocate = 2) + void useContextPooling(bool enabled, std::size_t preallocate = 2, std::size_t maxPoolSize = 10) { m_UseContextPooling = enabled; if (m_UseContextPooling) { + m_LogContextPool.setMaxSize(maxPoolSize); + if (preallocate > 0) { m_LogContextPool.preallocate(preallocate); diff --git a/Common++/header/ObjectPool.h b/Common++/header/ObjectPool.h index da2931ce11..b1299f4e75 100644 --- a/Common++/header/ObjectPool.h +++ b/Common++/header/ObjectPool.h @@ -150,6 +150,13 @@ namespace pcpp void preallocate(std::size_t count) { std::unique_lock lock(m_Mutex); + + if (m_MaxPoolSize < count) + { + throw std::invalid_argument("Preallocated objects cannot exceed the maximum pool size"); + } + + // If the pool is already larger than the requested count, we don't need to do anything. for (std::size_t i = m_Pool.size(); i < count; i++) { m_Pool.push(new T()); diff --git a/Tests/Pcap++Test/Tests/ObjectPoolTests.cpp b/Tests/Pcap++Test/Tests/ObjectPoolTests.cpp index e3c06ab4df..3255552a65 100644 --- a/Tests/Pcap++Test/Tests/ObjectPoolTests.cpp +++ b/Tests/Pcap++Test/Tests/ObjectPoolTests.cpp @@ -19,6 +19,9 @@ PTF_TEST_CASE(TestObjectPool) PTF_ASSERT_EQUAL(pool.size(), 1); PTF_ASSERT_EQUAL(pool.maxSize(), 1); + PTF_ASSERT_RAISES(pool.preallocate(2), std::invalid_argument, + "Preallocated objects cannot exceed the maximum pool size"); + pool.clear(); PTF_ASSERT_EQUAL(pool.size(), 0); PTF_ASSERT_EQUAL(pool.maxSize(), 1); From 1e5694c915b58d0be3b073d442920bbe48d05f54 Mon Sep 17 00:00:00 2001 From: Dimitar Krastev Date: Fri, 3 Jan 2025 14:28:01 +0200 Subject: [PATCH 44/52] Disabled logger context pooling in the tests as it is detected as a false positive memory leak. --- Tests/Packet++Test/main.cpp | 4 ++-- Tests/Pcap++Test/main.cpp | 6 +++--- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/Tests/Packet++Test/main.cpp b/Tests/Packet++Test/main.cpp index aec896dae7..ebf1838620 100644 --- a/Tests/Packet++Test/main.cpp +++ b/Tests/Packet++Test/main.cpp @@ -85,8 +85,8 @@ int main(int argc, char* argv[]) #endif // The logger singleton looks like a memory leak. Invoke it before starting the memory check - // Context pooling can cause issues if the logger's built-in context pool allocates new LogContext instances. - pcpp::Logger::getInstance(); + // Disables context pooling to avoid false positives in the memory leak check, as the contexts persist in the pool. + pcpp::Logger::getInstance().useContextPooling(false); // cppcheck-suppress knownConditionTrueFalse if (skipMemLeakCheck) diff --git a/Tests/Pcap++Test/main.cpp b/Tests/Pcap++Test/main.cpp index 4904cd7fff..204e34fee7 100644 --- a/Tests/Pcap++Test/main.cpp +++ b/Tests/Pcap++Test/main.cpp @@ -143,9 +143,9 @@ int main(int argc, char* argv[]) << " https://github.com/cpputest/cpputest/issues/786#issuecomment-148921958" << std::endl; #endif - // The logger singleton looks like a memory leak. Invoke it before starting the memory check - // Context pooling can cause issues if the logger's built-in context pool allocates new LogContext instances. - pcpp::Logger::getInstance(); + // The logger singleton looks like a memory leak. Invoke it before starting the memory check. + // Disables context pooling to avoid false positives in the memory leak check, as the contexts persist in the pool. + pcpp::Logger::getInstance().useContextPooling(false); // cppcheck-suppress knownConditionTrueFalse if (skipMemLeakCheck) From 6da344c214c96d169112cd5e626bfb791734221c Mon Sep 17 00:00:00 2001 From: Dimitar Krastev Date: Fri, 3 Jan 2025 14:28:42 +0200 Subject: [PATCH 45/52] Lint --- Common++/header/Logger.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Common++/header/Logger.h b/Common++/header/Logger.h index 81f7387fa1..bede6cb84d 100644 --- a/Common++/header/Logger.h +++ b/Common++/header/Logger.h @@ -327,7 +327,7 @@ namespace pcpp } /// @brief Controls if the logger should use a pool of LogContext objects. - /// + /// /// If enabled is set to false, preallocate and maxPoolSize are ignored. /// @param enabled True to enable context pooling, false to disable. /// @param preallocate The number of LogContext objects to preallocate in the pool. From 421a41994937f229744f5990b6c442f95de0c2dd Mon Sep 17 00:00:00 2001 From: Dimitar Krastev Date: Fri, 3 Jan 2025 15:28:25 +0200 Subject: [PATCH 46/52] Lint --- Common++/CMakeLists.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Common++/CMakeLists.txt b/Common++/CMakeLists.txt index 48344830ad..8ef1ca1f79 100644 --- a/Common++/CMakeLists.txt +++ b/Common++/CMakeLists.txt @@ -22,7 +22,7 @@ set( header/Logger.h header/LRUList.h header/MacAddress.h - header/ObjectPool.h + header/ObjectPool.h header/OUILookup.h header/PcapPlusPlusVersion.h header/PointerVector.h From 6915e6ad5d96137f50317ab6c07c31f4b08e8997 Mon Sep 17 00:00:00 2001 From: Dimitar Krastev Date: Fri, 3 Jan 2025 17:14:03 +0200 Subject: [PATCH 47/52] Updated logger tests. - Changed `pcpp::Logger::getInstance()` to use a cached `logger` variable. - Removed the need for fully qualified names in the logger test. - C-style casts to Cpp casts. --- Tests/Pcap++Test/Tests/LoggerTests.cpp | 57 ++++++++++++++++---------- 1 file changed, 35 insertions(+), 22 deletions(-) diff --git a/Tests/Pcap++Test/Tests/LoggerTests.cpp b/Tests/Pcap++Test/Tests/LoggerTests.cpp index e40b235ea8..6168f183bf 100644 --- a/Tests/Pcap++Test/Tests/LoggerTests.cpp +++ b/Tests/Pcap++Test/Tests/LoggerTests.cpp @@ -190,24 +190,34 @@ PTF_TEST_CASE(TestLoggerMultiThread) PTF_TEST_CASE(TestLogger) { + using pcpp::Logger; + using pcpp::LogLevel; + using pcpp::LogModule; + + auto& logger = Logger::getInstance(); + // cppcheck-suppress unusedVariable LoggerCleaner loggerCleaner; // verify all modules are on info log level - for (int module = 1; module < pcpp::NumOfLogModules; module++) + for (int moduleInt = 1; moduleInt < LogModule::NumOfLogModules; moduleInt++) { - PTF_ASSERT_EQUAL(pcpp::Logger::getInstance().getLogLevel((pcpp::LogModule)module), pcpp::Logger::Info, enum); - PTF_ASSERT_FALSE(pcpp::Logger::getInstance().isDebugEnabled((pcpp::LogModule)module)); + const LogModule moduleEnum = static_cast(moduleInt); + + PTF_ASSERT_EQUAL(logger.getLogLevel(moduleEnum), LogLevel::Info, enum); + PTF_ASSERT_FALSE(logger.isDebugEnabled(moduleEnum)); } // invoke debug and error logs - expect to see only the error log - pcpp::Logger::getInstance().setLogPrinter(&LogPrinter::logPrinter); + logger.setLogPrinter(&LogPrinter::logPrinter); + pcpp::invokeDebugLog(); PTF_ASSERT_EQUAL(LogPrinter::lastLogLevelSeen, 999); PTF_ASSERT_EQUAL(LogPrinter::lastLineSeen, 99999); PTF_ASSERT_NULL(LogPrinter::lastLogMessageSeen); PTF_ASSERT_NULL(LogPrinter::lastFilenameSeen); PTF_ASSERT_NULL(LogPrinter::lastMethodSeen); + pcpp::invokeErrorLog(); PTF_ASSERT_EQUAL(LogPrinter::lastLogLevelSeen, (int)pcpp::Logger::Error); PTF_ASSERT_EQUAL(*LogPrinter::lastLogMessageSeen, "error log"); @@ -216,9 +226,10 @@ PTF_TEST_CASE(TestLogger) PTF_ASSERT_EQUAL(LogPrinter::lastLineSeen, 21); // change one module log level - pcpp::Logger::getInstance().setLogLevel(pcpp::PacketLogModuleArpLayer, pcpp::Logger::Debug); - PTF_ASSERT_EQUAL(pcpp::Logger::getInstance().getLogLevel(pcpp::PacketLogModuleArpLayer), pcpp::Logger::Debug, enum); - PTF_ASSERT_TRUE(pcpp::Logger::getInstance().isDebugEnabled(pcpp::PacketLogModuleArpLayer)); + logger.setLogLevel(pcpp::PacketLogModuleArpLayer, pcpp::Logger::Debug); + PTF_ASSERT_EQUAL(logger.getLogLevel(pcpp::PacketLogModuleArpLayer), pcpp::LogLevel::Debug, + enum); + PTF_ASSERT_TRUE(logger.isDebugEnabled(pcpp::PacketLogModuleArpLayer)); // invoke debug and error logs - expect to see both pcpp::invokeDebugLog(); @@ -236,14 +247,16 @@ PTF_TEST_CASE(TestLogger) PTF_ASSERT_EQUAL(LogPrinter::lastLineSeen, 21); // verify the last error message - PTF_ASSERT_EQUAL(pcpp::Logger::getInstance().getLastError(), "error log"); + PTF_ASSERT_EQUAL(logger.getLastError(), "error log"); // change all modules log level - pcpp::Logger::getInstance().setAllModulesToLogLevel(pcpp::Logger::Debug); - for (int module = 1; module < pcpp::NumOfLogModules; module++) + logger.setAllModulesToLogLevel(LogLevel::Debug); + for (int moduleInt = 1; moduleInt < LogModule::NumOfLogModules; moduleInt++) { - PTF_ASSERT_EQUAL(pcpp::Logger::getInstance().getLogLevel((pcpp::LogModule)module), pcpp::Logger::Debug, enum); - PTF_ASSERT_TRUE(pcpp::Logger::getInstance().isDebugEnabled((pcpp::LogModule)module)); + auto const moduleEnum = static_cast(moduleInt); + + PTF_ASSERT_EQUAL(logger.getLogLevel(static_cast(moduleEnum)), pcpp::LogLevel::Debug, enum); + PTF_ASSERT_TRUE(logger.isDebugEnabled(static_cast(moduleEnum))); } // invoke debug log - expect to see it @@ -255,9 +268,9 @@ PTF_TEST_CASE(TestLogger) PTF_ASSERT_EQUAL(LogPrinter::lastLineSeen, 16); // suppress logs - PTF_ASSERT_TRUE(pcpp::Logger::getInstance().logsEnabled()) - pcpp::Logger::getInstance().suppressLogs(); - PTF_ASSERT_FALSE(pcpp::Logger::getInstance().logsEnabled()) + PTF_ASSERT_TRUE(logger.logsEnabled()) + logger.suppressLogs(); + PTF_ASSERT_FALSE(logger.logsEnabled()) // reset LogPrinter LogPrinter::clean(); @@ -270,32 +283,32 @@ PTF_TEST_CASE(TestLogger) // invoke another error log - expect to see it as the last error message although logs are suppressed pcpp::invokeErrorLog("2"); - PTF_ASSERT_EQUAL(pcpp::Logger::getInstance().getLastError(), "error log2"); + PTF_ASSERT_EQUAL(logger.getLastError(), "error log2"); // re-enable logs - pcpp::Logger::getInstance().enableLogs(); - PTF_ASSERT_TRUE(pcpp::Logger::getInstance().logsEnabled()) + logger.enableLogs(); + PTF_ASSERT_TRUE(logger.logsEnabled()) // invoke error log - expect to see it pcpp::invokeErrorLog(); - PTF_ASSERT_EQUAL(LogPrinter::lastLogLevelSeen, (int)pcpp::Logger::Error); + PTF_ASSERT_EQUAL(LogPrinter::lastLogLevelSeen, static_cast(pcpp::LogLevel::Error)); PTF_ASSERT_EQUAL(*LogPrinter::lastLogMessageSeen, "error log"); PTF_ASSERT_EQUAL(getLowerCaseFileName(*LogPrinter::lastFilenameSeen), "loggertests.cpp"); PTF_ASSERT_EQUAL(getMethodWithoutNamespace(*LogPrinter::lastMethodSeen), "invokeErrorLog"); - PTF_ASSERT_EQUAL(pcpp::Logger::getInstance().getLastError(), "error log"); + PTF_ASSERT_EQUAL(logger.getLastError(), "error log"); PTF_ASSERT_EQUAL(LogPrinter::lastLineSeen, 21); // reset LogPrinter LogPrinter::clean(); // reset the log printer - pcpp::Logger::getInstance().resetLogPrinter(); + logger.resetLogPrinter(); // disable std::cout for a bit std::cout.setstate(std::ios_base::failbit); // set debug log for a module, don't expect to see it in the custom log printer - pcpp::Logger::getInstance().setLogLevel(pcpp::PacketLogModuleArpLayer, pcpp::Logger::Debug); + logger.setLogLevel(pcpp::PacketLogModuleArpLayer, pcpp::LogLevel::Debug); pcpp::invokeDebugLog(); pcpp::invokeErrorLog(); PTF_ASSERT_EQUAL(LogPrinter::lastLogLevelSeen, 999); From cd3e36184e067631254d049aaffd40a1ce3970a9 Mon Sep 17 00:00:00 2001 From: Dimitar Krastev Date: Fri, 3 Jan 2025 17:14:40 +0200 Subject: [PATCH 48/52] Added tests for `shouldLog`. --- Tests/Pcap++Test/Tests/LoggerTests.cpp | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/Tests/Pcap++Test/Tests/LoggerTests.cpp b/Tests/Pcap++Test/Tests/LoggerTests.cpp index 6168f183bf..d338c1fd0d 100644 --- a/Tests/Pcap++Test/Tests/LoggerTests.cpp +++ b/Tests/Pcap++Test/Tests/LoggerTests.cpp @@ -206,6 +206,11 @@ PTF_TEST_CASE(TestLogger) PTF_ASSERT_EQUAL(logger.getLogLevel(moduleEnum), LogLevel::Info, enum); PTF_ASSERT_FALSE(logger.isDebugEnabled(moduleEnum)); + + PTF_ASSERT_TRUE(logger.shouldLog(LogLevel::Error, moduleEnum)); + PTF_ASSERT_TRUE(logger.shouldLog(LogLevel::Info, moduleEnum)); + PTF_ASSERT_FALSE(logger.shouldLog(LogLevel::Debug, moduleEnum)); + PTF_ASSERT_FALSE(logger.shouldLog(LogLevel::Off, moduleEnum)); } // invoke debug and error logs - expect to see only the error log @@ -257,6 +262,11 @@ PTF_TEST_CASE(TestLogger) PTF_ASSERT_EQUAL(logger.getLogLevel(static_cast(moduleEnum)), pcpp::LogLevel::Debug, enum); PTF_ASSERT_TRUE(logger.isDebugEnabled(static_cast(moduleEnum))); + + PTF_ASSERT_TRUE(logger.shouldLog(LogLevel::Error, moduleEnum)); + PTF_ASSERT_TRUE(logger.shouldLog(LogLevel::Info, moduleEnum)); + PTF_ASSERT_TRUE(logger.shouldLog(LogLevel::Debug, moduleEnum)); + PTF_ASSERT_FALSE(logger.shouldLog(LogLevel::Off, moduleEnum)); } // invoke debug log - expect to see it From 37a0f199bb48dea5df6914959229aafa53455df6 Mon Sep 17 00:00:00 2001 From: Dimitar Krastev Date: Fri, 3 Jan 2025 17:39:48 +0200 Subject: [PATCH 49/52] Moved iostream and iomanip to Logger.cpp as they are unnessesary in the header. --- Common++/header/Logger.h | 3 +-- Common++/src/Logger.cpp | 2 ++ 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/Common++/header/Logger.h b/Common++/header/Logger.h index bede6cb84d..6bce565ff9 100644 --- a/Common++/header/Logger.h +++ b/Common++/header/Logger.h @@ -5,9 +5,8 @@ #include #include #include -#include +#include #include -#include #include "DeprecationUtils.h" #include "ObjectPool.h" diff --git a/Common++/src/Logger.cpp b/Common++/src/Logger.cpp index cb49f1160a..d0dbc12d87 100644 --- a/Common++/src/Logger.cpp +++ b/Common++/src/Logger.cpp @@ -1,5 +1,7 @@ #include "Logger.h" +#include +#include #include #include From 28d5e4f652f63fc1b080053a7205f5dc6378dedc Mon Sep 17 00:00:00 2001 From: Dimitar Krastev Date: Fri, 3 Jan 2025 17:51:13 +0200 Subject: [PATCH 50/52] Lint --- Tests/Pcap++Test/Tests/LoggerTests.cpp | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/Tests/Pcap++Test/Tests/LoggerTests.cpp b/Tests/Pcap++Test/Tests/LoggerTests.cpp index d338c1fd0d..cbdbb19f23 100644 --- a/Tests/Pcap++Test/Tests/LoggerTests.cpp +++ b/Tests/Pcap++Test/Tests/LoggerTests.cpp @@ -206,7 +206,7 @@ PTF_TEST_CASE(TestLogger) PTF_ASSERT_EQUAL(logger.getLogLevel(moduleEnum), LogLevel::Info, enum); PTF_ASSERT_FALSE(logger.isDebugEnabled(moduleEnum)); - + PTF_ASSERT_TRUE(logger.shouldLog(LogLevel::Error, moduleEnum)); PTF_ASSERT_TRUE(logger.shouldLog(LogLevel::Info, moduleEnum)); PTF_ASSERT_FALSE(logger.shouldLog(LogLevel::Debug, moduleEnum)); @@ -215,14 +215,14 @@ PTF_TEST_CASE(TestLogger) // invoke debug and error logs - expect to see only the error log logger.setLogPrinter(&LogPrinter::logPrinter); - + pcpp::invokeDebugLog(); PTF_ASSERT_EQUAL(LogPrinter::lastLogLevelSeen, 999); PTF_ASSERT_EQUAL(LogPrinter::lastLineSeen, 99999); PTF_ASSERT_NULL(LogPrinter::lastLogMessageSeen); PTF_ASSERT_NULL(LogPrinter::lastFilenameSeen); PTF_ASSERT_NULL(LogPrinter::lastMethodSeen); - + pcpp::invokeErrorLog(); PTF_ASSERT_EQUAL(LogPrinter::lastLogLevelSeen, (int)pcpp::Logger::Error); PTF_ASSERT_EQUAL(*LogPrinter::lastLogMessageSeen, "error log"); @@ -232,8 +232,7 @@ PTF_TEST_CASE(TestLogger) // change one module log level logger.setLogLevel(pcpp::PacketLogModuleArpLayer, pcpp::Logger::Debug); - PTF_ASSERT_EQUAL(logger.getLogLevel(pcpp::PacketLogModuleArpLayer), pcpp::LogLevel::Debug, - enum); + PTF_ASSERT_EQUAL(logger.getLogLevel(pcpp::PacketLogModuleArpLayer), pcpp::LogLevel::Debug, enum); PTF_ASSERT_TRUE(logger.isDebugEnabled(pcpp::PacketLogModuleArpLayer)); // invoke debug and error logs - expect to see both From 83e0dc36923a621b08432021c135bd154aa5d621 Mon Sep 17 00:00:00 2001 From: Dimitar Krastev Date: Sat, 4 Jan 2025 13:17:47 +0200 Subject: [PATCH 51/52] Added iostream to examples that depended on transitively including iostream from Logger.h --- Examples/KniPong/main.cpp | 1 + Examples/PfRingExample-FilterTraffic/main.cpp | 1 + 2 files changed, 2 insertions(+) diff --git a/Examples/KniPong/main.cpp b/Examples/KniPong/main.cpp index e77db3188a..beb5f89e97 100644 --- a/Examples/KniPong/main.cpp +++ b/Examples/KniPong/main.cpp @@ -5,6 +5,7 @@ #include #include #include +#include #include #include diff --git a/Examples/PfRingExample-FilterTraffic/main.cpp b/Examples/PfRingExample-FilterTraffic/main.cpp index f070fceb67..3d17b3b72a 100644 --- a/Examples/PfRingExample-FilterTraffic/main.cpp +++ b/Examples/PfRingExample-FilterTraffic/main.cpp @@ -40,6 +40,7 @@ #include #include #include +#include #include // clang-format off From 117ff498de20bf5ea6ea26224699e0278b1cd545 Mon Sep 17 00:00:00 2001 From: Dimitar Krastev Date: Sun, 5 Jan 2025 10:49:38 +0200 Subject: [PATCH 52/52] Added to examples that depended on transitively including from Logger.h --- Examples/PfRingExample-FilterTraffic/main.cpp | 1 + 1 file changed, 1 insertion(+) diff --git a/Examples/PfRingExample-FilterTraffic/main.cpp b/Examples/PfRingExample-FilterTraffic/main.cpp index 3d17b3b72a..7f7583ac9c 100644 --- a/Examples/PfRingExample-FilterTraffic/main.cpp +++ b/Examples/PfRingExample-FilterTraffic/main.cpp @@ -41,6 +41,7 @@ #include #include #include +#include #include // clang-format off