rbtree: changeing design. now using callbacks

src/inotify.c

@@ -35,13 +35,12 @@ typedef struct inotify_event inoev;
 static int fd = 0;
 
 /* redblack tree */
-static rbtree tree;
+static rbtree tree = RBTREE_INIT(free, NULL, strcmp);
 
 static queue_t event_queue;
 
 static int addwatch(const char *path, const char *name) {
 	
-	rbnode *node;
 	char   *npath;
 	int     wd;
     
@@ -54,33 +53,22 @@ static int addwatch(const char *path, const char *name) {
 		return -1;
 	}
 
-	/* we must update to not introduce duplicated wd's (keys) */
+    rbtree_insert(&tree, wd, npath);
-	node = rbtree_search(&tree, wd);
+    
-	
+    dprint("added wd = %i on %s\n", wd, npath);
-	if (node == NULL) {
-		dprint("added wd = %i on %s\n", wd, npath);
-		rbtree_insert(&tree, wd, (void*)npath, strlen(npath)+1);
-	} else {
-		dprint("updated wd = %i from %s to %s\n", wd, (char*)node->data, npath);
-		free(node->data);
-		node->data = (void*) npath;
-        node->len = strlen(npath)+1;
-	}
 	
 	return wd;
 }
 
 static int rmwatch(unsigned int wd) {
 
-	void *data = rbtree_delete(&tree, wd);
+	int ret = rbtree_delete(&tree, wd);
 	
-	if (data == NULL)
+	if (!ret)
 		return 0;
 	
-	dprint("rmwatch: %i %s\n", wd, (char*) data);
     inotify_rm_watch(fd, wd);
-    if (data)
+
-        free(data);
 	return 1;
 }
 
