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

@@ -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 <errno.h>
@@ -370,6 +439,7 @@ int main(void)
 	fail_test();
 	example();
 	test1();
+	test2();
 
 	return 0;
 }

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;

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__, "")))