(CAUTION: I'm a novice.) I have a function that creates a Doubly Linked List which contains DNodes of Representative*'s, each Representative having 3 objects (string name, int visits, int donations) and am getting memory leaks.
I get the following memory leak messages after running the program:
https://textuploader.com/51tme
I get very similar errors in valgrind's memcheck. Each message seems to be triggered twice for the 5 times I create a new Representative (there are 10 total memory leaks). The function listed is always operator new within addToList(). Here's the addToList() function:
bool ListManager::addToList(string rep_list){
istringstream convertRepresentative(rep_list);
string whitespaces(" \t\f\v\n\r");
size_t found = rep_list.find_first_not_of(whitespaces);
if (rep_list.empty() || found == string::npos){
/* string is empty or only has whitespace */
cout << "\n| ERROR(addToList()): String is empty. No representatives added to list.\n| Function halted.\n";
return false;
}
string name;
int visits;
int donations;
Representative* temp; // comment out if using shared_ptr
if (convertRepresentative.fail()){
/* error in string input */
cout << "\n| ERROR(addToList()): Could not import string. No representatives added to list.\n| Function halted.\n";
convertRepresentative.clear();
return false;
}
else{
while (!convertRepresentative.eof()){
if (!(convertRepresentative >> name)){
cout << "\n| ERROR(addToList()): Could not import string. Incorrect format.\n| No representatives added to list. Function halted.\n";
rep_list.clear();
convertRepresentative.clear();
return false;
}
if (!(convertRepresentative >> visits)){
cout << "\n| ERROR(addToList()): Could not import string. Incorrect format.\n| No representatives added to list. Function halted.\n";
rep_list.clear();
convertRepresentative.clear();
return false;
}
if (!(convertRepresentative >> donations)){
cout << "\n| ERROR(addToList()): Could not import string. Incorrect format.\n| No representatives added to list. Function halted.\n";
rep_list.clear();
convertRepresentative.clear();
return false;
}
temp = new Representative(name, visits, donations);
ListDLL.insertTail(temp);
//shared_ptr<Representative> temp;
//temp.reset(new Representative(name, strength, speed));
//RosterDLL.insertTail(&*temp);
}
convertRepresentative.clear();
return true;
}
}
I assume I am to delete temp at some point, but deleting temp deletes the Representative data from inside the DNode. So I don't know when I am to delete temp and/or reset/clear it, whatever. As you can see in my commented code, I've tried smart pointers, but they do what I just described--they delete the needed information from inside the DNodes.
I'm new to this & I know you guys are strict on formatting, etc (sorry about my #include namespace std). I hope it's a simple fix. Please help.
EDIT: Per suggestion, here is DoubleLinkedList.h (you can see all the used files for this project in the first link):
#ifndef DOUBLELINKEDLIST_H_
#define DOUBLELINKEDLIST_H_
#include <stdexcept>
#include <iostream>
#include "Representative.h"
using namespace std;
struct DNode{
Representative* rep;
DNode* next;
DNode* prev;
DNode(Representative*& rep,
DNode* prev_val = NULL, DNode* next_val = NULL) :
rep(rep), next(next_val), prev(prev_val) {}
};
template<class T>
class DoubleLinkedList
{
public:
DNode* head;
DNode* tail;
int count;
DoubleLinkedList() :
head(NULL), count(0),
tail(NULL){
}
virtual ~DoubleLinkedList() {
clear();
}
bool find(Representative* rep){
bool found = false;
if (head == NULL) {
return found;
}
DNode* DNode_ptr = head;
while (DNode_ptr != NULL){
if (DNode_ptr->rep == rep){
found = true;
}
DNode_ptr = DNode_ptr->next;
}
return found;
}
void insertTail(Representative* rep) {
if (find(rep) == false) {
if (head == NULL) {
DNode* newDNode = new DNode(rep);
head = newDNode;
count++;
return;
}
else {
DNode* temp = head;
while (temp->next != NULL) {
temp = temp->next;
}
DNode* tailDNode = new DNode(rep);
temp->next = tailDNode;
tailDNode->next = NULL;
tailDNode->prev = temp;
count++;
return;
}
}
else {
return;
}
}
Representative* at(int index){
DNode* temp_ptr = head;
if (index < 0 || index > count - 1){
throw out_of_range("Out of Range");
}
else {
for (int i = 0; i < index; i++){
temp_ptr = temp_ptr->next;
}
return temp_ptr->rep;
}
}
int size(){
return count;
}
void clear(){
DNode* temporary_ptr = head;
while (temporary_ptr != NULL){
DNode* tmp_pointer = temporary_ptr;
temporary_ptr = temporary_ptr->next;
delete tmp_pointer;
}
count = 0;
head = NULL;
return;
}
//Rest of functions omitted
};
#endif /* DOUBLELINKKEDLIST_H_ */
If you can compile under C++11, which, considering it is highly supported by compilers, you should be able to, you should not be using raw pointers to manage memory at all.
If you do not want to transfer a lot of data and you are making your own class, you do not even need to use pointers in the first place. Just implement the move constructor and move asignment operator and use regular value objects instead of pointers to them.
If you really need to use pointers - an example can be making a collection of objects of some kind of a library, objects, which do not have the move constructor and move asignment operator implemented - use either std::unique_ptr<T> or std::shared_ptr<T> from the <memory> header. You won't need to call delete, because the smart pointers will take care of it.