From 5406fb29e28a440faad7842e4dc9a1681e81d862 Mon Sep 17 00:00:00 2001
From: John Jones <jmjatlanta@gmail.com>
Date: Mon, 31 Jul 2017 14:51:29 -0500
Subject: [PATCH] Chasing memory leak possibly in PeerStore

---
 peer/peerstore.c | 33 +++++++++++++++++++++------------
 test/test_peer.h | 24 +++++++++++++++++++-----
 2 files changed, 40 insertions(+), 17 deletions(-)

diff --git a/peer/peerstore.c b/peer/peerstore.c
index 491f02a..6646bb4 100644
--- a/peer/peerstore.c
+++ b/peer/peerstore.c
@@ -4,6 +4,10 @@
 #include "libp2p/peer/peerstore.h"
 #include "libp2p/utils/logger.h"
 
+/***
+ * Creates a new PeerEntry struct
+ * @returns the newly allocated struct or NULL
+ */
 struct PeerEntry* libp2p_peer_entry_new() {
 	struct PeerEntry* out = (struct PeerEntry*)malloc(sizeof(struct PeerEntry));
 	if (out != NULL) {
@@ -12,6 +16,10 @@ struct PeerEntry* libp2p_peer_entry_new() {
 	return out;
 }
 
+/***
+ * Frees resources
+ * @param in the PeerEntry to free
+ */
 void libp2p_peer_entry_free(struct PeerEntry* in) {
 	if (in != NULL) {
 		libp2p_peer_free(in->peer);
@@ -19,6 +27,11 @@ void libp2p_peer_entry_free(struct PeerEntry* in) {
 	}
 }
 
+/***
+ * Copies a PeerEntry
+ * @param in the PeerEntry to copy
+ * @returns a newly allocated PeerEntry with the values from "in"
+ */
 struct PeerEntry* libp2p_peer_entry_copy(struct PeerEntry* in) {
 	struct PeerEntry* out = libp2p_peer_entry_new();
 	if (out != NULL) {
@@ -60,9 +73,9 @@ int libp2p_peerstore_free(struct Peerstore* in) {
 			next = current->next;
 			libp2p_peer_entry_free((struct PeerEntry*)current->item);
 			current->item = NULL;
+			libp2p_utils_linked_list_free(current);
 			current = next;
 		}
-		libp2p_utils_linked_list_free(in->head_entry);
 		free(in);
 	}
 	return 1;
@@ -75,14 +88,14 @@ int libp2p_peerstore_free(struct Peerstore* in) {
  * @returns true(1) on success, otherwise false
  */
 int libp2p_peerstore_add_peer_entry(struct Peerstore* peerstore, struct PeerEntry* peer_entry) {
+	if (peer_entry == NULL)
+		return 0;
+
 	struct Libp2pLinkedList* new_item = libp2p_utils_linked_list_new();
 	if (new_item == NULL)
 		return 0;
-	new_item->item = libp2p_peer_entry_copy(peer_entry);
-	if (new_item->item == NULL) {
-		libp2p_utils_linked_list_free(new_item);
-		return 0;
-	}
+
+	new_item->item = peer_entry;
 	if (peerstore->head_entry == NULL) {
 		peerstore->head_entry = new_item;
 		peerstore->last_entry = new_item;
@@ -113,12 +126,9 @@ int libp2p_peerstore_add_peer(struct Peerstore* peerstore, const struct Libp2pPe
 	}
 
 	if (peer->id_size > 0) {
-		char peer_id[peer->id_size + 1];
-		memcpy(peer_id, peer->id, peer->id_size);
-		peer_id[peer->id_size] = 0;
 		if (peer->addr_head != NULL) {
 			char* address = ((struct MultiAddress*)peer->addr_head->item)->string;
-			libp2p_logger_debug("peerstore", "Adding peer %s with address %s to peer store\n", peer_id, address);
+			libp2p_logger_debug("peerstore", "Adding peer %s with address %s to peer store\n", peer->id, address);
 		}
 		struct PeerEntry* peer_entry = libp2p_peer_entry_new();
 		if (peer_entry == NULL) {
@@ -128,8 +138,7 @@ int libp2p_peerstore_add_peer(struct Peerstore* peerstore, const struct Libp2pPe
 		if (peer_entry->peer == NULL)
 			return 0;
 		retVal = libp2p_peerstore_add_peer_entry(peerstore, peer_entry);
-		libp2p_peer_entry_free(peer_entry);
-		libp2p_logger_debug("peerstore", "Adding peer %s to peerstore was a success\n", peer_id);
+		libp2p_logger_debug("peerstore", "Adding peer %s to peerstore was a success\n", peer->id);
 	}
 	return retVal;
 }
diff --git a/test/test_peer.h b/test/test_peer.h
index ad974a4..4275f18 100644
--- a/test/test_peer.h
+++ b/test/test_peer.h
@@ -24,9 +24,14 @@ int test_peer() {
  * Test the peerstore
  */
 int test_peerstore() {
+
+	// create a Peer
 	struct Libp2pPeer* peer = libp2p_peer_new();
-	peer->id = "Qmabcdefg";
+	peer->id = malloc(10);
+	strcpy(peer->id, "Qmabcdefg");
 	peer->id_size = strlen(peer->id);
+
+	// create a PeerStore
 	struct Peerstore* peerstore = libp2p_peerstore_new(peer);
 	struct PeerEntry* peer_entry = NULL;
 	struct PeerEntry* results = NULL;
@@ -36,21 +41,32 @@ int test_peerstore() {
 		goto exit;
 
 	// add a peer entry to the peerstore
+	/*
 	peer_entry = libp2p_peer_entry_new();
 	peer_entry->peer = libp2p_peer_new();
 	peer_entry->peer->id_size = 6;
 	peer_entry->peer->id = malloc(peer_entry->peer->id_size);
 	memcpy(peer_entry->peer->id, "ABC123", peer_entry->peer->id_size);
 	peer_entry->peer->connection_type = CONNECTION_TYPE_NOT_CONNECTED;
+	*/
 
+	if (!libp2p_peerstore_add_peer(peerstore, peer)) {
+		fprintf(stderr, "libp2p_peerstore_add_peer returned false\n");
+		goto exit;
+	}
+
+	/*
 	if (!libp2p_peerstore_add_peer_entry(peerstore, peer_entry))
 		goto exit;
+	*/
 
 	// now try to retrieve it
-	results = libp2p_peerstore_get_peer_entry(peerstore, (unsigned char*)"ABC123", 6);
+	results = libp2p_peerstore_get_peer_entry(peerstore, (unsigned char*)"Qmabcdefg", 9);
 
-	if (results == NULL || results->peer->id_size != 6)
+	if (results == NULL || results->peer->id_size != 9) {
+		fprintf(stderr, "libp2p_peerstore_get_peer_entry returned NULL or was the wrong size\n");
 		goto exit;
+	}
 
 	// cleanup
 	retVal = 1;
@@ -58,8 +74,6 @@ int test_peerstore() {
 	exit:
 	if (peerstore != NULL)
 		libp2p_peerstore_free(peerstore);
-	if (peer_entry != NULL)
-		libp2p_peer_entry_free(peer_entry);
 	if (peer != NULL)
 		libp2p_peer_free(peer);
 	return retVal;