From 41119003d44da7d0ece2024edbbdc45a65da0343 Mon Sep 17 00:00:00 2001 From: Ozan Tezcan Date: Mon, 12 Jun 2023 23:18:13 +0300 Subject: [PATCH] sc_log: improvements (#123) - Make sc_log_term() safe to call twice - sc_log_set_file() parameters will be copied to an internal buffer. - comment --- logger/log_test.c | 70 +++++++++++++++++++++++++++++++++++++++++++++++ logger/sc_log.c | 30 ++++++++++++++------ logger/sc_log.h | 20 ++++++++++++-- 3 files changed, 108 insertions(+), 12 deletions(-) diff --git a/logger/log_test.c b/logger/log_test.c index f01fe8b..da203ff 100644 --- a/logger/log_test.c +++ b/logger/log_test.c @@ -23,6 +23,8 @@ void test1(void) { int count = 0; + sc_log_term(); + sc_log_init(); sc_log_set_callback(&count, callback); assert(sc_log_set_level("errrorr") == -1); @@ -96,6 +98,73 @@ void test1(void) sc_log_term(); } +void verify_size(const char* filename, size_t expected) +{ + FILE *fp = fopen(filename, "rb"); + assert(fp != NULL); + fseek(fp, 0, SEEK_END); + assert((size_t) ftell(fp) == expected); + fclose(fp); +} + +void test2(void) +{ + int rc; + + char buf1[256] = {0}; + char buf2[257] = {0}; + char buf3[200] = {0}; + + memset(buf1, 'a', sizeof(buf1) - 1); + memset(buf2, 'b', sizeof(buf2) - 1); + memset(buf3, 'c', sizeof(buf3) - 1); + + sc_log_init(); + + sc_log_set_file("prev1.txt", "current1.txt"); + sc_log_info("h1+"); + + rc = sc_log_set_file(buf1, buf3); + assert(rc != 0); + sc_log_info("nolog1"); + + rc = sc_log_set_file("prev1.txt", "current1.txt"); + assert(rc == 0); + sc_log_info("h2+"); + + rc = sc_log_set_file(buf3, buf1); + assert(rc != 0); + sc_log_info("nolog2"); + + rc = sc_log_set_file("prev1.txt", "current1.txt"); + assert(rc == 0); + sc_log_info("h3+"); + + rc = sc_log_term(); + assert(rc == 0); + + char tmp[1024] = {0}; + FILE *fp = fopen("current1.txt", "rb"); + fread(tmp, sizeof(tmp), 1, fp); + fclose(fp); + + assert(strstr(tmp, "h1+") != NULL); + assert(strstr(tmp, "h2+") != NULL); + assert(strstr(tmp, "h3+") != NULL); + assert(strstr(tmp, "nolog1") == NULL); + assert(strstr(tmp, "nolog2") == NULL); + + sc_log_init(); + rc = sc_log_set_file(buf2, buf3); + assert(rc != 0); + rc = sc_log_set_file(buf3, buf2); + assert(rc != 0); + rc = sc_log_term(); + assert(rc == 0); + rc = sc_log_term(); + assert(rc != 0); +} + #ifdef SC_HAVE_WRAP #include @@ -370,6 +439,7 @@ int main(void) fail_test(); example(); test1(); + test2(); return 0; } diff --git a/logger/sc_log.c b/logger/sc_log.c index bf46948..34c07ba 100644 --- a/logger/sc_log.c +++ b/logger/sc_log.c @@ -152,13 +152,13 @@ void sc_log_mutex_unlock(struct sc_log_mutex *mtx) struct sc_log { FILE *fp; - const char *current_file; - const char *prev_file; + char current_file[256]; + char prev_file[256]; size_t file_size; struct sc_log_mutex mtx; sc_atomic enum sc_log_level level; - + bool init; bool to_stdout; void *arg; @@ -181,6 +181,8 @@ int sc_log_init(void) errno = rc; } + sc_log.init = true; + return rc; } @@ -188,17 +190,19 @@ int sc_log_term(void) { int rc = 0; + if (!sc_log.init) { + return -1; + } + if (sc_log.fp) { rc = fclose(sc_log.fp); if (rc != 0) { rc = -1; } - sc_log_mutex_term(&sc_log.mtx); } - sc_log = (struct sc_log){ - .fp = NULL, - }; + sc_log_mutex_term(&sc_log.mtx); + sc_log = (struct sc_log){0}; return rc; } @@ -263,13 +267,21 @@ int sc_log_set_file(const char *prev, const char *current) sc_log.fp = NULL; } - sc_log.prev_file = prev; - sc_log.current_file = current; + sc_log.prev_file[0] = '\0'; + sc_log.current_file[0] = '\0'; if (prev == NULL || current == NULL) { goto out; } + if (strlen(prev) >= sizeof(sc_log.prev_file) - 1 || + strlen(current) >= sizeof(sc_log.current_file) - 1) { + goto error; + } + + memcpy(sc_log.prev_file, prev, strlen(prev) + 1); + memcpy(sc_log.current_file, current, strlen(current) + 1); + fp = fopen(sc_log.current_file, "a+"); if (fp == NULL) { goto error; diff --git a/logger/sc_log.h b/logger/sc_log.h index b2c6f49..ec59cb0 100644 --- a/logger/sc_log.h +++ b/logger/sc_log.h @@ -88,26 +88,38 @@ int sc_log_init(void); int sc_log_term(void); /** + * Set thread name. + * * Call once from each thread if you want to set thread name. * @param name Thread name */ void sc_log_set_thread_name(const char *name); /** + * Set log level. + * * @param level One of "DEBUG", "INFO", "WARN", "ERROR", "OFF" * @return '0' on success, negative value on invalid level string */ int sc_log_set_level(const char *level); /** + * Enable stdout logging. + * * @param enable 'true' to enable, 'false' to disable logging to stdout. */ void sc_log_set_stdout(bool enable); /** - * Log files will be rotated. Latest logs will always be in the 'current_file'. + * Enable file logging. + * + * Log files will be rotated to prevent generating very big files. Once current + * log file reaches 'SC_LOG_FILE_SIZE' (see definition above), it will be + * renamed as `prev` and the latest logs will always be in the 'current' file. + * * e.g., sc_log_set_file("/tmp/log.0.txt", "/tmp/log-latest.txt"); - * To disable logging into file : sc_log_set_file(NULL, NULL); + * + * To disable logging into file: sc_log_set_file(NULL, NULL); * * @param prev file path for previous log file, 'NULL' to disable * @param current file path for latest log file, 'NULL' to disable @@ -116,6 +128,8 @@ void sc_log_set_stdout(bool enable); int sc_log_set_file(const char *prev, const char *current); /** + * Enabled logging to callback. + * * @param arg user arg to callback. * @param cb log callback. */ @@ -123,7 +137,7 @@ void sc_log_set_callback(void *arg, int (*cb)(void *arg, enum sc_log_level level, const char *fmt, va_list va)); -// e.g., sc_log_error("Errno : %d, reason : %s", errno, strerror(errno)); +// e.g., sc_log_error("Errno: %d, reason: %s", errno, strerror(errno)); #define sc_log_debug(...) (sc_log_log(SC_LOG_DEBUG, sc_log_ap(__VA_ARGS__, ""))) #define sc_log_info(...) (sc_log_log(SC_LOG_INFO, sc_log_ap(__VA_ARGS__, ""))) #define sc_log_warn(...) (sc_log_log(SC_LOG_WARN, sc_log_ap(__VA_ARGS__, "")))