This is an automated email from the ASF dual-hosted git repository. rmiddleton pushed a commit to branch location-info in repository https://gitbox.apache.org/repos/asf/logging-log4cxx.git
commit 538c0bdd36c2986f56dc43b9a9fa3440dd046e90 Author: Robert Middleton <[email protected]> AuthorDate: Sun Jan 2 13:10:45 2022 -0500 Updated the location info to use string_view if available --- src/main/cpp/locationinfo.cpp | 61 ++++++++++++--------- src/main/include/log4cxx/logger.h | 63 ++++++++++++++-------- .../include/log4cxx/spi/location/locationinfo.h | 35 ++++++++++-- src/site/markdown/usage.md | 10 ++++ src/test/cpp/throughput/CMakeLists.txt | 2 + .../resources/input/patternLayout14.properties | 4 +- src/test/resources/witness/patternLayout.14 | 20 +++---- 7 files changed, 135 insertions(+), 60 deletions(-) diff --git a/src/main/cpp/locationinfo.cpp b/src/main/cpp/locationinfo.cpp index ebfa212..707a4e4 100644 --- a/src/main/cpp/locationinfo.cpp +++ b/src/main/cpp/locationinfo.cpp @@ -19,12 +19,6 @@ #include <log4cxx/helpers/objectoutputstream.h> #include <log4cxx/helpers/pool.h> -#if defined(_WIN32) -#define SHORT_FILENAME_SPLIT_CHAR R"(\)" -#else -#define SHORT_FILENAME_SPLIT_CHAR R"(/)" -#endif - using namespace ::log4cxx::spi; using namespace log4cxx::helpers; @@ -42,21 +36,6 @@ const LocationInfo& LocationInfo::getLocationUnavailable() } /** - * filter short file name - * @param fileName - * @return shortFileName - */ -const std::string filterFileName(const char* const fileName){ - std::string fullFileName(fileName); - std::size_t found = fullFileName.rfind(SHORT_FILENAME_SPLIT_CHAR); - if (found != std::string::npos) { - return fullFileName.substr(found + 1); - } else { - return std::string(fileName); - } -} - -/** * Constructor. * @remarks Used by LOG4CXX_LOCATION to generate * location info for current code site @@ -66,11 +45,28 @@ LocationInfo::LocationInfo( const char* const fileName1, int lineNumber1 ) : lineNumber( lineNumber1 ), fileName( fileName1 ), - shortFileName(filterFileName(fileName)), + shortFileName(nullptr), methodName( methodName1 ) { } +LocationInfo::LocationInfo( const char* const fileName1, + int shortFilenameOffset, + const char* const methodName1, + int lineNumber1 ) + : lineNumber( lineNumber1 ), + fileName( fileName1 ), + methodName( methodName1 ) +{ + if( shortFilenameOffset < 0 || shortFilenameOffset > 300 ){ + // Arbitrary cap at 300 chars for the filename offset - anything greather + // than that sounds suspicous. + shortFileName = fileName; + }else{ + shortFileName = fileName + shortFilenameOffset; + } +} + /** * Default constructor. */ @@ -130,8 +126,25 @@ const char* LocationInfo::getFileName() const * Return the short file name of the caller. * @returns file name, may be null. */ -const std::string LocationInfo::getShortFileName() const{ - return shortFileName; +const char* LocationInfo::getShortFileName() const{ + if(shortFileName){ + return shortFileName; + } + + if(fileName == nullptr){ + return nullptr; + } + + const char* begin = fileName; + size_t pos = 0; + while( fileName[pos] != 0 ){ + if( fileName[pos] == LOG4CXX_SHORT_FILENAME_SPLIT_CHAR && + fileName[pos+1] != 0 ){ + begin = fileName + pos + 1; + } + pos++; + } + return begin; } /** diff --git a/src/main/include/log4cxx/logger.h b/src/main/include/log4cxx/logger.h index b74d872..b9b060d 100644 --- a/src/main/include/log4cxx/logger.h +++ b/src/main/include/log4cxx/logger.h @@ -1768,7 +1768,8 @@ Logs a message to a specified logger with a specified level. #define LOG4CXX_LOG(logger, level, message) do { \ if (logger->isEnabledFor(level)) {\ ::log4cxx::helpers::MessageBuffer oss_; \ - logger->forcedLog(level, oss_.str(oss_ << message), LOG4CXX_LOCATION); }} while (0) + LOG4CXX_LOCATION_CREATE;\ + logger->forcedLog(level, oss_.str(oss_ << message), location); }} while (0) /** Logs a message to a specified logger with a specified level, formatting utilizing libfmt @@ -1779,7 +1780,8 @@ Logs a message to a specified logger with a specified level, formatting utilizin */ #define LOG4CXX_LOG_FMT(logger, level, ...) do { \ if (logger->isEnabledFor(level)) {\ - logger->forcedLog(level, fmt::format( __VA_ARGS__ ), LOG4CXX_LOCATION); }} while (0) + LOG4CXX_LOCATION_CREATE;\ + logger->forcedLog(level, fmt::format( __VA_ARGS__ ), location); }} while (0) /** Logs a message to a specified logger with a specified level. @@ -1791,7 +1793,8 @@ Logs a message to a specified logger with a specified level. #define LOG4CXX_LOGLS(logger, level, message) do { \ if (logger->isEnabledFor(level)) {\ ::log4cxx::helpers::LogCharMessageBuffer oss_; \ - logger->forcedLog(level, oss_.str(oss_ << message), LOG4CXX_LOCATION); }} while (0) + LOG4CXX_LOCATION_CREATE;\ + logger->forcedLog(level, oss_.str(oss_ << message), location); }} while (0) #if !defined(LOG4CXX_THRESHOLD) || LOG4CXX_THRESHOLD <= 10000 /** @@ -1803,7 +1806,8 @@ Logs a message to a specified logger with the DEBUG level. #define LOG4CXX_DEBUG(logger, message) do { \ if (LOG4CXX_UNLIKELY(logger->isDebugEnabled())) {\ ::log4cxx::helpers::MessageBuffer oss_; \ - logger->forcedLog(::log4cxx::Level::getDebug(), oss_.str(oss_ << message), LOG4CXX_LOCATION); }} while (0) + LOG4CXX_LOCATION_CREATE;\ + logger->forcedLog(::log4cxx::Level::getDebug(), oss_.str(oss_ << message), location); }} while (0) /** Logs a message to a specified logger with the DEBUG level, formatting with libfmt @@ -1813,7 +1817,8 @@ Logs a message to a specified logger with the DEBUG level, formatting with libfm */ #define LOG4CXX_DEBUG_FMT(logger, ...) do { \ if (LOG4CXX_UNLIKELY(logger->isDebugEnabled())) {\ - logger->forcedLog(::log4cxx::Level::getDebug(), fmt::format( __VA_ARGS__ ), LOG4CXX_LOCATION); }} while (0) + LOG4CXX_LOCATION_CREATE;\ + logger->forcedLog(::log4cxx::Level::getDebug(), fmt::format( __VA_ARGS__ ), location); }} while (0) #else #define LOG4CXX_DEBUG(logger, message) #define LOG4CXX_DEBUG_FMT(logger, ...) @@ -1829,7 +1834,8 @@ Logs a message to a specified logger with the TRACE level. #define LOG4CXX_TRACE(logger, message) do { \ if (LOG4CXX_UNLIKELY(logger->isTraceEnabled())) {\ ::log4cxx::helpers::MessageBuffer oss_; \ - logger->forcedLog(::log4cxx::Level::getTrace(), oss_.str(oss_ << message), LOG4CXX_LOCATION); }} while (0) + LOG4CXX_LOCATION_CREATE;\ + logger->forcedLog(::log4cxx::Level::getTrace(), oss_.str(oss_ << message), location); }} while (0) /** Logs a message to a specified logger with the TRACE level, formatting with libfmt. @@ -1839,7 +1845,8 @@ Logs a message to a specified logger with the TRACE level, formatting with libfm */ #define LOG4CXX_TRACE_FMT(logger, ...) do { \ if (LOG4CXX_UNLIKELY(logger->isTraceEnabled())) {\ - logger->forcedLog(::log4cxx::Level::getTrace(), fmt::format( __VA_ARGS__ ), LOG4CXX_LOCATION); }} while (0) + LOG4CXX_LOCATION_CREATE;\ + logger->forcedLog(::log4cxx::Level::getTrace(), fmt::format( __VA_ARGS__ ), location); }} while (0) #else #define LOG4CXX_TRACE(logger, message) #define LOG4CXX_TRACE_FMT(logger, ...) @@ -1855,7 +1862,8 @@ Logs a message to a specified logger with the INFO level. #define LOG4CXX_INFO(logger, message) do { \ if (logger->isInfoEnabled()) {\ ::log4cxx::helpers::MessageBuffer oss_; \ - logger->forcedLog(::log4cxx::Level::getInfo(), oss_.str(oss_ << message), LOG4CXX_LOCATION); }} while (0) + LOG4CXX_LOCATION_CREATE;\ + logger->forcedLog(::log4cxx::Level::getInfo(), oss_.str(oss_ << message), location); }} while (0) /** Logs a message to a specified logger with the INFO level, formatting with libfmt. @@ -1866,7 +1874,8 @@ Logs a message to a specified logger with the INFO level, formatting with libfmt */ #define LOG4CXX_INFO_FMT(logger, ...) do { \ if (logger->isInfoEnabled()) {\ - logger->forcedLog(::log4cxx::Level::getInfo(), fmt::format( __VA_ARGS__ ), LOG4CXX_LOCATION); }} while (0) + LOG4CXX_LOCATION_CREATE;\ + logger->forcedLog(::log4cxx::Level::getInfo(), fmt::format( __VA_ARGS__ ), location); }} while (0) #else #define LOG4CXX_INFO(logger, message) #define LOG4CXX_INFO_FMT(logger, ...) @@ -1882,7 +1891,8 @@ Logs a message to a specified logger with the WARN level. #define LOG4CXX_WARN(logger, message) do { \ if (logger->isWarnEnabled()) {\ ::log4cxx::helpers::MessageBuffer oss_; \ - logger->forcedLog(::log4cxx::Level::getWarn(), oss_.str(oss_ << message), LOG4CXX_LOCATION); }} while (0) + LOG4CXX_LOCATION_CREATE;\ + logger->forcedLog(::log4cxx::Level::getWarn(), oss_.str(oss_ << message), location); }} while (0) /** Logs a message to a specified logger with the WARN level, formatting with libfmt @@ -1892,7 +1902,8 @@ Logs a message to a specified logger with the WARN level, formatting with libfmt */ #define LOG4CXX_WARN_FMT(logger, ...) do { \ if (logger->isWarnEnabled()) {\ - logger->forcedLog(::log4cxx::Level::getWarn(), fmt::format( __VA_ARGS__ ), LOG4CXX_LOCATION); }} while (0) + LOG4CXX_LOCATION_CREATE;\ + logger->forcedLog(::log4cxx::Level::getWarn(), fmt::format( __VA_ARGS__ ), location); }} while (0) #else #define LOG4CXX_WARN(logger, message) #define LOG4CXX_WARN_FMT(logger, ...) @@ -1908,7 +1919,8 @@ Logs a message to a specified logger with the ERROR level. #define LOG4CXX_ERROR(logger, message) do { \ if (logger->isErrorEnabled()) {\ ::log4cxx::helpers::MessageBuffer oss_; \ - logger->forcedLog(::log4cxx::Level::getError(), oss_.str(oss_ << message), LOG4CXX_LOCATION); }} while (0) + LOG4CXX_LOCATION_CREATE;\ + logger->forcedLog(::log4cxx::Level::getError(), oss_.str(oss_ << message), location); }} while (0) /** Logs a message to a specified logger with the ERROR level, formatting with libfmt @@ -1918,7 +1930,8 @@ Logs a message to a specified logger with the ERROR level, formatting with libfm */ #define LOG4CXX_ERROR_FMT(logger, ...) do { \ if (logger->isErrorEnabled()) {\ - logger->forcedLog(::log4cxx::Level::getError(), fmt::format( __VA_ARGS__ ), LOG4CXX_LOCATION); }} while (0) + LOG4CXX_LOCATION_CREATE;\ + logger->forcedLog(::log4cxx::Level::getError(), fmt::format( __VA_ARGS__ ), location); }} while (0) /** Logs a error if the condition is not true. @@ -1930,7 +1943,8 @@ Logs a error if the condition is not true. #define LOG4CXX_ASSERT(logger, condition, message) do { \ if (!(condition) && logger->isErrorEnabled()) {\ ::log4cxx::helpers::MessageBuffer oss_; \ - logger->forcedLog(::log4cxx::Level::getError(), oss_.str(oss_ << message), LOG4CXX_LOCATION); }} while (0) + LOG4CXX_LOCATION_CREATE;\ + logger->forcedLog(::log4cxx::Level::getError(), oss_.str(oss_ << message), location); }} while (0) /** Logs a error if the condition is not true, formatting with libfmt @@ -1941,7 +1955,8 @@ Logs a error if the condition is not true, formatting with libfmt */ #define LOG4CXX_ASSERT_FMT(logger, condition, ...) do { \ if (!(condition) && logger->isErrorEnabled()) {\ - logger->forcedLog(::log4cxx::Level::getError(), fmt::format( __VA_ARGS__ ), LOG4CXX_LOCATION); }} while (0) + LOG4CXX_LOCATION_CREATE;\ + logger->forcedLog(::log4cxx::Level::getError(), fmt::format( __VA_ARGS__ ), location); }} while (0) #else #define LOG4CXX_ERROR(logger, message) @@ -1960,7 +1975,8 @@ Logs a message to a specified logger with the FATAL level. #define LOG4CXX_FATAL(logger, message) do { \ if (logger->isFatalEnabled()) {\ ::log4cxx::helpers::MessageBuffer oss_; \ - logger->forcedLog(::log4cxx::Level::getFatal(), oss_.str(oss_ << message), LOG4CXX_LOCATION); }} while (0) + LOG4CXX_LOCATION_CREATE;\ + logger->forcedLog(::log4cxx::Level::getFatal(), oss_.str(oss_ << message), location); }} while (0) /** Logs a message to a specified logger with the FATAL level, formatting with libfmt @@ -1970,7 +1986,8 @@ Logs a message to a specified logger with the FATAL level, formatting with libfm */ #define LOG4CXX_FATAL_FMT(logger, ...) do { \ if (logger->isFatalEnabled()) {\ - logger->forcedLog(::log4cxx::Level::getFatal(), fmt::format( __VA_ARGS__ ), LOG4CXX_LOCATION); }} while (0) + LOG4CXX_LOCATION_CREATE;\ + logger->forcedLog(::log4cxx::Level::getFatal(), fmt::format( __VA_ARGS__ ), location); }} while (0) #else #define LOG4CXX_FATAL(logger, message) #define LOG4CXX_FATAL_FMT(logger, ...) @@ -1985,7 +2002,8 @@ Logs a localized message with no parameter. */ #define LOG4CXX_L7DLOG(logger, level, key) do { \ if (logger->isEnabledFor(level)) {\ - logger->l7dlog(level, key, LOG4CXX_LOCATION); }} while (0) + LOG4CXX_LOCATION_CREATE;\ + logger->l7dlog(level, key, location); }} while (0) /** Logs a localized message with one parameter. @@ -1997,7 +2015,8 @@ Logs a localized message with one parameter. */ #define LOG4CXX_L7DLOG1(logger, level, key, p1) do { \ if (logger->isEnabledFor(level)) {\ - logger->l7dlog(level, key, LOG4CXX_LOCATION, p1); }} while (0) + LOG4CXX_LOCATION_CREATE;\ + logger->l7dlog(level, key, location, p1); }} while (0) /** Logs a localized message with two parameters. @@ -2010,7 +2029,8 @@ Logs a localized message with two parameters. */ #define LOG4CXX_L7DLOG2(logger, level, key, p1, p2) do { \ if (logger->isEnabledFor(level)) {\ - logger->l7dlog(level, key, LOG4CXX_LOCATION, p1, p2); }} while (0) + LOG4CXX_LOCATION_CREATE;\ + logger->l7dlog(level, key, location, p1, p2); }} while (0) /** Logs a localized message with three parameters. @@ -2024,7 +2044,8 @@ Logs a localized message with three parameters. */ #define LOG4CXX_L7DLOG3(logger, level, key, p1, p2, p3) do { \ if (logger->isEnabledFor(level)) {\ - logger->l7dlog(level, key, LOG4CXX_LOCATION, p1, p2, p3); }} while (0) + LOG4CXX_LOCATION_CREATE;\ + logger->l7dlog(level, key, location, p1, p2, p3); }} while (0) /**@}*/ diff --git a/src/main/include/log4cxx/spi/location/locationinfo.h b/src/main/include/log4cxx/spi/location/locationinfo.h index 1979f56..9346476 100644 --- a/src/main/include/log4cxx/spi/location/locationinfo.h +++ b/src/main/include/log4cxx/spi/location/locationinfo.h @@ -56,6 +56,11 @@ class LOG4CXX_EXPORT LocationInfo const char* const functionName, int lineNumber); + LocationInfo( const char* const fileName, + int shortFileNameOffset, + const char* const functionName, + int lineNumber); + /** * Default constructor. */ @@ -92,7 +97,7 @@ class LOG4CXX_EXPORT LocationInfo * Return the short file name of the caller. * @returns file name, may be null. */ - const std::string getShortFileName() const; + const char* getShortFileName() const; /** * Returns the line number of the caller. @@ -114,7 +119,7 @@ class LOG4CXX_EXPORT LocationInfo const char* fileName; /** Caller's short file name. */ - const std::string shortFileName; + const char* shortFileName; /** Caller's method name. */ const char* methodName; @@ -124,7 +129,7 @@ class LOG4CXX_EXPORT LocationInfo } } -#if !defined(LOG4CXX_LOCATION) +#if !defined(LOG4CXX_LOCATION) && !LOG4CXX_DISABLE_LOCATION_INFO #if defined(_MSC_VER) #if _MSC_VER >= 1300 #define __LOG4CXX_FUNC__ __FUNCSIG__ @@ -141,9 +146,33 @@ class LOG4CXX_EXPORT LocationInfo #if !defined(__LOG4CXX_FUNC__) #define __LOG4CXX_FUNC__ "" #endif + +#if __cpp_lib_string_view || (_MSVC_LANG >= 201703L) +#include <string_view> +#if defined(_WIN32) +#define LOG4CXX_SHORT_FILENAME_SPLIT_CHAR '\\' +#else +#define LOG4CXX_SHORT_FILENAME_SPLIT_CHAR '/' +#endif + +#define LOG4CXX_LOCATION_CREATE ::std::string_view file_name{__FILE__};\ + const int short_filename_offset = file_name.find_last_of(LOG4CXX_SHORT_FILENAME_SPLIT_CHAR) + 1;\ + ::log4cxx::spi::LocationInfo location(__FILE__, \ + short_filename_offset, \ + __LOG4CXX_FUNC__, \ + __LINE__) #define LOG4CXX_LOCATION ::log4cxx::spi::LocationInfo(__FILE__, \ __LOG4CXX_FUNC__, \ __LINE__) +#else +#define LOG4CXX_LOCATION ::log4cxx::spi::LocationInfo(__FILE__, \ + __LOG4CXX_FUNC__, \ + __LINE__) +#define LOG4CXX_LOCATION_CREATE ::log4cxx::spi::LocationInfo location = LOG4CXX_LOCATION #endif +#else +#define LOG4CXX_LOCATION ::log4cxx::spi::LocationInfo::getLocationUnavailable() +#define LOG4CXX_LOCATION_CREATE ::log4cxx::spi::LocationInfo location = LOG4CXX_LOCATION +#endif // LOG4CXX_LOCATION #endif //_LOG4CXX_SPI_LOCATION_LOCATIONINFO_H diff --git a/src/site/markdown/usage.md b/src/site/markdown/usage.md index 0536f83..232aa7c 100644 --- a/src/site/markdown/usage.md +++ b/src/site/markdown/usage.md @@ -765,6 +765,16 @@ The levels are set as follows: Note that this has no effect on other macros, such as using the `LOG4CXX_LOG`, `LOG4CXX_LOGLS`, or `LOG4CXX_L7DLOG` family of macros. +# Removing location information {#removing-location information} + +Whenever you log a message with Log4cxx, metadata about the location of the +logging statement is captured as well through the preprocessor. This includes +the file name, the method name, and the line number. If you would not like to +include this information in your build but you still wish to keep the log +statements, define `LOG4CXX_DISABLE_LOCATION_INFO` in your build system. This +will allow log messages to still be created, but the location information +will be invalid. + # Logging Custom Types {#custom-types} Often, the data that needs to be logged is not just standard data types diff --git a/src/test/cpp/throughput/CMakeLists.txt b/src/test/cpp/throughput/CMakeLists.txt index 154a6ae..3315484 100644 --- a/src/test/cpp/throughput/CMakeLists.txt +++ b/src/test/cpp/throughput/CMakeLists.txt @@ -38,3 +38,5 @@ endif( WIN32 ) target_compile_definitions(throughputtests PRIVATE ${LOG4CXX_COMPILE_DEFINITIONS} ${APR_COMPILE_DEFINITIONS} ${APR_UTIL_COMPILE_DEFINITIONS} ) target_include_directories(throughputtests PRIVATE ${CMAKE_CURRENT_LIST_DIR} $<TARGET_PROPERTY:log4cxx,INCLUDE_DIRECTORIES>) target_link_libraries(throughputtests PRIVATE log4cxx ${APR_LIBRARIES} ${APR_SYSTEM_LIBS} Threads::Threads fmt::fmt) + +add_custom_target(run-throughput COMMAND throughputtests DEPENDS throughputtests) diff --git a/src/test/resources/input/patternLayout14.properties b/src/test/resources/input/patternLayout14.properties index 9daf406..7c3c831 100644 --- a/src/test/resources/input/patternLayout14.properties +++ b/src/test/resources/input/patternLayout14.properties @@ -15,7 +15,7 @@ # log4j.rootCategory=DEBUG, testAppender log4j.appender.testAppender=org.apache.log4j.FileAppender -log4j.appender.testAppender.File=output/temp +log4j.appender.testAppender.File=output/patternlayout log4j.appender.testAppender.Append=false log4j.appender.testAppender.layout=org.apache.log4j.PatternLayout -log4j.appender.testAppender.layout.ConversionPattern=%-5p %f(%L): %m%n +log4j.appender.testAppender.layout.ConversionPattern=%-5p %f: %m%n diff --git a/src/test/resources/witness/patternLayout.14 b/src/test/resources/witness/patternLayout.14 index c431909..e398ba3 100644 --- a/src/test/resources/witness/patternLayout.14 +++ b/src/test/resources/witness/patternLayout.14 @@ -1,10 +1,10 @@ -DEBUG patternlayouttest.cpp(539): Message 0 -DEBUG patternlayouttest.cpp(540): Message 0 -INFO patternlayouttest.cpp(542): Message 1 -INFO patternlayouttest.cpp(543): Message 1 -WARN patternlayouttest.cpp(545): Message 2 -WARN patternlayouttest.cpp(546): Message 2 -ERROR patternlayouttest.cpp(548): Message 3 -ERROR patternlayouttest.cpp(549): Message 3 -FATAL patternlayouttest.cpp(551): Message 4 -FATAL patternlayouttest.cpp(552): Message 4 +DEBUG patternlayouttest.cpp: Message 0 +DEBUG patternlayouttest.cpp: Message 0 +INFO patternlayouttest.cpp: Message 1 +INFO patternlayouttest.cpp: Message 1 +WARN patternlayouttest.cpp: Message 2 +WARN patternlayouttest.cpp: Message 2 +ERROR patternlayouttest.cpp: Message 3 +ERROR patternlayouttest.cpp: Message 3 +FATAL patternlayouttest.cpp: Message 4 +FATAL patternlayouttest.cpp: Message 4
