Cannot push C style strings into std::vector

Question

I'm trying to push some const char* into a vector, but the vector remains unpopulated after performing the operations I would presume to fill it.

Here's my attempt, where dict is my command-line argument.

test.cc

#include <iostream>
#include <string>
#include <vector>
#include <fstream>

using namespace std;

int main(int argc, char **argv) 
{
  ifstream dict;
  size_t dict_size;

  dict.open(argv[1]); // Dictionary

  vector<const char*> dictionary; 

  string line;
  getline(dict, line);

  while(!dict.fail()) {
    dictionary.push_back(line.c_str());
    getline(dict, line);
  }

  dict_size = dictionary.size();

  for(int i = 0; i < dict_size; i++)
      cout << "dictionary[" << i << "] is " << dictionary[i] << endl;
}

dict

Hello
World
Foo
Bar

After compiling this, I get the following output:

dictionary[0] is 
dictionary[1] is 
dictionary[2] is 
dictionary[3] is

However, if I change the dictionary's type to vector and push back line instead of line.c_str(), I get the expected output:

dictionary[0] is Hello
dictionary[1] is World
dictionary[2] is Foo
dictionary[3] is Bar

I'm not terribly familiar with C style strings, so maybe it has something to do with null termination?

Remember the C strings are not special pointers. They work like all other pointers. — chris, Mar 29 '15 at 18:52
You are making bad assumptions about the lifetime of a pointer returned from `std::string::c_str()`. See [What is std::string::c_str() lifetime?](http://stackoverflow.com/questions/6456359). So the answer to your question here is "No. It has nothing to do with null termination". — Drew Dormann, Mar 29 '15 at 18:52
Is there a reason you have to use `const char*` in your `std::vector` rather than a `std::string`? — Galik, Mar 29 '15 at 19:30
@Galik I am passing these to a function that requires arguments as `const char*`. I'm passing 118k of them, so converting from string to char* each time is expensive. — erip, Mar 29 '15 at 19:50
@erip That depends on how you're passing them. If you are passing them as a whole block then yes but if you're passing them one at a time using `std::string::c_str()` then probably not. The compiler will likely optimize the `c_str()` function call away completely. It is worth doing a comparison to see if there is any difference (remember to set compiler optimizations). — Galik, Mar 29 '15 at 20:08

score 4 · Accepted Answer · answered Mar 29 '15 at 18:59

4

You are storing dangling pointers.

std::string::c_str() isn't a pointer to some permanent copy of data — just think, that would be leaked!

Store the std::strings instead.

answered Mar 29 '15 at 18:59

Lightness Races in Orbit

378,754
76
643
1,055

Thanks, this makes sense. I decided to just store the `std::string` and convert to `std::string::c_str` when necessary. – erip Mar 30 '15 at 01:13

score 2 · Answer 2 · answered Mar 29 '15 at 19:00

2

Your code invokes undefined behavior, because after you do

dictionary.push_back(line.c_str());

On the next line that pointer may get deleted:

getline(dict, line); // line now is a different string

answered Mar 29 '15 at 19:00

Anton Savin

40,838
8
54
90

score 0 · Answer 3 · answered Mar 29 '15 at 19:46

You are pushing into the dictionary pointers that point to the same address and at the last iteration it fills the memory area with an empty string. If you don't care about memory leakage you can try like this:

#include <iostream>
#include <string>
#include <vector>
#include <fstream>

using namespace std;

int main(int argc, char **argv) 
{
  ifstream dict;
  size_t dict_size;

  dict.open(argv[1]); // Dictionary

  vector<char *> dictionary; 

 while(!dict.fail()) {
 string * line = new string();
 getline(dict, *line);
 if(line->length()>0)
 {
   dictionary.push_back((char *)line->c_str());
 }
}

  dict_size = dictionary.size();

 for(int i = 0; i < dict_size; i++)
      cout << "dictionary[" << i << "] is " << dictionary[i] << endl;
}

_"pointers that point to the same address"_ That's actually extremely unlikely. It's just that the old addresses become invalid. And your suggested solution is kinda silly no offence — Lightness Races in Orbit, Mar 30 '15 at 00:42

Cannot push C style strings into std::vector

3 Answers3