diff --git a/src/ctl/ctl.c b/src/ctl/ctl.c index f33df885c..3dc899937 100644 --- a/src/ctl/ctl.c +++ b/src/ctl/ctl.c @@ -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; @@ -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; } diff --git a/src/utils/utils_log.c b/src/utils/utils_log.c index 3c57ce033..57ab0ef93 100644 --- a/src/utils/utils_log.c +++ b/src/utils/utils_log.c @@ -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) { @@ -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, ";"); @@ -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\")", @@ -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; } @@ -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) { @@ -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; } @@ -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; diff --git a/test/ctl/ctl_api.cpp b/test/ctl/ctl_api.cpp index 0037d3dbb..a3050645d 100644 --- a/test/ctl/ctl_api.cpp +++ b/test/ctl/ctl_api.cpp @@ -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); } diff --git a/test/ctl/ctl_env_app.cpp b/test/ctl/ctl_env_app.cpp index 45d5b7829..d40af3b27 100644 --- a/test/ctl/ctl_env_app.cpp +++ b/test/ctl/ctl_env_app.cpp @@ -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] << " args..." @@ -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; } diff --git a/test/ctl/ctl_env_driver.cpp b/test/ctl/ctl_env_driver.cpp index bcc795354..3bde34a75 100644 --- a/test/ctl/ctl_env_driver.cpp +++ b/test/ctl/ctl_env_driver.cpp @@ -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"}); +} diff --git a/test/utils/utils_log.cpp b/test/utils/utils_log.cpp index 4d1b0554e..16d035861 100644 --- a/test/utils/utils_log.cpp +++ b/test/utils/utils_log.cpp @@ -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; @@ -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; @@ -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; @@ -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"); } @@ -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 { @@ -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", @@ -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 = ""; @@ -368,7 +372,8 @@ 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 = ""; @@ -376,7 +381,8 @@ TEST_F(test, pid_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; @@ -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; @@ -437,7 +444,8 @@ template 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"; @@ -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"; @@ -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; @@ -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; @@ -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";