From c0991eb012b13c2c587173221b6cfe369f1b264b Mon Sep 17 00:00:00 2001 From: Mark Watts Date: Sat, 21 Feb 2015 15:58:47 -0600 Subject: [PATCH] Replaced 'Trie' component with 'GNode' in Stage - see #34 --- key.c | 7 ++ key.h | 1 + stage.c | 126 +++++++++++++++++++++--------- stage.h | 12 +-- tagdb_fs.lc | 19 +++-- tests/acceptance_test.pl | 14 ++++ tests/test_stage.lc | 160 +++++++++++++++++++++------------------ 7 files changed, 217 insertions(+), 122 deletions(-) diff --git a/key.c b/key.c index b7ce98d..27972b2 100644 --- a/key.c +++ b/key.c @@ -37,6 +37,13 @@ void key_push_end (tagdb_key_t k, key_elem_t e) g_array_append_val(k, e); } +key_elem_t key_pop_front (tagdb_key_t k) +{ + key_elem_t res = g_array_index(k, key_elem_t, 0); + g_array_remove_index(k, 0); + return res; +} + tagdb_key_t key_copy (tagdb_key_t k) { tagdb_key_t res = key_new(); diff --git a/key.h b/key.h index 096b218..aad1b1d 100644 --- a/key.h +++ b/key.h @@ -17,6 +17,7 @@ tagdb_key_t make_key (key_elem_t *args, int nkeys); tagdb_key_t key_copy (tagdb_key_t k); void key_destroy (tagdb_key_t k); void key_push_end (tagdb_key_t k, key_elem_t e); +key_elem_t key_pop_front (tagdb_key_t k); key_elem_t key_ref (tagdb_key_t k, int index); int key_is_empty (tagdb_key_t k); void key_sort (tagdb_key_t k, GCompareFunc c); diff --git a/stage.c b/stage.c index 84ba591..988c94a 100644 --- a/stage.c +++ b/stage.c @@ -4,73 +4,131 @@ #define trie_index(__t) ((__t)->index) +/* Modifies the key given */ +GNode *_node_lookup(GNode *n, tagdb_key_t position); + /* Staging tags created with mkdir */ Stage *new_stage () { Stage *res = g_try_malloc0(sizeof(Stage)); - trie_index(res) = g_hash_table_new(g_direct_hash, g_direct_equal); if (!res) return NULL; - res->data = new_trie(); + res->tree = g_node_new(TO_SP(-1)); return res; } -AbstractFile* stage_lookup (Stage *s, tagdb_key_t position, file_id_t id) +gboolean stage_lookup (Stage *s, tagdb_key_t position, file_id_t id) { - return trie_retrieve(s->data, position, TO_SP(id)); + tagdb_key_t lookup_key = key_copy(position); + key_push_end(lookup_key, id); + GNode *n = _node_lookup(s->tree, lookup_key); + key_destroy(lookup_key); + if (n) + { + return (id == TO_S(n->data)); + } + return FALSE; } -void stage_add (Stage *s, tagdb_key_t position, AbstractFile *item) +GNode *_node_lookup (GNode *n, tagdb_key_t position) { - - tagdb_key_t index_key = key_copy(position); - key_push_end(index_key, get_file_id(item)); - KL(index_key, i) + if (key_is_empty(position)) { - gpointer k = TO_SP(key_ref(index_key, i)); - GList *l = g_hash_table_lookup(trie_index(s), k); - l = g_list_prepend(l, key_copy(position)); - g_hash_table_insert(trie_index(s), k, l); + return n; } - key_destroy(index_key); - trie_insert(s->data, position, TO_SP(get_file_id(item)), item); + key_elem_t child_id = key_pop_front(position); + GNode *child = g_node_find_child(n, G_TRAVERSE_ALL, TO_SP(child_id)); + + if (child) + { + return _node_lookup(child, position); + } + else + { + return child; + } } -void trie_remove_by_bucket_key (Stage *t, AbstractFile *s) +GNode *_node_lookup_create (GNode *n, tagdb_key_t position) { - gpointer id = TO_SP(get_file_id(s)); - GList *l = g_hash_table_lookup(trie_index(t), id); - LL(l, it) + if (key_is_empty(position)) + { + return n; + } + + key_elem_t child_id = key_pop_front(position); + GNode *child = g_node_find_child(n, G_TRAVERSE_ALL, TO_SP(child_id)); + + if (child) + { + return _node_lookup_create(child, position); + } + else { - stage_remove(t, it->data, s); - }LL_END; - g_hash_table_remove(trie_index(t), id); - g_list_free_full(l, (GDestroyNotify)key_destroy); + GNode *new_child = g_node_new(TO_SP(child_id)); + g_node_insert(n, 0, new_child); + return _node_lookup_create(new_child, position); + } } -void stage_remove (Stage *s, tagdb_key_t position, AbstractFile *f) +gboolean stage_add (Stage *s, tagdb_key_t position, file_id_t item) { - trie_remove(s->data, position, TO_SP(get_file_id(f))); + tagdb_key_t lookup_key = key_copy(position); + key_push_end(lookup_key, item); + _node_lookup_create(s->tree, lookup_key); + key_destroy(lookup_key); + return TRUE; +} + +gboolean stage_remove (Stage *s, tagdb_key_t position, file_id_t id) +{ + tagdb_key_t lookup_key = key_copy(position); + key_push_end(lookup_key, id); + GNode *n = _node_lookup(s->tree, lookup_key); + gboolean res = FALSE; + if (n) + { + g_node_destroy(n); + res = TRUE; + } + key_destroy(lookup_key); + return res; } -void stage_remove_tag (Stage *s, AbstractFile *t) +void stage_remove_all (Stage *s, file_id_t id) { - trie_remove_by_bucket_key(s, t); + GNode *n = s->tree; + while (n) + { + n = g_node_find(s->tree, G_IN_ORDER, G_TRAVERSE_ALL, TO_SP(id)); + if (n) + { + g_node_destroy(n); + } + } } GList *stage_list_position (Stage *s, tagdb_key_t position) { - return trie_retrieve_bucket_l(s->data, position); + tagdb_key_t lookup_key = key_copy(position); + GNode *n = _node_lookup(s->tree, lookup_key); + GList *res = NULL; + if (n) + { + GNode *it = n->children; + while (it) + { + res = g_list_prepend(res, it->data); + it = it->next; + } LL_END; + } + key_destroy(lookup_key); + return res; } void stage_destroy (Stage *s) { - HL(trie_index(s), it, k, v) - { - g_list_free_full(v, (GDestroyNotify) key_destroy); - } HL_END - g_hash_table_destroy(trie_index(s)); - trie_destroy(s->data); + g_node_destroy(s->tree); g_free(s); } diff --git a/stage.h b/stage.h index b381f50..68d1579 100644 --- a/stage.h +++ b/stage.h @@ -1,12 +1,12 @@ #ifndef STAGE_H #define STAGE_H -#include "trie.h" #include "tag.h" +#include "key.h" typedef struct { - Trie *data; + GNode *tree; GHashTable *index; } Stage; @@ -14,15 +14,15 @@ Stage *new_stage (); /* Adds the given name to the stage at the given posiiton */ -void stage_add (Stage *s, tagdb_key_t position, AbstractFile *item); +gboolean stage_add (Stage *s, tagdb_key_t position, file_id_t item); /* Removes the given name from the given position */ -void stage_remove (Stage *s, tagdb_key_t position, AbstractFile *f); +gboolean stage_remove (Stage *s, tagdb_key_t position, file_id_t f); GList *stage_list_position (Stage *s, tagdb_key_t position); -AbstractFile* stage_lookup (Stage *s, tagdb_key_t position, file_id_t id); -void stage_remove_tag (Stage *s, AbstractFile *f); +gboolean stage_lookup (Stage *s, tagdb_key_t position, file_id_t id); +void stage_remove_all (Stage *s, file_id_t f); void stage_destroy (Stage *s); #endif /* STAGE_H */ diff --git a/tagdb_fs.lc b/tagdb_fs.lc index 082d406..e4a8e8e 100644 --- a/tagdb_fs.lc +++ b/tagdb_fs.lc @@ -274,7 +274,7 @@ int _move_directory(const char *path, const char *newpath) { set_tag_name(DB, t, newbase); tagdb_key_t key = path_extract_key(newdir); - stage_add(STAGE, key, (AbstractFile*)t); + stage_add(STAGE, key, tag_id(t)); key_destroy(key); } } @@ -542,7 +542,7 @@ int make_a_file_and_return_its_real_path(const char *path, char **result) } } tagdb_key_t key = path_extract_key(dir); - stage_add(STAGE, key, (AbstractFile*)t); + stage_add(STAGE, key, tag_id(t)); key_destroy(key); tagdb_end_transaction(DB); } @@ -575,9 +575,9 @@ int make_a_file_and_return_its_real_path(const char *path, char **result) } file_id_t tag_id = tag_id(t); - stage_remove(STAGE, key, (AbstractFile *)t); - stage_remove_tag(STAGE, (AbstractFile *)t); - assert(stage_lookup(STAGE, key, tag_id) == NULL); + /*stage_remove(STAGE, key, (AbstractFile *)t);*/ + stage_remove_all(STAGE, tag_id); + assert(!stage_lookup(STAGE, key, tag_id)); if (t) { tagdb_begin_transaction(DB); @@ -756,11 +756,14 @@ if (_readdir_list_file(filler, buffer, (const char*)(__name)))\ t = get_tags_list(DB, tags); } - - if (tags) { - s = stage_list_position(STAGE, tags); + GList *tmp = stage_list_position(STAGE, tags); + LL(tmp, it) + { + s = g_list_prepend(s, retrieve_tag(DB, TO_S(it->data))); + } LL_END + g_list_free(tmp); } seen = g_hash_table_new(g_str_hash, g_str_equal); diff --git a/tests/acceptance_test.pl b/tests/acceptance_test.pl index 5269de3..4f54d92 100755 --- a/tests/acceptance_test.pl +++ b/tests/acceptance_test.pl @@ -1104,6 +1104,20 @@ sub cleanupTestDir ok(rename($g, $h), "file rename succeeded"); ok((-f $hh), "renamed file exists"); }, + move_directories_beneath_each_other => + sub { + # See issue #34 + my $f = "$testDirName/a"; + my $g = "$testDirName/b"; + my $fg = "$testDirName/a/b"; + my $gf = "$testDirName/b/a"; + mkdir($f); + mkdir($g); + ok(rename($f, $gf), "Rename $f to $gf succeeds"); + ok(rename($g, $fg), "Rename $g to $fg succeeds"); + ok((-d $fg), "$fg exists"); + ok((-d $gf), "$gf exists"); + }, ); my %tests = @tests_list; diff --git a/tests/test_stage.lc b/tests/test_stage.lc index 3107e57..4377a74 100644 --- a/tests/test_stage.lc +++ b/tests/test_stage.lc @@ -10,9 +10,7 @@ %(test Stage lookup) { Stage *s = new_stage(); - char *s1 = "boop"; - AbstractFile *f = malloc(sizeof(AbstractFile)); - abstract_file_init(f, s1); + file_id_t f = 0; tagdb_key_t k = key_new(); key_push_end(k, 23ll); @@ -20,18 +18,15 @@ key_push_end(k, 1ll); stage_add(s, k, f); - AbstractFile* p = stage_lookup(s, k, 0); - CU_ASSERT_STRING_EQUAL(p->name, s1); - free(f); + CU_ASSERT_TRUE(stage_lookup(s, k, 0)); key_destroy(k); stage_destroy(s); } -%(test Stage lookup_2) +%(test Stage lookup_jumbled_key_fails) { Stage *s = new_stage(); - AbstractFile *f = malloc(sizeof(AbstractFile)); - abstract_file_init(f, "boop"); + file_id_t f = 0; tagdb_key_t k = key_new(); key_push_end(k, 3ll); @@ -43,19 +38,16 @@ key_push_end(j, 3ll); key_push_end(j, 23ll); key_push_end(j, 1ll); - AbstractFile* p = stage_lookup(s, j, 0); - CU_ASSERT_NULL(p); + CU_ASSERT_FALSE(stage_lookup(s, j, 0)); stage_destroy(s); - free(f); key_destroy(k); key_destroy(j); } -%(test Stage lookup_3) +%(test Stage lookup_partial_key_fails) { Stage *s = new_stage(); - AbstractFile *f = malloc(sizeof(AbstractFile)); - abstract_file_init(f, "boop"); + file_id_t f = 0; tagdb_key_t k = key_new(); key_push_end(k, 3ll); @@ -66,100 +58,90 @@ tagdb_key_t j = key_new(); key_push_end(j, 3ll); key_push_end(j, 23ll); - AbstractFile* p = stage_lookup(s, j, 0); - CU_ASSERT_NULL(p); + CU_ASSERT_FALSE(stage_lookup(s, j, 0)); stage_destroy(s); - free(f); key_destroy(k); key_destroy(j); } -%(test Stage lookup_4) +%(test Stage lookup_key_with_additional_parts_fails) { Stage *s = new_stage(); - AbstractFile f; - abstract_file_init(&f, "boop"); + file_id_t f = 0; tagdb_key_t k = key_new(); key_push_end(k, 3ll); key_push_end(k, 1ll); key_push_end(k, 23ll); - stage_add(s, k, &f); + stage_add(s, k, f); tagdb_key_t j = key_new(); key_push_end(j, 3ll); key_push_end(j, 1ll); key_push_end(j, 23ll); key_push_end(j, 43ll); - AbstractFile* p = stage_lookup(s, j, 0); - CU_ASSERT_NULL(p); + CU_ASSERT_FALSE(stage_lookup(s, j, 0)); stage_destroy(s); key_destroy(k); key_destroy(j); } -%(test Stage insert_same_id) +%(test Stage insert_same_id_doesnt_duplicate_entries) { - /* only one shall rule them all! - * (Only one is stored when they share an id) - */ Stage *s = new_stage(); - char *s1 = "boop"; - AbstractFile f; - AbstractFile g; - abstract_file_init(&f, s1); - abstract_file_init(&g, s1); + file_id_t f = 0; tagdb_key_t k = key_new(); key_push_end(k, 3ll); key_push_end(k, 1ll); key_push_end(k, 23ll); - stage_add(s, k, &f); - stage_add(s, k, &g); + stage_add(s, k, f); + stage_add(s, k, f); + GList* p = stage_list_position(s, k); + CU_ASSERT_EQUAL(g_list_length(p), 1); + g_list_free(p); stage_destroy(s); key_destroy(k); } -%(test Stage remove_1) +%(test Stage remove_basic_succeeds) { Stage *s = new_stage(); - char *s1 = "boop"; - AbstractFile f; - abstract_file_init(&f, s1); + file_id_t f = 0; + tagdb_key_t k = key_new(); key_push_end(k, 3ll); key_push_end(k, 1ll); key_push_end(k, 2ll); - stage_add(s, k, &f); - stage_remove(s, k, &f); - GList *l = stage_list_position(s,k); + stage_add(s, k, f); + + stage_remove(s, k, f); + + GList *l = stage_list_position(s, k); CU_ASSERT_NULL(l); g_list_free(l); stage_destroy(s); key_destroy(k); } -%(test Stage remove_2) +%(test Stage remove_basic_succeeds_2) { Stage *s = new_stage(); - AbstractFile f; - AbstractFile g; - abstract_file_init(&f, "free"); - abstract_file_init(&g, "junk"); - set_file_id(&g, 1); + file_id_t f = 0; + file_id_t g = 1; tagdb_key_t k = key_new(); key_push_end(k, 3ll); key_push_end(k, 1ll); key_push_end(k, 2ll); - stage_add(s, k, &f); - stage_add(s, k, &g); - stage_remove(s, k, &f); - stage_remove(s, k, &g); + stage_add(s, k, f); + stage_add(s, k, g); + stage_remove(s, k, f); + stage_remove(s, k, g); GList *l = stage_list_position(s,k); CU_ASSERT_NULL(l); g_list_free(l); @@ -167,25 +149,29 @@ key_destroy(k); } -%(test Stage remove_1_from_2) +%(test Stage remove_one_from_two) { Stage *s = new_stage(); - AbstractFile f; - AbstractFile g; - abstract_file_init(&f, "free"); - abstract_file_init(&g, "junk"); - set_file_id(&g, 1); + file_id_t f = 0; + file_id_t g = 1; tagdb_key_t k = key_new(); key_push_end(k, 3ll); key_push_end(k, 1ll); key_push_end(k, 2ll); - stage_add(s, k, &f); - stage_add(s, k, &g); - stage_remove(s, k, &f); - GList *l = stage_list_position(s,k); - CU_ASSERT_STRING_EQUAL(abstract_file_get_name(l->data), "junk"); + stage_add(s, k, f); + stage_add(s, k, g); + stage_remove(s, k, f); + GList *l = stage_list_position(s, k); + if (l) + { + CU_ASSERT_EQUAL(TO_S(l->data), g); + } + else + { + CU_FAIL(); + } g_list_free(l); stage_destroy(s); key_destroy(k); @@ -194,12 +180,19 @@ %(test Stage insert_empty_key) { Stage *s = new_stage(); - AbstractFile f; - abstract_file_init(&f, "free"); + file_id_t f = 0; tagdb_key_t k = key_new(); - stage_add(s, k, &f); + stage_add(s, k, f); GList *l = stage_list_position(s,k); - CU_ASSERT_STRING_EQUAL(abstract_file_get_name(l->data), "free"); + printf("insert_empty_key l = %p\n", l); + if (l) + { + CU_ASSERT_EQUAL(TO_S(l->data), 0); + } + else + { + CU_FAIL(); + } g_list_free(l); stage_destroy(s); key_destroy(k); @@ -207,26 +200,45 @@ %(test Stage memory_leak) { - /* This one was discoverd in - * acceptance_test.pl - * this is a more minimal + /* This one was discovered in + * acceptance_test.pl. + * + * This is a more minimal * reproduction */ Stage *s = new_stage(); - AbstractFile f; - abstract_file_init(&f, "free"); + file_id_t f = 0; tagdb_key_t k = key_new(); key_push_end(k, 0ll); - stage_add(s, k, &f); + stage_add(s, k, f); - stage_remove_tag(s,&f); + stage_remove_all(s, f); stage_destroy(s); key_destroy(k); } +%(test Stage issue_34) +{ + Stage *s = new_stage(); + file_id_t f = 0; + file_id_t g = 1; + + tagdb_key_t k = key_new(); + key_push_end(k, 0ll); + stage_add(s, k, f); + + tagdb_key_t j = key_new(); + key_push_end(j, 1ll); + stage_add(s, j, g); + + key_destroy(k); + key_destroy(j); + stage_destroy(s); +} + int main () { %(run_tests);