std::vector mangles a char*

[kerzack]

char* cFileIO::readStringFromFile()
{
   string line;
   getline(*readFile, line);
   char str[70];
   strcpy(str, line.c_str());
   printf("%s\n", str);
   char* tosaveto = str;
   return tosaveto;
}

for(int x=0; x<map->numCountries; x++)
   {
      char* t = readStringFromFile();
      printf("%s\n", t);
      map->countryNames.push_back(t);
      printf("%s\n", map->countryNames[x]);
      map->countryRects.push_back(readRectFromFile());
   }

When I call push_back(t), the string gets mangled. The first time I printf it it's fine, but the second time it's mangled. Anyone have any ideas?

The rectangle vertices come out printed fine, it's just the char*s that don't.

[Paul McKenzie]

There is a lot wrong with your code:

char* cFileIO::readStringFromFile()
{
string line;
getline(*readFile, line);
char str[70];
strcpy(str, line.c_str());
printf("%s\n", str);
char* tosaveto = str;
return tosaveto;
}

You are returning the address of a local variable. This is undefined behaviour.

Second, you never showed what the definition of the vector is. Is it a vector<string>, a vector<char *>, ...?

If it is a vector<char *> make it a vector<string>.

Third, why are you using char * at all? What's wrong with just using std::string throughout your code?

Regards,

Paul McKenzie

[kerzack]

It is a vector<char*> and because it seemed to be the best idea for whatever reason. I just converted everything over to use string instead, everything works flawlessly. Thanks Paul

[MrDoomMaster]

It is a vector<char*> and because it seemed to be the best idea for whatever reason. I just converted everything over to use string instead, everything works flawlessly. Thanks Paul

I hope you never have to copy a vector of string objects. That's the downside to using std::string most of the time.

[Paul McKenzie]

It is a vector<char*> and because it seemed to be the best idea for whatever reason. I just converted everything over to use string instead, everything works flawlessly.

The reason why it now works (not withstanding the problem with returning the char* in your function) is that when you use a vector<char*> you are *not* pushing a string onto the vector. What you're doing is pusing a pointer value into the vector.

See the problem this causes in the code below:

#include <vector>
#include <iostream>
#include <cstring>

int main()
{
    std::vector<char *> Vect;
    char p[] = "abc";
    Vect.push_back(p);
    strcpy(p, "123");
    Vect.push_back(p);
    std::cout << Vect[0] << " " << Vect[1];
}

Output:
123 123

What happened to the "abc" that was placed in the vector? What was placed was a pointer value, not a string. So whatever that pointer value happens to point to, that is what will be in the vector.

If you want to store strings (actual strings, not char *), then you use a vector<string>. As a matter of fact, I would recommend never using a container based of char* if the goal is to use a string. It should be vector<string>, list<string>, map<string, whatever>, etc.

Regards,

Paul McKenzie

[Paul McKenzie]

I hope you never have to copy a vector of string objects. That's the downside to using std::string most of the time.

Usually vectors are passed by reference, so this should not be a problem. However, if the goal of the program is to actually copy strings, then copying vectors would be part of the design.

In any event, depending on the compiler, returning a vector of strings by value is usually optimized away and the penalty isn't incurred.

Regards,

Paul McKenzie

[kerzack]

Thanks for your help, I made changes to the code and it works and doesn't return incorrectly anymore. Also, I won't be trying to copy vectors, but if for some reason I have to I guess I'll jump that hurdle when I get to it.

[MrDoomMaster]

In any event, depending on the compiler, returning a vector of strings by value is usually optimized away and the penalty isn't incurred.

Can you explain this? If I do this:

std::vector<std::string> a;
// fill 'a' with some strings here

std::vector<std::string> b( a );

// eventually use both 'a' and 'b' later on

how can the compiler optimize that? Are you talking about NRVO for temporary copies?