How to return pointer to object & avoid memory leak?

Please support our C++ advertiser: Intel Parallel Studio Home
Thread Solved

Join Date: Feb 2008
Posts: 18
Reputation: Undertech is an unknown quantity at this point 
Solved Threads: 1
Undertech's Avatar
Undertech Undertech is offline Offline
Newbie Poster

How to return pointer to object & avoid memory leak?

 
0
  #1
Aug 16th, 2008
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):

  1. const Item* Library::addBook(const string& title, const string& author, const int nPages)
  2. {
  3. Item* book = new Book(title, author, nPages);
  4. lib->insert(book); //lib is a set<Item*> container
  5. return book;
  6. }

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.
Reply With Quote Quick reply to this message  
Join Date: Nov 2007
Posts: 978
Reputation: mitrmkar is just really nice mitrmkar is just really nice mitrmkar is just really nice mitrmkar is just really nice mitrmkar is just really nice 
Solved Threads: 208
mitrmkar mitrmkar is offline Offline
Posting Shark

Re: How to return pointer to object & avoid memory leak?

 
1
  #2
Aug 16th, 2008
Go through the stored items in the Library 's destructor and explicitly delete the items there.
Last edited by mitrmkar; Aug 16th, 2008 at 11:29 am.
Reply With Quote Quick reply to this message  
Join Date: Oct 2007
Posts: 1,951
Reputation: Duoas has much to be proud of Duoas has much to be proud of Duoas has much to be proud of Duoas has much to be proud of Duoas has much to be proud of Duoas has much to be proud of Duoas has much to be proud of Duoas has much to be proud of 
Solved Threads: 214
Featured Poster
Duoas's Avatar
Duoas Duoas is offline Offline
Posting Virtuoso

Re: How to return pointer to object & avoid memory leak?

 
1
  #3
Aug 16th, 2008
In addition, you should have a removeBook() (or some such named) method(s) for the user's convenience.
Reply With Quote Quick reply to this message  
Join Date: Feb 2008
Posts: 18
Reputation: Undertech is an unknown quantity at this point 
Solved Threads: 1
Undertech's Avatar
Undertech Undertech is offline Offline
Newbie Poster

Re: How to return pointer to object & avoid memory leak?

 
0
  #4
Aug 16th, 2008
Originally Posted by mitrmkar View Post
Go through the stored items in the Library 's destructor and explicitly delete the items there.
I have attempted this but I am getting an access violation.

  1. Library::~Library()
  2. {
  3. for(ItemSet::iterator itr = lib->begin();itr != lib->end();itr++) //Causes access violation
  4. delete *itr;
  5. delete lib; //No problem
  6. delete temp; //No problem
  7. }

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?
Reply With Quote Quick reply to this message  
Join Date: Dec 2005
Posts: 5,850
Reputation: Salem has a reputation beyond repute Salem has a reputation beyond repute Salem has a reputation beyond repute Salem has a reputation beyond repute Salem has a reputation beyond repute Salem has a reputation beyond repute Salem has a reputation beyond repute Salem has a reputation beyond repute Salem has a reputation beyond repute Salem has a reputation beyond repute Salem has a reputation beyond repute 
Solved Threads: 749
Team Colleague
Salem's Avatar
Salem Salem is offline Offline
Void main'ers are DOOMed

Re: How to return pointer to object & avoid memory leak?

 
1
  #5
Aug 16th, 2008
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.
Reply With Quote Quick reply to this message  
Join Date: Feb 2008
Posts: 18
Reputation: Undertech is an unknown quantity at this point 
Solved Threads: 1
Undertech's Avatar
Undertech Undertech is offline Offline
Newbie Poster

Re: How to return pointer to object & avoid memory leak?

 
0
  #6
Aug 16th, 2008
You haven't been trying to delete books anywhere else have you?
Deleting the same book twice is a sure recipe for disaster.
Nope. In fact, the memory leaks are a result of the books not being deleted anywhere. The addBooks method is the only place in the program that allocates memory for an instance of Item (as derived class Book).

