From c99bc830154040545a4e2170467adece3be7151d Mon Sep 17 00:00:00 2001 From: Chewing_Bever Date: Wed, 7 Dec 2022 23:20:39 +0100 Subject: [PATCH] refactor: start using void* with trie --- src/main.cpp | 45 ++++++++++++++++++------------------- trie/include/trie.h | 22 +++++++++++------- trie/src/trie.c | 42 +++++++++++++++++----------------- trie/src/trie_entry.c | 8 +++++++ trie/src/trie_node.c | 11 +++++---- trie/test/CMakeLists.txt | 5 +++++ trie/test/test_trie.c | 5 +++++ trie/test/test_trie_fuzzy.c | 2 +- 8 files changed, 83 insertions(+), 57 deletions(-) diff --git a/src/main.cpp b/src/main.cpp index a36505f..035b32a 100644 --- a/src/main.cpp +++ b/src/main.cpp @@ -125,25 +125,25 @@ int main() { // Serve an entry CROW_ROUTE(app, "/") - .methods(crow::HTTPMethod::Get)( - [trie](crow::response &res, std::string key) { - trie_rlock(trie); - Entry *entry; - TrieExitCode status = trie_search(trie, &entry, key.c_str()); + .methods( + crow::HTTPMethod::Get)([trie](crow::response &res, std::string key) { + trie_rlock(trie); + Entry *entry; + TrieExitCode status = trie_search(trie, (void **)&entry, key.c_str()); - if (status == Ok) { - if (entry->type == Redirect) { - res.redirect(entry->string); - } else if (entry->type == Paste) { - res.set_static_file_info("pastes/" + key); - } - } else { - res.code = 404; - } + if (status == Ok) { + if (entry->type == Redirect) { + res.redirect(entry->string); + } else if (entry->type == Paste) { + res.set_static_file_info("pastes/" + key); + } + } else { + res.code = 404; + } - res.end(); - trie_unlock(trie); - }); + res.end(); + trie_unlock(trie); + }); // Add a new Redirect with a short randomly generated key CROW_ROUTE(app, "/s/") @@ -214,15 +214,14 @@ int main() { TrieExitCode status = trie_add(trie, key.c_str(), new_entry); trie_unlock(trie); - if (status != Ok) { + switch (status) { + case Ok: + return crow::response(base_url + key); + case AlreadyPresent: return crow::response(crow::status::CONFLICT); - } - - if (!store_paste(key.c_str(), req.body.c_str())) { + default: return crow::response(crow::status::INTERNAL_SERVER_ERROR); } - - return crow::response(base_url + key); }); app.port(18080).multithreaded().run(); } diff --git a/trie/include/trie.h b/trie/include/trie.h index c32dadc..613aa68 100644 --- a/trie/include/trie.h +++ b/trie/include/trie.h @@ -1,12 +1,15 @@ #ifndef AD3_TERNARYTRIE #define AD3_TERNARYTRIE -#define ALPHABET_SIZE 256 #define DELIMITER '\0' -#define MAX(x, y) (((x) > (y)) ? (x) : (y)) -// Should not be higher than 254 or stuff will break -#define TRIE_MAX_SKIP_SIZE 8 +// The compiler aligns structs to be multiple of 8 bytes. Therefore, we have a +// few "free" bytes that are allocated whether we use them or not, so we might +// as, hence the seemingly weird number. This is the largest number we can use +// where the size of TernaryTrieNode is still 32 bytes. +// +// NOTE: Should not be higher than 254 or stuff will break. +#define TRIE_MAX_SKIP_SIZE 13 /** * The implementation of a Ternary Trie. @@ -17,6 +20,7 @@ #include #include +#include #include static const char charset[] = @@ -50,6 +54,8 @@ typedef enum trie_exit_code { Entry *entry_new(EntryType type, const char *string); +void entry_free(Entry *entry); + /** * Allocate & initialize a new trie, and populate it with the data from the * given data file. @@ -73,7 +79,7 @@ void trie_free(Trie *trie); * @param key key representing the entry * @return 0 if the search was successful, 1 if not found */ -TrieExitCode trie_search(Trie *trie, Entry **entry_ptr, const char *key); +TrieExitCode trie_search(Trie *trie, void **data_ptr, const char *key); /** * Add a string to this trie. @@ -83,7 +89,7 @@ TrieExitCode trie_search(Trie *trie, Entry **entry_ptr, const char *key); * @param entry entry to add * @return 0 if added, 1 if already in trie, something else if other errors */ -TrieExitCode trie_add(Trie *trie, const char *key, Entry *entry); +TrieExitCode trie_add(Trie *trie, const char *key, void *data); /** * Add an entry by generating a random string as the key. @@ -94,7 +100,7 @@ TrieExitCode trie_add(Trie *trie, const char *key, Entry *entry); * @return pointer to the generated key. This pointer is safe to use after * unlocking the trie, and should be freed manually. */ -TrieExitCode trie_add_random(Trie *trie, char **key_ptr, Entry *entry, +TrieExitCode trie_add_random(Trie *trie, char **key_ptr, void *data, bool secure); /** @@ -113,7 +119,7 @@ bool trie_remove(Trie *trie, const char *key); * @param trie * @return the number of entries in this trie */ -size_t trie_size(Trie *trie); +uint64_t trie_size(Trie *trie); /* * Acquire a read lock on the trie. diff --git a/trie/src/trie.c b/trie/src/trie.c index 9803b79..a0e94a1 100644 --- a/trie/src/trie.c +++ b/trie/src/trie.c @@ -10,12 +10,12 @@ typedef struct ttrie { TrieNode *root; - size_t size; + uint64_t size; char *file_path; pthread_rwlock_t lock; } Trie; -TrieExitCode trie_add_no_lock(Trie *trie, const char *key, Entry *entry); +TrieExitCode trie_add_no_lock(Trie *trie, const char *key, void *data); /** * Allocate and initialize an empty Trie @@ -138,7 +138,7 @@ SearchResult trie_search_node(Trie *trie, const char *key) { // At this point, we've either arrived at an empty child, or traversed through // the entire string. Therefore, all we have to do is check whether we're at // the end of the string and if node represents a string. - if (key[i] == DELIMITER && (*child_ptr)->represents) { + if (key[i] == DELIMITER && (*child_ptr)->data_size > 0) { out.parent = *node_ptr; out.child = *child_ptr; } @@ -153,14 +153,14 @@ SearchResult trie_search_node(Trie *trie, const char *key) { * @param string string to look up * @return true if the string is present in the trie, false otherwise */ -TrieExitCode trie_search(Trie *trie, Entry **entry_ptr, const char *key) { +TrieExitCode trie_search(Trie *trie, void **data_ptr, const char *key) { SearchResult res = trie_search_node(trie, key); if (res.child == NULL) { return NotFound; } - *entry_ptr = res.child->entry; + *data_ptr = res.child->data; return Ok; } @@ -173,7 +173,7 @@ TrieExitCode trie_search(Trie *trie, Entry **entry_ptr, const char *key) { * @return true if the string wasn't present in the trie and thus added, false * otherwise */ -TrieExitCode trie_add_no_lock(Trie *trie, const char *string, Entry *entry) { +TrieExitCode trie_add_no_lock(Trie *trie, const char *string, void *data) { size_t i = 0; uint8_t offset; TrieNode **node_ptr = &(trie->root); @@ -211,8 +211,8 @@ TrieExitCode trie_add_no_lock(Trie *trie, const char *string, Entry *entry) { continue; } - child_node->represents = true; - child_node->entry = entry; + child_node->data_size = sizeof(data); + child_node->data = data; trie->size++; return Ok; @@ -265,17 +265,17 @@ TrieExitCode trie_add_no_lock(Trie *trie, const char *string, Entry *entry) { i += offset; } while (string[i] != DELIMITER); - if ((*child_node_ptr)->represents) { + if ((*child_node_ptr)->data_size > 0) { return AlreadyPresent; } - (*child_node_ptr)->represents = true; - (*child_node_ptr)->entry = entry; + (*child_node_ptr)->data_size = sizeof(data); + (*child_node_ptr)->data = data; trie->size++; return Ok; } -TrieExitCode trie_add(Trie *trie, const char *key, Entry *entry) { +TrieExitCode trie_add(Trie *trie, const char *key, void *entry) { if (trie->file_path != NULL) { // Easiest way to make sure we don't add duplicate entries // We use an internal function that doesn't require a read lock, as we're @@ -290,12 +290,12 @@ TrieExitCode trie_add(Trie *trie, const char *key, Entry *entry) { return FileError; } - fputs(key, fp); - fputs(" ", fp); - fputc(entry_type_to_char(entry->type), fp); - fputs(" ", fp); - fputs(entry->string, fp); - fputs("\n", fp); + /* fputs(key, fp); */ + /* fputs(" ", fp); */ + /* fputc(entry_type_to_char(entry->type), fp); */ + /* fputs(" ", fp); */ + /* fputs(entry->string, fp); */ + /* fputs("\n", fp); */ fclose(fp); } @@ -305,7 +305,7 @@ TrieExitCode trie_add(Trie *trie, const char *key, Entry *entry) { return trie_add_no_lock(trie, key, entry); } -TrieExitCode trie_add_random(Trie *trie, char **key_ptr, Entry *entry, +TrieExitCode trie_add_random(Trie *trie, char **key_ptr, void *data, bool secure) { // Generate random key bool ok = false; @@ -324,7 +324,7 @@ TrieExitCode trie_add_random(Trie *trie, char **key_ptr, Entry *entry, ok = trie_search_node(trie, key).child == NULL; } - TrieExitCode return_value = trie_add(trie, key, entry); + TrieExitCode return_value = trie_add(trie, key, data); if (return_value == Ok) { *key_ptr = key; @@ -402,7 +402,7 @@ TrieExitCode trie_add_random(Trie *trie, char **key_ptr, Entry *entry, * @param trie trie to return size for * @return size of the trie */ -size_t trie_size(Trie *trie) { return trie->size; } +uint64_t trie_size(Trie *trie) { return trie->size; } int trie_rlock(Trie *trie) { return pthread_rwlock_rdlock(&trie->lock); } diff --git a/trie/src/trie_entry.c b/trie/src/trie_entry.c index 2ddec08..1038ed6 100644 --- a/trie/src/trie_entry.c +++ b/trie/src/trie_entry.c @@ -35,3 +35,11 @@ Entry *entry_new(EntryType type, const char *string) { return entry; } + +void entry_free(Entry *entry) { + if (entry->string != NULL) { + free(entry->string); + } + + free(entry); +} diff --git a/trie/src/trie_node.c b/trie/src/trie_node.c index bb1be5c..4e4343a 100644 --- a/trie/src/trie_node.c +++ b/trie/src/trie_node.c @@ -27,7 +27,8 @@ typedef struct tinode { * and its string pointer is initialized. */ typedef struct tnode { - Entry *entry; + void *data; + uint64_t data_size; TrieInnerNode *tree; uint8_t tree_size; @@ -36,8 +37,6 @@ typedef struct tnode { // nodes char string[TRIE_MAX_SKIP_SIZE]; uint8_t string_len; - - bool represents; } TrieNode; // Required for recursively freeing tree structure @@ -67,7 +66,7 @@ TrieNode *tnode_init() { node->tree_size = 0; node->string_len = 0; - node->represents = false; + node->data_size = 0; return node; } @@ -105,6 +104,10 @@ void tnode_free(TrieNode *node) { tinode_free_cascade(node->tree); } + if (node->data_size > 0) { + free(node->data); + } + // TODO properly free entry /* if (node->payload != NULL) { */ /* free(node->payload); */ diff --git a/trie/test/CMakeLists.txt b/trie/test/CMakeLists.txt index fdd8f31..e61c238 100644 --- a/trie/test/CMakeLists.txt +++ b/trie/test/CMakeLists.txt @@ -2,5 +2,10 @@ add_compile_options(-Wno-incompatible-pointer-types) add_executable(test_trie test_trie.c ../src/trie.c) add_test(NAME test_trie COMMAND test_trie) + add_executable(test_trie_fuzzy test_trie_fuzzy.c ../src/trie.c) add_test(NAME test_trie_fuzzy COMMAND test_trie_fuzzy) + +add_test(NAME test_leaks COMMAND valgrind --tool=memcheck --error-exitcode=1 --track-origins=yes --leak-check=full ./test_trie) +add_test(NAME test_fuzzy_leaks COMMAND valgrind --tool=memcheck --error-exitcode=1 --track-origins=yes --leak-check=full ./test_trie_fuzzy) + diff --git a/trie/test/test_trie.c b/trie/test/test_trie.c index 7a7e8d3..1cd376a 100644 --- a/trie/test/test_trie.c +++ b/trie/test/test_trie.c @@ -28,6 +28,8 @@ void test_add_one() { TEST_CHECK(trie_search(ct, &entry2, string) == Ok); TEST_CHECK(entry == entry2); TEST_SIZE(ct, 1); + + entry_free(entry); trie_free(ct); } @@ -52,6 +54,8 @@ void test_add_prefix() { TEST_CHECK(trie_search(ct, &entry3, s2) == Ok); TEST_CHECK(entry3 == entry2); + entry_free(entry1); + entry_free(entry2); trie_free(ct); } @@ -104,6 +108,7 @@ void test_add_more() { TEST_CHECK(trie_add(ct, twenty, NULL) == AlreadyPresent); TEST_CHECK(trie_add(ct, twentytwo, NULL) == AlreadyPresent); + entry_free(entry); trie_free(ct); } diff --git a/trie/test/test_trie_fuzzy.c b/trie/test/test_trie_fuzzy.c index 33a2e24..f5ab85d 100644 --- a/trie/test/test_trie_fuzzy.c +++ b/trie/test/test_trie_fuzzy.c @@ -10,7 +10,7 @@ void test_fuzzy() { int counter = 0; int res; - for (int len = 1; len < 25; len += 5) { + for (int len = 2; len < 25; len += 5) { for (int count = 10; count <= 500; count += 10) { for (int i = 0; i < 50; i++) { counter++;