| | |
How to return pointer to object & avoid memory leak?
Please support our C++ advertiser: Intel Parallel Studio Home
Thread Solved |
I have nearly finished an assignment that calls for implementing a media library. I have pinpointed my current solution for the addItem method in the library class as a memory leak, but so far I am not certain how to fix it. The addItem method I have so far is as follows (addBook shown; Book is a subclass of Item):
I understand that if I use new to allocate an instance of Item, I need to delete it. Of course, I can't delete it before the return statement, nor after. The addBook definition is part of the assignment and can't be changed.
c++ Syntax (Toggle Plain Text)
const Item* Library::addBook(const string& title, const string& author, const int nPages) { Item* book = new Book(title, author, nPages); lib->insert(book); //lib is a set<Item*> container return book; }
I understand that if I use new to allocate an instance of Item, I need to delete it. Of course, I can't delete it before the return statement, nor after. The addBook definition is part of the assignment and can't be changed.
•
•
•
•
Go through the stored items in theLibrary's destructor and explicitlydeletethe items there.
c++ Syntax (Toggle Plain Text)
Library::~Library() { for(ItemSet::iterator itr = lib->begin();itr != lib->end();itr++) //Causes access violation delete *itr; delete lib; //No problem delete temp; //No problem }
This is strange since I can iterate through lib fine in methods outside of the destructor. Is there something about the destructor that disallows iterators to do their magic?
You haven't been trying to delete books anywhere else have you?
Deleting the same book twice is a sure recipe for disaster.
I see your addBook() returns a pointer to a book (why?). What else are you doing with that book pointer?
If you need to remove a book, then you need another member function, say delBook(), which removes the book from the set, and then deletes it.
Deleting the same book twice is a sure recipe for disaster.
I see your addBook() returns a pointer to a book (why?). What else are you doing with that book pointer?
If you need to remove a book, then you need another member function, say delBook(), which removes the book from the set, and then deletes it.
•
•
•
•
You haven't been trying to delete books anywhere else have you?
Deleting the same book twice is a sure recipe for disaster.
•
•
•
•
I see your addBook() returns a pointer to a book (why?). What else are you doing with that book pointer?
•
•
•
•
If you need to remove a book, then you need another member function, say delBook(), which removes the book from the set, and then deletes it.
The biggest problem I am seeing now (now that the conceptual stage has been breached - thank you all so far for that) is implementing the proper deletion of the individual pointers. Accessing them via iterators within the destructor has been a failure (due to an inexplicable access violation). And I can think of no other place to do this than from within the destructor.
The reason for this is because I am not allowed to modify the driver routine in any way. The driver routine declares const* Item item, which it uses to reference the pointer that addBook() returns. item is not involved in any interaction with delete. The only delete in the routine is used to free the single instance of library. Of course, since library only contains a set of pointers to all of the items (books), deleting the set itself won't be enough.
The driver routine:
c++ Syntax (Toggle Plain Text)
// make no changes to this code, except to have it // print out your actual name instead of "your name" #include <iostream> #include "Library.h" using namespace std; static Library *library; .... int main(int argc, char **argv) { .... const Item *item; .... // create library instance library = new Library(); .... item = library->addBook(); if (item != NULL) { //Do Stuff with item pointer } .... delete library; //The only occurrence of delete in this routine .... }
By the way, the access violation (from the above ~library() example) reading location on my machine is always 0xfeeefeee if that is of any significance.
The violation is in xtree, at:
When I step through the code, the violation occurs immediately upon reaching the line in the main routine after everything else is done. I am unable to step into the destructor for library before the program halts (which is as shown in my prior post).
I use the same iterator elsewhere in the code with no ill effects. As far as I can tell, I am not trying to access any empty space with the iterator - library has not been deleted yet, nor have any of the elements of the lib set.
UPDATE: The base class of Book, Item, had a virtual destructor defined. The destructor does nothing other than delete a set container that was allocated in the constructor. I made it virtual because some of the derived classes had additional sets that needed to be deleted also.
I removed the virtual from ~Item() and now the iterator in ~library() no longer causes an access violation and deletes all of the elements.
This eliminated the majority of the memory leaks, but not all. The remainder (2 occurrences) seem to originate from the derived class Book, although this particular class does not add any additional dynamically allocated memory.
The violation is in xtree, at:
c++ Syntax (Toggle Plain Text)
iterator begin() { // return iterator for beginning of mutable sequence return (_TREE_ITERATOR(_Lmost())); //Access violation }
c++ Syntax (Toggle Plain Text)
delete library;
I use the same iterator elsewhere in the code with no ill effects. As far as I can tell, I am not trying to access any empty space with the iterator - library has not been deleted yet, nor have any of the elements of the lib set.
UPDATE: The base class of Book, Item, had a virtual destructor defined. The destructor does nothing other than delete a set container that was allocated in the constructor. I made it virtual because some of the derived classes had additional sets that needed to be deleted also.
I removed the virtual from ~Item() and now the iterator in ~library() no longer causes an access violation and deletes all of the elements.
This eliminated the majority of the memory leaks, but not all. The remainder (2 occurrences) seem to originate from the derived class Book, although this particular class does not add any additional dynamically allocated memory.
Last edited by Undertech; Aug 16th, 2008 at 10:35 pm. Reason: Updated with additonal findings
0xfeeefeee is one of the patterns filled in memory after it has been freed.
http://www.nobugs.org/developer/win3..._crt_heap.html
http://www.nobugs.org/developer/win3..._crt_heap.html
•
•
Join Date: Nov 2007
Posts: 978
Reputation:
Solved Threads: 208
•
•
•
•
The base class of Book, Item, had a virtual destructor defined. The destructor does nothing other than delete a set container that was allocated in the constructor. I made it virtual because some of the derived classes had additional sets that needed to be deleted also.
I removed the virtual from ~Item()
do a thing like ...
C++ Syntax (Toggle Plain Text)
Item* book = new Book(title, author, nPages); delete book; // ~Book() will not be invoked
Maybe you should post the complete code ...
I figured out why I was getting the access violation, and yes, you would have caught it if I had posted all of my code earlier! (I was deleting a private set twice - once through the Item destructor and then through the Book destructor. I removed the redundant delete and now it is working with the virtual base destructor.) Unfortunately, I am still getting that small bit of memory leak. I think it might be because the item pointer is going out of scope. If so, I am not sure how to work around it while keeping within the requirements of the assignment (cannot modify driver routine or library definitions).
This is the debug output for the below code:
Driver:
Item.h:
Item.cpp:
Book.h:
Book.cpp:
Library.h:
Library.cpp:
This is the debug output for the below code:
Detected memory leaks!
Dumping objects ->
{147} normal block at 0x003363E8, 11 bytes long.
Data: <class Book > 63 6C 61 73 73 20 42 6F 6F 6B 00
{146} normal block at 0x003363A0, 8 bytes long.
Data: < c3 > E8 63 33 00 00 00 00 00
Object dump complete.Driver:
c++ Syntax (Toggle Plain Text)
// make no changes to this code, except to have it // print out your actual name instead of "your name" #ifdef _WIN32 #define _CRTDBG_MAP_ALLOC #include <stdlib.h> #include <crtdbg.h> #endif #include <iostream> #include "Library.h" using namespace std; static Library *library; static void printItemSet(ostream& out, const ItemSet *itemSet) { if (itemSet != NULL && itemSet->size() > 0) for (ItemSet::const_iterator i = itemSet->begin(); i != itemSet->end(); i++) library->printItem(out, (Item *) *i); else out << "none" << endl << endl; } int main(int argc, char **argv) { ostream& out(cout); const Item *item; // create library instance library = new Library(); // add items to library cout << ">>> adding items to library:\n\n"; item = library->addBook("Book Title", "Joe Author", 249); if (item != NULL) { library->addKeywordForItem(item, "Keyword one"); library->addKeywordForItem(item, "Another keyword"); library->printItem(cout, item); } //Two more books added like the one above cout << ">>> books:\n\n"; printItemSet(out, library->books()); // print items for keyword cout << ">>> items for keyword \"Keyword one\"\n\n"; printItemSet(out, library->itemsForKeyword("Keyword one")); // items for artist cout << ">>> books by Author:\n\n"; printItemSet(cout, library->booksByAuthor("Author")); delete library; // in Visual Studio, reports on memory leaks in the Output window #ifdef _WIN32 _CrtDumpMemoryLeaks(); #endif }
Item.h:
c++ Syntax (Toggle Plain Text)
#pragma once #include <ostream> #include <set> #include <string> using namespace std; typedef set<string> StringSet; class Item { protected: string title; string artist; StringSet* keywords; public: Item(const string& title, const string& artist); virtual ~Item(); void addKeyword(const string& keyword) const; const string* getTitle() const; const string* getArtist() const; const StringSet* getKeywords() const; string printKeywords(const StringSet* keywords) const; virtual void print(ostream& out) const; }; ostream& operator<<(ostream& out, const Item* const item);
Item.cpp:
c++ Syntax (Toggle Plain Text)
#include "Item.h" Item::Item(const string& title, const string& artist) :title(title),artist(artist),keywords(new StringSet) { } Item::~Item() { delete keywords; } void Item::addKeyword(const string& keyword) const { keywords->insert(keyword); } const string* Item::getTitle() const { return &title; } const string* Item::getArtist() const { return &artist; } const StringSet* Item::getKeywords() const { return keywords; } string Item::printKeywords(const StringSet* keywords) const { string kwString; StringSet::const_iterator itr; for(itr = keywords->begin();itr != keywords->end();itr++) kwString.append(*itr + ", "); kwString.erase(kwString.end() - 2, kwString.end()); return kwString; } void Item::print(ostream& out) const { out << "Should not be printing this!" << endl; } ostream& operator<<(ostream& out, const Item* item) { item->print(out); return out; }
Book.h:
c++ Syntax (Toggle Plain Text)
#include "Item.h" using namespace std; class Book : public Item { public: Book(const string& title, const string& artist, const int nPages); ~Book(); const int getnPages() const; virtual void print(ostream& out) const; private: int numPages; }; ostream& operator<<(ostream& out, const Book* book);
Book.cpp:
c++ Syntax (Toggle Plain Text)
#include "Book.h" Book::Book(const string& title, const string& artist, const int nPages) :Item(title, artist), numPages(nPages) { } Book::~Book() { } const int Book::getnPages() const { return numPages; } void Book::print(ostream& out) const { out << "-Book-" << endl; out << "author: " << *this->getArtist() << endl; out << "pages: " << this->getnPages() << endl; out << "title: " << *this->getTitle() << endl; out << "keywords: " << this->printKeywords(this->getKeywords()) << endl; out << endl; } ostream& operator<<(ostream& out, const Book* book) { book->print(out); return out; }
Library.h:
c++ Syntax (Toggle Plain Text)
#pragma once #include <ostream> #include <set> #include <string> #include <typeinfo> #include "Item.h" #include "Book.h" using namespace std; typedef set<Item*> ItemSet; class Library { public: // Constructor Library(); // Destructor ~Library(); // general functions void addKeywordForItem(const Item* const item, const string& keyword); const ItemSet* itemsForKeyword(const string& keyword) const; void printItem(ostream& out, const Item* const item) const; // book-related functions const Item* addBook(const string& title, const string& author, int const nPages); const ItemSet* booksByAuthor(const string& author) const; const ItemSet* books() const; private: ItemSet* lib; ItemSet* temp; };
Library.cpp:
c++ Syntax (Toggle Plain Text)
#include "Library.h" Library::Library() :lib(new ItemSet), temp(new ItemSet) { } Library::~Library() { for(ItemSet::iterator itr = lib->begin();itr != lib->end();itr++) delete *itr; delete lib; delete temp; } // general functions void Library::addKeywordForItem(const Item* const item, const string& keyword) { item->addKeyword(keyword); } const ItemSet* Library::itemsForKeyword(const string& keyword) const { StringSet* kw; temp->clear(); for(ItemSet::iterator itr = lib->begin();itr != lib->end();itr++) { kw = const_cast<StringSet*>((*itr)->getKeywords()); if((kw->find(keyword)) != kw->end()) temp->insert(*itr); } return temp; } void Library::printItem(ostream& out, const Item* const item) const { out << item; } // book-related functions const Item* Library::addBook(const string& title, const string& author, const int nPages) { Item* book = new Book(title, author, nPages); lib->insert(book); return book; } const ItemSet* Library::booksByAuthor(const string& author) const { string book = "class Book"; temp->clear(); for(ItemSet::iterator itr = lib->begin();itr != lib->end();itr++) { if(book.compare(typeid(*(*itr)).name()) == 0) { if(author.compare(*(*itr)->getArtist()) == 0) temp->insert(*itr); } } return temp; } const ItemSet* Library::books() const { string book = "class Book"; temp->clear(); for(ItemSet::iterator itr = lib->begin();itr != lib->end();itr++) { if(book.compare(typeid(*(*itr)).name()) == 0) temp->insert(*itr); } return temp; }
![]() |
Other Threads in the C++ Forum
- Previous Thread: getline is being skipped
- Next Thread: c++ beginner help
| Thread Tools | Search this Thread |
api array based binary c++ c/c++ calculator char char* class classes code coding compile console conversion count database delete deploy desktop developer directshow dll download dynamic dynamiccharacterarray email encryption error file forms fstream function functions game givemetehcodez google graph gui homeworkhelp iamthwee ifstream input int integer java lib linkedlist linker linux list loop looping loops map math matrix memory multiple news number numbertoword output parameter pointer problem program programming project python random read recursion recursive reference return rpg sorting string strings struct temperature template templates test text text-file tree unix url variable vector video visualstudio win32 windows winsock wordfrequency wxwidgets