I see your addBook() returns a pointer to a book (why?). What else are you doing with that book pointer?
That was part of the skeleton code which I am not able to change. The driver routine passes this pointer to other functions, which do things like printing out the object data and adding additional data to the object. I am also not allowed to add to the parameter list, unfortunately.

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.
How would you 'remove' a book from the set? Given that the elements of the set are pointers to books, would you copy the pointer to a local variable, then perform set::erase on the set element and delete the local pointer?

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:

  1. // make no changes to this code, except to have it
  2. // print out your actual name instead of "your name"
  3.  
  4. #include <iostream>
  5. #include "Library.h"
  6.  
  7. using namespace std;
  8.  
  9. static Library *library;
  10. ....
  11. int main(int argc, char **argv)
  12. {
  13. ....
  14. const Item *item;
  15. ....
  16. // create library instance
  17. library = new Library();
  18. ....
  19. item = library->addBook();
  20. if (item != NULL) {
  21. //Do Stuff with item pointer
  22. }
  23. ....
  24. delete library; //The only occurrence of delete in this routine
  25. ....
  26. }
Reply With Quote Quick reply to this message  
Join Date: Feb 2008
Posts: 18
Reputation: Undertech is an unknown quantity at this point 
Solved Threads: 1
Undertech's Avatar
Undertech Undertech is offline Offline
Newbie Poster

Re: How to return pointer to object & avoid memory leak?

 
0
  #7
Aug 16th, 2008
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:
  1. iterator begin()
  2. { // return iterator for beginning of mutable sequence
  3. return (_TREE_ITERATOR(_Lmost())); //Access violation
  4. }
When I step through the code, the violation occurs immediately upon reaching the line
  1. delete library;
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.
Last edited by Undertech; Aug 16th, 2008 at 10:35 pm. Reason: Updated with additonal findings
Reply With Quote Quick reply to this message  
Join Date: Dec 2005
Posts: 5,850
Reputation: Salem has a reputation beyond repute Salem has a reputation beyond repute Salem has a reputation beyond repute Salem has a reputation beyond repute Salem has a reputation beyond repute Salem has a reputation beyond repute Salem has a reputation beyond repute Salem has a reputation beyond repute Salem has a reputation beyond repute Salem has a reputation beyond repute Salem has a reputation beyond repute 
Solved Threads: 749
Team Colleague
Salem's Avatar
Salem Salem is offline Offline
Void main'ers are DOOMed

Re: How to return pointer to object & avoid memory leak?

 
0
  #8
Aug 17th, 2008
0xfeeefeee is one of the patterns filled in memory after it has been freed.
http://www.nobugs.org/developer/win3..._crt_heap.html
Reply With Quote Quick reply to this message  
Join Date: Nov 2007
Posts: 978
Reputation: mitrmkar is just really nice mitrmkar is just really nice mitrmkar is just really nice mitrmkar is just really nice mitrmkar is just really nice 
Solved Threads: 208
mitrmkar mitrmkar is offline Offline
Posting Shark

Re: How to return pointer to object & avoid memory leak?

 
0
  #9
Aug 17th, 2008
Originally Posted by Undertech View Post
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()
Then the destructor of the derived class will not get called when you
do a thing like ...
  1. Item* book = new Book(title, author, nPages);
  2. delete book; // ~Book() will not be invoked

Maybe you should post the complete code ...
Reply With Quote Quick reply to this message  
Join Date: Feb 2008
Posts: 18
Reputation: Undertech is an unknown quantity at this point 
Solved Threads: 1
Undertech's Avatar
Undertech Undertech is offline Offline
Newbie Poster

Re: How to return pointer to object & avoid memory leak?

 
0
  #10
