From c7be8062154da62018b09207dc6986de5cda58e4 Mon Sep 17 00:00:00 2001 From: Vinayak Katoch Date: Wed, 22 Oct 2025 11:25:26 +0530 Subject: [PATCH] Refactor fastrpc_log lifecycle and improve runtime logging Initialize and deinitialize fastrpc_log during domain lifecycle instead of global init/deinit. Refactor HAP_debug_runtime for clarity and efficiency by adding early return when no logging is required, consolidating buffer allocation and message formatting, validating length, and ensuring deterministic cleanup. The changes also streamline logging logic by handling persist buffer for critical logs, enabling file logging only when configured, and invoking HAP_debug based on log mask, while reducing unnecessary heap allocations and improving control flow. Signed-off-by: Vinayak Katoch --- src/fastrpc_apps_user.c | 4 +-- src/fastrpc_log.c | 79 ++++++++++++++++++++++++----------------- 2 files changed, 49 insertions(+), 34 deletions(-) diff --git a/src/fastrpc_apps_user.c b/src/fastrpc_apps_user.c index 81fef0ab..d5a20a06 100644 --- a/src/fastrpc_apps_user.c +++ b/src/fastrpc_apps_user.c @@ -3219,6 +3219,7 @@ static void domain_deinit(int domain) { trace_marker_deinit(domain); deinitFileWatcher(domain); adspmsgd_stop(domain); + fastrpc_log_deinit(); fastrpc_mem_close(domain); apps_mem_deinit(domain); hlist[domain].state = 0; @@ -4000,6 +4001,7 @@ static int domain_init(int domain, int *dev) { } VERIFY(AEE_SUCCESS == (nErr = fastrpc_mem_open(domain))); VERIFY(AEE_SUCCESS == (nErr = apps_mem_init(domain))); + fastrpc_log_init(); if (dom == CDSP_DOMAIN_ID || dom == CDSP1_DOMAIN_ID || dom == GDSP0_DOMAIN_ID || dom == GDSP1_DOMAIN_ID) { panic_handle = get_adsp_current_process1_handle(domain); @@ -4111,7 +4113,6 @@ static void fastrpc_apps_user_deinit(void) { fastrpc_notif_deinit(); apps_mem_table_deinit(); fastrpc_wake_lock_deinit(); - fastrpc_log_deinit(); fastrpc_mem_deinit(); PL_DEINIT(apps_std); PL_DEINIT(rpcmem); @@ -4173,7 +4174,6 @@ static int fastrpc_apps_user_init(void) { VERIFY(AEE_SUCCESS == (nErr = PL_INIT(rpcmem))); fastrpc_mem_init(); fastrpc_context_table_init(); - fastrpc_log_init(); fastrpc_config_init(); pthread_mutex_init(&update_notif_list_mut, 0); VERIFYC(NULL != (hlist = calloc(NUM_DOMAINS_EXTEND, sizeof(*hlist))), diff --git a/src/fastrpc_log.c b/src/fastrpc_log.c index de773f4d..06067543 100644 --- a/src/fastrpc_log.c +++ b/src/fastrpc_log.c @@ -162,44 +162,59 @@ void HAP_debug_runtime(int level, const char *file, int line, const char *format, ...) { int len = 0; va_list argp; - char *buf = NULL, *log = NULL; - - /* - * Adding logs to persist buffer when level is set to - * RUNTIME_RPC_CRITICAL and fastrpc_log mask is disabled. - */ - if (((1 << level) & (fastrpc_logmask)) || - ((level == HAP_LEVEL_RPC_CRITICAL) && persist_buf.buf) || - log_userspace_file_fd != NULL) { - buf = (char *)malloc(sizeof(char) * MAX_FARF_LEN); - if (buf == NULL) { - return; - } - va_start(argp, format); - len = vsnprintf(buf, MAX_FARF_LEN, format, argp); - va_end(argp); - log = (char *)malloc(sizeof(char) * MAX_FARF_LEN); - if (log == NULL) { - return; - } - snprintf(log, MAX_FARF_LEN, "%d:%d:%s:%s:%d: %s", getpid(), gettid(), - __progname, file, line, buf); + char *buf = NULL; + const uint32_t level_mask = (1 << level); + const bool log_enabled = (level_mask & fastrpc_logmask); + const bool is_critical = (level == HAP_LEVEL_RPC_CRITICAL); + const bool need_persist = is_critical && persist_buf.buf; + const bool need_file_log = (log_userspace_file_fd != NULL); + + /* Early return if no logging is needed */ + if (!log_enabled && !need_persist && !need_file_log) { + return; } - print_dbgbuf_data(log, len); - if (((1 << level) & (fastrpc_logmask))) { - if (log_userspace_file_fd != NULL) { - fputs(log, log_userspace_file_fd); - fputs("\n", log_userspace_file_fd); - } - HAP_debug(buf, level, file, line); + /* Allocate buffer only when needed */ + buf = (char *)malloc(sizeof(char) * MAX_FARF_LEN); + if (buf == NULL) { + return; } - if (buf) { + + /* Format the message once */ + va_start(argp, format); + len = vsnprintf(buf, MAX_FARF_LEN, format, argp); + va_end(argp); + + /* Validate length */ + if (len <= 0 || len >= MAX_FARF_LEN) { free(buf); + return; } - if (log) { - free(log); + + /* Handle persist buffer for critical logs */ + if (need_persist || (log_enabled && IS_PERSIST_BUF_DATA(len, level))) { + print_dbgbuf_data(buf, len); + } + + /* Handle regular logging when enabled */ + if (log_enabled) { + /* File logging */ + if (need_file_log) { + char *filelog = (char *)malloc(sizeof(char) * MAX_FARF_LEN); + if (filelog) { + if (snprintf(filelog, MAX_FARF_LEN, "%d:%d:%s:%s:%d: %s", + getpid(), gettid(), __progname, file, line, buf) > 0) { + fputs(filelog, log_userspace_file_fd); + fputs("\n", log_userspace_file_fd); + } + free(filelog); + } + } + /* Debug output */ + HAP_debug(buf, level, file, line); } + + free(buf); } #ifdef __LE_TVM__