Is it safe to return a std::string by value?

Question

In the following code, a string is encapsulated within a class, Foo.

The call to Foo::getText() returns the string by value. This creates a second instance of the string object, but both string objects now point to the same char buffer on the heap.

When the Foo instance is deleted, the the encapsulated string is automatically destructed, and therefore the char buffer on the heap is deleted.

Even though getText() returns a string by value, the resulting string object still relies on the lifetime of the original string object in order to retain the char buffer to which they mutually point.

Doesn't this mean that the printing of the retval to the terminal is an invalid access to memory that has already been free'd on the heap?

class Foo
{
    Foo(const char* text) :
       str(text)
    {
    }

    std::string getText()
    {
        return str;
    }

    std::string str;
};

int main()
{
    Foo pFoo = new Foo("text");
    std::string retval = foo.getText();
    delete pFoo;
    cout << retval;  // invalid memory access to char buffer?
}

I think a lot of people assume that, because the string was returned by value, they need not be concerned about the lifetime of the original string within Foo. This problem isn't strictly related to strings, but really applies to any class with encapsulated pointers that are free'd upon destruction. But what's the best practice here when it comes to strings?

1. Never return string by value?
2. Only return strings by value if the lifetime of the original string is guaranteed?
3. Always make a copy of the string? return std::string(retval.c_str());
4. Enforce a contract with the caller of getText()?

EDIT:

I think I was misled by RVO. All three strings in this example return a c_str at the same address. Is RVO to blame?

class Obj
{
public:
    Obj() : s("text")
    {
        std::printf("%p\n", s.c_str());
    }

    std::string getText() { return s; }

    std::string s;
};

int main()
{
    Obj* pObj = new Obj();
    std::string s1(pObj->getText());
    std::string s2 = pObj->getText();
    delete pObj;
    std::printf("%p\n", s1.c_str());
    std::printf("%p\n", s2.c_str());
}

Result:

0x600022888
0x600022888
0x600022888

Answer 1

This creates a second instance of the string object, but both string objects now point to the same char buffer on the heap.

No, they don't.

std::strings own their contents. When you copy a std::string, you copy its buffer.

I think a lot of people assume that, because the string was returned by value, they need not be concerned about the lifetime of the original string within Foo.

And those people are right. There is no "sharing".

Your return by value is fine and you needn't think more about it.

Answer 2

I'd like to add some points to @LightnessRacesInOrbit's answer:

Never return string by value?

Never return local strings by reference or pointer. Actually, never return anything local by reference or pointer. By value is just fine.

Only return strings by value if the lifetime of the original string is guaranteed?

Again, you're thinking backwards. Only return by reference or pointer if the lifetime of the original is guaranteed.

Always make a copy of the string? return std::string(retval.c_str());

C++ does this automatically for you, if it can't move the string out (RVO/NRVO). No need to copy manually.

Enforce a contract with the caller of getText()?

Not needed as you get a copy anyway

I think a lot of people assume that, because the string was returned by value, they need not be concerned about the lifetime of the original string within Foo. This problem isn't strictly related to strings, but really applies to any class with encapsulated pointers that are free'd upon destruction.

And their assumption is correct. Any class with a (working) copy-constructor can be copied as often as needed and every copied instance is completely independent from the others. The copy-constructor takes care of copying the heap-space.