test and update the printf functionality

Signed-off-by: Ava Hahn <ava@aidanis.online>

.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

Makefile

@@ -12,20 +12,21 @@ 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
-so: $(if $(shell stat $(ALOG_LIB)), clean) $(ALOG_LIB)
+.PHONY: alog-so
+alog-so: $(if $(shell stat $(ALOG_LIB)), alog-clean) $(ALOG_LIB)
 
 $(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)
@@ -34,5 +35,5 @@ $(TARGET_DIR):
 	mkdir $(TARGET_DIR)
 
 .PHONY: clean
-clean:
+alog-clean:
 	rm $(ALOG_LIB) $(ALOG_OBJ)

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.

alog.c

@@ -24,6 +24,13 @@
 #include <time.h>
 
 #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;
+int *_alog_out_fds;
+int _alog_num_err_fds;
+int *_alog_err_fds;
 
 void _init_alog() {
   if (!ALOG_LOGGING_STATE) {
@@ -42,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();
@@ -133,46 +144,60 @@ 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;
   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);
-    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));
+    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);
-    size = snprintf(NULL, 0, msg_and_timestamp, fmt_list);
+
+    // 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_end(fmt_list);
+    va_copy(fmt_list, fmt);
     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_end(fmt_list);
+    va_copy(fmt_list, fmt);
     buffer = malloc(size + 1);
-    sprintf(buffer, message, fmt_list);
+    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?
       ;;
@@ -182,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
         ;;
       }
@@ -191,20 +218,31 @@ void alog (
   }
 
   free(buffer);
-  va_end(fmt_list);
+  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 */

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
@@ -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();
@@ -77,11 +78,13 @@ 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
 int printf(const char *format, ...);
 #endif // ALOG_HIJACK_PRINTF
+
 #endif // ALOG_H

tests/log_test.c

@@ -4,14 +4,24 @@
 #include <sys/types.h>
 #include <stdio.h>
 #include <string.h>
+#include <semaphore.h>
+#include <fcntl.h>
 
 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,23 @@ int main() {
     alog(ERROR, err_message);
     alog(PRINT, out_message);
 
-    exit(0);
+    if (sem_post(sem)) {
+      perror("cannot increment semaphore");
+      return 1;
+    }
+
+    // clean up
+    sem_close(sem);
+    deinit_alog();
+
   // 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 +96,8 @@ int main() {
 
 
     printf("out and err log printing are successfull.\n");
-  }
-
+    sem_close(sem);
     deinit_alog();
+  }
   return 0;
 }

tests/tests.mk

@@ -1,12 +1,13 @@
-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) -o $(TARGET_DIR)/$@ $(BUILD_DIR)/$@.o $(ALOG_LIB)
+	$(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)/$@
 
-run-tests: $(ALOG_TESTS)
+alog-tests: $(ALOG_TESTS)
 	for test in $^ ; do \
 		$(TARGET_DIR)/$$test ; \
 	done