fix(ll): prevent use after free when removing items from any type of ll

README.md

@@ -7,7 +7,8 @@ Example of working with a singly linked list (sll):
 ```c
 char *a = "one";
 char *b = "two";
-ds_sll_t *ll = ds_sll_new_node(a); // [ "one" ]
+ds_sll_t *ll = ds_sll_init();
+ds_sll_insert(ll, a); // [ "one" ]
 ds_sll_insert(ll, b); // [ "one", "two" ]
 ds_ll_foreach(ds_sll_t, ll) {
     puts(cur->data);
@@ -22,28 +23,28 @@ datatypes.
 
 ## Hash Map
 ```c
-struct complex {
+typedef struct {
     int num;
     char *str;
-};
+} complex ;
 
-struct complex *a = calloc(1, sizeof(struct complex));
+complex *a = calloc(1, sizeof(complex));
 a->num = 1;
 a->str = "abc";
-struct complex *b = calloc(1, sizeof(struct complex));
+complex *b = calloc(1, sizeof(complex));
 b->num = 2;
 b->str = "def";
 
 ds_hmp_t *hmp = ds_hmp_init(101);
 ds_hmp_insert(hmp, a->str, a); // [ (23)[ [ a->str, a ] ] ]
 ds_hmp_insert(hmp, b->str, b); // [ (23)[ [ a->str, a ] ], (58)[ [ b->str, b ] ]
-struct complex *pa = ds_hmp_remove(hmp, a->str); // a
+complex *pa = ds_hmp_remove(hmp, a->str); // a
 free(pa->str);
 free(pa);
