reading/writing from ostringstream changes original data

Question

Inside a test, I write a given value to an ostringstream. After that, I try to evaluate if the correct value was written. However, it seems that the original value I wrote into the stream is changed later on while reading from it.

I reduced my problem to the following code:

#include <sstream>
#include <cassert>
#include <iostream>

int main()
{
    std::ostringstream os;
    uint16_t data{ 123 };
    os.write(reinterpret_cast<const char*>(&data), sizeof(uint16_t));

    uint16_t data_returned;
    std::string data_str(os.str());
  
    std::copy(data_str.begin(), data_str.end(), &data_returned);

    assert(data == 123); // <- not true
    assert(data_returned == 123);
}

Here data seems to be changed (to 0) by std::copy().

I also put the code on godbolt: https://godbolt.org/z/bcb4PE

Even more strange, if I change uint16_t data{ 123 }; to const uint16_t data{ 123 };, everything is fine.

Seems like I am missing some insights on the mechanics of std::copy().

Remy Lebeau · Accepted Answer · 2021-01-26T17:38:24.433

std::copy() does not do what you think it does in this situation. It is just a wrapper for a logical loop that copies individual elements from one container to another. You are essentially telling it to copy 2 individual chars from data_str into a uint16_t[2] array, except that you don't actually have such as array, so you have undefined behavior and are corrupting stack memory.

This statement:

std::copy(data_str.begin(), data_str.end(), &data_returned);

Is basically doing this:

std::string::iterator iter = data_str.begin(), end = data_str.end();
uint16_t *dest = &data_returned;
while (iter != end) {
    *dest++ = *iter++;
}

Which is basically just the equivalent of doing this, in your example:

uint16_t *dest = &data_returned;
dest[0] = static_cast<uint16_t>(data_str[0]);
dest[1] = static_cast<uint16_t>(data_str[1]);

It is assigning the 1st byte to the whole uint16_t (which is why you see the value change), and then it is assigning the 2nd byte to the next whole uint16_t (corrupting the stack).

For what you are attempting, use std::memcpy() instead, eg:

std::memcpy(&data_returned, data_str.c_str(), sizeof(data_returned));

Otherwise, if you really want to use std::copy(), you need to make sure it knows to copy individual bytes into the destination, not whole uint16_ts, eg:

std::copy(data_str.begin(), data_str.end(), reinterpret_cast<char*>(&data_returned));

Since both input and output are trivial types, that should get optimized down to an appropriate std::memcpy()-equivalent copy.

Ted Lyngmo · Answer 2 · 2021-01-26T18:01:47.110

In order to use std::copy() this way, you need to cast &data_returned to a char*, or else std::copy() will treat it as an uint16_t* and populate two uint16_ts instead of the two chars you aimed for. This makes your program have undefined behavior and that's why you see a changed value in data. The program could just as well have crashed or done something completely different.

std::copy(data_str.begin(), data_str.end(), reinterpret_cast<char*>(&data_returned));

Here's your program compiled with two extra options (-ggdb -fsanitize=address) to make it easier to catch problems like this in the future: Demo

reading/writing from ostringstream changes original data

2 Answers2