From 643093d7e0ebe98466e9b8375dfd1a372a8e1fd4 Mon Sep 17 00:00:00 2001 From: Aidan Hahn Date: Mon, 28 Feb 2022 01:14:56 -0800 Subject: [PATCH 01/10] add unique names so that makefiles including these dont have to account for conflict Signed-off-by: Aidan Hahn --- Makefile | 4 ++-- tests/tests.mk | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/Makefile b/Makefile index 59e630d..25df43e 100644 --- a/Makefile +++ b/Makefile @@ -19,7 +19,7 @@ ALOG_LIB = $(TARGET_DIR)/alog.so include tests/tests.mk .PHONY: so -so: $(if $(shell stat $(ALOG_LIB)), clean) $(ALOG_LIB) +alog-so: $(if $(shell stat $(ALOG_LIB)), clean) $(ALOG_LIB) $(ALOG_LIB): $(TARGET_DIR) $(ALOG_OBJ) $(CC) $(LDFLAGS) -o $(ALOG_LIB) $(ALOG_OBJ) @@ -34,5 +34,5 @@ $(TARGET_DIR): mkdir $(TARGET_DIR) .PHONY: clean -clean: +alog-clean: rm $(ALOG_LIB) $(ALOG_OBJ) diff --git a/tests/tests.mk b/tests/tests.mk index 9cc1397..ef5360f 100644 --- a/tests/tests.mk +++ b/tests/tests.mk @@ -6,7 +6,7 @@ $(ALOG_TESTS): $(ALOG_LIB) $(CC) -o $(TARGET_DIR)/$@ $(BUILD_DIR)/$@.o $(ALOG_LIB) chmod +x $(TARGET_DIR)/$@ -run-tests: $(ALOG_TESTS) +alog-tests: $(ALOG_TESTS) for test in $^ ; do \ $(TARGET_DIR)/$$test ; \ done From 033f1d51732f77301f8abf30c24a7bf12869bf0f Mon Sep 17 00:00:00 2001 From: Aidan Hahn Date: Tue, 1 Mar 2022 23:21:43 -0800 Subject: [PATCH 02/10] add gitlab CI Signed-off-by: Aidan Hahn --- .gitlab-ci.yml | 33 +++++++++++++++++++++++++++++++++ 1 file changed, 33 insertions(+) create mode 100644 .gitlab-ci.yml diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml new file mode 100644 index 0000000..73a6738 --- /dev/null +++ b/.gitlab-ci.yml @@ -0,0 +1,33 @@ +image: debian:latest + +before_script: + - apt update + - apt install -y make + - apt install -y gcc + - export CC=$(which gcc) + +stages: + - build + - test + +build: + stage: build + script: + - echo "compiling with $CC" + - make alog-so + artifacts: + paths: + - target/alog.so + expire_in: 1 week + +test: + stage: test + script: + - make alog-tests + +valgrind: + stage: test + script: + - apt install -y valgrind + - make log_test + - valgrind -s --leak-check=full --show-leak-kinds=all --error-exitcode=1 target/log_test From 8dca2bac04f51d65c44b89e957343794a8b847f0 Mon Sep 17 00:00:00 2001 From: Aidan Hahn Date: Sun, 6 Mar 2022 23:39:55 -0800 Subject: [PATCH 03/10] use semaphore to remove race con in tests Signed-off-by: Aidan Hahn --- tests/log_test.c | 28 ++++++++++++++++++++++------ tests/tests.mk | 2 +- 2 files changed, 23 insertions(+), 7 deletions(-) diff --git a/tests/log_test.c b/tests/log_test.c index d73a223..37039ec 100644 --- a/tests/log_test.c +++ b/tests/log_test.c @@ -4,14 +4,24 @@ #include #include #include +#include +#include int out_fd[2]; int err_fd[2]; const char *out_message = "standard message"; const char *err_message = "error message"; +const char *sem_name = "/alog_testing_sem"; int main() { + // RPC sync mechanism. Start locked + sem_t *sem = sem_open(sem_name, O_CREAT, 0x777, 0); + if (sem_unlink(sem_name)) { + perror("couldnt unlink semaphore"); + // might as well proceed since it is a leak anyways + } + // make RPC pipe pipe(out_fd); pipe(err_fd); @@ -23,7 +33,9 @@ int main() { pid_t childpid; if ((childpid = fork()) == -1) { - perror("fork fail"); + perror("cannot fork"); + sem_close(sem); + deinit_alog(); return 1; } @@ -40,14 +52,18 @@ int main() { alog(ERROR, err_message); alog(PRINT, out_message); - exit(0); + if (sem_post(sem)) { + perror("cannot increment semaphore"); + return 1; + } // parent process } else { // we can do checks from here char out_read_buffer[128] = {0}; char err_read_buffer[128] = {0}; - - usleep(10); + if (sem_wait(sem)) { + perror("cannot decrement semaphore"); + } read(err_fd[0], err_read_buffer, 128); // cant check read len because of timestamp @@ -75,8 +91,8 @@ int main() { printf("out and err log printing are successfull.\n"); + sem_close(sem); + deinit_alog(); } - - deinit_alog(); return 0; } diff --git a/tests/tests.mk b/tests/tests.mk index ef5360f..7681cee 100644 --- a/tests/tests.mk +++ b/tests/tests.mk @@ -3,7 +3,7 @@ ALOG_TESTS = $(ALOG_TEST_SRCS:.c=) $(ALOG_TESTS): $(ALOG_LIB) $(CC) $(CFLAGS) -g -o $(BUILD_DIR)/$@.o -c tests/$@.c - $(CC) -o $(TARGET_DIR)/$@ $(BUILD_DIR)/$@.o $(ALOG_LIB) + $(CC) -o $(TARGET_DIR)/$@ $(BUILD_DIR)/$@.o $(ALOG_LIB) -lpthread chmod +x $(TARGET_DIR)/$@ alog-tests: $(ALOG_TESTS) From b7bedc76ff6ede5f1f0dedb61656b970906f90eb Mon Sep 17 00:00:00 2001 From: Aidan Hahn Date: Sun, 6 Mar 2022 23:46:03 -0800 Subject: [PATCH 04/10] fix heap leaks in test app Signed-off-by: Aidan Hahn --- tests/log_test.c | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/tests/log_test.c b/tests/log_test.c index 37039ec..fe96bfd 100644 --- a/tests/log_test.c +++ b/tests/log_test.c @@ -56,6 +56,11 @@ int main() { perror("cannot increment semaphore"); return 1; } + + // clean up + sem_close(sem); + deinit_alog(); + // parent process } else { // we can do checks from here From 08d387cae9460676327c19c65328b0d72f6ebe9c Mon Sep 17 00:00:00 2001 From: Aidan Hahn Date: Mon, 7 Mar 2022 00:15:11 -0800 Subject: [PATCH 05/10] reduce bad reads Signed-off-by: Aidan Hahn --- alog.c | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/alog.c b/alog.c index 9abf360..95ae87b 100644 --- a/alog.c +++ b/alog.c @@ -154,10 +154,9 @@ void alog ( // GET TIMESTAMP time_t t = time(NULL); struct tm * p = localtime(&t); - int timestamp_size = strftime(NULL, 0, "[%c]", p); - char *timestamp = malloc(sizeof(char) * (timestamp_size+1)); - strftime(timestamp, timestamp_size, "[%c]", p); - char *msg_and_timestamp = malloc(sizeof(char) * (timestamp_size + strlen(message) + 3)); + char *timestamp = malloc(strftime(NULL, 0, "[%c]", p) + 1); + strftime(timestamp, sizeof(timestamp), "[%c]", p); + char *msg_and_timestamp = malloc(sizeof(timestamp) + 1 + (strlen(message) + 1) + 2); sprintf(msg_and_timestamp, "%s %s\n", timestamp, message); free(timestamp); size = snprintf(NULL, 0, msg_and_timestamp, fmt_list); From ca1e2e3453bbfce94189bb0adb457f768e6bacc1 Mon Sep 17 00:00:00 2001 From: Aidan Hahn Date: Mon, 7 Mar 2022 00:43:37 -0800 Subject: [PATCH 06/10] portable makefiles Signed-off-by: Aidan Hahn --- Makefile | 7 ++++--- tests/tests.mk | 5 +++-- 2 files changed, 7 insertions(+), 5 deletions(-) diff --git a/Makefile b/Makefile index 25df43e..6ac8dd0 100644 --- a/Makefile +++ b/Makefile @@ -12,11 +12,12 @@ ifdef ALOG_DEBUG CFLAGS += -g endif -ALOG_SRC = alog.c +ALOG_TOP_DIR := $(dir $(lastword $(MAKEFILE_LIST))) +ALOG_SRC = $(ALOG_TOP_DIR)alog.c ALOG_OBJ = $(BUILD_DIR)/alog.o ALOG_LIB = $(TARGET_DIR)/alog.so -include tests/tests.mk +include $(dir $(lastword $(MAKEFILE_LIST)))tests/tests.mk .PHONY: so alog-so: $(if $(shell stat $(ALOG_LIB)), clean) $(ALOG_LIB) @@ -25,7 +26,7 @@ $(ALOG_LIB): $(TARGET_DIR) $(ALOG_OBJ) $(CC) $(LDFLAGS) -o $(ALOG_LIB) $(ALOG_OBJ) $(ALOG_OBJ): $(BUILD_DIR) - $(CC) $(CFLAGS) -fPIC -c alog.c -o $(ALOG_OBJ) + $(CC) $(CFLAGS) -fPIC -c $(ALOG_SRC) -o $(ALOG_OBJ) $(BUILD_DIR): mkdir $(BUILD_DIR) diff --git a/tests/tests.mk b/tests/tests.mk index 7681cee..f10e804 100644 --- a/tests/tests.mk +++ b/tests/tests.mk @@ -1,8 +1,9 @@ -ALOG_TEST_SRCS = $(shell find tests -iname "*.c" -exec basename {} \;) +ALOG_TEST_DIR := $(dir $(lastword $(MAKEFILE_LIST))) +ALOG_TEST_SRCS = $(shell find $(ALOG_TEST_DIR) -iname "*.c" -exec basename {} \;) ALOG_TESTS = $(ALOG_TEST_SRCS:.c=) $(ALOG_TESTS): $(ALOG_LIB) - $(CC) $(CFLAGS) -g -o $(BUILD_DIR)/$@.o -c tests/$@.c + $(CC) $(CFLAGS) -g -o $(BUILD_DIR)/$@.o -c $(ALOG_TEST_DIR)$@.c $(CC) -o $(TARGET_DIR)/$@ $(BUILD_DIR)/$@.o $(ALOG_LIB) -lpthread chmod +x $(TARGET_DIR)/$@ From 9ee1aacd836f4d70ed3e51957bb4e897b6281df6 Mon Sep 17 00:00:00 2001 From: Aidan Hahn Date: Mon, 7 Mar 2022 11:40:22 -0800 Subject: [PATCH 07/10] fix readme and add test instructions Signed-off-by: Aidan Hahn --- Makefile | 2 +- README.md | 6 ++++-- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/Makefile b/Makefile index 6ac8dd0..d999773 100644 --- a/Makefile +++ b/Makefile @@ -19,7 +19,7 @@ ALOG_LIB = $(TARGET_DIR)/alog.so include $(dir $(lastword $(MAKEFILE_LIST)))tests/tests.mk -.PHONY: so +.PHONY: alog-so alog-so: $(if $(shell stat $(ALOG_LIB)), clean) $(ALOG_LIB) $(ALOG_LIB): $(TARGET_DIR) $(ALOG_OBJ) diff --git a/README.md b/README.md index 76b8a43..dc86de0 100644 --- a/README.md +++ b/README.md @@ -17,14 +17,16 @@ Anything that is a file descriptor and can be written to with write() and fsync( # How to build ```bash -$ make so +$ make alog-so ``` # How to test ``` +$ make alog-tests +``` # Variables -The following (shell) variables can be set to influence behavior at runtime: +The following (shell) variables can be set at compile time: - **ALOG_DEBUG**: Set this variable to compile with debug symbols - **ALOG_HIJACK_PRINTF**: Set this variable to compile along with a printf implementation that leverages alog. Probably dont though. From 7ff43f0346c9bea1bbc63fc61c4cb8f2bd9941bb Mon Sep 17 00:00:00 2001 From: Aidan Hahn Date: Fri, 22 Jul 2022 09:52:52 -0700 Subject: [PATCH 08/10] fix include issues Signed-off-by: Aidan Hahn --- alog.c | 8 ++++++++ alog.h | 11 ++++++----- 2 files changed, 14 insertions(+), 5 deletions(-) diff --git a/alog.c b/alog.c index 95ae87b..4e9f7ae 100644 --- a/alog.c +++ b/alog.c @@ -25,6 +25,12 @@ #define TEST_ALOG_STATE() if (!ALOG_LOGGING_STATE) { _init_alog(); } +struct _global_logging_state *ALOG_LOGGING_STATE; +int _alog_num_out_fds; +int *_alog_out_fds; +int _alog_num_err_fds; +int *_alog_err_fds; + void _init_alog() { if (!ALOG_LOGGING_STATE) { ALOG_LOGGING_STATE = malloc(sizeof(struct _global_logging_state)); @@ -152,6 +158,7 @@ void alog ( int size; if (severity != PRINT) { // GET TIMESTAMP + // TODO: try to get this all into 1 call to sprintf time_t t = time(NULL); struct tm * p = localtime(&t); char *timestamp = malloc(strftime(NULL, 0, "[%c]", p) + 1); @@ -159,6 +166,7 @@ void alog ( char *msg_and_timestamp = malloc(sizeof(timestamp) + 1 + (strlen(message) + 1) + 2); sprintf(msg_and_timestamp, "%s %s\n", timestamp, message); free(timestamp); + // TODO: Why even use msg_and_timestamp if I am going to write it wholesale into buffer? size = snprintf(NULL, 0, msg_and_timestamp, fmt_list); buffer = malloc(size + 1); sprintf(buffer, msg_and_timestamp, fmt_list); diff --git a/alog.h b/alog.h index b7e8cfb..30d3093 100644 --- a/alog.h +++ b/alog.h @@ -30,7 +30,7 @@ struct _global_logging_state { * I am sticking with it here. * * Alloc'ed/Initialized by a call to init_alog()*/ -struct _global_logging_state *ALOG_LOGGING_STATE; +extern struct _global_logging_state *ALOG_LOGGING_STATE; /* alternative impl I didnt want to write * it would have required a bunch of getters and setters @@ -42,10 +42,10 @@ struct _global_logging_state *ALOG_LOGGING_STATE; #define LOG_DEBUG_MSGS_FIELD = 2; int flags = 0; */ -int _alog_num_out_fds; -int *_alog_out_fds; -int _alog_num_err_fds; -int *_alog_err_fds; +extern int _alog_num_out_fds; +extern int *_alog_out_fds; +extern int _alog_num_err_fds; +extern int *_alog_err_fds; typedef enum { // Not printed by default. Useful for debug info @@ -84,4 +84,5 @@ void alog(alog_sev, const char *, ...); #ifdef ALOG_HIJACK_PRINTF int printf(const char *format, ...); #endif // ALOG_HIJACK_PRINTF + #endif // ALOG_H From eeaa710f68417255c9c680ca39e0cef3efd0f77a Mon Sep 17 00:00:00 2001 From: Aidan Hahn Date: Fri, 29 Jul 2022 12:57:54 -0700 Subject: [PATCH 09/10] fix timestamp and printing Signed-off-by: Aidan Hahn --- alog.c | 30 ++++++++++++++++++++++-------- alog.h | 1 + 2 files changed, 23 insertions(+), 8 deletions(-) diff --git a/alog.c b/alog.c index 4e9f7ae..bd603b2 100644 --- a/alog.c +++ b/alog.c @@ -24,6 +24,7 @@ #include #define TEST_ALOG_STATE() if (!ALOG_LOGGING_STATE) { _init_alog(); } +#define MAX_TIMESTAMP 30 struct _global_logging_state *ALOG_LOGGING_STATE; int _alog_num_out_fds; @@ -48,6 +49,10 @@ void _init_alog() { _alog_err_fds[0] = 2; } +void init_alog() { + TEST_ALOG_STATE(); +} + // TODO: should I dont close stdin and stdout? void deinit_alog() { TEST_ALOG_STATE(); @@ -161,21 +166,31 @@ void alog ( // TODO: try to get this all into 1 call to sprintf time_t t = time(NULL); struct tm * p = localtime(&t); - char *timestamp = malloc(strftime(NULL, 0, "[%c]", p) + 1); - strftime(timestamp, sizeof(timestamp), "[%c]", p); - char *msg_and_timestamp = malloc(sizeof(timestamp) + 1 + (strlen(message) + 1) + 2); + int timestamp_len = strftime(NULL, MAX_TIMESTAMP, "[%c]", p) + 1; + char *timestamp = calloc(timestamp_len , sizeof(char)); + strftime(timestamp, timestamp_len+1, "[%c]", p); + + size = (timestamp_len + strlen(message) + 1); + char *msg_and_timestamp = calloc(size, sizeof(char)); sprintf(msg_and_timestamp, "%s %s\n", timestamp, message); free(timestamp); + // TODO: Why even use msg_and_timestamp if I am going to write it wholesale into buffer? - size = snprintf(NULL, 0, msg_and_timestamp, fmt_list); + size = vsnprintf(NULL, 0, msg_and_timestamp, fmt_list); + va_start(fmt_list, message); + va_end(fmt_list); buffer = malloc(size + 1); - sprintf(buffer, msg_and_timestamp, fmt_list); + vsprintf(buffer, msg_and_timestamp, fmt_list); + va_end(fmt_list); free(msg_and_timestamp); + // if severity is PRINT we avoid timestamp } else { - size = snprintf(NULL, 0, message, fmt_list); + size = vsnprintf(NULL, 0, message, fmt_list); + va_start(fmt_list, message); buffer = malloc(size + 1); - sprintf(buffer, message, fmt_list); + vsprintf(buffer, message, fmt_list); + va_end(fmt_list); } for (int i = 0; i < _alog_num_out_fds; i++) { @@ -198,7 +213,6 @@ void alog ( } free(buffer); - va_end(fmt_list); } #ifdef ALOG_HIJACK_PRINTF diff --git a/alog.h b/alog.h index 30d3093..ef2deb9 100644 --- a/alog.h +++ b/alog.h @@ -65,6 +65,7 @@ typedef enum { * err fds: stderr */ void _init_alog(); +void init_alog(); /* deinitializes all global state memory used in this library */ void deinit_alog(); From 56f87419f164e4fb0f1b79e53f308084432fdb7e Mon Sep 17 00:00:00 2001 From: Ava Hahn Date: Thu, 11 Aug 2022 11:15:42 -0700 Subject: [PATCH 10/10] test and update the printf functionality Signed-off-by: Ava Hahn --- Makefile | 2 +- alog.c | 45 +++++++++++++++++++++++++++++++-------------- alog.h | 5 +++-- 3 files changed, 35 insertions(+), 17 deletions(-) diff --git a/Makefile b/Makefile index d999773..63910eb 100644 --- a/Makefile +++ b/Makefile @@ -20,7 +20,7 @@ ALOG_LIB = $(TARGET_DIR)/alog.so include $(dir $(lastword $(MAKEFILE_LIST)))tests/tests.mk .PHONY: alog-so -alog-so: $(if $(shell stat $(ALOG_LIB)), clean) $(ALOG_LIB) +alog-so: $(if $(shell stat $(ALOG_LIB)), alog-clean) $(ALOG_LIB) $(ALOG_LIB): $(TARGET_DIR) $(ALOG_OBJ) $(CC) $(LDFLAGS) -o $(ALOG_LIB) $(ALOG_OBJ) diff --git a/alog.c b/alog.c index bd603b2..26aecbb 100644 --- a/alog.c +++ b/alog.c @@ -144,20 +144,19 @@ void alog_remove_target(int fd) { // TODO: use preprocessor directives to gate off the posix timestamp stuff // unless the right variable is passed in at compile time -void alog ( +int _alog ( alog_sev severity, const char * message, - ... + va_list fmt ) { - TEST_ALOG_STATE(); if (severity == DEBUG && !ALOG_LOGGING_STATE->log_debug_messages) { // nop :) - return; + return -1; } va_list fmt_list; - va_start(fmt_list, message); + va_copy(fmt_list, fmt); char *buffer; int size; @@ -177,8 +176,8 @@ void alog ( // TODO: Why even use msg_and_timestamp if I am going to write it wholesale into buffer? size = vsnprintf(NULL, 0, msg_and_timestamp, fmt_list); - va_start(fmt_list, message); va_end(fmt_list); + va_copy(fmt_list, fmt); buffer = malloc(size + 1); vsprintf(buffer, msg_and_timestamp, fmt_list); va_end(fmt_list); @@ -187,14 +186,18 @@ void alog ( // if severity is PRINT we avoid timestamp } else { size = vsnprintf(NULL, 0, message, fmt_list); - va_start(fmt_list, message); + va_end(fmt_list); + va_copy(fmt_list, fmt); buffer = malloc(size + 1); vsprintf(buffer, message, fmt_list); va_end(fmt_list); } + int written = 0; for (int i = 0; i < _alog_num_out_fds; i++) { - if (write(_alog_out_fds[i], buffer, size) < size) { + int num = write(_alog_out_fds[i], buffer, size); + written += num; + if (num < size) { // TODO: how to handle? probably cant log another message lmao // perhaps it should be yanked from the list? maybe not? ;; @@ -204,7 +207,9 @@ void alog ( if (severity == ERROR) { for (int i = 0; i < _alog_num_err_fds; i++) { - if (write(_alog_err_fds[i], buffer, size) < size) { + int num = write(_alog_err_fds[i], buffer, size); + written += num; + if (num < size) { // TODO: see above ;; } @@ -213,19 +218,31 @@ void alog ( } free(buffer); + return written; } +int alog ( + alog_sev severity, + const char * message, + ... +) { + TEST_ALOG_STATE(); + va_list fmt_list; + va_start(fmt_list, message); + int res = _alog(severity, message, fmt_list); + va_end(fmt_list); + return res; +} + + #ifdef ALOG_HIJACK_PRINTF int printf(const char *format, ...) { TEST_ALOG_STATE(); va_list fmt_list; va_start(fmt_list, format); - /* TODO: this is a duplicate call given the implementation of alog. - * Lets figure out a better way to handle this */ - int size = snprintf(NULL, 0, format, fmt_list); - alog(PRINT, NULL, format, fmt_list); + int i = _alog(PRINT, format, fmt_list); va_end(fmt_list); - return size; + return i; /* potentially accept a mem leak at end of program for alog state. * That is, if the program using this is not already using alog correctly * or if ALOG has been LD_PRELOAD'ed into a program to override its printf */ diff --git a/alog.h b/alog.h index ef2deb9..4b5c104 100644 --- a/alog.h +++ b/alog.h @@ -78,8 +78,9 @@ void alog_add_target(int, char); /* removes an fd from both out fds and err fds */ void alog_remove_target(int); -/* call to log a message. Takes a severity, a message, and format stuff */ -void alog(alog_sev, const char *, ...); +/* call to log a message. Takes a severity, a message, and format stuff + * returns number of bytes written*/ +int alog(alog_sev, const char *, ...); /* overrides all calls to printf with a call to alog */ #ifdef ALOG_HIJACK_PRINTF