.c_str() weirdness? Data changes without rhyme or reason?

Question

I have this simple function:

const wchar_t *StringManager::GetWCharTStar(int stringId)
{
    std::wstring originalString = StringManager::GetString(stringId);
    const wchar_t *retStr = originalString.c_str();
    return retStr;
}

At the second line of that function, I have the correct wchar_t*. However, when I go to return, the data switches to garbage data. There are no functions in between. What gives?!

Answer 1

originalString is allocated on the stack. The .c_str() method just returns a pointer to some contiguous internal memory of the wstring object. When the function returns, originalString goes out of scope and is destroyed, therefore the pointer value you return points to deleted memory.

If you need to do this, you should make a copy of the data into memory you allocate with new or malloc(), and then the caller must delete/free() that memory.

Answer 2

You are returning a pointer to a temporary. When originalString goes out of scope, the data your pointer is pointing to is going to be deleted.

Answer 3

This is a FAQ. You are returning a pointer to an object that is freed (originalString object) before you actually get to use it.

Answer 4

std::wstring originalString; is a local variable inside the body of your GetWCharTStar function.

As soon as you leave the scope of the GetWCharTStar() function, this local variable gets destroyed and the pointer you return is no more valid.

The following code might eventually work:

const wchar_t *StringManager::GetWCharTStar(int stringId)
{
    const std::wstring& originalString = StringManager::GetString(stringId);
    const wchar_t *retStr = originalString.c_str();
    return retStr;
}

Provided StringManager::GetString() returns a reference:

const std::wstring& StringManager::GetString(int stringId);

However, this is still risky as it assumes the strings managed by your StringManager class will never be relocated in memory. For example, if StringManager is implemented with the help of an std::vector then as soon as the vector needs to expand, its previous content gets copied elsewhere in a larger memory block and you end up holding a reference to an object that is no more there.

In other words, avoid returning handles to internal data.

Answer 5

The previous answers are mostly correct, except for one little detail. You're not returning a pointer to a destroyed object, you're returning a pointer owned by a destroyed object. When that object was destroyed, the object your pointer was pointing at was destroyed as well.