Aug 17th, 2008
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:
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:
  1. // make no changes to this code, except to have it
  2. // print out your actual name instead of "your name"
  3.  
  4. #ifdef _WIN32
  5. #define _CRTDBG_MAP_ALLOC
  6. #include <stdlib.h>
  7. #include <crtdbg.h>
  8. #endif
  9.  
  10. #include <iostream>
  11. #include "Library.h"
  12.  
  13. using namespace std;
  14.  
  15. static Library *library;
  16.  
  17. static void printItemSet(ostream& out, const ItemSet *itemSet)
  18. {
  19. if (itemSet != NULL && itemSet->size() > 0)
  20. for (ItemSet::const_iterator i = itemSet->begin(); i != itemSet->end(); i++)
  21. library->printItem(out, (Item *) *i);
  22. else
  23. out << "none" << endl << endl;
  24. }
  25.  
  26. int main(int argc, char **argv)
  27. {
  28. ostream& out(cout);
  29. const Item *item;
  30.  
  31. // create library instance
  32. library = new Library();
  33.  
  34. // add items to library
  35. cout << ">>> adding items to library:\n\n";
  36. item = library->addBook("Book Title", "Joe Author", 249);
  37. if (item != NULL) {
  38. library->addKeywordForItem(item, "Keyword one");
  39. library->addKeywordForItem(item, "Another keyword");
  40. library->printItem(cout, item);
  41. }
  42.  
  43. //Two more books added like the one above
  44.  
  45. cout << ">>> books:\n\n";
  46. printItemSet(out, library->books());
  47.  
  48. // print items for keyword
  49. cout << ">>> items for keyword \"Keyword one\"\n\n";
  50. printItemSet(out, library->itemsForKeyword("Keyword one"));
  51.  
  52. // items for artist
  53. cout << ">>> books by Author:\n\n";
  54. printItemSet(cout, library->booksByAuthor("Author"));
  55.  
  56. delete library;
  57.  
  58. // in Visual Studio, reports on memory leaks in the Output window
  59. #ifdef _WIN32
  60. _CrtDumpMemoryLeaks();
  61. #endif
  62. }

Item.h:
  1. #pragma once
  2.  
  3. #include <ostream>
  4. #include <set>
  5. #include <string>
  6.  
  7. using namespace std;
  8.  
  9. typedef set<string> StringSet;
  10.  
  11. class Item
  12. {
  13. protected:
  14. string title;
  15. string artist;
  16. StringSet* keywords;
  17.  
  18. public:
  19. Item(const string& title, const string& artist);
  20. virtual ~Item();
  21. void addKeyword(const string& keyword) const;
  22. const string* getTitle() const;
  23. const string* getArtist() const;
  24. const StringSet* getKeywords() const;
  25. string printKeywords(const StringSet* keywords) const;
  26. virtual void print(ostream& out) const;
  27. };
  28.  
  29. ostream& operator<<(ostream& out, const Item* const item);

Item.cpp:
  1. #include "Item.h"
  2.  
  3. Item::Item(const string& title, const string& artist)
  4. :title(title),artist(artist),keywords(new StringSet)
  5. {
  6. }
  7.  
  8. Item::~Item()
  9. {
  10. delete keywords;
  11. }
  12.  
  13. void Item::addKeyword(const string& keyword) const
  14. {
  15. keywords->insert(keyword);
  16. }
  17.  
  18. const string* Item::getTitle() const
  19. {
  20. return &title;
  21. }
  22.  
  23. const string* Item::getArtist() const
  24. {
  25. return &artist;
  26. }
  27.  
  28. const StringSet* Item::getKeywords() const
  29. {
  30. return keywords;
  31. }
  32.  
  33. string Item::printKeywords(const StringSet* keywords) const
  34. {
  35. string kwString;
  36. StringSet::const_iterator itr;
  37.  
  38. for(itr = keywords->begin();itr != keywords->end();itr++)
  39. kwString.append(*itr + ", ");
  40. kwString.erase(kwString.end() - 2, kwString.end());
  41. return kwString;
  42. }
  43.  
  44. void Item::print(ostream& out) const
  45. {
  46. out << "Should not be printing this!" << endl;
  47. }
  48.  
  49. ostream& operator<<(ostream& out, const Item* item)
  50. {
  51. item->print(out);
  52. return out;
  53. }

Book.h:
  1. #include "Item.h"
  2.  
  3. using namespace std;
  4.  
  5. class Book : public Item
  6. {
  7. public:
  8. Book(const string& title, const string& artist, const int nPages);
  9. ~Book();
  10. const int getnPages() const;
  11. virtual void print(ostream& out) const;
  12.  
  13. private:
  14. int numPages;
  15. };
  16.  
  17. ostream& operator<<(ostream& out, const Book* book);

