Skip to content

fix logger when enabled thru ctl #1503

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Aug 22, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 4 additions & 2 deletions src/ctl/ctl.c
Original file line number Diff line number Diff line change
Expand Up @@ -198,11 +198,13 @@ static void *ctl_parse_args(const struct ctl_argument *arg_proto, char *arg) {
* structure as required by the node callback
*/
static void *ctl_query_get_real_args(const umf_ctl_node_t *n, void *write_arg,
umf_ctl_query_source_t source) {
umf_ctl_query_source_t source,
size_t *size) {
void *real_arg = NULL;
switch (source) {
case CTL_QUERY_CONFIG_INPUT:
real_arg = ctl_parse_args(n->arg, write_arg);
*size = n->arg->dest_size;
break;
case CTL_QUERY_PROGRAMMATIC:
real_arg = write_arg;
Expand Down Expand Up @@ -251,7 +253,7 @@ static umf_result_t ctl_exec_query_write(void *ctx, const umf_ctl_node_t *n,
return UMF_RESULT_ERROR_INVALID_ARGUMENT;
}

void *real_arg = ctl_query_get_real_args(n, arg, source);
void *real_arg = ctl_query_get_real_args(n, arg, source, &size);
if (real_arg == NULL) {
return UMF_RESULT_ERROR_INVALID_ARGUMENT;
}
Expand Down
51 changes: 32 additions & 19 deletions src/utils/utils_log.c
Original file line number Diff line number Diff line change
Expand Up @@ -66,11 +66,11 @@ typedef struct {
utils_log_level_t level;
utils_log_level_t flushLevel;
FILE *output;
const char *file_name;
char file_name[MAX_FILE_PATH];
} utils_log_config_t;

utils_log_config_t loggerConfig = {false, false, LOG_ERROR,
LOG_ERROR, NULL, NULL};
LOG_ERROR, NULL, ""};

static const char *level_to_str(utils_log_level_t l) {
switch (l) {
Expand Down Expand Up @@ -257,10 +257,10 @@ void utils_log_init(void) {
const char *arg;
if (utils_parse_var(envVar, "output:stdout", NULL)) {
loggerConfig.output = stdout;
loggerConfig.file_name = "stdout";
strncpy(loggerConfig.file_name, "stdout", MAX_FILE_PATH);
} else if (utils_parse_var(envVar, "output:stderr", NULL)) {
loggerConfig.output = stderr;
loggerConfig.file_name = "stderr";
strncpy(loggerConfig.file_name, "stderr", MAX_FILE_PATH);
} else if (utils_parse_var(envVar, "output:file", &arg)) {
loggerConfig.output = NULL;
const char *argEnd = strstr(arg, ";");
Expand Down Expand Up @@ -289,7 +289,9 @@ void utils_log_init(void) {
loggerConfig.output = NULL;
return;
}
loggerConfig.file_name = file;
strncpy(loggerConfig.file_name, file, MAX_FILE_PATH - 1);
loggerConfig.file_name[MAX_FILE_PATH - 1] =
'\0'; // ensure null-termination
} else {
loggerConfig.output = stderr;
LOG_ERR("Logging output not set - logging disabled (UMF_LOG = \"%s\")",
Expand Down Expand Up @@ -506,17 +508,29 @@ static umf_result_t CTL_READ_HANDLER(output)(void *ctx,
/* suppress unused-parameter errors */
(void)source, (void)indexes, (void)ctx;

const char **arg_out = (const char **)arg;
if (arg_out == NULL || size < sizeof(const char *)) {
char *arg_out = (char *)arg;
if (arg_out == NULL) {
return UMF_RESULT_ERROR_INVALID_ARGUMENT;
}

if (loggerConfig.output == NULL) {
*arg_out = "disabled";
const char disabled[] = "disabled";
if (size < sizeof(disabled)) {
LOG_ERR("Invalid output argument size: %zu, expected at least %zu",
size, sizeof(disabled));
return UMF_RESULT_ERROR_INVALID_ARGUMENT;
}

strncpy(arg_out, disabled, size);
return UMF_RESULT_SUCCESS;
}
if (size < strlen(loggerConfig.file_name)) {
LOG_ERR("Invalid output argument size: %zu, expected at least %zu",
size, strlen(loggerConfig.file_name));
return UMF_RESULT_ERROR_INVALID_ARGUMENT;
}

*arg_out = loggerConfig.file_name;
strncpy(arg_out, loggerConfig.file_name, size);
return UMF_RESULT_SUCCESS;
}

Expand All @@ -525,16 +539,13 @@ static umf_result_t CTL_WRITE_HANDLER(output)(void *ctx,
void *arg, size_t size,
umf_ctl_index_utlist_t *indexes) {
/* suppress unused-parameter errors */
(void)source, (void)indexes, (void)ctx;
(void)source, (void)indexes, (void)ctx, (void)size;

const char *arg_in = *(const char **)arg;
if (size < sizeof(const char *)) {
return UMF_RESULT_ERROR_INVALID_ARGUMENT;
}
const char *arg_in = (const char *)arg;

FILE *oldHandle = loggerConfig.output;
const char *oldName =
loggerConfig.file_name ? loggerConfig.file_name : "disabled";
*loggerConfig.file_name == '\0' ? loggerConfig.file_name : "disabled";

if (arg_in == NULL) {
if (loggerConfig.output) {
Expand All @@ -543,7 +554,7 @@ static umf_result_t CTL_WRITE_HANDLER(output)(void *ctx,
fclose(oldHandle);
}
loggerConfig.output = NULL;
loggerConfig.file_name = NULL;
loggerConfig.file_name[0] = '\0';
}
return UMF_RESULT_SUCCESS;
}
Expand All @@ -552,16 +563,18 @@ static umf_result_t CTL_WRITE_HANDLER(output)(void *ctx,

if (strcmp(arg_in, "stdout") == 0) {
newHandle = stdout;
loggerConfig.file_name = "stdout";
strncpy(loggerConfig.file_name, "stdout", MAX_FILE_PATH);
} else if (strcmp(arg_in, "stderr") == 0) {
newHandle = stderr;
loggerConfig.file_name = "stderr";
strncpy(loggerConfig.file_name, "stderr", MAX_FILE_PATH);
} else {
newHandle = fopen(arg_in, "a");
if (!newHandle) {
return UMF_RESULT_ERROR_INVALID_ARGUMENT;
}
loggerConfig.file_name = arg_in;
strncpy(loggerConfig.file_name, arg_in, MAX_FILE_PATH - 1);
loggerConfig.file_name[MAX_FILE_PATH - 1] =
'\0'; // ensure null-termination
}

loggerConfig.output = newHandle;
Expand Down
22 changes: 12 additions & 10 deletions test/ctl/ctl_api.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -414,21 +414,23 @@ TEST_F(test, ctl_logger_basic_rw) {
UMF_RESULT_SUCCESS);
EXPECT_EQ(flush_get, 2);

const char *out_name = "stdout";
ASSERT_EQ(umfCtlSet("umf.logger.output", &out_name, sizeof(out_name)),
UMF_RESULT_SUCCESS);
const char *out_get = NULL;
ASSERT_EQ(umfCtlGet("umf.logger.output", &out_get, sizeof(out_get)),
const char out_name[] = "stdout";
ASSERT_EQ(
umfCtlSet("umf.logger.output", (void *)out_name, sizeof(out_name)),
UMF_RESULT_SUCCESS);
const char out_get[256] = "";
ASSERT_EQ(umfCtlGet("umf.logger.output", (void *)out_get, sizeof(out_get)),
UMF_RESULT_SUCCESS);
EXPECT_STREQ(out_get, "stdout");
}

TEST_F(test, ctl_logger_output_file) {
const char *file_name = "ctl_log.txt";
ASSERT_EQ(umfCtlSet("umf.logger.output", &file_name, sizeof(file_name)),
UMF_RESULT_SUCCESS);
const char *out_get = NULL;
ASSERT_EQ(umfCtlGet("umf.logger.output", &out_get, sizeof(out_get)),
const char file_name[] = "ctl_log.txt";
ASSERT_EQ(
umfCtlSet("umf.logger.output", (void *)file_name, sizeof(file_name)),
UMF_RESULT_SUCCESS);
const char out_get[256] = "";
ASSERT_EQ(umfCtlGet("umf.logger.output", (void *)out_get, sizeof(out_get)),
UMF_RESULT_SUCCESS);
EXPECT_STREQ(out_get, file_name);
}
Expand Down
30 changes: 30 additions & 0 deletions test/ctl/ctl_env_app.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,32 @@ static int test_env_defaults(int argc, char **argv) {
return 0;
}

static int test_logger(int argc, char **argv) {
char buf[256] = {0};
int level = 0;

if (argc != 2) {
std::cerr << "expected two arguments" << std::endl;
std::cerr << "Usage: logger log_output log_level" << std::endl;
return 1;
}
umfCtlGet("umf.logger.output", buf, sizeof(buf));
if (strcmp(buf, argv[0]) != 0) {
std::cerr << "Expected log_output to be '" << argv[0] << "', but got '"
<< buf << "'" << std::endl;
return 1;
}

umfCtlGet("umf.logger.level", &level, sizeof(level));
if (level != atoi(argv[1])) {
std::cerr << "Expected log_level to be '" << argv[1] << "', but got '"
<< level << "'" << std::endl;
return 1;
}

return 0;
}

int main(int argc, char **argv) {
if (argc < 2) {
std::cerr << "Usage: " << argv[0] << " <test_name> args..."
Expand All @@ -53,5 +79,9 @@ int main(int argc, char **argv) {
if (strcmp(test_name, "env_defaults") == 0) {
return test_env_defaults(argc, argv);
}

if (strcmp(test_name, "logger") == 0) {
return test_logger(argc, argv);
}
return 1;
}
5 changes: 5 additions & 0 deletions test/ctl/ctl_env_driver.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -121,3 +121,8 @@ TEST_F(test, ctl_env_plus_file) {
"opt_two_value2", "umf.pool.default.test_pool.opt_three",
"second"});
}

TEST_F(test, ctl_env_logger) {
run_case({{"UMF_CONF", "umf.logger.output=stdout;umf.logger.level=0"}},
{"logger", "stdout", "0"});
}
48 changes: 30 additions & 18 deletions test/utils/utils_log.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -144,7 +144,8 @@ void helper_checkConfig(utils_log_config_t *expected, utils_log_config_t *is) {

TEST_F(test, parseEnv_errors) {
expected_message = "";
loggerConfig = {false, false, LOG_ERROR, LOG_ERROR, NULL, ""};
loggerConfig =
utils_log_config_t{false, false, LOG_ERROR, LOG_ERROR, NULL, ""};

expect_fput_count = 0;
expected_stream = stderr;
Expand Down Expand Up @@ -217,8 +218,8 @@ TEST_F(test, parseEnv) {
flushLevel.first + ";" +
output.first + ";" +
timestamp.first + ";" + pid.first;
b = loggerConfig = {false, false, LOG_ERROR,
LOG_ERROR, NULL, ""};
b = loggerConfig = utils_log_config_t{
false, false, LOG_ERROR, LOG_ERROR, NULL, ""};
expect_fput_count = 0;
expect_fopen_count = 0;
expected_stream = stderr;
Expand Down Expand Up @@ -290,8 +291,8 @@ TEST_F(test, log_levels) {
expected_stream = stderr;
for (int i = LOG_DEBUG; i <= LOG_ERROR; i++) {
for (int j = LOG_DEBUG; j <= LOG_ERROR; j++) {
loggerConfig = {false, false, (utils_log_level_t)i,
LOG_DEBUG, stderr, ""};
loggerConfig = utils_log_config_t{
false, false, (utils_log_level_t)i, LOG_DEBUG, stderr, ""};
if (i > j) {
expect_fput_count = 0;
expect_fflush_count = 0;
Expand All @@ -314,7 +315,8 @@ TEST_F(test, log_outputs) {
expect_fflush_count = 1;
expected_message = "[DEBUG UMF] " + MOCK_FN_NAME + ": example log\n";
for (auto o : outs) {
loggerConfig = {false, false, LOG_DEBUG, LOG_DEBUG, o, ""};
loggerConfig =
utils_log_config_t{false, false, LOG_DEBUG, LOG_DEBUG, o, ""};
expected_stream = o;
helper_test_log(LOG_DEBUG, MOCK_FN_NAME.c_str(), "%s", "example log");
}
Expand All @@ -325,8 +327,8 @@ TEST_F(test, flush_levels) {
expect_fput_count = 1;
for (int i = LOG_DEBUG; i <= LOG_ERROR; i++) {
for (int j = LOG_DEBUG; j <= LOG_ERROR; j++) {
loggerConfig = {false, false, LOG_DEBUG, (utils_log_level_t)i,
stderr, ""};
loggerConfig = utils_log_config_t{
false, false, LOG_DEBUG, (utils_log_level_t)i, stderr, ""};
if (i > j) {
expect_fflush_count = 0;
} else {
Expand All @@ -343,7 +345,8 @@ TEST_F(test, flush_levels) {
TEST_F(test, long_log) {
expect_fput_count = 1;
expect_fflush_count = 1;
loggerConfig = {false, false, LOG_DEBUG, LOG_DEBUG, stderr, ""};
loggerConfig =
utils_log_config_t{false, false, LOG_DEBUG, LOG_DEBUG, stderr, ""};
expected_message = "[DEBUG UMF] " + MOCK_FN_NAME + ": " +
std::string(8189 - MOCK_FN_NAME.size(), 'x') + "\n";
helper_test_log(LOG_DEBUG, MOCK_FN_NAME.c_str(), "%s",
Expand All @@ -358,7 +361,8 @@ TEST_F(test, long_log) {
TEST_F(test, timestamp_log) {
expect_fput_count = 1;
expect_fflush_count = 1;
loggerConfig = {true, false, LOG_DEBUG, LOG_DEBUG, stderr, ""};
loggerConfig =
utils_log_config_t{true, false, LOG_DEBUG, LOG_DEBUG, stderr, ""};
// TODO: for now we do not check output message,
// as it requires more sophisticated message validation (a.k.a regrex)
expected_message = "";
Expand All @@ -368,15 +372,17 @@ TEST_F(test, timestamp_log) {
TEST_F(test, pid_log) {
expect_fput_count = 1;
expect_fflush_count = 1;
loggerConfig = {false, true, LOG_DEBUG, LOG_DEBUG, stderr, ""};
loggerConfig =
utils_log_config_t{false, true, LOG_DEBUG, LOG_DEBUG, stderr, ""};
// TODO: for now we do not check output message,
// as it requires more sophisticated message validation (a.k.a regrex)
expected_message = "";
helper_test_log(LOG_DEBUG, MOCK_FN_NAME.c_str(), "%s", "example log");
}

TEST_F(test, log_fatal) {
loggerConfig = {false, false, LOG_DEBUG, LOG_DEBUG, NULL, ""};
loggerConfig =
utils_log_config_t{false, false, LOG_DEBUG, LOG_DEBUG, NULL, ""};
expected_stream = stderr;
expect_fput_count = 1;
expect_fflush_count = 1;
Expand All @@ -390,7 +396,8 @@ TEST_F(test, log_macros) {
expected_stream = stderr;
expect_fput_count = 1;
expect_fflush_count = 1;
loggerConfig = {false, false, LOG_DEBUG, LOG_DEBUG, stderr, ""};
loggerConfig =
utils_log_config_t{false, false, LOG_DEBUG, LOG_DEBUG, stderr, ""};

expected_message = "[DEBUG UMF] TestBody: example log\n";
fput_count = 0;
Expand Down Expand Up @@ -437,7 +444,8 @@ template <typename... Args> void helper_test_plog(Args... args) {
}

TEST_F(test, plog_basic) {
loggerConfig = {false, false, LOG_DEBUG, LOG_DEBUG, stderr, ""};
loggerConfig =
utils_log_config_t{false, false, LOG_DEBUG, LOG_DEBUG, stderr, ""};
expected_stream = stderr;
errno = 1;
strerr = "test error";
Expand All @@ -453,7 +461,8 @@ TEST_F(test, plog_basic) {
}

TEST_F(test, plog_invalid) {
loggerConfig = {false, false, LOG_DEBUG, LOG_DEBUG, stderr, ""};
loggerConfig =
utils_log_config_t{false, false, LOG_DEBUG, LOG_DEBUG, stderr, ""};
expected_stream = stderr;
errno = INVALID_ERRNO;
strerr = "test error";
Expand All @@ -469,7 +478,8 @@ TEST_F(test, plog_invalid) {
}

TEST_F(test, plog_long_message) {
loggerConfig = {false, false, LOG_DEBUG, LOG_DEBUG, stderr, ""};
loggerConfig =
utils_log_config_t{false, false, LOG_DEBUG, LOG_DEBUG, stderr, ""};
expected_stream = stderr;
expect_fput_count = 1;
expect_fflush_count = 1;
Expand All @@ -490,7 +500,8 @@ TEST_F(test, plog_long_message) {
}

TEST_F(test, plog_long_error) {
loggerConfig = {false, false, LOG_DEBUG, LOG_DEBUG, stderr, ""};
loggerConfig =
utils_log_config_t{false, false, LOG_DEBUG, LOG_DEBUG, stderr, ""};
expected_stream = stderr;
expect_fput_count = 1;
expect_fflush_count = 1;
Expand All @@ -516,7 +527,8 @@ TEST_F(test, log_pmacros) {
expected_stream = stderr;
expect_fput_count = 1;
expect_fflush_count = 1;
loggerConfig = {false, false, LOG_DEBUG, LOG_DEBUG, stderr, ""};
loggerConfig =
utils_log_config_t{false, false, LOG_DEBUG, LOG_DEBUG, stderr, ""};
errno = 1;
strerr = "test error";

Expand Down
Loading