chore: refactor makefiles and fix a few bad bugs

Makefile

@@ -1,78 +1,76 @@
-CC ?= gcc
-AR ?= ar
-LINTER ?= clang-format
-
-LIB := libhash
-
-PREFIX := /usr/local
-INCDIR := $(PREFIX)/include
-LIBDIR := $(PREFIX)/lib
-SRCDIR := src
-DEPSDIR := deps
-TESTDIR := t
-EXAMPLEDIR := examples
-LINCDIR := include
-
-DYNAMIC_TARGET := $(LIB).so
-STATIC_TARGET := $(LIB).a
-EXAMPLE_TARGET := example
-TEST_TARGET := test
-
-SRC := $(wildcard $(SRCDIR)/*.c)
-TEST_DEPS := $(wildcard $(DEPSDIR)/tap.c/*.c)
-DEPS := $(filter-out $(wildcard $(DEPSDIR)/tap.c/*), $(wildcard $(DEPSDIR)/*/*.c))
-OBJ := $(addprefix obj/, $(notdir $(SRC:.c=.o)) $(notdir $(DEPS:.c=.o)))
-
-CFLAGS := -I$(LINCDIR) -I$(DEPSDIR) -Wall -Wextra -pedantic -std=c17
-LIBS := -lm
-
-TESTS := $(wildcard $(TESTDIR)/*.c)
-
-SEPARATOR := ---------------------------
-
-all: $(DYNAMIC_TARGET) $(STATIC_TARGET)
-
+include Makefile.config
+
+.PHONY: all obj install uninstall clean unit_test unit_test_dev valgrind fmt
+.DELETE_ON_ERROR:
+
+PREFIX          := /usr/local
+INCDIR          := $(PREFIX)/include
+LIBDIR          := $(PREFIX)/lib
+SRCDIR          := src
+DEPSDIR         := deps
+TESTDIR         := t
+EXAMPLEDIR      := examples
+INCDIR          := include
+
+DYNAMIC_TARGET  := $(LIBNAME).so
+STATIC_TARGET   := $(LIBNAME).a
+EXAMPLE_TARGET  := example
+TEST_TARGET     := test
+
+SRC             := $(wildcard $(SRCDIR)/*.c)
+TESTS           := $(wildcard $(TESTDIR)/*.c)
+DEPS            := $(filter-out $(wildcard $(DEPSDIR)/libtap/*), $(wildcard $(DEPSDIR)/*/*.c))
+TEST_DEPS       := $(wildcard $(DEPSDIR)/libtap/*.c)
+OBJ             := $(addprefix obj/, $(notdir $(SRC:.c=.o)) $(notdir $(DEPS:.c=.o)))
+
+INCLUDES        := -I$(INCDIR) -I$(DEPSDIR) -I$(SRCDIR)
+LIBS            := -lm
+CFLAGS          := -Wall -Wextra -pedantic -std=c17 $(INCLUDES)
+
+$(DYNAMIC_TARGET): CFLAGS += -shared
 $(DYNAMIC_TARGET): $(OBJ)
-	$(CC) $(CFLAGS) $(OBJ) -shared $(LIBS) -o $(DYNAMIC_TARGET)
+	$(CC) $(CFLAGS) $^ $(LIBS) -o $@
 
 $(STATIC_TARGET): $(OBJ)
-	$(AR) rcs $@ $(OBJ)
+	$(AR) rcs $@ $^
 
-obj/%.o: $(SRCDIR)/%.c $(LINCDIR)/$(LIB).h | obj
+obj/%.o: $(SRCDIR)/%.c $(INCDIR)/$(LIBNAME).h | obj
 	$(CC) $< -c $(CFLAGS) -o $@
 
 obj/%.o: $(DEPSDIR)/*/%.c | obj
 	$(CC) $< -c $(CFLAGS) -o $@
 
+$(EXAMPLE_TARGET): $(STATIC_TARGET)
+	$(CC) $(CFLAGS) $(EXAMPLEDIR)/main.c $< $(LIBS) -o $@
+
+all: $(DYNAMIC_TARGET) $(STATIC_TARGET)
+
 obj:
-	mkdir -p obj
+	@mkdir -p obj
 
 install: $(STATIC_TARGET)
-	mkdir -p ${LIBDIR} && cp -f ${STATIC_TARGET} ${LIBDIR}/$(STATIC_TARGET)
-	mkdir -p ${INCDIR} && cp -r $(LINCDIR)/$(LIB).h ${INCDIR}
+	@mkdir -p ${LIBDIR} && cp -f ${STATIC_TARGET} ${LIBDIR}/$@
+	@mkdir -p ${INCDIR} && cp -r $(INCDIR)/$(LIBNAME).h ${INCDIR}
 
 uninstall:
-	rm -f ${LIBDIR}/$(STATIC_TARGET)
-	rm -f ${INCDIR}/libys.h
-
-$(EXAMPLE_TARGET): $(STATIC_TARGET)
-	$(CC) $(CFLAGS) $(EXAMPLEDIR)/main.c $(STATIC_TARGET) $(LIBS) -o $(EXAMPLE_TARGET)
+	@rm -f ${LIBDIR}/$(STATIC_TARGET)
+	@rm -f ${INCDIR}/libys.h
 
 clean:
-	rm -f $(OBJ) $(STATIC_TARGET) $(DYNAMIC_TARGET) $(EXAMPLE_TARGET) $(TEST_TARGET)
+	@rm -f $(OBJ) $(STATIC_TARGET) $(DYNAMIC_TARGET) $(EXAMPLE_TARGET) $(TEST_TARGET)
 
-test: $(STATIC_TARGET)
-	$(foreach test,$(TESTS),					  																											\
-		$(MAKE) .compile_test file=$(test); 																										\
-		printf "\033[1;32m\nRunning test $(patsubst $(TESTDIR)/%,%,$(test))...\n$(SEPARATOR)\n\033[0m";	\
-		./test;\
- 	)
-	rm $(TEST_TARGET)
+unit_test: $(STATIC_TARGET)
+	$(CC) $(CFLAGS) $(TESTS) $(TEST_DEPS) $(STATIC_TARGET) -I$(SRCDIR) $(LIBS) -o $(TEST_TARGET)
+	./$(TEST_TARGET)
+	$(MAKE) clean
 
-.compile_test:
-	$(CC) $(CFLAGS) $(file) $(TEST_DEPS) $(STATIC_TARGET) -I$(SRCDIR) -I$(DEPSDIR) $(LIBS) -o $(TEST_TARGET)
+unit_test_dev:
+	ls $(SRCDIR)/*.{h,c} $(TESTDIR)/*.{h,c} | entr -s 'make -s unit_test'
 
-lint:
-	$(LINTER) -i $(wildcard $(SRCDIR)/*) $(wildcard $(TESTDIR)/*) $(wildcard $(LINCDIR)/*) $(wildcard $(EXAMPLEDIR)/*)
+valgrind: $(STATIC_TARGET)
+	$(CC) $(CFLAGS) $(TESTS) $(TEST_DEPS) $< $(LIBS) -o $(TEST_TARGET)
+	$(VALGRIND) --leak-check=full --track-origins=yes -s ./$(TEST_TARGET)
+	@$(MAKE) clean
 
-.PHONY: clean test .compile_test all obj install uninstall lint
+fmt:
+	@$(FMT) -i $(wildcard $(SRCDIR)/*) $(wildcard $(TESTDIR)/*) $(wildcard $(INCDIR)/*) $(wildcard $(EXAMPLEDIR)/*)

Makefile.config

@@ -0,0 +1,6 @@
+CC       ?= gcc
+AR       ?= ar
+FMT      ?= clang-format
+VALGRIND ?= valgrind
+
+LIBNAME  := libhash

include/libhash.h

@@ -1,8 +1,10 @@
 #ifndef LIBHASH_H
 #define LIBHASH_H
 
-#define HT_DEFAULT_CAPACITY 50
-#define HS_DEFAULT_CAPACITY 50
+#include "list.h"
+
+#define HT_DEFAULT_CAPACITY 53
+#define HS_DEFAULT_CAPACITY 53
 
 /**
  * A free function that will be invoked a hashmap value any time it is removed.
@@ -51,6 +53,8 @@ typedef struct {
    * however they want.
    */
   free_fn *free_value;
+
+  node_t *occupied_buckets;
 } hash_table;
 
 /**
@@ -108,6 +112,15 @@ void ht_delete_table(hash_table *ht);
  */
 int ht_delete(hash_table *ht, const char *key);
 
