Unable to return the contents of a vector as a char* pointer

Question

I am trying to store a char* into a struct's char* field. I have tried different things but none of them worked. The problematic piece of code is shown below:

pInfo is the object of the struct PlayerInfo.

PlayerInfo *pInfo = (PlayerInfo*)malloc(sizeof(PlayerInfo));

The char* I get from GetAddress is stored in the Address field of PlayerInfo.

pInfo->Address = GetAddress(pInfo->playerId);

The GetAddress function is shown below. It converts integers to strings, stores them in a vector and returns the vector as a char* using &retChar[0].

char* GetAddress(int playerId)
{
  std::string strPlayerId = std::to_string(playerId);
  std::string strGroupId = std::to_string(group.GetGroupId());
  std::string retAddress = strPlayerId + ":" + strGroupId + ":" + GenRandomChar();

  //From -- http://stackoverflow.com/questions/347949/convert-stdstring-to-const-char-or-char
  std::vector<char> retChar(retAddress.begin(), retAddress.end());
  retChar.push_back('\0');

  for(std::vector<char>::const_iterator i = retChar.begin(); i != retChar.end(); ++i)
      std::cout << "retChar is " << *i << std::endl;
  return &retChar[0];
}

When I print the contents, only garbage is printed. I tried printing the memory contents from gdb, but that also did not help.

char* address = GetAddress(pInfo->playerId);
std::cout << "address is " << *address << std::endl;
std::cout << "address is " << pInfo->Address << std::endl;
std::cout << "address is " << *(pInfo->Address) << std::endl;

Why are you using `malloc` and not `new`? Second, you are returning a pointer to data which is cleaned up by `vector` when it goes out of scope. — Yakk - Adam Nevraumont, May 26 '14 at 18:25
Thanks. I did not realize that. I think I will just pass a reference and store the contents of the vector into the reference. — Sarvavyapi, May 26 '14 at 18:29
@Sarvavyapi a reference won't necessarily help you need to do some memory management - the easiest way is to make the field in PlayerInfo a std::string or if need a vector — mmmmmm, May 26 '14 at 19:27

πάντα ῥεῖ · Accepted Answer · 2014-05-26T19:32:48.723

4

The problem is, that your function scope local variable

 std::vector<char> retChar;

goes out of scope and is destroyed after your function returned.

Thus using the returned pointer return &retChar[0]; is calling undefined behavior.

The better choice would be to pass the pointer where to copy the data as a reference

 void GetAddress(int playerId, char*& result) {

     std::vector<char> retChar;
     // ...

     std::copy(retChar.begin(),result);
 }

and ensure result buffer is big enough to receive the copied data.

NOTE:
The above suggestion just solves the 1st level of your current problem. The probably better idea is to change your function simply to deal with std::string instead of using std::vector<char> and raw char* pointers (if your use case allows to refactor this):

Make PlayerInfo::Address member a std::string type

struct PlayerInfo {
    // ...
    std::string Address;
};

and define your GetAddress() function as follows

std::string GetAddress(int playerId) {
  std::ostringstream result;
  result << playerId << ":" group.GetGroupId() << ":" << GenRandomChar();
  return result.str();
}

and use the results std::string::c_str() method if you really need a const char* value to pass it elsewhere.

edited May 26 '14 at 19:32

answered May 26 '14 at 18:28

πάντα ῥεῖ

1
13
116
190

The problem is stated correctly in that the char* points to something unknown but the better fix is to pass back a vector and only get char* when needed i.e. to pass to a C function - Then the vector does all the memory management – mmmmmm May 26 '14 at 19:00
Well std::vector is still a reasonable thing to use - depends on the use – mmmmmm May 26 '14 at 19:24
1

@Mark _'depends on the use'_ Of course, but what it sounds from the OP's usage `std::string` is appropriate. I'll put another update clarifying (otherwise I mentioned _'(if your use case allows to refactor this)'_). – πάντα ῥεῖ May 26 '14 at 19:34
1

Yes, I can refactor the code to use a string instead of a char*. This is a personal project and I wanted to try out all such scenarios so that I learn and become comfortable with them. – Sarvavyapi May 27 '14 at 01:51
1

@Sarvavyapi Well, then the final answer might help you to proceed successfully ;) Glad to help if I could ... – πάντα ῥεῖ May 27 '14 at 02:00

score 3 · Answer 2 · edited May 23 '17 at 12:11

I think the idea of using std::vector<char> in the selected answer to How to convert a std::string to const char* or char*? is that the std::vector<char> is your writable character array, not that you should extract a char * from it. (You can, however, copy the contents into a different memory location identified by a char *, as you have already seen.)

But I would ask why you are storing a char * in the Address member of PlayerInfo. Why don't you make that member an std::string and change the return type of getAddress() to std::string, in which case getAddress can simply return retAddress?

Alternatively, you can declare getAddress like this: void getAddress(int playerId, std::string& retAddress) and the code inside the function is even simpler, because you don't have to declare retAddress as a local variable inside the function.

Unable to return the contents of a vector as a char* pointer

2 Answers2