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
Solved

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

Posted on 2009-04-11
7
866 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: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
Space-Age Communications Transitions to DevOps

ViaSat, a global provider of satellite and wireless communications, securely connects businesses, governments, and organizations to the Internet. Learn how ViaSat’s Network Solutions Engineer, drove the transition from a traditional network support to a DevOps-centric model.

 
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

Free Tool: Path Explorer

An intuitive utility to help find the CSS path to UI elements on a webpage. These paths are used frequently in a variety of front-end development and QA automation tasks.

One of a set of tools we're offering as a way of saying thank you for being a part of the community.

Question has a verified solution.

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

Jaspersoft Studio is a plugin for Eclipse that lets you create reports from a datasource.  In this article, we'll go over creating a report from a default template and setting up a datasource that connects to your database.
A long time ago (May 2011), I have written an article showing you how to create a DLL using Visual Studio 2005 to be hosted in SQL Server 2005. That was valid at that time and it is still valid if you are still using these versions. You can still re…
The goal of this video is to provide viewers with basic examples to understand and use switch statements in the C programming language.
This tutorial covers a step-by-step guide to install VisualVM launcher in eclipse.

808 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