+#define HT_ITER_START(ht)               \
+  node_t *head = ht->occupied_buckets;  \
+  while (head != &LIST_SENTINEL_NODE) { \
+    ht_entry *entry = ht->entries[head->value];
+
+#define HT_ITER_END  \
+  head = head->next; \
+  }
+
 typedef struct {
   /**
    * Max number of keys which may be stored in the hash set. Adjustable.

scripts/entrypoint.sh

@@ -1,6 +1,6 @@
 #!/bin/sh -l
 
-ret="$(make -s test 2>/dev/null)"
+ret="$(make -s unit_test 2>/dev/null)"
 
 echo "$ret"
 

src/hash.c

@@ -46,7 +46,9 @@ static int h_hash(const char *key, const int prime, const int capacity) {
  */
 int h_resolve_hash(const char *key, const int capacity, const int attempt) {
   const int hash_a = h_hash(key, H_PRIME_1, capacity);
-  const int hash_b = h_hash(key, H_PRIME_2, capacity);
+  int hash_b = h_hash(key, H_PRIME_2, capacity);
 
-  return (hash_a + attempt * (hash_b == 0 ? 1 : hash_b)) % capacity;
+  // Prevent infinite cycling when hash_b == num buckets.
+  if (hash_b % capacity == 0) hash_b = 1;
+  return (hash_a + (attempt * hash_b)) % capacity;
 }

src/hash_table.c

@@ -25,16 +25,15 @@ static void __ht_delete_table(hash_table *ht);
  * @param base_capacity
  * @return int
  */
-static void ht_resize(hash_table *ht, const int base_capacity) {
-  if (base_capacity < 0) {
-    return;
+static void ht_resize(hash_table *ht, int base_capacity) {
+  if (base_capacity < HT_DEFAULT_CAPACITY) {
+    base_capacity = HT_DEFAULT_CAPACITY;
   }
 
   hash_table *new_ht = ht_init(base_capacity, ht->free_value);
 
   for (unsigned int i = 0; i < ht->capacity; i++) {
     ht_entry *r = ht->entries[i];
-
     if (r != NULL && r != &HT_SENTINEL_ENTRY) {
       __ht_insert(new_ht, r->key, r->value);
     }
@@ -44,15 +43,17 @@ static void ht_resize(hash_table *ht, const int base_capacity) {
   ht->count = new_ht->count;
 
   const int tmp_capacity = ht->capacity;
-
   ht->capacity = new_ht->capacity;
   new_ht->capacity = tmp_capacity;
 
   ht_entry **tmp_entries = ht->entries;
   ht->entries = new_ht->entries;
   new_ht->entries = tmp_entries;
+  ht->occupied_buckets = new_ht->occupied_buckets;
 
-  ht_delete_table(new_ht);
+  // Cannot free values - we may still be using them.
+  free(new_ht->entries);
+  free(new_ht);
 }
 
 /**
@@ -121,47 +122,46 @@ static void __ht_insert(hash_table *ht, const char *key, void *value) {
   ht_entry *new_entry = ht_entry_init(key, value);
 
   int idx = h_resolve_hash(new_entry->key, ht->capacity, 0);
-
   ht_entry *current_entry = ht->entries[idx];
+  // If there was a hash collision, we need to perform double hashing and
+  // partial linear probing by incrementing this index and hashing it until we
+  // find a bucket.
   int i = 1;
-
-  // i.e. if there was a collision
   while (current_entry != NULL && current_entry != &HT_SENTINEL_ENTRY) {
-    // update existing key/value
+    // If the keys match, then we've inserted this key before. Use this bucket.
     if (strcmp(current_entry->key, key) == 0) {
-      ht_delete_entry(current_entry, ht->free_value);
+      ht_delete_entry(current_entry, NULL);
       ht->entries[idx] = new_entry;
-
       return;
     }
 
-    // TODO verify i is 1..
     idx = h_resolve_hash(new_entry->key, ht->capacity, i);
     current_entry = ht->entries[idx];
     i++;
   }
 
   ht->entries[idx] = new_entry;
+  list_prepend(&ht->occupied_buckets, idx);
   ht->count++;
 }
 
 static int __ht_delete(hash_table *ht, const char *key) {
   const int load = ht->count * 100 / ht->capacity;
 
-  if (load < 10) {
+  // TODO: const
+  if (load < 30) {
     ht_resize_down(ht);
   }
 
   int i = 0;
   int idx = h_resolve_hash(key, ht->capacity, i);
 
   ht_entry *current_entry = ht->entries[idx];
-
   while (current_entry != NULL && current_entry != &HT_SENTINEL_ENTRY) {
     if (strcmp(current_entry->key, key) == 0) {
       ht_delete_entry(current_entry, ht->free_value);
       ht->entries[idx] = &HT_SENTINEL_ENTRY;
-
+      list_remove(&ht->occupied_buckets, idx);
       ht->count--;
 
       return 1;
@@ -199,7 +199,7 @@ hash_table *ht_init(int base_capacity, free_fn *free_value) {
   ht->count = 0;
   ht->entries = calloc((size_t)ht->capacity, sizeof(ht_entry *));
   ht->free_value = free_value;
-
+  ht->occupied_buckets = list_node_create_head();
   return ht;
 }
 

src/list.h

@@ -0,0 +1,62 @@
+#ifndef LIBHASH_LIST_H
+#define LIBHASH_LIST_H
+
+#include <stdio.h>
+#include <stdlib.h>
+
+typedef struct node node_t;
+struct node {
+  int value;
+  node_t *next;
+};
+
+static node_t LIST_SENTINEL_NODE = {0, NULL};
+
+static node_t *list_node_create_head(void) { return &LIST_SENTINEL_NODE; }
+
+static node_t *list_node_create(const int value) {
+  node_t *n = (node_t *)malloc(sizeof(node_t));
+  n->value = value;
+  return n;
+}
+
+static void list_prepend(node_t **head, int value) {
+  // TODO: xmalloc
+  node_t *new_node = (node_t *)malloc(sizeof(node_t));
+  new_node->value = value;
+
+  node_t *tmp = *head;
+  *head = new_node;
+  new_node->next = tmp;
+}
+
+static void list_remove(node_t **head, int value) {
+  node_t *current = *head;
+  node_t *prev = NULL;
+
+  while (current != NULL) {
+    if (current->value == value) {
+      if (prev == NULL) {
+        *head = current->next;
+      } else {
+        prev->next = current->next;
+      }
+      free(current);
+      return;
+    }
+    prev = current;
+    current = current->next;
+  }
+}
+
+// TODO: Use and test
+static void free_list(node_t *head) {
+  node_t *tmp;
+  while (head) {
+    tmp = head;
+    head = head->next;
+    free(tmp);
+  }
+}
+
+#endif /* LIBHASH_LIST_H */

src/prime.h

@@ -1,7 +1,7 @@
-#ifndef PRIME_H
-#define PRIME_H
+#ifndef LIBHASH_PRIME_H
+#define LIBHASH_PRIME_H
 
 int is_prime(const int x);
 int next_prime(int x);
 
-#endif /* PRIME_H */
+#endif /* LIBHASH_PRIME_H */