Solved

Cant Find (Simple?) Memory Leak in C++.

Posted on 2009-04-11
7
863 Views
Last Modified: 2013-12-17
Hi, I'm currently a C plus plus student and my assignments must be memory leak free. I've spent so long trying to figure out where this memory leak is and I have a feeling it should be really easy to find. I'm using Visual C plus plus 2008 Express Edition with its built in memory leak dumper. Someone who knows their way around C or C plus plus (which is probably more than I know) would probably find this really fast. Attached is all my code and an example of a memory leak dump.

Things to note:
1. It seems I have a memory leak for each winery I add.
2. I dont have a destructor for the private struct node definition in list.h (I dont know if this is important because I tried adding one and it didnt work. Maybe I'm doing it wrong?)
3. I also noticed that right after a new winery is inserted, the destructor for the winery class is called. I have no idea why this is or if it's important.

Thank you for your help.
//============================= list.h =====================================
#ifndef _LIST_
#define _LIST_
 
#include <ostream>
#include "winery.h"
 
using namespace std;
 
class list
{
public:
	list(void);				// constructor
	virtual ~list(void);	// destructor
	void displayByName(ostream& out) const;
	void displayByRating(ostream& out) const;
	void insert(const winery& toInsert);
	winery * const find(const char * const name) const;
	bool remove(const char * const name);
 
private:
	struct node
	{
		node(const winery& winery)
		:item(winery), nextByName(0), nextByRating(0)
		{}		// constructor
		winery item;
		node * nextByName;
		node * nextByRating;
	};
 
	node * headByName;
	node * headByRating;
};
 
#endif // _LIST_
 
 
//========================== winery.h ======================================
#ifndef _WINERY_
#define _WINERY_
 
#include <ostream>
 
class winery
{
public:
	winery(const char * const name, const char * const location, const int acres, const int rating);
	virtual ~winery(void);
	const char * const getName() const;
	const char * const getLocation() const;
	const int getAcres() const;
	const int getRating() const;
 
	// display headings for lists of wineries
	static void displayHeadings(std::ostream& out);
 
	friend std::ostream& operator<<(std::ostream& out, winery *w);
 
private:
	char	*name;
	char	*location;
	int		acres;
	int		rating;
};
 
#endif // _WINERY_
 
 
//================================== list.cpp =============================
#include "list.h"
#include "winery.h"
#include <cstring>
 
list::list()
:headByName (0), headByRating(0)
{
	// your code here
}
 
list::~list()
{
	do{
		node *temp;
		temp = headByName;
		headByName = headByName->nextByName;
		delete temp;
	}while(headByName);
}
 
void list::displayByName(ostream& out) const
{
 
	node *tempNode;
	tempNode = headByName;
	winery::displayHeadings(out);
	while(tempNode != 0){
        out<<&tempNode->item;
        tempNode = tempNode->nextByName;
	}
	// your code here
 
}
 
void list::displayByRating(ostream& out) const
{
 
	node *tempNode;
	tempNode = headByRating;
	winery::displayHeadings(out);
	while(tempNode != 0){
        out<<&tempNode->item;
        tempNode = tempNode->nextByRating;
	}
	// your code here
 
}
 
void list::insert(const winery &toInsert)
{
	// your code here
	winery *theWinery = new winery (toInsert.getName(), toInsert.getLocation(), toInsert.getAcres(), toInsert.getRating());
	
	node *theNode, *tempNode, *tempNode2;
	bool nameIn = false, rateIn = false;
 
	theNode = new node(*theWinery);
 
	if (headByName == 0){
	    headByName = theNode;
	    headByRating = theNode;
	    return;
	}else{
	    tempNode = headByName;
	    if(strcmp(theWinery->getName(), tempNode->item.getName()) < 0){
            //new node becomes the new first node.
            theNode->nextByName = headByName;
            headByName = theNode;
            nameIn = true;
        }
 
		while(!nameIn && tempNode->nextByName != NULL){
            if(strcmp(theWinery->getName(), tempNode->item.getName()) > 0 && strcmp(theWinery->getName(), tempNode->nextByName->item.getName()) < 0){
                //new node goes between current node and next node
                theNode->nextByName = tempNode->nextByName;
                tempNode->nextByName = theNode;
                nameIn = true;
            }else{
                tempNode = tempNode->nextByName;
            }
        }
 
        tempNode2 = headByRating;
		if(theWinery->getRating() > tempNode2->item.getRating()){
            //new node becomes the new first node.
            theNode->nextByRating = headByRating;
            headByRating = theNode;
            rateIn = true;
        }
                        
        while(!rateIn && tempNode2->nextByRating != NULL){
            if(theWinery->getRating() < tempNode2->item.getRating() && theWinery->getRating() > tempNode2->nextByRating->item.getRating()){
                //new node goes between current node and next node
                theNode->nextByRating = tempNode2->nextByRating;
                tempNode2->nextByRating = theNode;
                rateIn = true;
            }else{
                tempNode2 = tempNode2->nextByRating;
            }
        }
 
        if(!rateIn){
            tempNode2->nextByRating = theNode;
        }
        if(!nameIn){
            tempNode->nextByName = theNode;
        }
        return;
	}
 
}
 
winery * const list::find(const char * const name) const
{
	// your code here, return the appropriate value
	node * tempNode = headByName;
	while(tempNode != 0){
        if(strcmp(name, tempNode->item.getName()) == 0){
            node * const constNode = tempNode;
            return &constNode->item;
        }else{
            tempNode = tempNode->nextByName;
        }
	}
	return 0;
}
 
bool list::remove (const char * const name)
{
	// your code here, return the appropriate value
	
	node *tempNode, *tempNode2;
	tempNode = headByName;
	bool removed = false;
 
	if(strcmp(name, tempNode->item.getName()) == 0){
        headByName = tempNode->nextByName;
        removed = true;
    }
	while(!removed && tempNode->nextByName != 0){
        if(strcmp(name, tempNode->item.getName()) == 0){
            tempNode2->nextByName = tempNode->nextByName;
            removed = true;
        }else{
            tempNode2 = tempNode;
            tempNode = tempNode->nextByName;
        }
	}
 
	if(!removed && strcmp(name, tempNode->item.getName()) == 0){
        tempNode2->nextByName = 0;
        removed = true;
    }
    if(removed){
        if(headByRating == tempNode){
            headByRating = headByRating->nextByRating;
            delete tempNode;
        }else{
            tempNode2 = headByRating;
            while(tempNode2->nextByRating != 0){
                if(tempNode2->nextByRating == tempNode){
                    tempNode2->nextByRating = tempNode2->nextByRating->nextByRating;
                    delete tempNode;
                    break;
                }else{
                    tempNode2 = tempNode2->nextByRating;
                }
            }
        }
    }
 
	return removed;
}
 
 
//=================================== winery.cpp ==========================
#include <cstring>
#include "winery.h"
 
using namespace std;
 
winery::winery(const char * const name, const char * const location, const int acres, const int rating)
: name (new char[strlen(name)+1]), location (new char[strlen(location)+1]), acres (acres), rating (rating)
{
	strcpy(this->name, name);
	strcpy(this->location, location);
	// your code here, or in this constructor's initialization list
}
 
winery::~winery()
{
	delete [] this->name;
	delete [] this->location;
}
 
const char * const winery::getName() const
{
	// your code here, return the appropriate value
	return this->name;
}
 
const char * const winery::getLocation() const
{
	// your code here, return the appropriate value
	return this->location;
}
 
const int winery::getAcres() const
{
	// your code here, return the appropriate value
	return this->acres;
}
 
const int winery::getRating() const
{
	// your code here, return the appropriate value
	return this->rating;
}
 
void winery::displayHeadings(ostream& out)
{
	out.width(26);
	out<<left<<"name";
	out.width(19);
	out<<left<<"location";
    out.width(5);
	out<<right<<"acres";
	out.width(7);
	out<<right<<"rating";
	out.width(26);
	out<<endl<<left<<"----";
	out.width(19);
	out<<left<<"--------";
    out.width(5);
	out<<right<<"-----";
	out.width(7);
	out<<right<<"------";
	out<<endl;
	// your code here
}
 
ostream& operator<<(ostream& out, winery *w)
{
	// your code here
	out.width(26);
	out<<left<<w->getName();
	out.width(19);
	out<<left<<w->getLocation();
	out.width(5);
	out<<right<<w->getAcres();
	out.width(7);
	out<<right<<w->getRating()<<endl;
	return out;
}
 
 
//================================= lab1driver.cpp ===========================
#ifdef _WIN32
#define _CRTDBG_MAP_ALLOC
#include <stdlib.h>
#include <crtdbg.h>
#endif
 
#include <iostream>
 
#include "winery.h"
#include "list.h"
 
using namespace std;
 
int main()
{
    list	*wineries = new list();
	winery	*wPtr;
 
    cout << "\nCS260 - Lab1 - Your Name" << endl << endl;
 
	wineries->insert(winery("Lopez Island Vinyard", "San Juan Islands", 7, 95));
	wineries->insert(winery("Gallo", "Napa Valley", 200, 25));
	wineries->insert(winery("Cooper Mountain", "Willamette Valley", 100, 47));
	wineries->insert(winery("Duck Pond Cellars", "Willamette Valley", 845, 70));
	wineries->insert(winery("Del Rio", "Bear Creek Valley", 200, 37));
	wineries->insert(winery("Weisinger's of Ashland", "Bear Creek Valley", 6, 83));
	wineries->insert(winery("LongSword", "Applegate Valley", 16, 85));
 
	cout << "\n+++ list by name\n";
	wineries->displayByName(cout);
	cout << "\n+++ list by rating\n";
	wineries->displayByRating(cout);
 
	cout << "\n>>> removing Cooper Mountain\n";
	wineries->remove("Cooper Mountain");
 
	cout << "\n+++ list by name\n";
	wineries->displayByName(cout);
	cout << "\n+++ list by rating\n";
	wineries->displayByRating(cout);
 
	cout << "\n>>> inserting San Juan Vinyards\n";
	wineries->insert(winery("San Juan Vinyards", "San Juan Islands", 20, 90));
 
	cout << "\n+++ list by name\n";
	wineries->displayByName(cout);
	cout << "\n+++ list by rating\n";
	wineries->displayByRating(cout);
 
	cout << "\n>>> search for \"Gallo\"\n\n";
	wPtr = wineries->find("Gallo");
	if (wPtr != 0)
		cout << wPtr;
	else
		cout << "not found" << endl;
 
	cout << "\n>>> search for \"No Such\"\n\n";
	wPtr = wineries->find("No Such");
	if (wPtr != 0)
		cout << wPtr;
	else
		cout << "not found" << endl;
 
	cout << endl;
	delete wineries;
	
// in Visual Studio, reports on memory leaks in the Output window
#ifdef _WIN32
	_CrtDumpMemoryLeaks();
#endif
	cin.get();
 
	return 0;
}
 
 
 
//=================== example of memory leak dump after execution ===============
Detected memory leaks!
Dumping objects ->
{183} normal block at 0x003364A8, 20 bytes long.
 Data: <  A  d3 He3     > 18 9A 41 00 F8 64 33 00 48 65 33 00 14 00 00 00 
{157} normal block at 0x00336978, 20 bytes long.
 Data: <  A  i3  j3     > 18 9A 41 00 C8 69 33 00 10 6A 33 00 10 00 00 00 
{151} normal block at 0x00336828, 20 bytes long.
 Data: <  A xh3  h3     > 18 9A 41 00 78 68 33 00 D0 68 33 00 06 00 00 00 
{145} normal block at 0x003366E8, 20 bytes long.
 Data: <  A 8g3  g3     > 18 9A 41 00 38 67 33 00 80 67 33 00 C8 00 00 00 
{139} normal block at 0x003365A0, 20 bytes long.
 Data: <  A  e3 @f3 M   > 18 9A 41 00 F0 65 33 00 40 66 33 00 4D 03 00 00 
{133} normal block at 0x00336458, 20 bytes long.
 Data: <  A  d3  d3 d   > 18 9A 41 00 A8 64 33 00 F8 64 33 00 64 00 00 00 
{127} normal block at 0x00336320, 20 bytes long.
 Data: <  A pc3  c3     > 18 9A 41 00 70 63 33 00 B8 63 33 00 C8 00 00 00 
{121} normal block at 0x003361D0, 20 bytes long.
 Data: <  A  b3 xb3     > 18 9A 41 00 20 62 33 00 78 62 33 00 07 00 00 00 
Object dump complete.

Open in new window

0
Comment
Question by:nlhess2003
  • 3
  • 3
7 Comments
 
LVL 45

Expert Comment

by:Kdo
ID: 24123951
Hi nlhess2003,

It appears that when you add an item to the list, you instantiate both winery and node objects.

When you remove an item from the list, you delete the node, but not the winery object.



Good Luck,
Kent
0
 

Author Comment

by:nlhess2003
ID: 24123978
I did notice this, but I'm not sure how to delete the winery object.
I tried this for my list destructor:
list::~list()
{
      do{
            node *temp;
            temp = headByName;
            headByName = headByName->nextByName;
            delete temp->item;               //But the winery in this node is not a pointer. So this doesnt work.
            delete temp;
      }while(headByName);
}
I also tried delete &temp->item;  but that just made my program crash.
0
 

Author Comment

by:nlhess2003
ID: 24123989
Also I noticed that when temp is deleted in the list destructor, the winery destructor is being called right after. I think this is supposed to happen but I'm not sure how it is.
0
Master Your Team's Linux and Cloud Stack!

The average business loses $13.5M per year to ineffective training (per 1,000 employees). Keep ahead of the competition and combine in-person quality with online cost and flexibility by training with Linux Academy.

 
LVL 4

Accepted Solution

by:
lhl60 earned 500 total points
ID: 24133560
As kdo points out your problem is here

void list::insert(const winery &toInsert)
{
      // your code here
      winery *theWinery = new winery (toInsert.getName(), toInsert.getLocation(), toInsert.getAcres(),
..
..
}

theWinery is never deleted,

winery is a pointer everywhere in the code except in  node, so  why not change
      
struct node
      {
            node(const winery& winery)
            :item(winery), nextByName(0), nextByRating(0)
            {}            // constructor
            winery item;
            node * nextByName;
            node * nextByRating;
      };

to
      struct node
      {
           node(const winery* winery)
            :item(winery), nextByName(0), nextByRating(0)
            {}            // constructor
            winery *item;
            node * nextByName;
            node * nextByRating;
~node(){delete item;}
};

void list::insert(const winery &toInsert)   to
void list::insert(const winery *toInsert)

you might have to change a few "."' to "->"' and delete some &





0
 
LVL 4

Expert Comment

by:lhl60
ID: 24133578
continued...

will also make this part more redable

void list::insert(const winery &toInsert)
{
      // your code here
      winery *theWinery = new winery (toInsert.getName(), toInsert.getLocation(), toInsert.getAcres(), toInsert.getRating());
      
      node *theNode, *tempNode, *tempNode2;
      bool nameIn = false, rateIn = false;
 
      theNode = new node(theWinery);

0
 

Author Comment

by:nlhess2003
ID: 24134045
Thank you so much lhl60. In the end I decided that I will just have to change my professor's code. It just makes more sense to use a pointer there. Thanks again.
0
 
LVL 4

Expert Comment

by:lhl60
ID: 24136422
no problem, glad to help
good luck
0

Featured Post

3 Use Cases for Connected Systems

Our Dev teams are like yours. They’re continually cranking out code for new features/bugs fixes, testing, deploying, testing some more, responding to production monitoring events and more. It’s complex. So, we thought you’d like to see what’s working for us.

Question has a verified solution.

If you are experiencing a similar issue, please ask a related question

Suggested Solutions

Update (December 2011): Since this article was published, the things have changed for good for Android native developers. The Sequoyah Project (http://www.eclipse.org/sequoyah/) automates most of the tasks discussed in this article. You can even fin…
How to install Selenium IDE and loops for quick automated testing. Get Selenium IDE from http://seleniumhq.org Go to that link and select download selenium in the right hand columnThat will then direct you to their download page.From that page s…
The goal of this video is to provide viewers with basic examples to understand how to create, access, and change arrays in the C programming language.
The viewer will learn how to use and create keystrokes in Netbeans IDE 8.0 for Windows.

813 members asked questions and received personalized solutions in the past 7 days.

Join the community of 500,000 technology professionals and ask your questions.

Join & Ask a Question

Need Help in Real-Time?

Connect with top rated Experts

18 Experts available now in Live!

Get 1:1 Help Now