From 8d2aeab0167a7e5145d07659c6d0e5a03ef9fd41 Mon Sep 17 00:00:00 2001 From: John Jones Date: Fri, 23 Dec 2016 10:49:30 -0500 Subject: [PATCH] Fixed various memory leaks --- .gitignore | 1 + blocks/blockstore.c | 30 +++++++++++++++++ importer/importer.c | 56 ++++++++++++++++++++++++-------- include/ipfs/blocks/blockstore.h | 1 + merkledag/merkledag.c | 50 ++++++++++------------------ merkledag/node.c | 3 -- repo/fsrepo/fs_repo.c | 13 +++++++- test/node/test_importer.h | 2 ++ test/storage/test_unixfs.h | 16 ++++++++- unixfs/unixfs.c | 6 ++++ 10 files changed, 127 insertions(+), 51 deletions(-) diff --git a/.gitignore b/.gitignore index b146679..ab89455 100644 --- a/.gitignore +++ b/.gitignore @@ -7,3 +7,4 @@ *.o .settings/language.settings.xml test/test_ipfs +main/ipfs diff --git a/blocks/blockstore.c b/blocks/blockstore.c index 0dc1b29..333a6c5 100644 --- a/blocks/blockstore.c +++ b/blocks/blockstore.c @@ -277,3 +277,33 @@ int ipfs_blockstore_put_node(const struct Node* node, const struct FSRepo* fs_re free(filename); return 1; } + +/*** + * Find a UnixFS struct based on its hash + * @param hash the hash to look for + * @param hash_length the length of the hash + * @param unix_fs the struct to fill + * @param fs_repo where to look for the data + * @returns true(1) on success + */ +int ipfs_blockstore_get_node(const unsigned char* hash, size_t hash_length, struct Node** node, const struct FSRepo* fs_repo) { + // get datastore key, which is a base32 key of the multihash + unsigned char* key = ipfs_blockstore_hash_to_base32(hash, hash_length); + + char* filename = ipfs_blockstore_path_get(fs_repo, (char*)key); + + size_t file_size = os_utils_file_size(filename); + unsigned char buffer[file_size]; + + FILE* file = fopen(filename, "rb"); + size_t bytes_read = fread(buffer, 1, file_size, file); + fclose(file); + + int retVal = ipfs_node_protobuf_decode(buffer, bytes_read, node); + + free(key); + free(filename); + + return retVal; +} + diff --git a/importer/importer.c b/importer/importer.c index e8977e3..e1564d9 100644 --- a/importer/importer.c +++ b/importer/importer.c @@ -23,31 +23,59 @@ size_t ipfs_import_chunk(FILE* file, struct Node* parent_node, struct FSRepo* fs unsigned char buffer[MAX_DATA_SIZE]; size_t bytes_read = fread(buffer, 1, MAX_DATA_SIZE, file); - // put the file bits into a new UnixFS file + // structs used by this method struct UnixFS* new_unixfs = NULL; - ipfs_unixfs_new(&new_unixfs); + struct Node* new_node = NULL; + struct NodeLink* new_link = NULL; + + // put the file bits into a new UnixFS file + if (ipfs_unixfs_new(&new_unixfs) == 0) + return 0; new_unixfs->data_type = UNIXFS_FILE; - ipfs_unixfs_add_data(&buffer[0], bytes_read, new_unixfs); + if (ipfs_unixfs_add_data(&buffer[0], bytes_read, new_unixfs) == 0) { + ipfs_unixfs_free(new_unixfs); + return 0; + } // protobuf the UnixFS size_t protobuf_size = ipfs_unixfs_protobuf_encode_size(new_unixfs); + if (protobuf_size == 0) { + ipfs_unixfs_free(new_unixfs); + return 0; + } unsigned char protobuf[protobuf_size]; size_t bytes_written = 0; - ipfs_unixfs_protobuf_encode(new_unixfs, protobuf, protobuf_size, &bytes_written); - // we're done with the object - ipfs_unixfs_free(new_unixfs); + if (ipfs_unixfs_protobuf_encode(new_unixfs, protobuf, protobuf_size, &bytes_written) == 0) { + ipfs_unixfs_free(new_unixfs); + return 0; + } // create a new node - struct Node* new_node = NULL; - ipfs_node_new_from_data(protobuf, bytes_written, &new_node); - ipfs_node_set_hash(new_node, new_unixfs->hash, new_unixfs->hash_length); + if (ipfs_node_new_from_data(protobuf, bytes_written, &new_node) == 0) { + return 0; + } + if (ipfs_node_set_hash(new_node, new_unixfs->hash, new_unixfs->hash_length) == 0) { + ipfs_node_free(new_node); + return 0; + } + // we're done with the UnixFS object + ipfs_unixfs_free(new_unixfs); // persist size_t size_of_node = 0; - ipfs_merkledag_add(new_node, fs_repo, &size_of_node); + if (ipfs_merkledag_add(new_node, fs_repo, &size_of_node) == 0) { + ipfs_node_free(new_node); + return 0; + } // put link in parent node - struct NodeLink* new_link = NULL; - ipfs_node_link_create("", new_node->hash, new_node->hash_size, &new_link); + if (ipfs_node_link_create("", new_node->hash, new_node->hash_size, &new_link) == 0) { + ipfs_node_free(new_node); + return 0; + } new_link->t_size = size_of_node; *total_size += new_link->t_size; - ipfs_node_add_link(parent_node, new_link); + // NOTE: disposal of this link object happens when the parent is disposed + if (ipfs_node_add_link(parent_node, new_link) == 0) { + ipfs_node_free(new_node); + return 0; + } ipfs_node_free(new_node); if (bytes_read != MAX_DATA_SIZE) { // We have read everything, now save the parent_node, @@ -138,6 +166,8 @@ int ipfs_import(int argc, char** argv) { retVal = ipfs_cid_hash_to_base58(directory_node->hash, directory_node->hash_size, buffer, buffer_len); if (retVal == 0) { printf("Unable to generate hash\n"); + ipfs_node_free(directory_node); + ipfs_repo_fsrepo_free(fs_repo); return 0; } printf("added %s %s\n", buffer, argv[2]); diff --git a/include/ipfs/blocks/blockstore.h b/include/ipfs/blocks/blockstore.h index 77f4b00..28f8a2f 100644 --- a/include/ipfs/blocks/blockstore.h +++ b/include/ipfs/blocks/blockstore.h @@ -60,5 +60,6 @@ int ipfs_blockstore_get_unixfs(const unsigned char* hash, size_t hash_length, st * Put a struct Node in the blockstore */ int ipfs_blockstore_put_node(const struct Node* node, const struct FSRepo* fs_repo, size_t* bytes_written); +int ipfs_blockstore_get_node(const unsigned char* hash, size_t hash_length, struct Node** node, const struct FSRepo* fs_repo); #endif diff --git a/merkledag/merkledag.c b/merkledag/merkledag.c index 32ce78c..aef5d06 100644 --- a/merkledag/merkledag.c +++ b/merkledag/merkledag.c @@ -20,20 +20,22 @@ int ipfs_merkledag_add(struct Node* node, struct FSRepo* fs_repo, size_t* bytes_ // taken from merkledag.go line 59 int retVal = 0; - // compute the hash - size_t protobuf_size = ipfs_node_protobuf_encode_size(node); - unsigned char protobuf[protobuf_size]; - size_t bytes_encoded; - retVal = ipfs_node_protobuf_encode(node, protobuf, protobuf_size, &bytes_encoded); - - node->hash_size = 32; - node->hash = (unsigned char*)malloc(node->hash_size); + // compute the hash if necessary if (node->hash == NULL) { - return 0; - } - if (libp2p_crypto_hashing_sha256(protobuf, bytes_encoded, &node->hash[0]) == 0) { - free(node->hash); - return 0; + size_t protobuf_size = ipfs_node_protobuf_encode_size(node); + unsigned char protobuf[protobuf_size]; + size_t bytes_encoded; + retVal = ipfs_node_protobuf_encode(node, protobuf, protobuf_size, &bytes_encoded); + + node->hash_size = 32; + node->hash = (unsigned char*)malloc(node->hash_size); + if (node->hash == NULL) { + return 0; + } + if (libp2p_crypto_hashing_sha256(protobuf, bytes_encoded, &node->hash[0]) == 0) { + free(node->hash); + return 0; + } } // write to block store & datastore @@ -56,8 +58,6 @@ int ipfs_merkledag_add(struct Node* node, struct FSRepo* fs_repo, size_t* bytes_ */ int ipfs_merkledag_get(const unsigned char* hash, size_t hash_size, struct Node** node, const struct FSRepo* fs_repo) { int retVal = 1; - //struct Block* block; - struct UnixFS* unix_fs; size_t key_length = 100; unsigned char key[key_length]; @@ -67,27 +67,11 @@ int ipfs_merkledag_get(const unsigned char* hash, size_t hash_size, struct Node* if (retVal == 0) return 0; - // we have the record from the db. Go get the UnixFS from the blockstore - retVal = ipfs_repo_fsrepo_unixfs_read(hash, hash_size, &unix_fs, fs_repo); + // we have the record from the db. Go get the node from the blockstore + retVal = ipfs_repo_fsrepo_node_read(hash, hash_size, node, fs_repo); if (retVal == 0) { return 0; } - // now convert the block into a node - if (ipfs_node_protobuf_decode(unix_fs->bytes, unix_fs->bytes_size, node) == 0) { - ipfs_unixfs_free(unix_fs); - return 0; - } - - // set the cid on the node - if (ipfs_node_set_hash(*node, hash, hash_size) == 0) { - ipfs_unixfs_free(unix_fs); - ipfs_node_free(*node); - return 0; - } - - // free resources - ipfs_unixfs_free(unix_fs); - return 1; } diff --git a/merkledag/node.c b/merkledag/node.c index 8314c26..2434b3d 100644 --- a/merkledag/node.c +++ b/merkledag/node.c @@ -375,9 +375,6 @@ int ipfs_node_set_data(struct Node * N, unsigned char * Data, size_t data_size) { return 0; } - N->encoded = NULL; - N->hash = NULL; - N->hash_size = 0; N->data = malloc(sizeof(unsigned char) * data_size); if (N->data == NULL) return 0; diff --git a/repo/fsrepo/fs_repo.c b/repo/fsrepo/fs_repo.c index 5e4d8e4..c615104 100644 --- a/repo/fsrepo/fs_repo.c +++ b/repo/fsrepo/fs_repo.c @@ -613,7 +613,18 @@ int ipfs_repo_fsrepo_node_write(const struct Node* node, const struct FSRepo* fs } int ipfs_repo_fsrepo_node_read(const unsigned char* hash, size_t hash_length, struct Node** node, const struct FSRepo* fs_repo) { - return 0; + int retVal = 0; + + // get the base32 hash from the database + // We do this only to see if it is in the database + size_t fs_key_length = 100; + unsigned char fs_key[fs_key_length]; + retVal = fs_repo->config->datastore->datastore_get((const char*)hash, hash_length, fs_key, fs_key_length, &fs_key_length, fs_repo->config->datastore); + if (retVal == 0) // maybe it doesn't exist? + return 0; + // now get the block from the blockstore + retVal = ipfs_blockstore_get_node(hash, hash_length, node, fs_repo); + return retVal; } diff --git a/test/node/test_importer.h b/test/node/test_importer.h index f71930e..285e4bd 100644 --- a/test/node/test_importer.h +++ b/test/node/test_importer.h @@ -64,6 +64,7 @@ int test_import_large_file() { if (write_node->hash[i] != cid_test[i]) { printf("Hashes should be the same each time, and do not match at position %d, should be %02x but is %02x\n", i, cid_test[i], write_node->hash[i]); ipfs_repo_fsrepo_free(fs_repo); + ipfs_node_free(write_node); return 0; } } @@ -210,6 +211,7 @@ int test_import_small_file() { if (write_node->hash[i] != cid_test[i]) { printf("Hashes do not match at position %d, should be %02x but is %02x\n", i, cid_test[i], write_node->hash[i]); ipfs_repo_fsrepo_free(fs_repo); + ipfs_node_free(write_node); return 0; } } diff --git a/test/storage/test_unixfs.h b/test/storage/test_unixfs.h index c1ab839..1ea4357 100644 --- a/test/storage/test_unixfs.h +++ b/test/storage/test_unixfs.h @@ -15,6 +15,7 @@ int test_unixfs_encode_decode() { retVal = ipfs_unixfs_protobuf_encode(unixfs, buffer, buffer_size, &bytes_written); if (retVal == 0) { + ipfs_unixfs_free(unixfs); return 0; } @@ -22,18 +23,25 @@ int test_unixfs_encode_decode() { struct UnixFS* results = NULL; retVal = ipfs_unixfs_protobuf_decode(buffer, bytes_written, &results); if (retVal == 0) { + ipfs_unixfs_free(unixfs); return 0; } // compare if (results->data_type != unixfs->data_type) { + ipfs_unixfs_free(unixfs); + ipfs_unixfs_free(results); return 0; } if (results->block_size_head != unixfs->block_size_head) { + ipfs_unixfs_free(unixfs); + ipfs_unixfs_free(results); return 0; } + ipfs_unixfs_free(unixfs); + ipfs_unixfs_free(results); return 1; } @@ -71,15 +79,21 @@ int test_unixfs_encode_smallfile() { size_t bytes_written; ipfs_unixfs_protobuf_encode(unixfs, protobuf, protobuf_size, &bytes_written); + int retVal = 1; + if (bytes_written != 41) { printf("Length should be %lu, but is %lu\n", 41LU, bytes_written); + retVal = 0; } for(int i = 0; i < bytes_written; i++) { if (expected_results[i] != protobuf[i]) { printf("Byte at position %d should be %02x but is %02x\n", i, expected_results[i], protobuf[i]); + retVal = 0; } } - return 1; + ipfs_unixfs_free(unixfs); + + return retVal; } diff --git a/unixfs/unixfs.c b/unixfs/unixfs.c index acb9420..514375b 100644 --- a/unixfs/unixfs.c +++ b/unixfs/unixfs.c @@ -53,6 +53,12 @@ int ipfs_unixfs_new(struct UnixFS** obj) { int ipfs_unixfs_free(struct UnixFS* obj) { if (obj != NULL) { + if (obj->hash != NULL) { + free(obj->hash); + } + if (obj->bytes != NULL) { + free(obj->bytes); + } free(obj); obj = NULL; }