From 4d26f13c264a9df6126c3394968d49cd50dc8c38 Mon Sep 17 00:00:00 2001 From: MeirGavish Date: Sun, 28 Jun 2026 06:54:47 +0300 Subject: [PATCH] Add logging macros that include the function name (#557) * Added function name logging to mgba_logger * Addendum to no-op defines * Fixed compile errors * Added "()" to function name in log to make it clearer it's a function * Corrected documentation * clang-format...? * Removed MGBA_LOG_LEVEL_MASK magic number * Added CHECK_NULL_ARG macros * clang-format...? * clang-format for real * Added documentation for CHECK_NULL_ARG_VOID and CHECK_NULL_ARG_RET * Fixed typo Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> * Fixed macro safety by wrappinh with do-while Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> * Fixed format string cut risk in mgba_func_printf * clang-format * clang-format some more * clang-format for real * Fixed DEBUG -> INFO in comment Co-authored-by: Rickey * Moved NULL-check macros to util.h and renamed them * Small additions - macro rename + clang-format * Truncate the string instead of the function name * Cleaned up code from previous commit * clang-format * Added mgba_logger_available check to mgba_func_printf * Updated documentation for latest change * Changed CHECK_NULL_ARG functions to generic RETURN_ON_ERROR_VAL functions. * clang-format * Changed back to specifically check NULL * clang-format * Changed "=" into "==" in the error message * clang-format * Fixed forgotten "==" --------- Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> Co-authored-by: Rickey --- include/mgba_logger.h | 33 ++++++++++++++++++++++++++++++- include/util.h | 46 +++++++++++++++++++++++++++++++++++++++++++ source/mgba_logger.c | 36 +++++++++++++++++++++++++++++---- 3 files changed, 110 insertions(+), 5 deletions(-) diff --git a/include/mgba_logger.h b/include/mgba_logger.h index 7e10194..8e6a9de 100644 --- a/include/mgba_logger.h +++ b/include/mgba_logger.h @@ -17,7 +17,7 @@ * mgba -l 14 game.rom # INFO, WARN, and ERROR * ``` * - * @note You have to fight with other logs in mgba and DEBUG can get messy. + * @note You have to fight with other logs in mgba and INFO can get messy. * * @note FATAL does kill the game. Use with care. */ @@ -54,19 +54,50 @@ bool mgba_logger_init(void); */ void mgba_printf(MgbaLogLevel level, const char* fmt, ...); +/** + * @brief Print to mgba log with a format string and function name + * + * @param level + * @param func_name Function name - prepended to the log string "(): " + * @param fmt Format string + * @param ... variadic arguments + * + * @note for all logs, it's cutoff at the hard mgba limit of 0x100 + */ +void mgba_func_printf(MgbaLogLevel level, const char* func_name, const char* fmt, ...); + // clang-format off #ifdef MGBA_LOGGING + #define MGBA_FATAL(...) mgba_printf(MGBA_LOG_FATAL, __VA_ARGS__) #define MGBA_ERROR(...) mgba_printf(MGBA_LOG_ERROR, __VA_ARGS__) #define MGBA_WARN(...) mgba_printf(MGBA_LOG_WARN, __VA_ARGS__) #define MGBA_INFO(...) mgba_printf(MGBA_LOG_INFO, __VA_ARGS__) #define MGBA_DEBUG(...) mgba_printf(MGBA_LOG_DEBUG, __VA_ARGS__) + +#define MGBA_FUNC_LOG(level, ...) mgba_func_printf(level, __func__, __VA_ARGS__) + +#define MGBA_FUNC_FATAL(...) MGBA_FUNC_LOG(MGBA_LOG_FATAL, __VA_ARGS__) +#define MGBA_FUNC_ERROR(...) MGBA_FUNC_LOG(MGBA_LOG_ERROR, __VA_ARGS__) +#define MGBA_FUNC_WARN(...) MGBA_FUNC_LOG(MGBA_LOG_WARN, __VA_ARGS__) +#define MGBA_FUNC_INFO(...) MGBA_FUNC_LOG(MGBA_LOG_INFO, __VA_ARGS__) +#define MGBA_FUNC_DEBUG(...) MGBA_FUNC_LOG(MGBA_LOG_DEBUG, __VA_ARGS__) + #else + #define MGBA_FATAL(...) ((void)0) #define MGBA_ERROR(...) ((void)0) #define MGBA_WARN(...) ((void)0) #define MGBA_INFO(...) ((void)0) #define MGBA_DEBUG(...) ((void)0) + +#define MGBA_FUNC_LOG(level, ...) ((void)0) + +#define MGBA_FUNC_FATAL(...) ((void)0) +#define MGBA_FUNC_ERROR(...) ((void)0) +#define MGBA_FUNC_WARN(...) ((void)0) +#define MGBA_FUNC_INFO(...) ((void)0) +#define MGBA_FUNC_DEBUG(...) ((void)0) #endif // clang-format on diff --git a/include/util.h b/include/util.h index 0ace660..9b7b9a3 100644 --- a/include/util.h +++ b/include/util.h @@ -8,6 +8,9 @@ #define UTIL_H #include +#ifdef MGBA_LOGGING +#include "mgba_logger.h" +#endif /** * @def GBAL_UNUSED @@ -56,6 +59,49 @@ // so it needs at least this number of chars to be able to display any suffixed number #define SUFFIXED_NUM_MIN_REQ_CHARS 4 +#ifdef MGBA_LOGGING +#define LOG_ERROR(...) MGBA_FUNC_ERROR(__VA_ARGS__) +#else +// TODO: Add a define to conditionally compile print error to console and add it to the tests? +#define LOG_ERROR(...) ((void)(0)) +#endif + +/** + * @brief Checks if @p param is NULL and prints error message and returns in case it is. + * Useful for checking arguments to a function or errors during control flow. + * + * This version is for a void function, while @ref GBAL_RETURN_ON_ERROR_VAL_RET is for one with + * a return value. + */ +#define GBAL_RETURN_IF_NULL_VOID(param) \ + do \ + { \ + if ((param) == NULL) \ + { \ + LOG_ERROR("Unexpected value: %s == NULL", #param); \ + return; \ + } \ + } while (0) + +/** + * @brief Checks if @p param is equal to NULL + * and prints error message and returns in case it is. + * Useful for checking arguments to a function or errors during control flow. + * @param ret_val The value to return in case @p param is equal to NULL. + * + * This version is for a function that returns a value while @ref GBAL_RETURN_ON_ERROR_VAL_VOID + * is for a void function. + */ +#define GBAL_RETURN_IF_NULL_RET(param, ret_val) \ + do \ + { \ + if ((param) == NULL) \ + { \ + LOG_ERROR("Unexpected value: %s == NULL", #param); \ + return (ret_val); \ + } \ + } while (0) + /** * @brief Avoid overflow when adding two u32 integers * diff --git a/source/mgba_logger.c b/source/mgba_logger.c index 572cb37..293d1b6 100644 --- a/source/mgba_logger.c +++ b/source/mgba_logger.c @@ -16,6 +16,7 @@ static const u32 MGBA_ENABLE_MAGIC = 0xC0DE; static const u32 MGBA_ENABLE_OK = 0x1DEA; static const u32 MGBA_LOG_SEND = 0x100; static const u32 MGBA_LOG_BUFFER_SIZE = 0x100; +static const u16 MGBA_LOG_LEVEL_MASK = 0x7; static bool mgba_logger_available = false; @@ -26,18 +27,42 @@ bool mgba_logger_init(void) return mgba_logger_available; } -void mgba_printf(MgbaLogLevel level, const char* fmt, ...) +static void mgba_vprintf(MgbaLogLevel level, const char* fmt, va_list args) { if (!mgba_logger_available || fmt == NULL) return; + vsnprintf(MGBA_REG_DEBUG_STRING, MGBA_LOG_BUFFER_SIZE, fmt, args); + + *MGBA_REG_DEBUG_FLAGS = ((uint16_t)level & MGBA_LOG_LEVEL_MASK) | MGBA_LOG_SEND; +} + +void mgba_printf(MgbaLogLevel level, const char* fmt, ...) +{ va_list args; va_start(args, fmt); - vsnprintf(MGBA_REG_DEBUG_STRING, MGBA_LOG_BUFFER_SIZE, fmt, args); + mgba_vprintf(level, fmt, args); va_end(args); - - *MGBA_REG_DEBUG_FLAGS = ((uint16_t)level & 0x7) | MGBA_LOG_SEND; } + +void mgba_func_printf(MgbaLogLevel level, const char* func_name, const char* fmt, ...) +{ + if (!mgba_logger_available || func_name == NULL || fmt == NULL) + { + // The one place where we can't log the error. + return; + } + + char printed_str_buff[MGBA_LOG_BUFFER_SIZE]; + + // Expand the format first so the full string is truncated in case it's too long + va_list args; + va_start(args, fmt); + vsnprintf(printed_str_buff, sizeof(printed_str_buff), fmt, args); + va_end(args); + mgba_printf(level, "%s(): %s", func_name, printed_str_buff); +} + #else // Noop stubs @@ -50,4 +75,7 @@ void mgba_printf(MgbaLogLevel level, const char* fmt, ...) { } +void mgba_func_printf(MgbaLogLevel level, const char* func_name, const char* fmt, ...) +{ +} #endif