Prevent last element in char * vector from changing

Question

I am reading strings in C++ using fread where I am reading and storing shortSiteText in siteNames_. siteNames_ is declared as std::vector<char*> siteNames_; I use siteNames_ in other functions but because shortSiteText is a pointer, when I call the delete command on it, the last entry in siteNames_ is changed. How do I prevent this?

for (unsigned int i = 0; i <= nSites; i ++){
   fread((char *) &shortSiteTextLength, 1, sizeof shortSiteTextLength, baseFile_);
   shortSiteText = new char[shortSiteTextLength];
   fread(shortSiteText,1, shortSiteTextLength,baseFile_);
   siteNames_.push_back(shortSiteText);
}
delete [] shortSiteText;

I tried to use the dereference operator: siteNames_.push_back(*shortSiteText); but that generates a compiler error.

NOTE: I must use fread and char* due to legacy code.

Answer 1

You cannot delete[] anything you push into the vector until the associated vector element is done with.

It's not clear to me what the intent of this code is - you are only deleting the last-used value of shortSitetext anyhow, so this does not do what you think (which is trying to avoid leaking memory by matching new with delete, I think).

Remove the last line of code, and manually clean up the vector when you are done with it by iterating over the elements calling delete[] for each, then clear() the vector.

Or use boost::ptr_vector, which will do this automagically for you.

Or use vector<string>, to decouple the legacy char* code from your modern, non raw pointer using C++ world. You can push_back const char* directly to a vector<string> like so:

const char* str;
// init str to the value you wish

vector<string> vec;
vec.push_back(str);

Answer 2

Don't delete shortSiteText until you want to remove the data from the vector.

You've created a chunk of memory, set the data, and saved the pointer in your vector. The memory you delete is the same memory that the vector element points to.

Just removed the delete [] shortSiteText; line.

But, be sure that when you're done with the vector, to carefully remove and delete each pointer.

Answer 3

Let's zoom on this:

shortSiteText = new char[shortSiteTextLength];
siteNames_.push_back(shortSiteText);
delete [] shortSiteText;

Explanation: The second line just pushes a pointer to the array, not the array itself. The first line then desallocate the array, on which the last element of siteNames still points to; this leads to undefined behavior when you use this element.

Hack: Remove delete [] shortSiteText

Real fix: You encounter this problem because you try to manage object ownerships yourself. Don't ! Here, you can use std::string and still be able to work with legacy code, with the c_str() member function.

To quote a friend of mine:

As a general rule, if you are a beginner and your code contains the word 'char', you have a bug.