Splitting a std::string into two const char*s resulting in the second const char* overwriting the first

Question

I am taking a line of input which is separated by a space and trying to read the data into two integer variables.

for instance: "0 1" should give child1 == 0, child2 == 1.

The code I'm using is as follows:

int separator = input.find(' ');
const char* child1_str = input.substr(0, separator).c_str(); // Everything is as expected here.
const char* child2_str = input.substr(
    separator+1,  //Start with the next char after the separator
    input.length()-(separator+1) // And work to the end of the input string.
    ).c_str();     // But now child1_str is showing the same location in memory as child2_str!
int child1 = atoi(child1_str);
int child2 = atoi(child2_str);      // and thus are both of these getting assigned the integer '1'.
// do work

What's happening is perplexing me to no end. I'm monitoring the sequence with the Eclipse debugger (gdb). When the function starts, child1_str and child2_str are shown to have different memory locations (as they should). After splitting the string at separator and getting the first value, child1_str holds '0' as expected.

However, the next line, which assigns a value to child2_str not only assigns the correct value to child2_str, but also overwrites child1_str. I don't even mean the character value is overwritten, I mean that the debugger shows child1_str and child2_str to share the same location in memory.

What the what?

1) Yes, I'll be happy to listen to other suggestions to convert a string to an int -- this was how I learned to do it a long time ago, and I've never had a problem with it, so never needed to change, however:

2) Even if there's a better way to perform the conversion, I would still like to know what's going on here! This is my ultimate question. So even if you come up with a better algorithm, the selected answer will be the one that helps me understand why my algorithm fails.

3) Yes, I know that std::string is C++ and const char* is standard C. atoi requires a c string. I'm tagging this as C++ because the input will absolutely be coming as a std::string from the framework I am using.

Answer 1

First, the superior solutions.

In C++11 you can use the newfangled std::stoi function:

int child1 = std::stoi(input.substr(0, separator));

Failing that, you can use boost::lexical_cast:

int child1 = boost::lexical_cast<int>(input.substr(0, separator));

Now, an explanation.

input.substr(0, separator) creates a temporary std::string object that dies at the semicolon. Calling c_str() on that temporary object gives you a pointer that is only valid as long as the temporary lives. This means that, on the next line, the pointer is already invalid. Dereferencing that pointer has undefined behaviour. Then weird things happens, as is often the case with undefined behaviour.

Answer 2

The value returned by c_str() is invalid after the string is destructed. So when you run this line:

const char* child1_str = input.substr(0, separator).c_str();

The substr function returns a temporary string. After the line is run, this temporary string is destructed and the child1_str pointer becomes invalid. Accessing that pointer results in undefined behavior.

What you should do is assign the result of substr to a local std::string variable. Then you can call c_str() on that variable, and the result will be valid until the variable is destructed (at the end of the block).

Answer 3

Others have already pointed out the problem with your current code. Here's how I'd do the conversion:

std::istringstream buffer(input);

buffer >> child1 >> child2;

Much simpler and more straightforward, not to mention considerably more flexible (e.g., it'll continue to work even if the input has a tab or two spaces between the numbers).

Answer 4

input.substr returns a temporary std::string. Since you are not saving it anywhere, it gets destroyed. Anything that happens afterwards depends solely on your luck.

I recommend using an istringstream.