Evan Cooper
Evan Cooper

Reputation: 124

"Use & to create a pointer to a member" when using c_str

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

Upvotes: 1

Views: 453

Answers (4)

BoBTFish
BoBTFish

Reputation: 19767

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.

Upvotes: 5

Sebastian Redl
Sebastian Redl

Reputation: 72044

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.

Upvotes: 1

segfault
segfault

Reputation: 71

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

Upvotes: 0

Vittorio Romeo
Vittorio Romeo

Reputation: 93324

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*>.

Upvotes: 2

Related Questions