-ds_hmp_free(&hmp);
+ds_hmp_free(&hmp, NULL);
 free(b);
 ```
 
 # TODO
 - [ ] more data structures
-- [ ] tests
+- [x] tests

ds.c

@@ -3,6 +3,18 @@
 
 #include "ds.h"
 
+ds_sll_t
+*ds_sll_init(void)
+{
+	return ds_sll_new_node(NULL);
+}
+
+ds_dll_t
+*ds_dll_init(void)
+{
+	return ds_dll_new_node(NULL);
+}
+
 ds_sll_t
 *ds_sll_new_node(void *data)
 {
@@ -27,33 +39,45 @@ ds_dll_t
   return node;
 }
 
-void
+int
 ds_sll_insert(ds_sll_t *ll, void *data)
 {
+	if (!ll || !data) {
+		return -1;
+	}
+
 	ds_ll_foreach(ds_sll_t, ll) {
 		if (!cur->data) {
 			cur->data = data;
-			return;
+			return 0;
 		} else if (!cur->next) {
 			cur->next = ds_sll_new_node(data);
-			return;
-		}
+			return 0;
 		}
 	}
 
-void
+	return -1;
+}
+
+int
 ds_dll_insert(ds_dll_t *ll, void *data)
 {
+	if (!ll || !data) {
+		return -1;
+	}
+
 	ds_ll_foreach(ds_dll_t, ll) {
 		if (!cur->data) {
 			cur->data = data;
-			return;
+			return 0;
 		} else if (!cur->next) {
 			cur->next = ds_dll_new_node(data);
 			cur->next->prev = cur;
-			return;
+			return 0;
 		}
 	}
+
+	return -1;
 }
 
 void
@@ -63,25 +87,18 @@ void
 	void *data;
 	ds_sll_t *cur, *rm;
 
+	rm = NULL;
 	if (!ll || !*ll) {
 		return NULL;
 	}
 
-	for (i = -1, cur = *ll; cur; i++, cur = cur->next) {
-		if (i + 1 == idx) {
 	if (idx == 0) {
-				rm = cur;
-				if (!rm) {
-					return NULL;
-				}
-				*ll = cur->next;
+		rm = *ll;
+		*ll = rm->next;
 	} else {
+		for (i = -1, cur = *ll; cur; i++, cur = cur->next) {
+			if (idx == i + 1) {
 				rm = cur->next;
-				if (!rm) {
-					return NULL;
-				}
-				cur->next = cur->next->next;
-			}
 				break;
 			}
 		}
@@ -90,6 +107,9 @@ void
 			return NULL;
 		}
 
+		cur->next = rm->next;
+	}
+
 	data = rm->data;
 	free(rm);
 	return data;
@@ -102,31 +122,21 @@ void
 	void *data;
 	ds_dll_t *cur, *rm;
 
+	rm = NULL;
 	if (!ll || !*ll) {
 		return NULL;
 	}
 
-	for (i = -1, cur = *ll;; i++, cur = cur->next) {
-		if (i + 1 == idx) {
 	if (idx == 0) {
-				rm = cur;
-				if (!rm) {
-					return NULL;
-				}
-				if (cur->next) {
-					cur->next->prev = NULL;
-					*ll = cur->next;
+		rm = *ll;
+		*ll = rm->next;
+		if (rm->next) {
+			rm->next->prev = NULL;
 		}
 	} else {
-				rm = cur->next;
-				if (!rm) {
-					return NULL;
-				}
-				cur->next = cur->next->next;
-				if (cur->next) {
-					cur->next->prev = cur;
-				}
-			}
+		for (i = 0, cur = *ll; cur; i++, cur = cur->next) {
+			if (i == idx) {
+				rm = cur;
 				break;
 			}
 		}
@@ -135,6 +145,12 @@ void
 			return NULL;
 		}
 
+		rm->prev->next = rm->next;
+		if (rm->next) {
+			rm->next->prev = rm->prev;
+		}
+	}
+
 	data = rm->data;
 	free(rm);
 	return data;
@@ -145,6 +161,10 @@ ds_hmp_t
 {
 	ds_hmp_t *hmp;
 
+	if (!data_len) {
+		return NULL;
+	}
+
 	hmp = calloc(1, sizeof(ds_hmp_t));
 	hmp->data_len = data_len;
 	hmp->data = calloc(data_len, sizeof(ds_sll_t **));
@@ -152,21 +172,24 @@ ds_hmp_t
 	return hmp;
 }
 
-void
-ds_hmp_free(ds_hmp_t **hmp)
+int
+ds_hmp_free(ds_hmp_t **hmp, void kv_callback(_ds_hmp_kv_t *kv))
 {
 	int i;
 	_ds_hmp_kv_t *kv;
 	ds_sll_t *ll;
 
 	if (!hmp || !*hmp) {
-		return;
+		return -1;
 	}
 
 	for (i = 0; i < (*hmp)->data_len; i++) {
 		ll = (*hmp)->data[i];
 		while (ll && ll->data) {
 			if ((kv = ds_sll_remove(&ll, 0))) {
+				if (kv_callback) {
+					kv_callback(kv);
+				}
 				free(kv);
 			}
 		}
@@ -176,6 +199,8 @@ ds_hmp_free(ds_hmp_t **hmp)
 	}
 	free((*hmp)->data);
 	free(*hmp);
+
+	return 0;
 }
 
 int
@@ -191,25 +216,36 @@ _ds_hmp_gen_hash(char *str)
   return hash;
 }
 
-void
+int
 ds_hmp_insert(ds_hmp_t *hmp, char *key, void *data)
 {
 	_ds_hmp_kv_t *kv;
 	ds_sll_t *ll;
 	unsigned hash_pos;
 
+	if (!hmp || !key || !data) {
+		return -1;
+	}
+
 	kv = malloc(sizeof(_ds_hmp_kv_t));
 	kv->key = key;
 	kv->val = data;
 
 	hash_pos = _ds_hmp_gen_hash(key) % hmp->data_len;
 	if (!hmp->data[hash_pos]) {
-		hmp->data[hash_pos] = calloc(1, sizeof(ds_sll_t));
+		hmp->data[hash_pos] = ds_sll_init();
+		if (!hmp->data[hash_pos]) {
+			return -1;
+		}
 	}
 
 	/* get the ll and put the data into it */
 	ll = hmp->data[hash_pos];
-	ds_sll_insert(ll, kv);
+	if (ds_sll_insert(ll, kv) != 0) {
+		return -1;
+	}
+
+	return 0;
 }
 
 void
@@ -218,6 +254,10 @@ void
 	ds_sll_t *cur, *ll;
 	unsigned hash_pos;
 
+	if (!hmp || !key) {
+		return NULL;
+	}
+
 	hash_pos = _ds_hmp_gen_hash(key) % hmp->data_len;
 	ll = hmp->data[hash_pos];
 	if (!ll) {
@@ -242,6 +282,10 @@ void
 	ds_sll_t *ll, *cur;
 	_ds_hmp_kv_t *kv;
 
+	if (!hmp || !key) {
+		return NULL;
+	}
+
 	hash_pos = _ds_hmp_gen_hash(key) % hmp->data_len;
 	ll = hmp->data[hash_pos];
 	if (!ll) {

ds.h

@@ -24,6 +24,20 @@ typedef struct {
 
 #define ds_ll_foreach(t, ll) for (t *cur = ll; cur; cur = cur->next)
 
+/**
+ * @brief initialize a new singly linked list
+ *
+ * @return the new linked list
+ */
+ds_sll_t *ds_sll_init(void);
+
+/**
+ * @brief initialize a new doubly linked list
+ *
+ * @return the new linked list
+ */
+ds_dll_t *ds_dll_init(void);
+
 /**
  * @brief create a new allocated node for a singly linked list
  *
@@ -46,8 +60,9 @@ ds_dll_t *ds_dll_new_node(void *data);
  *
  * @param ll singly linked list
  * @param data data you want to add
+ * @return 0 on success
  */
-void ds_sll_insert(ds_sll_t *ll, void *data);
+int ds_sll_insert(ds_sll_t *ll, void *data);
 
 /**
  * @brief data to insert into a doubly linked list. This will use
@@ -55,8 +70,9 @@ void ds_sll_insert(ds_sll_t *ll, void *data);
  *
  * @param ll doubly linked list
  * @param data data you want to add
+ * @return 0 on success
  */
-void ds_dll_insert(ds_dll_t *ll, void *data);
+int ds_dll_insert(ds_dll_t *ll, void *data);
 
 /**
  * @brief remove an index from a singly linked list
@@ -88,8 +104,9 @@ ds_hmp_t *ds_hmp_init(int data_len);
  * @brief free all data allocated by ds_hmp_init and ds_hmp_insert
  *
  * @param hmp pointer to the hashmap
+ * @return 0 on success
  */
-void ds_hmp_free(ds_hmp_t **hmp);
+int ds_hmp_free(ds_hmp_t **hmp, void kv_callback(_ds_hmp_kv_t *kv));
 
 /**
  * @brief generate a numerical hash from a given string. You shouldn't need to
@@ -107,8 +124,9 @@ int _ds_hmp_gen_hash(char *str);
  * @param hmp the hashmap to insert into
  * @param key the key
  * @param data the data
+ * @return 0 on success
  */
-void ds_hmp_insert(ds_hmp_t *hmp, char *key, void *data);
+int ds_hmp_insert(ds_hmp_t *hmp, char *key, void *data);
 
 /**
  * @brief get something from a hashmap using it's key

test.c

@@ -1,42 +1,73 @@
-#include "ds.h"
 #include <stdio.h>
 #include <stdlib.h>
+#include <string.h>
 
-struct complex {
+#include "ds.h"
+
+#define test(name, tests) do {																				\
+		const char *_test_name = name;																		\
+		int _tests_passed = 0;																						\
+		int	_tests = 0;																										\
+		tests																															\
+		_tests_passed += _tests;																					\
+		printf("%s: %d/%d tests passed\n", name, _tests_passed, _tests);	\
+	} while (0)
+
+#define it(message, test) do {											\
+	_tests++;																					\
+	if (!(test)) {																		\
+		printf("%s %s FAILED\n", _test_name, message);	\
+		_tests_passed--;																\
+	}																									\
+} while (0)
+
+typedef struct {
 	int num;
 	char *str;
-};
+} complex;
 
 int
 main(int argc, char *argv[])
 {
+	test("sll",
 		char *a = "one";
-	char *b = "two";
-	ds_sll_t *ll = ds_sll_new_node(a); // [ "one" ]
-	ds_sll_insert(ll, b); // [ "one", "two" ]
-	ds_ll_foreach(ds_sll_t, ll) {
-		puts(cur->data);
-	}
-	ds_sll_remove(&ll, 1234);
-	char *pa = ds_sll_remove(&ll, 0); // [ "two" ]
-	char *pb = ds_sll_remove(&ll, 0); // [ ]
 
-	struct complex *a2 = calloc(1, sizeof(struct complex));
-	a2->num = 1;
-	a2->str = "abc";
-	struct complex *b2 = calloc(1, sizeof(struct complex));
-	b2->num = 2;
-	b2->str = "def";
+		ds_sll_t *tmp;
+		it("ensures creates a new linked list", tmp = ds_sll_init());
+		free(tmp);
+
+		ds_sll_t *ll = ds_sll_new_node(a);
+		it("inserts an invalid item", ds_sll_insert(ll, NULL) == -1);
+		it("removes an invalid index", ds_sll_remove(&ll, 1234) == NULL);
+		it("removes an item", a == ds_sll_remove(&ll, 0));
+	);
+
+	test("dll",
+		char *a = "one";
+
+		ds_dll_t *tmp;
+		it("ensures creates a new linked list", tmp = ds_dll_init());
+		free(tmp);
+
+		ds_dll_t *ll = ds_dll_new_node(a);
+		it("inserts an invalid item", ds_dll_insert(ll, NULL) == -1);
+		it("removes an invalid index", ds_dll_remove(&ll, 1234) == NULL);
+		it("removes an item", a == ds_dll_remove(&ll, 0));
+	);
+
+	test("hmp",
+		complex *a = calloc(1, sizeof(complex));
+		a->num = 1;
+		a->str = "abc";
 
 		ds_hmp_t *hmp = ds_hmp_init(101);
-	ds_hmp_insert(hmp, a2->str, a2); // [ (23)[ [ a->str, a ] ] ]
-	ds_hmp_insert(hmp, b2->str, b2); // [ (23)[ [ a->str, a ] ], (58)[ [ b->str, b ] ]
-	struct complex *pa2 = ds_hmp_remove(hmp, a2->str); // [ (23)[ [ a->str, a ] ]
-	struct complex *pb2 = ds_hmp_remove(hmp, b2->str); // [ (58)[ [ b->str, b ] ]
-	ds_hmp_free(&hmp);
-	puts(pa2->str);
-	puts(pb2->str);
+		it("inserts an item", ds_hmp_insert(hmp, a->str, a) == 0);
+		it("generates a hash", _ds_hmp_gen_hash("a") == 177670);
+		it("removes a non-existent item", ds_hmp_remove(hmp, "???") == NULL);
+		it("removes an invalid item", ds_hmp_remove(hmp, NULL) == NULL);
+		it("removes an item", a == ds_hmp_remove(hmp, a->str));
+		ds_hmp_free(&hmp, NULL);
 
-	free(pa2);
-	free(pb2);
+		free(a);
+	);
 }