Solved

memory allocation/deallocation for a vector of pointers

Posted on 2004-09-02
16
957 Views
Last Modified: 2013-12-14
I have code like the following:

vector <Widget *> GetWidgets(){
   vector <Widgets *> widgets;
   for(i=0;i<n;i++){
     Widget * widget = new Widget();
     widgets.push_back(widgets);
}

void DeleteWidget(Widget*& widget_p){
    if(widget_p){
       delete widget_p;
       widget_p = 0;
  }
}

main(){
  for(some loop){
    vector <Widget *> widgets = GetWidgets();
      ... do stuff with widgets...
 
     for_each(widgets.begin(), widgets.end(), DeleteWidget);
  }
}


------
Before I added the last line, I understandably had a memory leak.
Now, the code runs and the memory leak seems to be gone,  but I get the
following message during run time:

*** malloc[24383]: Deallocation of a pointer not malloced: 0x2809420; This could be a double free(), or free() called with the middle of an allocated block; Try setting environment variable MallocHelp to see tools to help debug
staring loop contents

It seems that I am only getting this message for one widget each run of the loop (the rest delete without the error)
What's going on here, how do I fix it, or is there a much better way altogether to delete my vector of
widget pointers?

Thanks!
0
Comment
Question by:axmeno
  • 5
  • 3
  • 2
  • +3
16 Comments
 
LVL 11

Accepted Solution

by:
avizit earned 100 total points
ID: 11968490
can you try this i

vector <Widget *>::iterator vec_iterator;

vec_iterator = my_vector.begin();

while (vec_iterator != my_vector.end()){
    DeleteWidget( *vec_iterator);  
    ++vec_iterator;
}


instead of

 for_each(widgets.begin(), widgets.end(), DeleteWidget);
0
 

Author Comment

by:axmeno
ID: 11968562
No dice.  This seems to do the same thing -- same memory usage and same error message.
0
 

Author Comment

by:axmeno
ID: 11968680
Left out a piece of information that may be relevant.

one peice  of the "do stuff to widgets" was

for_each(widgets.begin(), widgets.end(), DeleteBadWidget);
widgets.erase(remove(widgets.begin(),widgets.end(),static_cast<Widget*>(0)), widgets.end());

void DeleteBadWidget(Widget*& widget_p){
    if(some condition){
       delete widget_p;
       widget_p = 0;
  }
}

Just one bad widget is getting deleted here.  This is probably the one that's giving the error message -- am I deleting
it a second time?  How do I get it out of the widgets vector the first time, so I don't do that?

If I take out the widgets.erase line, or replace these two lines with the code suggested above as well, I get a bus error.
I'm not completely clear on what this line is doing anyways...can someone explain?

Thanks!


0
NAS Cloud Backup Strategies

This article explains backup scenarios when using network storage. We review the so-called “3-2-1 strategy” and summarize the methods you can use to send NAS data to the cloud

 
LVL 30

Assisted Solution

by:Axter
Axter earned 100 total points
ID: 11968835
Try copying and pasting your actual code here, so we can see exactly what your using.

The code you posted in your question has many compile errors, so I'm sure it's not a copy and paste of your original code.
0
 
LVL 11

Expert Comment

by:avizit
ID: 11968839
but in your

void DeleteWidget(Widget*& widget_p){
    if(widget_p){
       delete widget_p;
       widget_p = 0;
  }


you are checking if widget_p is 0 . so if it was deleted earlier the code wont do a  delete widget_p; on this..

anything I think you can put a cout<< on both the function

DeleteWidget() and DeleteBadWidget()
 
to see your program abended in which function .





0
 

Author Comment

by:axmeno
ID: 11968846
The last comment on the intermediate removal was a red herring, I've removed it and still have the error message.  I'll try to directly extract the actually relevant code and paste it.
0
 
LVL 30

Expert Comment

by:Axter
ID: 11968847
Modified version of your code, which does work to create and delete all instances, and therefore has no memory leaks.

int QtyExistingInstance = 0;

class Widget
{
public:
      Widget()
      {
            ++QtyExistingInstance;
      }
      ~Widget()
      {
            --QtyExistingInstance;
      }
};

vector <Widget *> GetWidgets(int n){
  vector <Widget *> widgets;
  for(int i=0;i<n;i++){
    Widget * widget = new Widget();
    widgets.push_back(widget);
  }
  return widgets;
}

void DeleteWidget(Widget*& widget_p){
   if(widget_p){
      delete widget_p;
      widget_p = 0;
 }
}

int main(){
 for(int i = 0;i < 3;++i){
   vector <Widget *> widgets = GetWidgets(11);
     for_each(widgets.begin(), widgets.end(), DeleteWidget);
 }
 cout << "Value of QtyExistingInstance = " << QtyExistingInstance << endl;
 return 0;
}
0
 
LVL 30

Expert Comment

by:Axter
ID: 11968859
If you have a memory leak, it's more then likely caused by code you still have not shown us yet.

Please copy and paste the code in a comment, and post it so we can see **exactly** what you're compiling.
0
 

Author Comment

by:axmeno
ID: 11969152
Okay, here' s the essense of the code.  This compiles by itself and generates the error  (though, it runs just fine, outputting this error, as does
my more complex code).  Suggestions on how to do this in a more sensible way are welcome.


#include <vector>
#include <algorithm>

using namespace std;

class Galaxy{
 public:
  Galaxy(){};
};

void Delete(Galaxy*& pgalaxy){
  if(pgalaxy){
    delete pgalaxy;
    pgalaxy =0;
  }
}

vector <Galaxy *> GetGalaxies(){
  vector <Galaxy *> galaxies;
  Galaxy * g = 0;
  int ngals = 100;
  for(int i=0;i<ngals;i++){
    g = new Galaxy();
    galaxies.push_back(g);
  }
  delete g;
  return galaxies;  
}

int main(){
  vector <Galaxy*> galaxies = GetGalaxies();
  for_each(galaxies.begin(),galaxies.end(),Delete);
  galaxies.erase(remove(galaxies.begin(),galaxies.end(),static_cast<Galaxy*>(0)), galaxies.end());
}

0
 

Author Comment

by:axmeno
ID: 11969172
Aha!  For some reason I had an extraneous delete g in there.  All is well now.
Anyways, if someone has something worthwhile to say about the basic way I'm
doing things above, I'm still happy to award some points.

Cheers...
0
 
LVL 3

Assisted Solution

by:Indrawati
Indrawati earned 100 total points
ID: 11969252
void Delete(Galaxy*& pgalaxy){
  if(pgalaxy){
    delete pgalaxy;
    pgalaxy =0;
  }
}

C++ delete already checked if the pointer is null before deleting it, so there's no need for you to check it again. So maybe you can change it to:
void Delete(Galaxy*& pgalaxy){
    delete pgalaxy;
    pgalaxy =0;
}


vector <Galaxy *> GetGalaxies()
instead of returning by value, maybe you can return by const reference instead to prevent unnecessary copying? Something like: const vector <Galaxy *>& GetGalaxies()

galaxies.erase(remove(galaxies.begin(),galaxies.end(),static_cast<Galaxy*>(0)), galaxies.end());
I don't understand this... You have already performed for_each and delete on the line before it, so all the elements in galazies should be zero now. If you want to erase all the elements in the vector, you can use galazies.clear();
0
 
LVL 3

Assisted Solution

by:teratoma
teratoma earned 100 total points
ID: 11978076
If you used boost::shared_pointer (www.boost.org) you wouldn't have to delete anything.
0
 

Expert Comment

by:s_senthil_kumar
ID: 12029659
How about a typedef for vector<Widget *>

typedef vector<Widget *> WidgetList;

1. That helps to improve code readability, you can write
WidgetList GetGalaxies() {...}

2. It allows you to change it from a vector to something else, say a list, whithout modifying code at a hundred places.

And Indrawati, the author can't return by reference as he/she is creating the vector inside the function, you can't return references to local objects.
0

Featured Post

3 Use Cases for Connected Systems

Our Dev teams are like yours. They’re continually cranking out code for new features/bugs fixes, testing, deploying, testing some more, responding to production monitoring events and more. It’s complex. So, we thought you’d like to see what’s working for us.

Question has a verified solution.

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

Suggested Solutions

Title # Comments Views Activity
c++ syntax question 9 48
gdb doesn't stop on breakpoint 2 67
oracle 11g 23 80
How to convert Structure to vector of byte and vice versa 5 14
Introduction This article is the first in a series of articles about the C/C++ Visual Studio Express debugger.  It provides a quick start guide in using the debugger. Part 2 focuses on additional topics in breakpoints.  Lastly, Part 3 focuses on th…
Go is an acronym of golang, is a programming language developed Google in 2007. Go is a new language that is mostly in the C family, with significant input from Pascal/Modula/Oberon family. Hence Go arisen as low-level language with fast compilation…
The viewer will learn how to pass data into a function in C++. This is one step further in using functions. Instead of only printing text onto the console, the function will be able to perform calculations with argumentents given by the user.
The viewer will be introduced to the member functions push_back and pop_back of the vector class. The video will teach the difference between the two as well as how to use each one along with its functionality.

770 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