diff --git a/src/include/zf_internal/zf_log.h b/src/include/zf_internal/zf_log.h index 46d8b31..802607d 100644 --- a/src/include/zf_internal/zf_log.h +++ b/src/include/zf_internal/zf_log.h @@ -62,9 +62,11 @@ enum zf_log_comp { _Static_assert(ZF_LC_NUM_COMPONENTS * ZF_LL_NUM_LEVELS <= 64, "64-bit mask is not large enough for all components and levels"); +#define ZF_LOG_FILE_NAME_SIZE 256 extern uint64_t zf_log_level; extern int zf_log_format; +extern char zf_log_file_name[ZF_LOG_FILE_NAME_SIZE]; struct zf_stack; static constexpr zf_stack* NO_STACK = NULL; diff --git a/src/lib/zf/attr.c b/src/lib/zf/attr.c index 9ff9fb5..212edff 100644 --- a/src/lib/zf/attr.c +++ b/src/lib/zf/attr.c @@ -136,11 +136,6 @@ int zf_attr_alloc(struct zf_attr** attr_out) zf_attr_from_str(attr, default_attr_str) < 0 ) { return -EINVAL; } - - /* Set global attributes */ - zf_log_level = attr->log_level; - zf_log_format = attr->log_format; - return 0; } @@ -220,6 +215,8 @@ int zf_attr_set_int(struct zf_attr* attr, const char* name, int64_t val) } int* pi = (int*) get_field(attr, f); *pi = (int) val; + if( strcmp("log_format", f->name) == 0 ) + zf_log_format = (int)val; return 0; } case zf_attr_type_str: @@ -234,6 +231,8 @@ int zf_attr_set_int(struct zf_attr* attr, const char* name, int64_t val) { uint64_t* pb = (uint64_t*) get_field(attr, f); *pb = (uint64_t) val; + if( strcmp("log_level", f->name) == 0 ) + zf_log_level = (uint64_t)val; return 0; } default: @@ -255,10 +254,16 @@ int zf_attr_get_int(struct zf_attr* attr, const char* name, int64_t* val) return -ENOENT; switch( f->type ) { case zf_attr_type_int: - *val = *(int*)get_field(attr, f); + if( strcmp("log_format", f->name) == 0 ) + *val = zf_log_format; + else + *val = *(int*)get_field(attr, f); return 0; case zf_attr_type_bitmask: - *val = *(uint64_t*)get_field(attr, f); + if( strcmp("log_level", f->name) == 0 ) + *val = zf_log_level; + else + *val = *(uint64_t*)get_field(attr, f); return 0; default: return -EINVAL; @@ -279,12 +284,23 @@ int zf_attr_set_str(struct zf_attr* attr, const char* name, const char* val) char** p = get_field_str(attr, f); free(*p); if( val != NULL ) { + if( strcmp("log_file", f->name) == 0 ) { + int rc = zf_log_redirect(val); + if( rc < 0 ) { + zf_log_stack_err(NO_STACK, "%s: Failed to redirect logging: %s\n", + __func__, strerror(-rc)); + /* Failure here is non-fatal. */ + *p=NULL; + return rc; + } + } *p = strdup(val); if( *p == NULL ) return -ENOMEM; } else *p = NULL; + return 0; } return -ENOENT; @@ -300,7 +316,12 @@ int zf_attr_get_str(struct zf_attr* attr, const char* name, char** val) switch( f->type ) { case zf_attr_type_str: { char** p = get_field_str(attr, f); - if( *p != NULL ) { + if( strcmp("log_file", f->name) == 0 ) { + *val = strdup(zf_log_file_name); + if( *val == NULL ) + return -ENOMEM; + } + else if( *p != NULL ) { *val = strdup(*p); if( *val == NULL ) return -ENOMEM; diff --git a/src/lib/zf/doxygen/040_using.dox b/src/lib/zf/doxygen/040_using.dox index a0dad0e..4dbdbeb 100644 --- a/src/lib/zf/doxygen/040_using.dox +++ b/src/lib/zf/doxygen/040_using.dox @@ -113,8 +113,13 @@ production libraries: attribute, as described in the \ref attributes chapter. - As a convenience, the `-l` option to the `zf_debug` wrapper will set the \attrref{log_level} attribute to the specified value. -- Changing the \attrref{log_level} attribute while the application is running - has no effect. +- After library initialization, the \attrref{log_level} and \attrref{log_format} + attributes can be changed programmatically while the application is running using + \attrref{zf_attr_set_int}. Similarly, \attrref{log_file} can be modified + using \attrref{zf_attr_set_str}. +- \attrref{log_level}, \attrref{log_format}, and \attrref{log_file} are global + attributes and may be modified by any thread. Therefore, any user setting + values for these attributes from both threads must ensure thread safety. \section using_general General diff --git a/src/lib/zf/log.c b/src/lib/zf/log.c index 37c2aa3..31599b7 100644 --- a/src/lib/zf/log.c +++ b/src/lib/zf/log.c @@ -29,6 +29,7 @@ uint64_t zf_log_level = ZF_LCL_ALL_ERR; int zf_log_format; static FILE* zf_log_file; +char zf_log_file_name[ZF_LOG_FILE_NAME_SIZE] = "/dev/stderr"; static uint64_t zf_log_level_get() { @@ -135,14 +136,42 @@ void zf_log(struct zf_stack* st, const char* fmt, ...) va_end(va); } +static int zf_log_set_file_name(const char *file) +{ + if( strnlen(file, ZF_LOG_FILE_NAME_SIZE) > ZF_LOG_FILE_NAME_SIZE-1 ) { + zf_log_stack_err(NO_STACK, "%s: File name is longer than %i characters: %s\n", + __func__, ZF_LOG_FILE_NAME_SIZE-1, strerror(-EINVAL)); + return -EINVAL; + } + + strncpy(zf_log_file_name, file, sizeof(zf_log_file_name)); + + return 0; +} + int zf_log_redirect(const char* file) { - FILE* f = fopen(file, "a"); - if( ! f ) + char* previous_file_name = strdup(zf_log_file_name); + if( !previous_file_name ) return -errno; + int rc = zf_log_set_file_name(file); + if( rc < 0 ) { + free(previous_file_name); + return rc; + } + + FILE* f = fopen(zf_log_file_name, "a"); + if( !f ) { + zf_log_set_file_name(previous_file_name); + return -errno; + } + zf_log_stderr(); /* close any existing file */ zf_log_file = f; + + free(previous_file_name); + return fileno(f); } @@ -159,6 +188,7 @@ int zf_log_replace_stderr(const char* file) if( fd < 0 ) return fd; int rc = dup2(fd, STDERR_FILENO); + zf_log_set_file_name(file); close(fd); return rc; } diff --git a/src/lib/zf/zf.c b/src/lib/zf/zf.c index cf95260..c9b472c 100644 --- a/src/lib/zf/zf.c +++ b/src/lib/zf/zf.c @@ -57,10 +57,17 @@ int zf_init(void) return rc; } + /* Set global attributes */ + if( attr->log_level != ZF_LCL_ALL_ERR ) + zf_log_level = attr->log_level; + if( attr->log_format != ZF_LCL_ALL_ERR ) + zf_log_format = attr->log_format; + if( attr->log_to_kmsg ) rc = zf_log_replace_stderr("/dev/kmsg"); else if( attr->log_file ) rc = zf_log_redirect(attr->log_file); + if( rc < 0 ) { zf_log_stack_err(NO_STACK, "%s: Failed to redirect logging: %s\n", __func__, strerror(-rc)); diff --git a/src/tests/zf_unit/zfattrs.c b/src/tests/zf_unit/zfattrs.c index 1aad70e..634c7e7 100644 --- a/src/tests/zf_unit/zfattrs.c +++ b/src/tests/zf_unit/zfattrs.c @@ -13,9 +13,10 @@ struct attr_value { }; struct attr_value test_attrs[] = { - { "log_level", 0x7777 }, - { "log_format", 0 }, + { "log_level", 0xFFFF }, + { "log_format", 15 }, { "log_to_kmsg", 1 }, + { "log_file", 0 }, { "tcp_delayed_ack", 0 }, { "tcp_wait_for_time_wait", 1 }, { "tcp_timewait_ms", ZF_TCP_TIMEWAIT_TIME_MS / 2 }, @@ -34,12 +35,14 @@ struct attr_value test_attrs[] = { { "alt_count", 1 }, }; +char *tmp_log_dir; static int set_test_attrs(struct zf_attr* attr) { int num_attrs = sizeof(test_attrs) / sizeof(test_attrs[0]); unsigned int seed = zf_frc64(); int random = rand_r(&seed); + static int file_uid=0; /* Make sure the 32-bit random number will cover all attributes */ zf_assert_le(num_attrs, 32); @@ -55,7 +58,16 @@ static int set_test_attrs(struct zf_attr* attr) if( (random & (1 << i)) == 0 ) continue; - rc = zf_attr_set_int(attr, test_attrs[i].name, test_attrs[i].value); + if( strcmp("log_file", test_attrs[i].name)==0 ) { + char tmp_file[40]=""; + strncat(tmp_file, tmp_log_dir, sizeof(tmp_file)-1); + int slen=strlen(tmp_file); + snprintf(&tmp_file[slen],sizeof(tmp_file)-slen,"/file_%i.log",file_uid++); + rc = zf_attr_set_str(attr, test_attrs[i].name, tmp_file); + } + else + rc = zf_attr_set_int(attr, test_attrs[i].name, test_attrs[i].value); + if( rc != 0 ) return rc; } @@ -161,19 +173,87 @@ static void test(struct zf_stack* stack, struct zf_attr* attr, int main(void) { + char log_template_1[]="/tmp/logdir1XXXXXX"; + + tmp_log_dir = mkdtemp(log_template_1); + if( tmp_log_dir == NULL ) + return errno; + + char non_existent_dir[] = "/path/to/non-existent-dir"; + plan(TESTS_PER_ITER * ITERS); int rc = zf_init(); if( rc != 0 ) return rc; - for( unsigned int i = 0; i < ITERS; ++i ) { - struct zf_stack* stack; - struct zf_attr* attr; + struct zf_stack* stack1; + struct zf_attr* attr1, *attr2, *attr3; + int64_t log_level1, log_level2, log_level3; + int64_t log_format1, log_format2, log_format3; + char* log_file1_check=NULL, *log_file1=NULL, *log_file2=NULL, *log_file3=NULL; + + ZF_TRY(init(&stack1, &attr1)); + + ZF_TRY( zf_attr_get_str(attr1, "log_file", &log_file1) ); + ZF_TRY( zf_attr_get_int(attr1, "log_level", &log_level1) ); + ZF_TRY( zf_attr_get_int(attr1, "log_format", &log_format1) ); + + // test a non-existent path + char tmp_file[50]=""; + strncat(tmp_file, non_existent_dir, sizeof(tmp_file)-1); + int slen=strlen(tmp_file); + snprintf(&tmp_file[slen],sizeof(tmp_file)-slen,"/my_log_file.log"); + rc = zf_attr_set_str(attr1, "log_file", tmp_file); + cmp_ok(rc, "==", -2, "Expected behaviour 'No such file or directory' when providing a non-existent path"); + + ZF_TRY( zf_attr_get_str(attr1, "log_file", &log_file1_check) ); + cmp_ok(strcmp(log_file1, log_file1_check), "==", 0, "Expected behaviour after trying invalid path: log_file_name has not changed."); + + // Not changin existing global attributes when allocating new set of attributes + rc = zf_attr_alloc(&attr2); + if( rc != 0 ) + return rc; + + test(stack1, attr1, 1); + + ZF_TRY( zf_attr_get_str(attr2, "log_file", &log_file2) ); + ZF_TRY( zf_attr_get_int(attr2, "log_level", &log_level2) ); + ZF_TRY( zf_attr_get_int(attr2, "log_format", &log_format2) ); + + cmp_ok(strcmp(log_file1, log_file2), "==", 0, "Expected behaviour after allocating new set of attributes: log_file_name has not changed."); + cmp_ok(log_level1, "==", log_level2, "Expected behaviour after allocating new set of attributes: log_level has not changed."); + cmp_ok(log_format1, "==", log_format2, "Expected behaviour after allocating new set of attributes: log_format has not changed."); + + test(stack1, attr2, 2); + + attr3 = zf_attr_dup(attr1); + + ZF_TRY( zf_attr_get_str(attr3, "log_file", &log_file3) ); + ZF_TRY( zf_attr_get_int(attr3, "log_level", &log_level3) ); + ZF_TRY( zf_attr_get_int(attr3, "log_format", &log_format3) ); + + cmp_ok(strcmp(log_file1, log_file3), "==", 0, "Expected behaviour after duplicating attributes: log_file_name has not changed."); + cmp_ok(log_level1, "==", log_level3, "Expected behaviour after duplicating attributes: log_level has not changed."); + cmp_ok(log_format1, "==", log_format3, "Expected behaviour after duplicating attributes: log_format has not changed."); + + test(stack1, attr3, 3); + + ZF_TRY(fini(stack1, attr2)); + + zf_attr_free(attr1); + + zf_attr_free(attr3); + + free(log_file1); + free(log_file1_check); + free(log_file2); + free(log_file3); - ZF_TRY(init(&stack, &attr)); - test(stack, attr, i); - ZF_TRY(fini(stack, attr)); + for( unsigned int i = 4; i < ITERS; ++i ) { + ZF_TRY(init(&stack1, &attr1)); + test(stack1, attr1, i); + ZF_TRY(fini(stack1, attr1)); } zf_deinit();