1

I'm trying to add a char* to a vector of char*'s by casting it over from a string. Here's the code I'm using:

vector<char*> actionLog;

// lots of code

int value = ...

// lots of code

string str = "string";
cout << str << value << endl;
str += std::to_string(player->scrap);
actionLog.push_back(str.c_str());

The problem is that I get the specified "Use & to create a pointer to a member" error for the push_back line. str.c_str should return a char*, which is the type that actionLog uses. I'm either incorrect about how c_str works, or doing something else wrong. Pushing to actionLog with

actionLog.push_back("something");

Works fine, but not what I mentioned. What am I doing wrong?

EDIT: I was actually using c_str() as a function, I just copied it incorrectly

Evan Cooper
  • 104
  • 1
  • 11
  • 3
    why are you not storing the string itself in the vector? even the `const char*` from `c_str()` might not be valid forever and your program might crash or use undefined behavior. – xander Mar 16 '17 at 09:31
  • Please don't edit your question to invalidate other's answer! – Danh Mar 16 '17 at 09:46

4 Answers4

5

There are actually several problems with what you are trying to do.

  1. Firstly, c_str is a member function. You would have to call it, using (): str.c_str().
  2. c_str() returns a const char*, so you won't be able to store it in a vector<char*>. This is so you can't break the std::string by changing it's internals in ways it doesn't expect.
  3. You really should not store the result of c_str(). It only remains valid until you do some non-const operation on the std::string it came from. I.e. if you make a change to the content of the std::string, then try to use the corresponding element in the vector, you have Undefined Behaviour! And from the way you have laid out your example, it looks like the lifetime of the string will be much shorter than the lifetime of the vector, so the vector would point to something that doesn't even exist any more.

Maybe it's better to just use a std::vector<std::string>. If you don't need the original string again after this, you could even std::move it into the vector, and avoid extra copying.

As an aside, please reconsider your use of what are often considered bad practices: using namespace std; and endl (those are links to explanations). The latter is a bit contentious, but at least understand why and make an informed decision.

Community
  • 1
  • 1
BoBTFish
  • 19,167
  • 3
  • 49
  • 76
  • I read through both links, and while I do recognize why those can be considered bad practice, I don't find it to be a necessary change at the moment. I will likely change it some time soon, when I don't have a feature deadline breathing down my back – Evan Cooper Mar 16 '17 at 10:21
2

std::basic_string::c_str() is a member function, not a data member - you need to invoke it by using ().

The correct code is:

actionLog.push_back(str.c_str());

Note that std::basic_string::c_str() returns a pointer to const char - your actionLog vectors should be of type std::vector<const char*>.

Vittorio Romeo
  • 90,666
  • 33
  • 258
  • 416
  • I copied the code incorrectly, I was using 'c_str()', but right as I was about read this answer, I found that the issue was not using 'const'. Thanks! I'll accept this as correct when I can – Evan Cooper Mar 16 '17 at 09:28
1

Vittorio's answer tells you what you did wrong in the details. But I would argue that what you're doing wrong is really using a vector<char*> instead of a vector<string> in the first place.

With the vector of pointers, you have to worry about lifetime of the underlying strings, about them getting invalidated, changed from under you, and so on. The name actionLog suggests that the thing is long-lived, and the code you use to add to it suggests that str is a local helper variable used to build the log string, and nothing else. The moment str goes out of scope, the vector contains a dangling pointer.

Change the vector to a vector<string>, do a actionLog.push_back(str), and don't worry about lifetimes or invalidation.

Sebastian Redl
  • 69,373
  • 8
  • 123
  • 157
  • I originally used char* because 1) I am using other functions that require char*'s and I can't change them, 2) I'm very tired, and 3) I hadn't even thought about this particular issue until running into it, and since I'm very used to C, I like char* – Evan Cooper Mar 16 '17 at 09:34
0

You forgot (), c_str is a method, not a data member. Just write actionLog.push_back(str.c_str());

segfault
  • 71
  • 4