From a4f3471707368fd13db0f587da07c8ff3039b6b2 Mon Sep 17 00:00:00 2001 From: Alexandre Quesnel <131881+aquesnel@users.noreply.github.com> Date: Wed, 23 Dec 2020 02:57:18 +0000 Subject: [PATCH] Fixing code location log level filtering --- common/log.c | 83 ++++++++++++++++++++++++++----------- common/log.h | 115 ++++++++++++++++++++++++++++----------------------- 2 files changed, 123 insertions(+), 75 deletions(-) diff --git a/common/log.c b/common/log.c index ef462f84..c7747397 100644 --- a/common/log.c +++ b/common/log.c @@ -523,27 +523,53 @@ internal_log_config_copy(struct log_config *dest, const struct log_config *src) bool_t internal_log_is_enabled_for_level(const enum logLevels log_level, - const bool_t override_destination_level) + const bool_t override_destination_level, + const enum logLevels override_log_level) { /* Is log initialized? */ if (g_staticLogConfig == NULL) { return 0; } - - /* Is there at least one log destination which will accept the message based on the log level? */ - return (g_staticLogConfig->fd >= 0 - && (override_destination_level || log_level <= g_staticLogConfig->log_level)) - || (g_staticLogConfig->enable_syslog - && (override_destination_level || log_level <= g_staticLogConfig->syslog_level)) - || (g_staticLogConfig->enable_console - && (override_destination_level || log_level <= g_staticLogConfig->console_level)); + else if (g_staticLogConfig->fd < 0 + && !g_staticLogConfig->enable_syslog + && !g_staticLogConfig->enable_console) + { + /* all logging outputs are disabled */ + return 0; + } + else if (override_destination_level) + { + /* Override is enabled - should the message should be logged? */ + return log_level <= override_log_level; + } + /* Override is disabled - Is there at least one log destination + * which will accept the message based on the log level? */ + else if (g_staticLogConfig->fd >= 0 + && log_level <= g_staticLogConfig->log_level) + { + return 1; + } + else if (g_staticLogConfig->enable_syslog + && log_level <= g_staticLogConfig->syslog_level) + { + return 1; + } + else if (g_staticLogConfig->enable_console + && log_level <= g_staticLogConfig->console_level) + { + return 1; + } + else + { + return 0; + } } bool_t internal_log_location_overrides_level(const char *function_name, const char *file_name, - const enum logLevels log_level) + enum logLevels *log_level_return) { struct log_logger_level *logger = NULL; int i; @@ -561,7 +587,8 @@ internal_log_location_overrides_level(const char *function_name, || (logger->logger_type == LOG_TYPE_FUNCTION && 0 == g_strncmp(logger->logger_name, function_name, LOGGER_NAME_SIZE))) { - return (log_level <= logger->log_level); + *log_level_return = logger->log_level; + return 1; } } @@ -756,6 +783,7 @@ log_hexdump_with_location(const char *function_name, int offset; char *dump_buffer; enum logReturns rv; + enum logLevels override_log_level; bool_t override_destination_level = 0; /* Start the dump on a new line so that the first line of the dump is @@ -780,8 +808,8 @@ log_hexdump_with_location(const char *function_name, override_destination_level = internal_log_location_overrides_level( function_name, file_name, - log_level); - if (!internal_log_is_enabled_for_level(log_level, override_destination_level)) + &override_log_level); + if (!internal_log_is_enabled_for_level(log_level, override_destination_level, override_log_level)) { return LOG_STARTUP_OK; } @@ -901,21 +929,24 @@ log_message_with_location(const char *function_name, va_list ap; enum logReturns rv; char buff[LOG_BUFFER_SIZE]; + enum logLevels override_log_level = LOG_LEVEL_NEVER; bool_t override_destination_level = 0; if (g_staticLogConfig == NULL) { g_writeln("The log reference is NULL - log not initialized properly " "when called from [%s(%s:%d)]", - function_name, file_name, line_number); + (function_name != NULL ? function_name : "unknown_function"), + (file_name != NULL ? file_name : "unknown_file"), + line_number); return LOG_ERROR_NO_CFG; } override_destination_level = internal_log_location_overrides_level( function_name, file_name, - level); - if (!internal_log_is_enabled_for_level(level, override_destination_level)) + &override_log_level); + if (!internal_log_is_enabled_for_level(level, override_destination_level, override_log_level)) { return LOG_STARTUP_OK; } @@ -924,7 +955,7 @@ log_message_with_location(const char *function_name, function_name, file_name, line_number, msg); va_start(ap, msg); - rv = internal_log_message(level, override_destination_level, buff, ap); + rv = internal_log_message(level, override_destination_level, override_log_level, buff, ap); va_end(ap); return rv; } @@ -936,14 +967,15 @@ log_message(const enum logLevels lvl, const char *msg, ...) enum logReturns rv; va_start(ap, msg); - rv = internal_log_message(lvl, 0, msg, ap); + rv = internal_log_message(lvl, 0, LOG_LEVEL_NEVER, msg, ap); va_end(ap); return rv; } enum logReturns internal_log_message(const enum logLevels lvl, - bool_t override_destination_level, + const bool_t override_destination_level, + const enum logLevels override_log_level, const char *msg, va_list ap) { @@ -967,7 +999,7 @@ internal_log_message(const enum logLevels lvl, return LOG_ERROR_FILE_NOT_OPEN; } - if (!internal_log_is_enabled_for_level(lvl, override_destination_level)) + if (!internal_log_is_enabled_for_level(lvl, override_destination_level, override_log_level)) { return LOG_STARTUP_OK; } @@ -1009,20 +1041,25 @@ internal_log_message(const enum logLevels lvl, #endif #endif - if (g_staticLogConfig->enable_syslog && (override_destination_level || lvl <= g_staticLogConfig->syslog_level)) + if (g_staticLogConfig->enable_syslog + && ((override_destination_level && lvl <= override_log_level) + || (!override_destination_level && lvl <= g_staticLogConfig->syslog_level))) { /* log to syslog*/ /* %s fix compiler warning 'not a string literal' */ syslog(internal_log_xrdp2syslog(lvl), "%s", buff + 20); } - if (g_staticLogConfig->enable_console && (override_destination_level || lvl <= g_staticLogConfig->console_level)) + if (g_staticLogConfig->enable_console + && ((override_destination_level && lvl <= override_log_level) + || (!override_destination_level && lvl <= g_staticLogConfig->console_level))) { /* log to console */ g_printf("%s", buff); } - if (override_destination_level || lvl <= g_staticLogConfig->log_level) + if ((override_destination_level && lvl <= override_log_level) + || (!override_destination_level && lvl <= g_staticLogConfig->log_level)) { /* log to application logfile */ if (g_staticLogConfig->fd >= 0) diff --git a/common/log.h b/common/log.h index 8d237435..f9304fcf 100644 --- a/common/log.h +++ b/common/log.h @@ -36,7 +36,8 @@ enum logLevels LOG_LEVEL_WARNING, /* for describing recoverable error states in a request or method */ LOG_LEVEL_INFO, /* for low verbosity and high level descriptions of normal operations */ LOG_LEVEL_DEBUG, /* for medium verbosity and low level descriptions of normal operations */ - LOG_LEVEL_TRACE /* for high verbosity and low level descriptions of normal operations (eg. method or wire tracing) */ + LOG_LEVEL_TRACE, /* for high verbosity and low level descriptions of normal operations (eg. method or wire tracing) */ + LOG_LEVEL_NEVER, }; /* startup return values */ @@ -69,53 +70,53 @@ enum logReturns #define LOG_PER_LOGGER_LEVEL /** - * @brief Logging macro for messages that are for an XRDP developper to + * @brief Logging macro for messages that are for an XRDP developer to * understand and debug XRDP code. - * - * Note: all log levels are relavant to help a developper understand XRDP at + * + * Note: all log levels are relavant to help a developer understand XRDP at * different levels of granularity. - * + * * Note: the logging function calls are removed when XRDP_DEBUG is NOT defined. - * - * Note: when the build is configured with --enable-xrdpdebug, then - * the log level can be configured per the source file name or method name + * + * Note: when the build is configured with --enable-xrdpdebug, then + * the log level can be configured per the source file name or method name * (with the suffix "()") in the [LoggingPerLogger] * section of the configuration file. - * + * * For example: - * ``` + * ``` * [LoggingPerLogger] * xrdp.c=DEBUG * main()=WARNING * ``` - * + * * @param lvl, the log level * @param msg, the log text as a printf format c-string * @param ... the arguments for the printf format c-string */ #define LOG_DEVEL(log_level, args...) \ - log_message_with_location(__func__, __FILE__, __LINE__, log_level, args); + log_message_with_location(__func__, __FILE__, __LINE__, log_level, args); /** * @brief Logging macro for messages that are for a systeam administrator to * configure and run XRDP on their machine. - * - * Note: the logging function calls contain additional code location info when + * + * Note: the logging function calls contain additional code location info when * XRDP_DEBUG is defined. - * + * * @param lvl, the log level * @param msg, the log text as a printf format c-string * @param ... the arguments for the printf format c-string */ #define LOG(log_level, args...) \ - log_message_with_location(__func__, __FILE__, __LINE__, log_level, args); + log_message_with_location(__func__, __FILE__, __LINE__, log_level, args); /** - * @brief Logging macro for logging the contents of a byte array using a hex + * @brief Logging macro for logging the contents of a byte array using a hex * dump format. - * + * * Note: the logging function calls are removed when XRDP_DEBUG is NOT defined. - * + * * @param log_level, the log level * @param message, a message prefix for the hex dump. Note: no printf like * formatting is done to this message. @@ -123,12 +124,13 @@ enum logReturns * @param length, the length of the byte array to log */ #define LOG_DEVEL_HEXDUMP(log_level, message, buffer, length) \ - log_hexdump_with_location(__func__, __FILE__, __LINE__, log_level, message, buffer, length); + log_hexdump_with_location(__func__, __FILE__, __LINE__, log_level, message, buffer, length); #else #define LOG_DEVEL(log_level, args...) #define LOG(log_level, args...) log_message(log_level, args); #define LOG_DEVEL_HEXDUMP(log_level, message, buffer, length) + #endif enum log_logger_type @@ -206,7 +208,7 @@ internal_log_text2level(const char *buf); * also init its content. * @return LOG_STARTUP_OK or LOG_ERROR_MALLOC */ -struct log_config* +struct log_config * internalInitAndAllocStruct(void); /** @@ -218,32 +220,41 @@ internal_log_config_dump(struct log_config *config); /** * the log function that all files use to log an event. * @param lvl, the loglevel + * @param override_destination_level, if true then the destination log level is not used + * @param override_log_level, the loglevel instead of the destination log level if override_destination_level is true * @param msg, the logtext. - * @param ... + * @param ap, the values for the message format arguments * @return */ enum logReturns -internal_log_message(const enum logLevels lvl, bool_t force_log, const char *msg, va_list args); +internal_log_message(const enum logLevels lvl, + const bool_t override_destination_level, + const enum logLevels override_log_level, + const char *msg, + va_list ap); /** * @param log_level, the log level - * @param override_destination_level, if true then the destinatino log level is ignored. + * @param override_destination_level, if true then the destination log level is ignored. + * @param override_log_level, the log level to use instead of the destination log level + * if override_destination_level is true * @return true if at least one log destination will accept a message logged at the given level. */ bool_t internal_log_is_enabled_for_level(const enum logLevels log_level, - const bool_t override_destination_level); - + const bool_t override_destination_level, + const enum logLevels override_log_level); + /** * @param function_name, the function name (typicaly the __func__ macro) * @param file_name, the file name (typicaly the __FILE__ macro) - * @param log_level, the log level + * @param[out] log_level_return, the log level to use instead of the destination log level * @return true if the logger location overrides the destination log levels */ bool_t -internal_log_location_overrides_level(const char *function_name, +internal_log_location_overrides_level(const char *function_name, const char *file_name, - const enum logLevels log_level); + enum logLevels *log_level_return); /*End of internal functions*/ #endif @@ -262,8 +273,8 @@ log_start(const char *iniFile, const char *applicationName); * An alternative log_start where the caller gives the params directly. * @param config * @return - * - * @post to avoid memory leaks, the config argument must be free'ed using + * + * @post to avoid memory leaks, the config argument must be free'ed using * `log_config_free()` */ enum logReturns @@ -283,27 +294,27 @@ log_start_from_param(const struct log_config *src_log_config); * * @return pointer to struct log_config. */ -struct log_config* +struct log_config * log_config_init_for_console(enum logLevels lvl, const char *override_name); /** - * Read configuration from a file and store the values in the returned + * Read configuration from a file and store the values in the returned * log_config. * @param file * @param applicationName, the application name used in the log events. * @param section_prefix, prefix for the logging sections to parse * @return */ -struct log_config* -log_config_init_from_config(const char *iniFilename, - const char *applicationName, +struct log_config * +log_config_init_from_config(const char *iniFilename, + const char *applicationName, const char *section_prefix); /** * Free the memory for the log_config struct. */ enum logReturns -log_config_free(struct log_config* config); +log_config_free(struct log_config *config); /** * Function that terminates all logging @@ -314,9 +325,9 @@ log_end(void); /** * the log function that all files use to log an event. - * + * * Please prefer to use the LOG and LOG_DEVEL macros instead of this function directly. - * + * * @param lvl, the loglevel * @param msg, the logtext. * @param ... @@ -326,11 +337,11 @@ enum logReturns log_message(const enum logLevels lvl, const char *msg, ...) printflike(2, 3); /** - * the log function that all files use to log an event, + * the log function that all files use to log an event, * with the function name and file line. * * Please prefer to use the LOG and LOG_DEVEL macros instead of this function directly. - * + * * @param function_name, the function name (typicaly the __func__ macro) * @param file_name, the file name (typicaly the __FILE__ macro) * @param line_number, the line number in the file (typicaly the __LINE__ macro) @@ -340,20 +351,20 @@ log_message(const enum logLevels lvl, const char *msg, ...) printflike(2, 3); * @return */ enum logReturns -log_message_with_location(const char *function_name, - const char *file_name, - const int line_number, - const enum logLevels lvl, - const char *msg, +log_message_with_location(const char *function_name, + const char *file_name, + const int line_number, + const enum logLevels lvl, + const char *msg, ...) printflike(5, 6); enum logReturns -log_hexdump_with_location(const char *function_name, - const char *file_name, - const int line_number, - const enum logLevels log_level, - const char *msg, - const char *p, +log_hexdump_with_location(const char *function_name, + const char *file_name, + const int line_number, + const enum logLevels log_level, + const char *msg, + const char *p, int len); /**