Book.cpp:
  1. #include "Book.h"
  2.  
  3. Book::Book(const string& title, const string& artist, const int nPages)
  4. :Item(title, artist), numPages(nPages)
  5. {
  6. }
  7.  
  8. Book::~Book()
  9. {
  10. }
  11.  
  12. const int Book::getnPages() const
  13. {
  14. return numPages;
  15. }
  16.  
  17. void Book::print(ostream& out) const
  18. {
  19. out << "-Book-" << endl;
  20. out << "author: " << *this->getArtist() << endl;
  21. out << "pages: " << this->getnPages() << endl;
  22. out << "title: " << *this->getTitle() << endl;
  23. out << "keywords: " << this->printKeywords(this->getKeywords()) << endl;
  24. out << endl;
  25. }
  26.  
  27. ostream& operator<<(ostream& out, const Book* book)
  28. {
  29. book->print(out);
  30. return out;
  31. }

Library.h:
  1. #pragma once
  2.  
  3. #include <ostream>
  4. #include <set>
  5. #include <string>
  6. #include <typeinfo>
  7. #include "Item.h"
  8. #include "Book.h"
  9.  
  10. using namespace std;
  11.  
  12. typedef set<Item*> ItemSet;
  13.  
  14. class Library
  15. {
  16.  
  17. public:
  18. // Constructor
  19. Library();
  20.  
  21. // Destructor
  22. ~Library();
  23.  
  24. // general functions
  25.  
  26. void addKeywordForItem(const Item* const item, const string& keyword);
  27. const ItemSet* itemsForKeyword(const string& keyword) const;
  28. void printItem(ostream& out, const Item* const item) const;
  29.  
  30. // book-related functions
  31.  
  32. const Item* addBook(const string& title, const string& author, int const nPages);
  33. const ItemSet* booksByAuthor(const string& author) const;
  34. const ItemSet* books() const;
  35.  
  36. private:
  37. ItemSet* lib;
  38. ItemSet* temp;
  39. };

Library.cpp
:
  1. #include "Library.h"
  2.  
  3. Library::Library()
  4. :lib(new ItemSet), temp(new ItemSet)
  5. {
  6. }
  7.  
  8. Library::~Library()
  9. {
  10. for(ItemSet::iterator itr = lib->begin();itr != lib->end();itr++)
  11. delete *itr;
  12. delete lib;
  13. delete temp;
  14. }
  15.  
  16. // general functions
  17.  
  18. void Library::addKeywordForItem(const Item* const item, const string& keyword)
  19. {
  20. item->addKeyword(keyword);
  21. }
  22.  
  23. const ItemSet* Library::itemsForKeyword(const string& keyword) const
  24. {
  25. StringSet* kw;
  26.  
  27. temp->clear();
  28. for(ItemSet::iterator itr = lib->begin();itr != lib->end();itr++)
  29. {
  30. kw = const_cast<StringSet*>((*itr)->getKeywords());
  31. if((kw->find(keyword)) != kw->end())
  32. temp->insert(*itr);
  33. }
  34. return temp;
  35. }
  36.  
  37. void Library::printItem(ostream& out, const Item* const item) const
  38. {
  39. out << item;
  40. }
  41.  
  42. // book-related functions
  43.  
  44. const Item* Library::addBook(const string& title, const string& author, const int nPages)
  45. {
  46. Item* book = new Book(title, author, nPages);
  47. lib->insert(book);
  48. return book;
  49. }
  50.  
  51. const ItemSet* Library::booksByAuthor(const string& author) const
  52. {
  53. string book = "class Book";
  54.  
  55. temp->clear();
  56. for(ItemSet::iterator itr = lib->begin();itr != lib->end();itr++)
  57. {
  58. if(book.compare(typeid(*(*itr)).name()) == 0)
  59. {
  60. if(author.compare(*(*itr)->getArtist()) == 0)
  61. temp->insert(*itr);
  62. }
  63. }
  64. return temp;
  65. }
  66.  
  67. const ItemSet* Library::books() const
  68. {
  69. string book = "class Book";
  70.  
  71. temp->clear();
  72. for(ItemSet::iterator itr = lib->begin();itr != lib->end();itr++)
  73. {
  74. if(book.compare(typeid(*(*itr)).name()) == 0)
  75. temp->insert(*itr);
  76. }
  77. return temp;
  78. }
Reply With Quote Quick reply to this message  
Reply

This thread has been marked solved.
Perhaps start a new thread instead?
Message:



Other Threads in the C++ Forum
Thread Tools Search this Thread



About Us | Contact Us | Advertise | DaniWeb | Acceptable Use Policy | RSS Feed

©2003 - 2009 DaniWeb® LLC