@@ -222,7 +210,7 @@ void notify_exit() {
 
 	fd = 0;
     
-	rbtree_free(&tree, NULL);
+	rbtree_free(&tree);
 	
 	if (event_queue) {
 		queue_destroy(event_queue);
@@ -245,7 +233,7 @@ int notify_add_watch(const char *path) {
 
 int notify_rm_watch(const char *path) {
 	
-	rbnode *node = rbtree_cmp_search(&tree, (void*)path, strlen(path));
+	rbnode *node = rbtree_cmp_search(&tree, path);
 	
 	if (node == NULL) {
 		dprint("remove watch: can't find %s\n", path);

src/rbtree.c

@@ -18,13 +18,12 @@
 #define is_red(n) ((n) != NULL && (n)->color == RB_RED)
 #define swap(n,d,q) ((n)->child[(n)->child[d] == (q)])
 
-static rbnode* node_alloc(uint key, void *ptr, size_t len) {
+static rbnode* node_alloc(uint key, void *data) {
 	
 	rbnode *n = xmalloc(sizeof(rbnode));
 	
 	n->key      = key;
-	n->data     = ptr;
+	n->data     = data;
-    n->len      = len;
 	n->color    = RB_RED;
 	n->child[0] = NULL;
 	n->child[1] = NULL;
@@ -35,22 +34,18 @@ static rbnode* node_alloc(uint key, void *ptr, size_t len) {
 /*
  * Recursivly deallocate a tree.
  */
-static void node_dealloc(rbnode *n, void (*action)(rbnode *)) {
+static void node_dealloc(rbnode *n, void (*dealloc)(void *)) {
 	
 	if (!n)
 		return;
 
-	if (action) {
+    if (dealloc)
-		action(n);
+        dealloc(n->data);
-	} else if (n->data) {
-		xfree(n->data);
-        n->data = NULL;
-    }
 	
-	node_dealloc(n->child[0], action);
+	node_dealloc(n->child[0], dealloc);
-	node_dealloc(n->child[1], action);
+	node_dealloc(n->child[1], dealloc);
 	
-	xfree(n);
+	free(n);
 }
 
 /*
@@ -61,40 +56,23 @@ static void rbwalk(rbnode *n, void (*action)(rbnode *)) {
 	if (n == NULL)
 		return;
 	
-	action(n);
-	
 	rbwalk(n->child[0], action);
+    action(n);
 	rbwalk(n->child[1], action);
 }
 
-static inline int datacmp(void *d1, void *d2, size_t l) {
+static rbnode* rbcmp(rbnode *n, int (*cmp_fn)(const char *, const char *), const void *cmpdata) {
 
-    if (d1 == NULL || d2 == NULL)
+    if (!n)
-        return d1 == d2;
+        return NULL;
-        
-    return !memcmp(d1, d2, l);
-}
 
-/*
+    if (cmp_fn(n->data, cmpdata) == 0)
- * Compares every node's data member with cmpdata along the
+        return n;
- * path. comparison is done at memory level and returns the first node that match.
- */
-static rbnode* rbcmp(rbnode *n, void *cmpdata, size_t len) {
-	
-	rbnode *r;
-	
-	if (!n)
-		return NULL;
 
-    dprint("CMP %s - %s\n", (char*)n->data, (char*)cmpdata);
+    rbnode *r = rbcmp(n->child[0], cmp_fn, cmpdata);
-
-	if (datacmp(n->data, cmpdata, len))
-		return n;
 	
-	r = rbcmp(n->child[0], cmpdata, len);
+	if (!r)
-	
+		r = rbcmp(n->child[1], cmp_fn, cmpdata);
-	if (r == NULL)
-		r = rbcmp(n->child[1], cmpdata, len);
 	
 	return r;
 }
@@ -147,42 +125,32 @@ rbnode* rbtree_search(rbtree *tree, uint key) {
 	return n;
 }
 
-rbnode* rbtree_cmp_search(rbtree *tree, void *cmpdata, size_t len) {
+rbnode* rbtree_cmp_search(rbtree *tree, const void *cmpdata) {
 	
-	if (tree == NULL)
+	if (tree == NULL || tree->cmp_fn == NULL)
 		return NULL;
 				
-	return rbcmp(tree->root, cmpdata, len);
+	return rbcmp(tree->root, tree->cmp_fn, cmpdata);
 }
 
 void rbtree_walk(rbtree *tree, void (*action)(rbnode *)) {
 	
-	if (tree == NULL)
+	if (tree == NULL || action == NULL)
 		return;
 			
 	rbwalk(tree->root, action);
 }
 
-void rbtree_free(rbtree *tree, void (*action)(rbnode *)) {
+void rbtree_free(rbtree *tree) {
 		
 	if (tree == NULL)
 		return;
 	
-	node_dealloc(tree->root, action);
+	node_dealloc(tree->root, tree->delete_fn);
 	tree->root = NULL;
 }
 
-/*
+int rbtree_insert(rbtree *tree, uint key, void *data) {
- * duplicate keys result in the tree remains unchanged
- * this can cause memory leaks as data fields can (should) 
- * be heap allocated, and the client expects us to keep track of it.
- *
- * for general purposes, we should notify client about it so 
- * then they can choose what to do
- *
- * the function now returns -1 in that situation // H Hautakoski
- */
-int rbtree_insert(rbtree *tree, uint key, void *data, size_t len) {
 	
 	rbnode head = {0};
 	
@@ -195,9 +163,7 @@ int rbtree_insert(rbtree *tree, uint key, void *data, size_t len) {
 	unsigned char dir = 0, dir2, last, inserted = 0;
 	
 	if (tree->root == NULL) {
-		tree->root = node_alloc(key, data, len);
+		tree->root = node_alloc(key, data);
-		if (tree->root == NULL)
-			return 0;
         inserted = 1;
         goto done;
 	}
@@ -210,9 +176,7 @@ int rbtree_insert(rbtree *tree, uint key, void *data, size_t len) {
 		
 	for(;;) {
 		if (q == NULL) {
-			p->child[dir] = q = node_alloc(key, data, len);
+			p->child[dir] = q = node_alloc(key, data);
-			if (q == NULL)
-				return 0;
             inserted = 1;
 		} else if (is_red(q->child[0]) && is_red(q->child[1])) {
 			/* color flip case */
@@ -241,20 +205,25 @@ int rbtree_insert(rbtree *tree, uint key, void *data, size_t len) {
 		g = p, p = q;
 		q = q->child[dir];
 	}
-		
+
+    if (!inserted) {
+        if (tree->update_fn)
+            tree->update_fn(q->data, data);
+        if (tree->delete_fn)
+            tree->delete_fn(q->data);
+        q->data = data;
+    }
+
 	tree->root = head.child[1];
-    
+
 done:
 	/* root should be black */
 	tree->root->color = RB_BLACK;
-	
+
-    if (!inserted)
+    return inserted;
-        return -1;
-    
-	return 1;
 }
 
-void* rbtree_delete(rbtree *tree, uint key) {
+int rbtree_delete(rbtree *tree, uint key) {
 	
 	rbnode head = {0};
 	
@@ -264,14 +233,10 @@ void* rbtree_delete(rbtree *tree, uint key) {
 	/* found item */
 	rbnode *f = NULL;
 	
-	/* pointer to the data member of the node we delete,
-       returned so it can be free'd */
-	void *ret = NULL;
-	
 	unsigned char dir = 1, dir2, last;
 	
 	if (rbtree_is_empty(tree))
-		return NULL;
+		return 0;
 	
 	q = &head;
 	g = p = NULL;
@@ -324,15 +289,16 @@ void* rbtree_delete(rbtree *tree, uint key) {
 	
 	/* remove if found */
 	if (f) {
-        ret = f->data;
+        if (tree->delete_fn)
+            tree->delete_fn(f->data);
+
         if (f != q) {
             f->key  = q->key;
             f->data = q->data;
-            f->len  = q->len;
         }
         swap(p, 1, q) = swap(q, 0, NULL);
         xfree(q);
+        return 1;
 	}
-	
+	return 0;
-	return ret;
 }

src/rbtree.h

@@ -22,28 +22,34 @@ typedef unsigned int uint;
 /* node definition */
 typedef struct _rbn {
 	uint 		 key;
-	void		*data;
+    void        *data;
-    size_t       len;
 	struct _rbn *child[2];
 	color_t      color;
 } rbnode;
 
 typedef struct {
 	rbnode *root;
+    /* user defined operations */
+    void (*delete_fn)(void *);
+    void (*update_fn)(void *, void *);
+    /* note: char* is used here to make derefernce easier inside the function */
+    int  (*cmp_fn)(const char *, const char *);
 } rbtree;
 
+#define RBTREE_INIT(delete, update, cmp) { NULL, delete, update, cmp}
+
 int rbtree_is_empty(rbtree *tree);
 
 rbnode* rbtree_search(rbtree *tree, uint key);
 
-rbnode* rbtree_cmp_search(rbtree *tree, void *cmpdata, size_t len);
+rbnode* rbtree_cmp_search(rbtree *tree, const void *data);
 
 void rbtree_walk(rbtree *tree, void (*action)(rbnode *));
 
-void rbtree_free(rbtree *tree, void (*action)(rbnode *));
+void rbtree_free(rbtree *tree);
 
-int rbtree_insert(rbtree *tree, uint key, void *data, size_t len);
+int rbtree_insert(rbtree *tree, uint key, void *data);
 
-void* rbtree_delete(rbtree *tree, uint key);
+int rbtree_delete(rbtree *tree, uint key);
 
 #endif /* __RBTREE_H */

test/t_rbtree.c

@@ -5,6 +5,7 @@
  * function is done.
  */
 
+#include <string.h>
 #include <stdio.h>
 #include <stdlib.h>
 #include "unit.h"
@@ -14,7 +15,7 @@
 #define MAX_VAL 500
 #define NODES   20
 
-#define is_red(n) (n != NULL && n->color == RB_RED)
+#define is_red(n) ((n) != NULL && (n)->color == RB_RED)
 
 static int rb_assert(rbnode *node) {
 	
@@ -58,12 +59,76 @@ static int rb_assert(rbnode *node) {
 	return 0;
 }
 
+static int vcmp(const char *a, const char *b);
+static void vfree(void *ptr);
+static void vupdate(void *old, void *new);
+
 /* data */
-static rbtree tree;
+static rbtree tree = RBTREE_INIT(vfree, vupdate, vcmp);
-static uint  keyref[NODES];
+static int   keyref[NODES];
 static char *dataref[NODES];
-static uint search_key = -1;
+
-static char search_data[32];
+static int vcmp(const char *a, const char *b) {
+
+    printf("cmp: %s -> %s\n", a, b); 
+    
+    return strcmp(a, b);
+}
+
+static void vupdate(void *old, void *new) {
+
+    int i;
+
+    printf("update: %s -> %s\n", (char*)old, (char*)new);
+
+    for(i=0; i < NODES; i++) {
+
+        if (keyref[i] == -1)
+            continue;
+        
+        if (old == dataref[i]) {
+            dataref[i] = new;
+            break;
+        }
+    }
+}
+
+static void vfree(void *ptr) {
+
+    int i;
+
+    printf("free: %s\n", (char*)ptr);
+
+    for(i=0; i < NODES; i++) {
+
+        if (keyref[i] == -1)
+            continue;
+        
+        if (ptr == dataref[i]) {
+            keyref[i]  = -1;
+            dataref[i] = NULL;
+            break;
+        }
+    }
+
+    free(ptr);
+}
+
+static void vwalk(rbnode *node) {
+
+    int i, found = 0;
+    
+    /* check if this node exist in the reference list */
+    for(i=0; i < NODES; i++) {
+        
+        if (node->key == keyref[i] && node->data == dataref[i]) {
+            found = 1;
+            break;
+        }
+    }
+
+    assert(found);
+}
 
 void test_insert() {
 
@@ -77,79 +142,95 @@ void test_insert() {
 	while(i < NODES) {
 		
 		ckey = (uint) (rand() % MAX_VAL);
-        data = utest_ran_string(32);
+        data = utest_ran_string(16);
 		
         /* insert into rbtree and assert it */
-		ret = rbtree_insert(&tree, ckey, data, 33);
+		ret = rbtree_insert(&tree, ckey, data);
         rb_assert(tree.root);
-        
+
-        dprint("INSERT: %i %s\n", ckey, data);
+        if (!ret)
-        
-        /* ignore duplicate key */
-        if (ret == -1) {
-            free(data);
             continue;
-        }
+        
+        printf("insert: %i %s\n", ckey, data);
         
 		keyref[i] = ckey;
         dataref[i] = data;
         
-		if (i == ((NODES/2))) {
-			search_key = ckey;
-            memcpy(&search_data, data, 33);
-        }
-
         i++;
 	}
-    
+
     /* insert duplicate key */
-    assert(rbtree_insert(&tree, search_key, "duplicate", 10) == -1);
+    rbtree_insert(&tree, keyref[rand() % NODES], strdup("---- update ----"));
     rb_assert(tree.root);
 }
 
 void test_delete() {
 
-    int i;
+    int i, key;
-    uint key;
-    char *data, *dref;
 
-    /* remove some values */	
+    /* remove some values */
 	for(i=0; i < 10; i++) {
 
-        key = keyref[(NODES/2)+i];
+        do
-        dref = dataref[(NODES/2)+i];
+            key = keyref[rand() % NODES];
+        while(key < 0);
 
-	    data = rbtree_delete(&tree, key);
+	    rbtree_delete(&tree, key);
-        assert_string(data, dref);
-        free(data);
         rb_assert(tree.root);
 	}
 }
 
 void test_search() {
 
+    int index;
+
     rbnode *n;
 
+    do
+        index = rand() % NODES;
+    while(keyref[index] < 0);
+
+    printf("search: expecting to find key %i\n", keyref[index]);
+
     /* search for a key we know exists */
-    n = rbtree_search(&tree, search_key);
+    n = rbtree_search(&tree, keyref[index]);
-    
+    rb_assert(tree.root);
+
     assert(n != NULL);
-	assert(n->key == search_key);
+    assert(n->key == keyref[index]);
+    assert_string(n->data, dataref[index]);
+
+    printf("search: expecting to not find key: %i\n", MAX_VAL+512);
 
     /* search for a key we now don't exist */
     n = rbtree_search(&tree, MAX_VAL+512);
+    rb_assert(tree.root);
 
     assert(n == NULL);
 }
 
 void test_cmp_search() {
 
+    char *search;
     rbnode *n;
 
-    n = rbtree_cmp_search(&tree, &search_data, 32);
+    do
+        search = dataref[rand()%NODES];
+    while(search == NULL);
 
+    printf("cmp_search: searching for %s\n", search);
+    
+    n = rbtree_cmp_search(&tree, search);
+    
+    rb_assert(tree.root);
     assert(n != NULL);
-    assert_string(n->data, search_data);
+    assert_string(n->data, search);
+}
+
+void test_walk() {
+
+    rbtree_walk(&tree, vwalk);
+    rb_assert(tree.root);
 }
 
 int main(int argc, char **argv) {
@@ -157,20 +238,27 @@ int main(int argc, char **argv) {
 	tree.root = NULL;
 	
     /* a new tree is empty */
-	assert(rbtree_is_empty(&tree) == 1);
+	assert(rbtree_is_empty(&tree));
 
     test_insert();
+    
     test_search();
     test_cmp_search();
+    test_walk();
+    
     test_delete();
+    
+    test_search();
+    test_cmp_search();
+    test_walk();
 
     /* free the tree */
-    rbtree_free(&tree, NULL);
+    rbtree_free(&tree);
     
 	/* tree is now empty again */
-	assert(rbtree_is_empty(&tree) == 1);
+	assert(rbtree_is_empty(&tree));
 
     printf("-- TEST PASS --\n");
-    
+
 	return 0;
 }