Fix threadpool after ripping out cleanup function suite

3 files changed, 73 insertions, 234 deletions

diff --git a/src/encryption.c b/src/encryption.c
index 6df1ff3..2ead9cf 100644
--- a/src/encryption.c
+++ b/src/encryption.c
@@ -212,7 +212,8 @@ int maketmp(const char * const dest) {
  * @retval (int)[-1, 0] 0 on success, -1 on error
  */
 int linkto(const char * const target, int tgfd) {
-    if(!target || tgfd < 0 || access(target, F_OK) != -1) ERRRET(EINVAL, -1);
+    if(!target || tgfd < 0) ERRRET(EINVAL, -1);
+    if(access(target, F_OK) != -1) ERRRET(EEXIST, -1);
     char *path = NULL;
 
     asprintf(&path, "/proc/self/fd/%d", tgfd);
diff --git a/src/shared.c b/src/shared.c
index 9a147eb..1310ef2 100644
--- a/src/shared.c
+++ b/src/shared.c
@@ -46,26 +46,12 @@
 typedef int (*gcallback)(void*);    //!< Generic callback signature
 typedef void (*fcallback)(void*);   //!< free()-like callback signature
 
-/**
- * @brief A locally defined structure designed for easier function cleanup
- *
- */
-typedef struct cl {
-    fcallback *callbacks;   //!< An array of free()-like callbacks. Actual Type: fcallback callbacks[]
-    void * *arguments;      //!< An array of void pointers. Actual Type: void *arguments[]
-    int size;               //!< The size of each array
-    int used;               //!< The current number of used elements in each array
-} cleanup;
-// While the cleanup thing is useful, it's also a complete fucking mess. Swtich to error gotos asap
-
 #include <stdlib.h>
 #include <string.h>
 #include <unistd.h>
 #include <errno.h>
 #include <error.h>
 
-#include <threads.h>
-
 /**
  * @brief Read the entire contents of a file descriptor into a malloc()'ed buffer
  *
@@ -241,138 +227,4 @@ char * vxdirname(const char * const path) {
         return actual;
 }
 
-
-/**
- * @brief Initialize a cleanup object
- *
- * @param loc The cleanup object to be initialized
- * @param callbacks An array of free()-like callbacks. Must be `size` elements long
- * @param arguments An array of void pointers. Must be `size` elements long
- * @param size The number of elements the callbacks and arguments array are long
- * @retval (int)[-1, 0] Returns 0 on success, -1 on error
- */
-int cleanup_init(cleanup *loc, fcallback callbacks[], void *arguments[], int size) {
-    if(!loc || !callbacks || !arguments || size <= 0) ERRRET(EINVAL, -1);
-
-    loc->callbacks = callbacks;
-    loc->arguments = arguments;
-    loc->size = size;
-    loc->used = 0;
-
-    return 0;
-}
-
-/**
- * @brief Register a new callback and argument onto a cleanup stack
- *
- * @param loc The cleanup object to modify
- * @param cb A free()-like callback to run
- * @param arg A piece of data for the callback to run
- * @retval (int)[-1, 0] Returns 0 on success, -1 on error
- */
-int cleanup_register(cleanup *loc, fcallback cb, void *arg) {
-    if(!loc || !cb) ERRRET(EINVAL, -1);
-    if(loc->used >= loc->size || loc->used < 0) ERRRET(ENOMEM, -1);
-
-    loc->callbacks[loc->used] = cb;
-    loc->arguments[loc->used] = arg;
-    loc->used++;
-
-    return 0;
-}
-
-/**
- * @brief Conditionally register a callback and argument
- *
- * @param loc The cleanup object to modify
- * @param cb A free()-like callback to run
- * @param arg A piece of data for the callback to run
- * @param flag Whether or not the register should take place. Will not run if `flag` is non-zero
- * @retval (int)[-1, 0] Returns 0 on success or skip, -1 on error
- */
-int cleanup_cndregister(cleanup *loc, fcallback cb, void *arg, unsigned char flag) {
-    if(flag) return 0;
-    return cleanup_register(loc, cb, arg);
-}
-
-/**
- * @brief Clear a cleanup object
- * @attention Does not free any registered callbacks or arguments, just marks them as available space
- *
- * @param loc The cleanup object to modify
- * @retval (int)[-1, 0] Returns 0 on success, -1 on error
- */
-int cleanup_clear(cleanup *loc) {
-    if(!loc) ERRRET(EINVAL, -1);
-    loc->used = 0;
-    return 0;
-}
-
-/**
- * @brief Fires all the registered callbacks and arguments in a cleanup object in FIFO (stack) order
- *
- * @param loc The cleanup object to fire
- * @retval (int)[-1, 0] Returns 0 on success, -1 on error
- */
-int cleanup_fire(cleanup *loc) {
-    if(!loc) ERRRET(EINVAL, -1);
-
-    for(int i = (loc->used - 1); i >= 0; i--) {
-        if(loc->callbacks[i] == NULL) {
-            error(0, EINVAL, "cleanup_fire: refusing to run null callback...");
-            continue;
-        }
-
-        loc->callbacks[i](loc->arguments[i]);
-    }
-    cleanup_clear(loc);
-
-    return 0;
-}
-
-/**
- * @brief Conditionally fires a cleanup object
- *
- * @param loc The cleanup object in question
- * @param flag Whether the object should be fired. Will skip firing if non-zero
- * @retval (int)[-1, 0] Returns 0 on success, -1 on error
- */
-int cleanup_cndfire(cleanup *loc, unsigned char flag) {
-    if(flag)
-        return cleanup_fire(loc);
-    return 0;
-}
-
-/**
- * @brief Initializes a set of variables suitable for use in the cleanup macros
- * @param size The number of elements long each array should be
- */
-#define cleanup_CREATE(size) \
-cleanup __CLEANUP; \
-fcallback __CLEANUP_FUNCS[(size)]; \
-void *__CLEANUP_ARGS[(size)]; \
-unsigned char __FLAG = 0; \
-cleanup_init(&__CLEANUP, __CLEANUP_FUNCS, __CLEANUP_ARGS, (size))
-
-//! Register a callback-argument pair using the local cleanup object
-#define cleanup_REGISTER(cb, arg)       cleanup_register(&__CLEANUP, (cb), (arg))
-//! Conditionally register a callback-argument pair using the local cleanup object
-#define cleanup_CNDREGISTER(cb, arg)    cleanup_cndregister(&__CLEANUP, (cb), (arg), __FLAG)
-//! Clean the local cleanup object
-#define cleanup_CLEAR()                 cleanup_clear(&__CLEANUP)
-//! Fire the local cleanup object
-#define cleanup_FIRE()                  cleanup_fire(&__CLEANUP)
-//! Conditionally fire the local cleanup object
-#define cleanup_CNDFIRE()               cleanup_cndfire(&__CLEANUP, __FLAG)
-//! Set the local cleanup flag to a non-zero number
-#define cleanup_MARK()                  (__FLAG = 1)
-//! Set the local cleanup flag to zero
-#define cleanup_UNMARK()                (__FLAG = 0)
-//! Check if the local cleanup flag is non-zero
-#define cleanup_ERRORFLAGGED            (__FLAG != 0)
-//! Conditionally execute some `code` if the local cleanup flag has not been marked
-#define cleanup_CNDEXEC(code)           while(!cleanup_ERRORFLAGGED) {code; break;}
-//! Conditionally fire the local cleanup object and return `ret`
-#define cleanup_CNDFIRERET(ret)         do {cleanup_CNDFIRE(); return ret;} while (0)
-
 #endif
\ No newline at end of file
diff --git a/src/threadpool.c b/src/threadpool.c
index c266964..959c060 100644
--- a/src/threadpool.c
+++ b/src/threadpool.c
@@ -15,6 +15,7 @@
 #include "shared.c"
 
 #include <threads.h>
+#include <stdint.h>
 #include <stdlib.h>
 #include <errno.h>
 #include <error.h>
@@ -44,9 +45,9 @@ typedef struct tqnode {
  *
  */
 typedef struct taskqueue {
-    tqnode *start;      //!< The first element of the queue
-    tqnode *end;        //!< The final element of the queue
-    unsigned int size;  //!< The number of elements in the queue
+    tqnode *start;  //!< The first element of the queue
+    tqnode *end;    //!< The final element of the queue
+    size_t size;    //!< The number of elements in the queue
 } taskqueue;
 
 /**
@@ -54,13 +55,14 @@ typedef struct taskqueue {
  *
  */
 typedef struct ctqueue {
-    mtx_t mutex;            //!< A mutex for locking sensitive resources
-    cnd_t cond;             //!< A conditional for waiting on / sending a signal
-    unsigned char canceled; //!< Whether the threads are currently canceled or not
-
-    taskqueue *tq;          //!< A taskqueue to be accessed concurrently
-    thrd_t *thrdarr;        //!< An array of threads to be dispatched as consumers
-    int talen;              //!< The length of the thread array
+    mtx_t mutex;        //!< A mutex for locking sensitive resources
+    cnd_t cond;         //!< A conditional for waiting on / sending a signal
+    uint8_t canceled;   //!< Whether the threads are currently canceled or not
+    taskqueue *tq;      //!< A taskqueue to be accessed concurrently
+
+    thrd_t *thrdarr;    //!< An array of threads to be dispatched as consumers
+    int talen;          //!< The length of the thread array
+        // Consider making these another linked list or stack or something
 } ctqueue;
 
 /**
@@ -71,12 +73,11 @@ typedef struct ctqueue {
  * @param data Data to be passed to the callback. May be null
  * @retval (task*)[NULL, task*] Returns a task object with set parameters. Returns `null` and sets errno on error
  */
-task * task_init(gcallback callback, fcallback freecb, void *data) {
+task * task_new(gcallback callback, fcallback freecb, void *data) {
     if(callback == NULL) ERRRET(EINVAL, NULL);
 
     task *tsk = calloc(1, sizeof(*tsk));
-    if(!tsk)
-        return NULL;
+    if(!tsk) return NULL;
 
     tsk->callback = callback;
     tsk->freecb = freecb;
@@ -91,12 +92,10 @@ task * task_init(gcallback callback, fcallback freecb, void *data) {
  * @param tsk A task object to free. Frees data associated with task via `freecb` value specified in its creation. May be null
  */
 void task_free(void *tsk) {
-    task *real = (task *)tsk;
-    if(!real)
-        return;
+    task *real = tsk;
+    if(!real) return;
 
-    if(real->freecb != NULL)
-        real->freecb(real->data);
+    if(real->freecb) real->freecb(real->data);
     free(real);
 
     return;
@@ -120,18 +119,18 @@ int task_fire(task *tsk) {
  * @retval (int) Returns value of the callback. Returns -1 and sets errno on error
  */
 int task_fired(task *tsk) {
+    if(!tsk) return -1;
     int retval = task_fire(tsk);
-    if(errno == EINVAL && retval == -1) {return -1;}
     task_free(tsk);
     return retval;
 }
 
 
-tqnode * tqnode_init(tqnode *next, tqnode *prev, task *tsk) {
+tqnode * tqnode_new(tqnode *next, tqnode *prev, task *tsk) {
     if(!tsk) ERRRET(EINVAL, NULL);
+
     tqnode *node = calloc(1, sizeof(*node));
-    if(!node)
-        return NULL;
+    if(!node) return NULL;
 
     node->next = next;
     node->prev = prev;
@@ -140,10 +139,19 @@ tqnode * tqnode_init(tqnode *next, tqnode *prev, task *tsk) {
     return node;
 }
 
+// Create a tqnode and task at the same time. Returns a valid tqnode with a valid task on success, NULL on error. Does not call task_free on error
+tqnode * tqnode_newtask(tqnode *next, tqnode *prev, gcallback callback, fcallback freecb, void *data) {
+    task *tsk = task_new(callback, freecb, data);
+    if(!tsk) return NULL;
+
+    tqnode *node = tqnode_new(next, prev, tsk);
+    if(!node) free(tsk);
+    return node;
+}
+
 void tqnode_free(void *tqn) {
-    tqnode *real = (tqnode *)tqn;
-    if(!real)
-        return;
+    tqnode *real = tqn;
+    if(!real) return;
 
     task_free(real->task);
     free(real);
@@ -157,10 +165,9 @@ void tqnode_free(void *tqn) {
  *
  * @retval (taskqueue*)[NULL, taskqueue*] Returns a new taskqueue object. Returns `null` and sets errno on error
  */
-taskqueue * taskqueue_init(void) {
+taskqueue * taskqueue_new(void) {
     taskqueue *tq = calloc(1, sizeof(*tq));
-    if(!tq)
-        return NULL;
+    if(!tq) return NULL;
 
     tq->start = NULL;
     tq->end = NULL;
@@ -175,8 +182,7 @@ taskqueue * taskqueue_init(void) {
  * @param tq A taskqueue to be freed. May be null
  */
 void taskqueue_free(void *tq) {
-    if(!tq)
-        return;
+    if(!tq) return;
 
     for(tqnode *p = ((taskqueue*)tq)->start, *n; p != NULL;) {
         n = p->next;
@@ -192,9 +198,8 @@ int taskqueue_handlefirst(taskqueue *tq, task *tsk) {
     if(!tq || !tsk) ERRRET(EINVAL, -1);
     if(tq->size) {return 0;}
 
-    tqnode *first = tqnode_init(NULL, NULL, tsk);
-    if(!first)
-        return -1;
+    tqnode *first = tqnode_new(NULL, NULL, tsk);
+    if(!first) return -1;
 
     tq->start = first;
     tq->end = first;
@@ -214,13 +219,12 @@ int taskqueue_push(taskqueue *tq, task *tsk) {
     if(!tq || !tsk) ERRRET(EINVAL, -1);
 
     int hf;
-    if((hf = taskqueue_handlefirst(tq, tsk)))
-        return (hf >= 0) ? 0 : -1;
+    if((hf = taskqueue_handlefirst(tq, tsk))) return (hf >= 0) ? 0 : -1;
+
+    tqnode *curstart = tq->start, *newstart = tqnode_new(tq->start, NULL, tsk);
+    if(!newstart) return -1;
 
-    tqnode *newstart = tqnode_init(tq->start, NULL, tsk);
-    if(!newstart)
-        return -1;
-    tq->start->prev = newstart;
+    curstart->prev = newstart;
     tq->start = newstart;
     tq->size++;
 
@@ -264,13 +268,12 @@ int taskqueue_pushfront(taskqueue *tq, task *tsk) {
     if(!tq || !tsk) ERRRET(EINVAL, -1);
 
     int hf;
-    if((hf = taskqueue_handlefirst(tq, tsk)))
-        return (hf >= 0) ? 0 : -1;
+    if((hf = taskqueue_handlefirst(tq, tsk))) return (hf >= 0) ? 0 : -1;
 
-    tqnode *newend = tqnode_init(NULL, tq->end, tsk);
-    if(!newend)
-        return -1;
-    tq->end->next = newend;
+    tqnode *end =tq->end, *newend = tqnode_new(NULL, tq->end, tsk);
+    if(!newend) return -1;
+
+    end->next = newend;
     tq->end = newend;
     tq->size++;
 
@@ -323,15 +326,15 @@ int taskqueue_size(taskqueue *tq) {
     mtx_unlock(&(ctq)->mutex); \
 } while (0)
 
-static void ___ucl_mtxdestroy(void *mtx) {
-    if(!mtx) return;
-    mtx_destroy((mtx_t *)mtx);
+static void mtxd_helper(mtx_t *mutex) {
+    if(!mutex) return;
+    mtx_destroy(mutex);
     return;
 }
 
-static void ___ucl_cnddestroy(void *cond) {
-    if(cond) return;
-    cnd_destroy((cnd_t *)cond);
+static void cndd_helper(cnd_t *cond) {
+    if(!cond) return;
+    cnd_destroy(cond);
     return;
 }
 
@@ -343,47 +346,32 @@ static void ___ucl_cnddestroy(void *cond) {
  */
 ctqueue * ctqueue_init(int nthreads) {
     if(nthreads <= 0) ERRRET(EINVAL, NULL);
-    cleanup_CREATE(6);
 
     ctqueue *ctq = calloc(1, sizeof(*ctq));
-    if(!ctq)
-        return NULL;
-    cleanup_REGISTER(free, ctq);
+    if(!ctq) return NULL;
 
     ctq->canceled = 0;
     ctq->talen = nthreads;
 
-    cleanup_CNDEXEC(
-        ctq->tq = taskqueue_init();
-        if(!ctq->tq)
-            cleanup_MARK();
-        cleanup_CNDREGISTER(taskqueue_free, ctq->tq);
-    );
+    ctq->tq = taskqueue_new();
+    if(!ctq->tq) goto ERR_ctqueue_init;
 
-    cleanup_CNDEXEC(
-        if(mtx_init(&ctq->mutex, mtx_plain) != thrd_success)
-            cleanup_MARK();
-        cleanup_CNDREGISTER(___ucl_mtxdestroy, (void*)&ctq->mutex);
-    );
+    if(mtx_init(&ctq->mutex, mtx_plain) != thrd_success) goto ERR_ctqueue_init;
+    if(cnd_init(&ctq->cond) != thrd_success) goto ERR_ctqueue_init;
 
-    cleanup_CNDEXEC(
-        if(cnd_init(&ctq->cond) != thrd_success)
-            cleanup_MARK();
-        cleanup_CNDREGISTER(___ucl_cnddestroy, (void*)&ctq->cond);
-    );
+    ctq->thrdarr = calloc(ctq->talen, sizeof(thrd_t));
+    if(!ctq->thrdarr) goto ERR_ctqueue_init;
 
-    cleanup_CNDEXEC(
-        ctq->thrdarr = calloc(ctq->talen, sizeof(thrd_t));
-        if(!ctq->thrdarr)
-            cleanup_MARK();
-        cleanup_CNDREGISTER(free, ctq->thrdarr);
-    )
+    return ctq;
 
-    cleanup_CNDFIRE();
-    if(cleanup_ERRORFLAGGED)
-        return NULL;
+ERR_ctqueue_init:
+    free(ctq->thrdarr);
+    cndd_helper(&ctq->cond);
+    mtxd_helper(&ctq->mutex);
+    taskqueue_free(ctq->tq);
+    free(ctq);
 
-    return ctq;
+    return NULL;
 }
 
 /**
@@ -410,8 +398,7 @@ int ctqueue_cancel(ctqueue *ctq) {
  * @param ctq The concurrent taskqueue to free. May be null
  */
 void ctqueue_free(void *ctq) {
-    if(!ctq)
-        return;
+    if(!ctq) return;
 
     ctqueue *real = (ctqueue *)ctq;
     ctqueue_cancel(real);
@@ -485,8 +472,7 @@ static int __CTQ_CONSUMER(void *ctq) {
 
     for(task *ctask = NULL;;) {
         ctask = ctqueue_waitpop(real);
-        if(!ctask)
-            break;
+        if(!ctask) break;
 
         task_fire(ctask);
         task_free(ctask);