Skip to content

Commit 78b57dd

Browse files
lukaszstolarczuklukaszstolarczuk
authored
Merge pull request #1503 from lplewa/ctl_logger2
fix logger when enabled thru ctl
2 parents 6e154f9 + f13f95a commit 78b57dd

File tree

6 files changed

+113
-49
lines changed

6 files changed

+113
-49
lines changed

‎src/ctl/ctl.c‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -198,11 +198,13 @@ static void *ctl_parse_args(const struct ctl_argument *arg_proto, char *arg) {
198198
* structure as required by the node callback
199199
*/
200200
static void *ctl_query_get_real_args(const umf_ctl_node_t *n, void *write_arg,
201-
umf_ctl_query_source_t source) {
201+
umf_ctl_query_source_t source,
202+
size_t *size) {
202203
void *real_arg = NULL;
203204
switch (source) {
204205
case CTL_QUERY_CONFIG_INPUT:
205206
real_arg = ctl_parse_args(n->arg, write_arg);
207+
*size = n->arg->dest_size;
206208
break;
207209
case CTL_QUERY_PROGRAMMATIC:
208210
real_arg = write_arg;
@@ -251,7 +253,7 @@ static umf_result_t ctl_exec_query_write(void *ctx, const umf_ctl_node_t *n,
251253
return UMF_RESULT_ERROR_INVALID_ARGUMENT;
252254
}
253255

254-
void *real_arg = ctl_query_get_real_args(n, arg, source);
256+
void *real_arg = ctl_query_get_real_args(n, arg, source, &size);
255257
if (real_arg == NULL) {
256258
return UMF_RESULT_ERROR_INVALID_ARGUMENT;
257259
}

‎src/utils/utils_log.c‎

Lines changed: 32 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -66,11 +66,11 @@ typedef struct {
6666
utils_log_level_t level;
6767
utils_log_level_t flushLevel;
6868
FILE *output;
69-
const char *file_name;
69+
char file_name[MAX_FILE_PATH];
7070
} utils_log_config_t;
7171

7272
utils_log_config_t loggerConfig = {false, false, LOG_ERROR,
73-
LOG_ERROR, NULL, NULL};
73+
LOG_ERROR, NULL, ""};
7474

7575
static const char *level_to_str(utils_log_level_t l) {
7676
switch (l) {
@@ -257,10 +257,10 @@ void utils_log_init(void) {
257257
const char *arg;
258258
if (utils_parse_var(envVar, "output:stdout", NULL)) {
259259
loggerConfig.output = stdout;
260-
loggerConfig.file_name = "stdout";
260+
strncpy(loggerConfig.file_name, "stdout", MAX_FILE_PATH);
261261
} else if (utils_parse_var(envVar, "output:stderr", NULL)) {
262262
loggerConfig.output = stderr;
263-
loggerConfig.file_name = "stderr";
263+
strncpy(loggerConfig.file_name, "stderr", MAX_FILE_PATH);
264264
} else if (utils_parse_var(envVar, "output:file", &arg)) {
265265
loggerConfig.output = NULL;
266266
const char *argEnd = strstr(arg, ";");
@@ -289,7 +289,9 @@ void utils_log_init(void) {
289289
loggerConfig.output = NULL;
290290
return;
291291
}
292-
loggerConfig.file_name = file;
292+
strncpy(loggerConfig.file_name, file, MAX_FILE_PATH - 1);
293+
loggerConfig.file_name[MAX_FILE_PATH - 1] =
294+
'\0'; // ensure null-termination
293295
} else {
294296
loggerConfig.output = stderr;
295297
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,
506508
/* suppress unused-parameter errors */
507509
(void)source, (void)indexes, (void)ctx;
508510

509-
const char **arg_out = (const char **)arg;
510-
if (arg_out == NULL || size < sizeof(const char *)) {
511+
char *arg_out = (char *)arg;
512+
if (arg_out == NULL) {
511513
return UMF_RESULT_ERROR_INVALID_ARGUMENT;
512514
}
513515

514516
if (loggerConfig.output == NULL) {
515-
*arg_out = "disabled";
517+
const char disabled[] = "disabled";
518+
if (size < sizeof(disabled)) {
519+
LOG_ERR("Invalid output argument size: %zu, expected at least %zu",
520+
size, sizeof(disabled));
521+
return UMF_RESULT_ERROR_INVALID_ARGUMENT;
522+
}
523+
524+
strncpy(arg_out, disabled, size);
516525
return UMF_RESULT_SUCCESS;
517526
}
527+
if (size < strlen(loggerConfig.file_name)) {
528+
LOG_ERR("Invalid output argument size: %zu, expected at least %zu",
529+
size, strlen(loggerConfig.file_name));
530+
return UMF_RESULT_ERROR_INVALID_ARGUMENT;
531+
}
518532

519-
*arg_out = loggerConfig.file_name;
533+
strncpy(arg_out, loggerConfig.file_name, size);
520534
return UMF_RESULT_SUCCESS;
521535
}
522536

@@ -525,16 +539,13 @@ static umf_result_t CTL_WRITE_HANDLER(output)(void *ctx,
525539
void *arg, size_t size,
526540
umf_ctl_index_utlist_t *indexes) {
527541
/* suppress unused-parameter errors */
528-
(void)source, (void)indexes, (void)ctx;
542+
(void)source, (void)indexes, (void)ctx, (void)size;
529543

530-
const char *arg_in = *(const char **)arg;
531-
if (size < sizeof(const char *)) {
532-
return UMF_RESULT_ERROR_INVALID_ARGUMENT;
533-
}
544+
const char *arg_in = (const char *)arg;
534545

535546
FILE *oldHandle = loggerConfig.output;
536547
const char *oldName =
537-
loggerConfig.file_name ? loggerConfig.file_name : "disabled";
548+
*loggerConfig.file_name == '\0' ? loggerConfig.file_name : "disabled";
538549

539550
if (arg_in == NULL) {
540551
if (loggerConfig.output) {
@@ -543,7 +554,7 @@ static umf_result_t CTL_WRITE_HANDLER(output)(void *ctx,
543554
fclose(oldHandle);
544555
}
545556
loggerConfig.output = NULL;
546-
loggerConfig.file_name = NULL;
557+
loggerConfig.file_name[0] = '\0';
547558
}
548559
return UMF_RESULT_SUCCESS;
549560
}
@@ -552,16 +563,18 @@ static umf_result_t CTL_WRITE_HANDLER(output)(void *ctx,
552563

553564
if (strcmp(arg_in, "stdout") == 0) {
554565
newHandle = stdout;
555-
loggerConfig.file_name = "stdout";
566+
strncpy(loggerConfig.file_name, "stdout", MAX_FILE_PATH);
556567
} else if (strcmp(arg_in, "stderr") == 0) {
557568
newHandle = stderr;
558-
loggerConfig.file_name = "stderr";
569+
strncpy(loggerConfig.file_name, "stderr", MAX_FILE_PATH);
559570
} else {
560571
newHandle = fopen(arg_in, "a");
561572
if (!newHandle) {
562573
return UMF_RESULT_ERROR_INVALID_ARGUMENT;
563574
}
564-
loggerConfig.file_name = arg_in;
575+
strncpy(loggerConfig.file_name, arg_in, MAX_FILE_PATH - 1);
576+
loggerConfig.file_name[MAX_FILE_PATH - 1] =
577+
'\0'; // ensure null-termination
565578
}
566579

567580
loggerConfig.output = newHandle;

‎test/ctl/ctl_api.cpp‎

Lines changed: 12 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -414,21 +414,23 @@ TEST_F(test, ctl_logger_basic_rw) {
414414
UMF_RESULT_SUCCESS);
415415
EXPECT_EQ(flush_get, 2);
416416

417-
const char *out_name = "stdout";
418-
ASSERT_EQ(umfCtlSet("umf.logger.output", &out_name, sizeof(out_name)),
419-
UMF_RESULT_SUCCESS);
420-
const char *out_get = NULL;
421-
ASSERT_EQ(umfCtlGet("umf.logger.output", &out_get, sizeof(out_get)),
417+
const char out_name[] = "stdout";
418+
ASSERT_EQ(
419+
umfCtlSet("umf.logger.output", (void *)out_name, sizeof(out_name)),
420+
UMF_RESULT_SUCCESS);
421+
const char out_get[256] = "";
422+
ASSERT_EQ(umfCtlGet("umf.logger.output", (void *)out_get, sizeof(out_get)),
422423
UMF_RESULT_SUCCESS);
423424
EXPECT_STREQ(out_get, "stdout");
424425
}
425426

426427
TEST_F(test, ctl_logger_output_file) {
427-
const char *file_name = "ctl_log.txt";
428-
ASSERT_EQ(umfCtlSet("umf.logger.output", &file_name, sizeof(file_name)),
429-
UMF_RESULT_SUCCESS);
430-
const char *out_get = NULL;
431-
ASSERT_EQ(umfCtlGet("umf.logger.output", &out_get, sizeof(out_get)),
428+
const char file_name[] = "ctl_log.txt";
429+
ASSERT_EQ(
430+
umfCtlSet("umf.logger.output", (void *)file_name, sizeof(file_name)),
431+
UMF_RESULT_SUCCESS);
432+
const char out_get[256] = "";
433+
ASSERT_EQ(umfCtlGet("umf.logger.output", (void *)out_get, sizeof(out_get)),
432434
UMF_RESULT_SUCCESS);
433435
EXPECT_STREQ(out_get, file_name);
434436
}

‎test/ctl/ctl_env_app.cpp‎

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,32 @@ static int test_env_defaults(int argc, char **argv) {
4141
return 0;
4242
}
4343

44+
static int test_logger(int argc, char **argv) {
45+
char buf[256] = {0};
46+
int level = 0;
47+
48+
if (argc != 2) {
49+
std::cerr << "expected two arguments" << std::endl;
50+
std::cerr << "Usage: logger log_output log_level" << std::endl;
51+
return 1;
52+
}
53+
umfCtlGet("umf.logger.output", buf, sizeof(buf));
54+
if (strcmp(buf, argv[0]) != 0) {
55+
std::cerr << "Expected log_output to be '" << argv[0] << "', but got '"
56+
<< buf << "'" << std::endl;
57+
return 1;
58+
}
59+
60+
umfCtlGet("umf.logger.level", &level, sizeof(level));
61+
if (level != atoi(argv[1])) {
62+
std::cerr << "Expected log_level to be '" << argv[1] << "', but got '"
63+
<< level << "'" << std::endl;
64+
return 1;
65+
}
66+
67+
return 0;
68+
}
69+
4470
int main(int argc, char **argv) {
4571
if (argc < 2) {
4672
std::cerr << "Usage: " << argv[0] << " <test_name> args..."
@@ -53,5 +79,9 @@ int main(int argc, char **argv) {
5379
if (strcmp(test_name, "env_defaults") == 0) {
5480
return test_env_defaults(argc, argv);
5581
}
82+
83+
if (strcmp(test_name, "logger") == 0) {
84+
return test_logger(argc, argv);
85+
}
5686
return 1;
5787
}

‎test/ctl/ctl_env_driver.cpp‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -121,3 +121,8 @@ TEST_F(test, ctl_env_plus_file) {
121121
"opt_two_value2", "umf.pool.default.test_pool.opt_three",
122122
"second"});
123123
}
124+
125+
TEST_F(test, ctl_env_logger) {
126+
run_case({{"UMF_CONF", "umf.logger.output=stdout;umf.logger.level=0"}},
127+
{"logger", "stdout", "0"});
128+
}

‎test/utils/utils_log.cpp‎

Lines changed: 30 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -144,7 +144,8 @@ void helper_checkConfig(utils_log_config_t *expected, utils_log_config_t *is) {
144144

145145
TEST_F(test, parseEnv_errors) {
146146
expected_message = "";
147-
loggerConfig = {false, false, LOG_ERROR, LOG_ERROR, NULL, ""};
147+
loggerConfig =
148+
utils_log_config_t{false, false, LOG_ERROR, LOG_ERROR, NULL, ""};
148149

149150
expect_fput_count = 0;
150151
expected_stream = stderr;
@@ -217,8 +218,8 @@ TEST_F(test, parseEnv) {
217218
flushLevel.first + ";" +
218219
output.first + ";" +
219220
timestamp.first + ";" + pid.first;
220-
b = loggerConfig = {false, false, LOG_ERROR,
221-
LOG_ERROR, NULL, ""};
221+
b = loggerConfig = utils_log_config_t{
222+
false, false, LOG_ERROR, LOG_ERROR, NULL, ""};
222223
expect_fput_count = 0;
223224
expect_fopen_count = 0;
224225
expected_stream = stderr;
@@ -290,8 +291,8 @@ TEST_F(test, log_levels) {
290291
expected_stream = stderr;
291292
for (int i = LOG_DEBUG; i <= LOG_ERROR; i++) {
292293
for (int j = LOG_DEBUG; j <= LOG_ERROR; j++) {
293-
loggerConfig = {false, false, (utils_log_level_t)i,
294-
LOG_DEBUG, stderr, ""};
294+
loggerConfig = utils_log_config_t{
295+
false, false, (utils_log_level_t)i, LOG_DEBUG, stderr, ""};
295296
if (i > j) {
296297
expect_fput_count = 0;
297298
expect_fflush_count = 0;
@@ -314,7 +315,8 @@ TEST_F(test, log_outputs) {
314315
expect_fflush_count = 1;
315316
expected_message = "[DEBUG UMF] " + MOCK_FN_NAME + ": example log\n";
316317
for (auto o : outs) {
317-
loggerConfig = {false, false, LOG_DEBUG, LOG_DEBUG, o, ""};
318+
loggerConfig =
319+
utils_log_config_t{false, false, LOG_DEBUG, LOG_DEBUG, o, ""};
318320
expected_stream = o;
319321
helper_test_log(LOG_DEBUG, MOCK_FN_NAME.c_str(), "%s", "example log");
320322
}
@@ -325,8 +327,8 @@ TEST_F(test, flush_levels) {
325327
expect_fput_count = 1;
326328
for (int i = LOG_DEBUG; i <= LOG_ERROR; i++) {
327329
for (int j = LOG_DEBUG; j <= LOG_ERROR; j++) {
328-
loggerConfig = {false, false, LOG_DEBUG, (utils_log_level_t)i,
329-
stderr, ""};
330+
loggerConfig = utils_log_config_t{
331+
false, false, LOG_DEBUG, (utils_log_level_t)i, stderr, ""};
330332
if (i > j) {
331333
expect_fflush_count = 0;
332334
} else {
@@ -343,7 +345,8 @@ TEST_F(test, flush_levels) {
343345
TEST_F(test, long_log) {
344346
expect_fput_count = 1;
345347
expect_fflush_count = 1;
346-
loggerConfig = {false, false, LOG_DEBUG, LOG_DEBUG, stderr, ""};
348+
loggerConfig =
349+
utils_log_config_t{false, false, LOG_DEBUG, LOG_DEBUG, stderr, ""};
347350
expected_message = "[DEBUG UMF] " + MOCK_FN_NAME + ": " +
348351
std::string(8189 - MOCK_FN_NAME.size(), 'x') + "\n";
349352
helper_test_log(LOG_DEBUG, MOCK_FN_NAME.c_str(), "%s",
@@ -358,7 +361,8 @@ TEST_F(test, long_log) {
358361
TEST_F(test, timestamp_log) {
359362
expect_fput_count = 1;
360363
expect_fflush_count = 1;
361-
loggerConfig = {true, false, LOG_DEBUG, LOG_DEBUG, stderr, ""};
364+
loggerConfig =
365+
utils_log_config_t{true, false, LOG_DEBUG, LOG_DEBUG, stderr, ""};
362366
// TODO: for now we do not check output message,
363367
// as it requires more sophisticated message validation (a.k.a regrex)
364368
expected_message = "";
@@ -368,15 +372,17 @@ TEST_F(test, timestamp_log) {
368372
TEST_F(test, pid_log) {
369373
expect_fput_count = 1;
370374
expect_fflush_count = 1;
371-
loggerConfig = {false, true, LOG_DEBUG, LOG_DEBUG, stderr, ""};
375+
loggerConfig =
376+
utils_log_config_t{false, true, LOG_DEBUG, LOG_DEBUG, stderr, ""};
372377
// TODO: for now we do not check output message,
373378
// as it requires more sophisticated message validation (a.k.a regrex)
374379
expected_message = "";
375380
helper_test_log(LOG_DEBUG, MOCK_FN_NAME.c_str(), "%s", "example log");
376381
}
377382

378383
TEST_F(test, log_fatal) {
379-
loggerConfig = {false, false, LOG_DEBUG, LOG_DEBUG, NULL, ""};
384+
loggerConfig =
385+
utils_log_config_t{false, false, LOG_DEBUG, LOG_DEBUG, NULL, ""};
380386
expected_stream = stderr;
381387
expect_fput_count = 1;
382388
expect_fflush_count = 1;
@@ -390,7 +396,8 @@ TEST_F(test, log_macros) {
390396
expected_stream = stderr;
391397
expect_fput_count = 1;
392398
expect_fflush_count = 1;
393-
loggerConfig = {false, false, LOG_DEBUG, LOG_DEBUG, stderr, ""};
399+
loggerConfig =
400+
utils_log_config_t{false, false, LOG_DEBUG, LOG_DEBUG, stderr, ""};
394401

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

439446
TEST_F(test, plog_basic) {
440-
loggerConfig = {false, false, LOG_DEBUG, LOG_DEBUG, stderr, ""};
447+
loggerConfig =
448+
utils_log_config_t{false, false, LOG_DEBUG, LOG_DEBUG, stderr, ""};
441449
expected_stream = stderr;
442450
errno = 1;
443451
strerr = "test error";
@@ -453,7 +461,8 @@ TEST_F(test, plog_basic) {
453461
}
454462

455463
TEST_F(test, plog_invalid) {
456-
loggerConfig = {false, false, LOG_DEBUG, LOG_DEBUG, stderr, ""};
464+
loggerConfig =
465+
utils_log_config_t{false, false, LOG_DEBUG, LOG_DEBUG, stderr, ""};
457466
expected_stream = stderr;
458467
errno = INVALID_ERRNO;
459468
strerr = "test error";
@@ -469,7 +478,8 @@ TEST_F(test, plog_invalid) {
469478
}
470479

471480
TEST_F(test, plog_long_message) {
472-
loggerConfig = {false, false, LOG_DEBUG, LOG_DEBUG, stderr, ""};
481+
loggerConfig =
482+
utils_log_config_t{false, false, LOG_DEBUG, LOG_DEBUG, stderr, ""};
473483
expected_stream = stderr;
474484
expect_fput_count = 1;
475485
expect_fflush_count = 1;
@@ -490,7 +500,8 @@ TEST_F(test, plog_long_message) {
490500
}
491501

492502
TEST_F(test, plog_long_error) {
493-
loggerConfig = {false, false, LOG_DEBUG, LOG_DEBUG, stderr, ""};
503+
loggerConfig =
504+
utils_log_config_t{false, false, LOG_DEBUG, LOG_DEBUG, stderr, ""};
494505
expected_stream = stderr;
495506
expect_fput_count = 1;
496507
expect_fflush_count = 1;
@@ -516,7 +527,8 @@ TEST_F(test, log_pmacros) {
516527
expected_stream = stderr;
517528
expect_fput_count = 1;
518529
expect_fflush_count = 1;
519-
loggerConfig = {false, false, LOG_DEBUG, LOG_DEBUG, stderr, ""};
530+
loggerConfig =
531+
utils_log_config_t{false, false, LOG_DEBUG, LOG_DEBUG, stderr, ""};
520532
errno = 1;
521533
strerr = "test error";
522534

0 commit comments

Comments
 (0)