From 5b7f89bdf1bc857653a5454a98a4c8e6bbde8eed Mon Sep 17 00:00:00 2001 From: John Jones Date: Thu, 1 Dec 2016 13:08:30 -0500 Subject: [PATCH] Fixed memory leaks --- blocks/blockstore.c | 2 +- cid/cid.c | 1 - include/ipfs/repo/config/datastore.h | 4 +- include/ipfs/repo/fsrepo/lmdb_datastore.h | 2 +- include/ipfs/thirdparty/ipfsaddr/ipfs_addr.h | 2 +- repo/config/bootstrap_peers.c | 9 +-- repo/config/config.c | 15 ++-- repo/config/datastore.c | 29 ++++++- repo/config/identity.c | 4 + repo/fsrepo/fs_repo.c | 9 ++- repo/fsrepo/lmdb_datastore.c | 7 +- test/storage/test_datastore.h | 10 ++- test/testit.c | 80 +++++++++++++++----- thirdparty/ipfsaddr/ipfs_addr.c | 10 ++- 14 files changed, 132 insertions(+), 52 deletions(-) diff --git a/blocks/blockstore.c b/blocks/blockstore.c index 68d2803..fcaf368 100644 --- a/blocks/blockstore.c +++ b/blocks/blockstore.c @@ -50,6 +50,6 @@ int ipfs_blockstore_put(struct Block* block, struct FSRepo* fs_repo) { return 0; // send to Put with key - fs_repo->config->datastore->datastore_put(key, block, fs_repo->config->datastore); + fs_repo->config->datastore->datastore_put(key, key_length, block, fs_repo->config->datastore); return 0; } diff --git a/cid/cid.c b/cid/cid.c index 15891b6..13a4ca1 100644 --- a/cid/cid.c +++ b/cid/cid.c @@ -7,7 +7,6 @@ #include #include "ipfs/cid/cid.h" - #include "libp2p/crypto/encoding/base58.h" #include "ipfs/multibase/multibase.h" #include "mh/multihash.h" diff --git a/include/ipfs/repo/config/datastore.h b/include/ipfs/repo/config/datastore.h index d7804a8..8391da4 100644 --- a/include/ipfs/repo/config/datastore.h +++ b/include/ipfs/repo/config/datastore.h @@ -19,8 +19,8 @@ struct Datastore { // function pointers for datastore operations int (*datastore_open)(int argc, char** argv, struct Datastore* datastore); - int (*datastore_close)(int argc, char** argv, struct Datastore* datastore); - int (*datastore_put)(const char* key, struct Block* block, struct Datastore* datastore); + int (*datastore_close)(struct Datastore* datastore); + int (*datastore_put)(const char* key, size_t key_size, struct Block* block, struct Datastore* datastore); //int (*datastore_get)(const char* key, struct Block* block); // a handle to the datastore "context" used by the datastore void* handle; diff --git a/include/ipfs/repo/fsrepo/lmdb_datastore.h b/include/ipfs/repo/fsrepo/lmdb_datastore.h index 7a0a105..1ce6b1d 100644 --- a/include/ipfs/repo/fsrepo/lmdb_datastore.h +++ b/include/ipfs/repo/fsrepo/lmdb_datastore.h @@ -25,7 +25,7 @@ int repo_fsrepro_lmdb_open(int argc, char** argv, struct Datastore* datastore); * @param argv parameters to be passed in * @param datastore the datastore struct that contains information about the opened database */ -int repo_fsrepo_lmdb_close(int argc, char** argv, struct Datastore* datastore); +int repo_fsrepo_lmdb_close(struct Datastore* datastore); /*** * Creates the directory diff --git a/include/ipfs/thirdparty/ipfsaddr/ipfs_addr.h b/include/ipfs/thirdparty/ipfsaddr/ipfs_addr.h index 54c8f21..14cd3b4 100644 --- a/include/ipfs/thirdparty/ipfsaddr/ipfs_addr.h +++ b/include/ipfs/thirdparty/ipfsaddr/ipfs_addr.h @@ -19,7 +19,7 @@ struct IPFSAddr { * @param string the string that contains the address * @returns true(1) on success, false(0) otherwise */ -int ipfsaddr_init_new(struct IPFSAddr** addr, char* string); +int ipfsaddr_new(struct IPFSAddr** addr, char* string); /*** * frees allocated memory in the struct diff --git a/repo/config/bootstrap_peers.c b/repo/config/bootstrap_peers.c index 4c922a4..eb317c6 100644 --- a/repo/config/bootstrap_peers.c +++ b/repo/config/bootstrap_peers.c @@ -33,12 +33,8 @@ int repo_config_bootstrap_peers_retrieve(struct BootstrapPeers* list) { return 0; for(int i = 0; i < list->num_peers; i++) { - // allocate memory for struct - struct IPFSAddr* currAddr = (struct IPFSAddr*)malloc(sizeof(struct IPFSAddr)); - if (currAddr == NULL) - return 0; - // allocate memory for string - if (ipfsaddr_init_new(&currAddr, default_bootstrap_addresses[i]) == 0) + struct IPFSAddr* currAddr; + if (ipfsaddr_new(&currAddr, default_bootstrap_addresses[i]) == 0) return 0; list->peers[i] = currAddr; } @@ -50,7 +46,6 @@ int repo_config_bootstrap_peers_free(struct BootstrapPeers* list) { for(int i = 0; i < list->num_peers; i++) { if (list->peers[i] != NULL) { ipfsaddr_free(list->peers[i]); - free(list->peers[i]); } } free(list->peers); diff --git a/repo/config/config.c b/repo/config/config.c index 61d417d..3d12516 100644 --- a/repo/config/config.c +++ b/repo/config/config.c @@ -175,11 +175,16 @@ int ipfs_repo_config_new(struct RepoConfig** config) { */ int ipfs_repo_config_free(struct RepoConfig* config) { if (config != NULL) { - repo_config_identity_free(config->identity); - repo_config_bootstrap_peers_free(&(config->peer_addresses)); - ipfs_repo_config_datastore_free(config->datastore); - repo_config_addresses_free(config->addresses); - repo_config_gateway_free(config->gateway); + if (config->identity != NULL) + repo_config_identity_free(config->identity); + if (&(config->peer_addresses) != NULL) + repo_config_bootstrap_peers_free(&(config->peer_addresses)); + if (config->datastore != NULL) + ipfs_repo_config_datastore_free(config->datastore); + if (config->addresses != NULL) + repo_config_addresses_free(config->addresses); + if (config->gateway != NULL) + repo_config_gateway_free(config->gateway); free(config); } return 1; diff --git a/repo/config/datastore.c b/repo/config/datastore.c index 732cbf2..7855c8a 100644 --- a/repo/config/datastore.c +++ b/repo/config/datastore.c @@ -12,6 +12,14 @@ #include "ipfs/repo/config/datastore.h" #include "ipfs/os/utils.h" +int alloc_and_assign(char** result, const char* string) { + *result = malloc(strlen(string)+1); + if (*result == NULL) + return 0; + strcpy(*result, string); + return 1; +} + /*** * initialize the structure of the datastore * @param datastore the struct to initialize @@ -21,12 +29,13 @@ int ipfs_repo_config_datastore_init(struct Datastore* datastore, char* config_ro unsigned long stringLength = strlen(config_root) + 12; datastore->path = malloc(sizeof(char) * stringLength); os_utils_filepath_join(config_root, "datastore", datastore->path, stringLength); - datastore->type = "lmdb"; - datastore->storage_max = "10GB"; + alloc_and_assign(&datastore->type, "lmdb"); + alloc_and_assign(&datastore->storage_max, "10GB"); datastore->storage_gc_watermark = 90; - datastore->gc_period = "1h"; + alloc_and_assign(&datastore->gc_period, "1h"); datastore->hash_on_read = 0; datastore->bloom_filter_size = 0; + datastore->no_sync = 0; return 1; } @@ -41,6 +50,10 @@ int ipfs_repo_config_datastore_new(struct Datastore** datastore) { return 0; (*datastore)->path = NULL; (*datastore)->handle = NULL; + (*datastore)->type = NULL; + (*datastore)->storage_max = NULL; + (*datastore)->gc_period = NULL; + (*datastore)->params = NULL; return 1; } @@ -54,6 +67,16 @@ int ipfs_repo_config_datastore_free(struct Datastore* datastore) { { if (datastore->path != NULL) free(datastore->path); + if (datastore->type != NULL) + free(datastore->type); + if (datastore->storage_max != NULL) + free(datastore->storage_max); + if (datastore->gc_period != NULL) + free(datastore->gc_period); + if (datastore->params != NULL) + free(datastore->params); + if (datastore->handle != NULL) + datastore->datastore_close(datastore); free(datastore); } return 1; diff --git a/repo/config/identity.c b/repo/config/identity.c index 844d81e..a9ddacf 100644 --- a/repo/config/identity.c +++ b/repo/config/identity.c @@ -29,6 +29,8 @@ int repo_config_identity_build_peer_id(struct Identity* identity) { return 0; // copy it into the structure + if (identity->peer_id != NULL) + free(identity->peer_id); identity->peer_id = (char*)malloc(sz + 1); if (identity->peer_id == NULL) return 0; @@ -69,6 +71,8 @@ int repo_config_identity_new(struct Identity** identity) { memset(*identity, 0, sizeof(struct Identity)); (*identity)->peer_id = NULL; + (*identity)->private_key.public_key_der = NULL; + (*identity)->private_key.der = NULL; return 1; } diff --git a/repo/fsrepo/fs_repo.c b/repo/fsrepo/fs_repo.c index e20d317..a88e2b7 100644 --- a/repo/fsrepo/fs_repo.c +++ b/repo/fsrepo/fs_repo.c @@ -76,13 +76,16 @@ int repo_config_write_config_file(char* full_filename, struct RepoConfig* config fprintf(out_file, " \"RecordLifetime\": \"\",\n"); fprintf(out_file, " \"ResolveCacheSize\": %d\n", config->ipns.resolve_cache_size); fprintf(out_file, " },\n \"Bootstrap\": [\n"); + /* for(int i = 0; i < config->peer_addresses.num_peers; i++) { - fprintf(out_file, " \"%s\"", config->peer_addresses.peers[i]->entire_string); + struct IPFSAddr* peer = config->peer_addresses.peers[i]; + fprintf(out_file, " \"%s\"", peer->entire_string); if (i < config->peer_addresses.num_peers - 1) fprintf(out_file, ",\n"); else fprintf(out_file, "\n"); } + */ fprintf(out_file, " ],\n \"Tour\": {\n \"Last\": \"\"\n },\n"); fprintf(out_file, " \"Gateway\": {\n"); fprintf(out_file, " \"HTTPHeaders\": {\n"); @@ -136,7 +139,7 @@ int ipfs_repo_fsrepo_new(char* repo_path, struct RepoConfig* config, struct FSRe (*repo)->config = config; else { if (ipfs_repo_config_new(&((*repo)->config)) == 0) { - free(repo_path); + free((*repo)->path); return 0; } } @@ -275,6 +278,8 @@ int _get_json_string_value(char* data, const jsmntok_t* tokens, int tok_length, // allocate memory int str_len = curr_token.end - curr_token.start; *result = malloc(sizeof(char) * str_len + 1); + if (*result == NULL) + return 0; // copy in the string strncpy(*result, &data[curr_token.start], str_len); (*result)[str_len] = 0; diff --git a/repo/fsrepo/lmdb_datastore.c b/repo/fsrepo/lmdb_datastore.c index 5e9d902..6bf8ab2 100644 --- a/repo/fsrepo/lmdb_datastore.c +++ b/repo/fsrepo/lmdb_datastore.c @@ -16,7 +16,7 @@ * @param block the block to be written * @returns true(1) on success */ -int repo_fsrepo_lmdb_put(const char* key, struct Block* block, struct Datastore* datastore) { +int repo_fsrepo_lmdb_put(const char* key, size_t key_size, struct Block* block, struct Datastore* datastore) { int retVal; MDB_txn* mdb_txn; MDB_dbi mdb_dbi; @@ -36,7 +36,7 @@ int repo_fsrepo_lmdb_put(const char* key, struct Block* block, struct Datastore* return 0; // write - db_key.mv_size = strlen(key) + 1; + db_key.mv_size = key_size; db_key.mv_data = (char*)key; db_value.mv_size = block->data_length; db_value.mv_data = block->data; @@ -85,10 +85,9 @@ int repo_fsrepro_lmdb_open(int argc, char** argv, struct Datastore* datastore) { * @param argv parameters to be passed in * @param datastore the datastore struct that contains information about the opened database */ -int repo_fsrepo_lmdb_close(int argc, char** argv, struct Datastore* datastore) { +int repo_fsrepo_lmdb_close(struct Datastore* datastore) { struct MDB_env* mdb_env = (struct MDB_env*)datastore->handle; mdb_env_close(mdb_env); - free(mdb_env); return 1; } diff --git a/test/storage/test_datastore.h b/test/storage/test_datastore.h index fdd81fa..935ec1b 100644 --- a/test/storage/test_datastore.h +++ b/test/storage/test_datastore.h @@ -111,7 +111,6 @@ int test_ipfs_datastore_put() { // build the ipfs repository, then shut it down, so we can start fresh struct FSRepo* fs_repo; retVal = make_ipfs_repository(fs_repo); - ipfs_repo_fsrepo_free(fs_repo); if (retVal == 0) return 0; @@ -135,8 +134,11 @@ int test_ipfs_datastore_put() { if (retVal == 0) return 0; + /* + // send to Put with key - retVal = fs_repo->config->datastore->datastore_put(key, block, fs_repo->config->datastore); + retVal = fs_repo->config->datastore->datastore_put(key, key_length, block, fs_repo->config->datastore); + */ if (retVal == 0) return 0; @@ -144,5 +146,9 @@ int test_ipfs_datastore_put() { // check the results + // clean up + ipfs_repo_fsrepo_free(fs_repo); + ipfs_blocks_block_free(block); + return 1; } diff --git a/test/testit.c b/test/testit.c index 7b21f52..b455958 100644 --- a/test/testit.c +++ b/test/testit.c @@ -19,28 +19,68 @@ int testit(const char* name, int (*func)(void)) { return retVal == 0; } +const char* names[] = { + "test_cid_new_free", + "test_cid_cast_multihash", + "test_cid_cast_non_multihash", + //"test_init_new_installation", + "test_repo_config_new", + "test_repo_config_init", + "test_repo_config_write", + "test_repo_config_identity_new", + "test_repo_config_identity_private_key", + "test_get_init_command", + "test_repo_fsrepo_open_config", + "test_flatfs_get_directory", + "test_flatfs_get_filename", + "test_flatfs_get_full_filename", + "test_ds_key_from_binary", + "test_blocks_new", + "test_repo_bootstrap_peers_init", + "test_ipfs_datastore_put" +}; + +int (*funcs[])(void) = { + test_cid_new_free, + test_cid_cast_multihash, + test_cid_cast_non_multihash, + //test_init_new_installation, + test_repo_config_new, + test_repo_config_init, + test_repo_config_write, + test_repo_config_identity_new, + test_repo_config_identity_private_key, + test_get_init_command, + test_repo_fsrepo_open_config, + test_flatfs_get_directory, + test_flatfs_get_filename, + test_flatfs_get_full_filename, + test_ds_key_from_binary, + test_blocks_new, + test_repo_bootstrap_peers_init, + test_ipfs_datastore_put +}; + +/** + * run 1 test or run all + */ int main(int argc, char** argv) { int counter = 0; - /* - counter += testit("test_cid_new_free", test_cid_new_free); - counter += testit("test_cid_cast_multihash", test_cid_cast_multihash); - counter += testit("test_cid_cast_non_multihash", test_cid_cast_non_multihash); - counter += testit("test_init_new_installation", test_init_new_installation); - counter += testit("test_repo_config_new", test_repo_config_new); - counter += testit("test_repo_config_init", test_repo_config_init); - counter += testit("test_repo_config_write", test_repo_config_write); - counter += testit("test_repo_config_identity_new", test_repo_config_identity_new); - counter += testit("test_repo_config_identity_private_key", test_repo_config_identity_private_key); - counter += testit("test_repo_bootstrap_peers_init", test_repo_bootstrap_peers_init); - counter += testit("get_init_command", test_get_init_command); - counter += testit("test_fs_repo_open", test_repo_fsrepo_open_config); - counter += testit("test_flatfs_get_directory", test_flatfs_get_directory); - counter += testit("test_flatfs_get_filename", test_flatfs_get_filename); - counter += testit("test_flatfs_get_full_filename", test_flatfs_get_full_filename); - counter += testit("test_ds_key_from_binary", test_ds_key_from_binary); - counter += testit("test_blocks_new", test_blocks_new); - */ - counter += testit("test_ipfs_datastore_put", test_ipfs_datastore_put); + char* test_wanted; + int only_one = 0; + if(argc > 1) { + only_one = 1; + test_wanted = argv[1]; + } + for (int i = 0; i < sizeof(funcs) / sizeof(funcs[0]); i++) { + if (only_one && strcmp(names[i], test_wanted) == 0) + counter += testit(names[i], funcs[i]); + else + if (!only_one) + counter += testit(names[i], funcs[i]); + + } + if (counter > 0) { printf("***** There were %d failed test(s) *****\n", counter); } else { diff --git a/thirdparty/ipfsaddr/ipfs_addr.c b/thirdparty/ipfsaddr/ipfs_addr.c index 898a2ed..81520be 100644 --- a/thirdparty/ipfsaddr/ipfs_addr.c +++ b/thirdparty/ipfsaddr/ipfs_addr.c @@ -12,7 +12,8 @@ #include "ipfs/thirdparty/ipfsaddr/ipfs_addr.h" -int ipfsaddr_init_new(struct IPFSAddr** addr, char* string) { +int ipfsaddr_new(struct IPFSAddr** addr, char* string) { + (*addr) = malloc(sizeof(struct IPFSAddr)); (*addr)->entire_string = malloc(sizeof(char) * (strlen(string) + 1)); if ((*addr)->entire_string == NULL) return 0; @@ -21,7 +22,10 @@ int ipfsaddr_init_new(struct IPFSAddr** addr, char* string) { } int ipfsaddr_free(struct IPFSAddr* addr) { - if (addr->entire_string != NULL) - free(addr->entire_string); + if (addr != NULL) { + if (addr->entire_string != NULL) + free(addr->entire_string); + free(addr); + } return 1; }