RDKEMW-14300 : Move the log4c init to logger init#55
Conversation
There was a problem hiding this comment.
Pull request overview
This PR refactors the RDK logger initialization by moving log4c initialization from an automatically executed constructor function to an explicit call within rdk_logger_init(). This change makes the initialization sequence more predictable and controllable. Additionally, it improves error message formatting by adding missing newline characters to stderr fprintf statements, and adjusts test timing to better capture CPU metrics.
Changes:
- Removed
__attribute__((constructor))function that automatically initialized log4c at program startup - Moved
rdk_dbg_priv_init()call to be explicitly invoked withinrdk_logger_init() - Added missing newline characters to three fprintf error messages for consistency
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| src/rdk_logger_init.c | Removed constructor function and added explicit call to rdk_dbg_priv_init() within rdk_logger_init() |
| src/rdk_debug_priv.c | Added newline characters to error messages for consistent formatting |
| test/rdk_logger_cpu_test.c | Increased usleep delay from 1ms to 10ms for more stable CPU metric collection |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Reason for change: Move the log4c init to logger init Test Procedure: Verify logging Risks: Medium Signed-off-by: Karunakaran A <karunakaran_amirthalingam@cable.comcast.com>
karuna2git
force-pushed
the
topic/reviewLog4c
branch
from
42b43f1 to
6b708ca
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| rdk_Error rdk_logger_init(const char* debugConfigFile) | ||
| { | ||
| rdk_Error ret = RDK_SUCCESS; | ||
| pthread_mutex_lock(&gInitMutex); | ||
| if (!isLogInited) | ||
| { | ||
| if (NULL == debugConfigFile) | ||
| { | ||
| debugConfigFile = DEBUG_CONF_FILE; | ||
| } | ||
|
|
||
| rdk_dbg_priv_init(); | ||
| /* Perform Logger Internal Init */ | ||
| ret = rdk_dbg_priv_config(debugConfigFile); | ||
|
|
There was a problem hiding this comment.
Removing the shared-library constructor means log4c/rdk internal initialization now only happens if callers explicitly invoke rdk_logger_init (or RDK_LOGGER_INIT). Any existing consumers that call rdk_logger_msg_printf/rdk_logger_is_logLevel_enabled without first initializing will now run log4c APIs before log4c_init(), which is a behavior change and can break logging at runtime. Consider keeping a safe lazy-init path (e.g., ensure rdk_dbg_priv_init runs before any log4c usage) or clearly enforcing/validating initialization and returning an error when logging is used pre-init.
Reason for change: Move the log4c init to logger init
Test Procedure: Verify logging
Risks: Medium