Memory management problems 3

[fugigoose]

I have a vector of char arrays declared as such:

vector <char*> aliasList;

I add stuff to it like this (alias is a const char* passed to the add function):

aliasList.push_back(new char[strlen(alias)]);
strcpy(aliasList.back(), alias);

So being that I dynamically added it, i assume i would have to delete all of them on close, like such:

if (aliasList.size()){
for (int i = 0; i < aliasList.size(); i++){
delete [] aliasList[i];
}
aliasList.clear();
}

The 'delete [] aliasList' generates a huge error with a big fat X and a loud alert sound. It says:

"Damage: After normal block #228" etc.

Can anyone tell me why it doesn't like that line? I've also tried this:

delete aliasList[i]

It doesn't work either, although being each element in the vector is an array of chars, the first is the one I would think I should use.

If someone could either tell me why it doesnt like that code or if its even necessary to delete these elements, that be awesome. Thanks!

 

[ArkM]

You allocate too small arrays for your string: you forget about string zero terminator. True size for new is strlen(alias)+1. Then strcpy() crashes your heap with this zero terminator. Now your program is in a big trouble (as you see...

 

[timmay3141]

Here's the problem

aliasList.push_back(new char[strlen(alias)]);
strcpy(aliasList.back(), alias);

Doing it this way will mean there is no space for a null terminating character, which is very bad. strcpy will assume that you need a null terminating character and write one extra char on the end, which is more than you allocated and causes the damage. Do this instead:

aliasList.push_back(new char[strlen(alias) + 1]);

You might consider std::string as well.

 

[timmay3141]

lol, ArkM you beat me to this one AND the atoi one. We came up with the same solutions separately on both, yet you posted before me both times. Pretty funny.

 

[xwb]

As timmay mentioned, why not use string instead of char[]. Then you don't have to worry about deleting them when you tidy up

vector<string> aliaslist;

// To allocate
alisalist.push_back (string (alias));

// To clear
aliaslist.clear ();

// or if you have a really big list - this swaps it 
// with the one on the stack.  For some reason 
// unknown to myself, this is actually faster than a clear.
{
   vector<string> emptylist;
   swap (aliaslist, emptylist);
}

 

[fugigoose]

Thanks a bunch guys, that fixed it right up, plus using string made the code a lot neater and probably more efficient. I was not aware you could use vectors like that. Right now i have another vector. It's a vector of pointers to instances of class cls_image, declared as such:

vector <cls_image*> imageCache;

And I add to it like this:

imageCache.push_back(new cls_image);
imageCache.back()->width = 100;
imageCache.back()->height = 100;
//etc.

And at the end I loop through imageCache, and delete imageCache then clear(). So now looking at how you had me do the aliasList vector, could i simply make a vector of class instances themselves?

vector <cls_image> imageCache;

And add to it like this:

imageCache.push_back(cls_image);
imageCache.width = 100;
imageCache.height = 100;
//etc.

And thus avoiding having to do the loop and delete thing at shutdown and simply clear the vector?

Would this work, or is it better the way i have it? Thanks.

 

[globos]

One things to know about STL containers is that they "owns" their items, i.e. containers manage their items with value semantics. To this end, STL containers make assumptions about the type of the items managed.

Say, if you have a vector<G>, then 'G' generic parameter must have :
- An empty constructor : G ()
- A copy construtor : G (const G& other)
- operator= defined : G& operator= (const G& other)
'const' for the two last routines is not mandatory.

A vector<G> instance owns copies of objects of type G, these copies are made thanks to the three routines below.

Back to your case, if your actual generic parameter is a 'cls_image*' then you not need to have those three routines defined on your class cls_image(because type 'cls_image*' already has those routines pre-defined).
But if your actual generic parameter is simply 'cls_image' then you should define these routines :

//Basic cls_image skeleton that will work with STL containers
class cls_image
{
public:
  cls_image ()
  {
    //FIXME: implement me
  }

  cls_image (const cls_image& other)
  {
    //FIXME: implement me
  }

  cls_image& operator= (const cls_image& other)
  {
    //FIXME: implement me
    return *this;
  }

};

A last thing, vector<cls_image*> is a polymorphic vector, but vector<cls_image> is not.

--
Globos