Solved

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

Posted on 2009-04-11
7
854 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
Comment Utility
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
Comment Utility
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
Comment Utility
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
How your wiki can always stay up-to-date

Quip doubles as a “living” wiki and a project management tool that evolves with your organization. As you finish projects in Quip, the work remains, easily accessible to all team members, new and old.
- Increase transparency
- Onboard new hires faster
- Access from mobile/offline

 
LVL 4

Accepted Solution

by:
lhl60 earned 500 total points
Comment Utility
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
Comment Utility
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
Comment Utility
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
Comment Utility
no problem, glad to help
good luck
0

Featured Post

Enabling OSINT in Activity Based Intelligence

Activity based intelligence (ABI) requires access to all available sources of data. Recorded Future allows analysts to observe structured data on the open, deep, and dark web.

Join & Write a Comment

This article describes relatively difficult and non-obvious issues that are likely to arise when creating COM class in Visual Studio and deploying it by professional MSI-authoring tools. It is assumed that the reader is already familiar with the cla…
Real-time is more about the business, not the technology. In day-to-day life, to make real-time decisions like buying or investing, business needs the latest information(e.g. Gold Rate/Stock Rate). Unlike traditional days, you need not wait for a fe…
The viewer will learn how to use NetBeans IDE 8.0 for Windows to connect to a MySQL database. Open Services Panel: Create a new connection using New Connection Wizard: Create a test database called eetutorial: Create a new test tabel called ee…
The viewer will learn how to use and create keystrokes in Netbeans IDE 8.0 for Windows.

763 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

6 Experts available now in Live!

Get 1:1 Help Now