From 9882c28743f8d1669e966afddfcbeb628d948c54 Mon Sep 17 00:00:00 2001 From: John Jones Date: Mon, 2 Jan 2017 00:38:09 -0500 Subject: [PATCH] Fixed memory leaks Several tests had memory leaks. As well found a few leaks within the importer and resolver areas. --- importer/importer.c | 6 +++++- importer/resolver.c | 16 ++++++++++++++-- os/utils.c | 3 +++ test/node/test_resolver.h | 16 +++++++++++++++- 4 files changed, 37 insertions(+), 4 deletions(-) diff --git a/importer/importer.c b/importer/importer.c index 04ea505..a77dc94 100644 --- a/importer/importer.c +++ b/importer/importer.c @@ -268,12 +268,14 @@ int ipfs_import_file(const char* root_dir, const char* fileName, struct Node** p free(file); if (path != NULL) free (path); + os_utils_free_file_list(first); } else { // process this file FILE* file = fopen(fileName, "rb"); retVal = ipfs_node_new(parent_node); - if (retVal == 0) + if (retVal == 0) { return 0; + } // add all nodes (will be called multiple times for large files) while ( bytes_read == MAX_DATA_SIZE) { @@ -345,6 +347,8 @@ int ipfs_import_files(int argc, char** argv) { ipfs_import_print_node_results(directory_entry, filename); // cleanup ipfs_node_free(directory_entry); + free(path); + free(filename); current = current->next; } diff --git a/importer/resolver.c b/importer/resolver.c index 5850dc3..96fa5ee 100644 --- a/importer/resolver.c +++ b/importer/resolver.c @@ -64,6 +64,13 @@ const char* ipfs_resolver_remove_path_prefix(const char* path) { * @returns what we are looking for, or NULL if it wasn't found */ struct Node* ipfs_resolver_get(const char* path, struct Node* from, const struct FSRepo* fs_repo) { + + /** + * Memory management notes: + * If we find what we're looking for, we clean up "from" and return the object + * If we don't find what we're looking for, but we can continue the search, we clean up "from" + * If we don't find what we're looking for, and we cannot continue, we do not clean up "from" + */ // remove unnecessary stuff if (from == NULL) path = ipfs_resolver_remove_path_prefix(path); @@ -90,10 +97,13 @@ struct Node* ipfs_resolver_get(const char* path, struct Node* from, const struct // we have the root node, now see if we want this or something further down int pos = strlen(path_section); if (pos == strlen(path)) { + free(path_section); return current_node; } else { // look on... - return ipfs_resolver_get(&path[pos+1], current_node, fs_repo); // the +1 is the slash + free(path_section); + struct Node* newNode = ipfs_resolver_get(&path[pos+1], current_node, fs_repo); // the +1 is the slash + return newNode; } } else { // we don't have a current node, and we don't have a hash. Something is wrong @@ -115,6 +125,7 @@ struct Node* ipfs_resolver_get(const char* path, struct Node* from, const struct if (strlen(path_section) == strlen(path)) { // we are at the end of our search ipfs_node_free(from); + free(path_section); return current_node; } else { char* next_path_section; @@ -123,7 +134,8 @@ struct Node* ipfs_resolver_get(const char* path, struct Node* from, const struct // if we're at the end of the path, return the node // continue looking for the next part of the path ipfs_node_free(from); - return ipfs_resolver_get(next_path_section, current_node, fs_repo); + struct Node* newNode = ipfs_resolver_get(next_path_section, current_node, fs_repo); + return newNode; } } curr_link = curr_link->next; diff --git a/os/utils.c b/os/utils.c index a65d2e7..76e162a 100644 --- a/os/utils.c +++ b/os/utils.c @@ -125,6 +125,9 @@ int os_utils_free_file_list(struct FileList* first) { struct FileList* last = NULL; while (next != NULL) { last = next->next; + if (next->file_name != NULL) { + free(next->file_name); + } free(next); next = last; } diff --git a/test/node/test_resolver.h b/test/node/test_resolver.h index 787a64d..9888a74 100644 --- a/test/node/test_resolver.h +++ b/test/node/test_resolver.h @@ -8,12 +8,18 @@ int test_resolver_get() { drop_and_build_repository(ipfs_path); + // this should point to a test directory with files and directories + char* home_dir = os_utils_get_homedir(); + char* test_dir = malloc(strlen(home_dir) + 10); + + os_utils_filepath_join(home_dir, "ipfstest", test_dir, strlen(home_dir) + 10); + int argc = 4; char* argv[argc]; argv[0] = "ipfs"; argv[1] = "add"; argv[2] = "-r"; - argv[3] = "/Users/JohnJones/ipfstest"; + argv[3] = test_dir; ipfs_import_files(argc, (char**)argv); @@ -24,6 +30,8 @@ int test_resolver_get() { // find something that is already in the repository struct Node* result = ipfs_resolver_get("/ipfs/QmbMecmXESf96ZNry7hRuzaRkEBhjqXpoYfPCwgFzVGDzB", NULL, fs_repo); if (result == NULL) { + free(test_dir); + ipfs_repo_fsrepo_free(fs_repo); return 0; } @@ -32,8 +40,14 @@ int test_resolver_get() { // find something by path result = ipfs_resolver_get("/ipfs/QmZBvycPAYScBoPEzm35zXHt6gYYV5t9PyWmr4sksLPNFS/hello_world.txt", NULL, fs_repo); if (result == NULL) { + free(test_dir); + ipfs_repo_fsrepo_free(fs_repo); return 0; } + ipfs_node_free(result); + free(test_dir); + ipfs_repo_fsrepo_free(fs_repo); + return 1; }