Want to win a PS4? Go Premium and enter to win our High-Tech Treats giveaway. Enter to Win

x
?
Solved

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

Posted on 2009-04-11
7
Medium Priority
?
879 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
[X]
Welcome to Experts Exchange

Add your voice to the tech community where 5M+ people just like you are talking about what matters.

  • Help others & share knowledge
  • Earn cash & points
  • Learn & ask questions
  • 3
  • 3
7 Comments
 
LVL 46

Expert Comment

by:Kent Olsen
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
Technology Partners: We Want Your Opinion!

We value your feedback.

Take our survey and automatically be enter to win anyone of the following:
Yeti Cooler, Amazon eGift Card, and Movie eGift Card!

 
LVL 4

Accepted Solution

by:
lhl60 earned 2000 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

What is SQL Server and how does it work?

The purpose of this paper is to provide you background on SQL Server. It’s your self-study guide for learning fundamentals. It includes both the history of SQL and its technical basics. Concepts and definitions will form the solid foundation of your future DBA expertise.

Question has a verified solution.

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

Today I had a very interesting conundrum that had to get solved quickly. Needless to say, it wasn't resolved quickly because when we needed it we were very rushed, but as soon as the conference call was over and I took a step back I saw the correct …
This is a short and sweet, but (hopefully) to the point article. There seems to be some fundamental misunderstanding about the function prototype for the "main" function in C and C++, more specifically what type this function should return. I see so…
The goal of this video is to provide viewers with basic examples to understand and use pointers in the C programming language.
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…

618 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