C++ function returning string gives garbage in the text of the string

Question

So I have a function returning a std::string as follows:

string ReadShaderSource(const char* filename, GLint& shaderSize) // Load the shader source code.
{
    ifstream::pos_type size;
    string text;

    ifstream file(filename, ios::in | ios::binary | ios::ate);
    if (file.is_open())
    {
        size = file.tellg();
        shaderSize = (GLuint)size;

        text.resize(size);
        file.seekg(0, ios::beg);
        file.read(&text[0], text.size());
        file.close();

        return text;
    }
    else
    {
        SDL_ShowSimpleMessageBox(SDL_MESSAGEBOX_ERROR, "Fatal Error!",
            "Could not load the shader source code from the file.", NULL);
        Lunar::Exit();
    }

    return "";

}

But When I call the function like this:

const char* testStr = ReadShaderSource("test.glsl", size).c_str();

The value of testStr is full of this:

0x036fdcd8 "îþîþîþîþîþîþîþîþîþîþîþîþîþîþîþîþîþîþîþîþîþîþîþîþîþîþîþîþîþîþîþîþîþîþîþîþîþîþîþîþîþîþîþîþîþîþîþîþîþîþîþîþîþîþîþîþîþîþîþîþîþîþîþîþîþîþîþîþîþîþîþîþîþîþîþîþîþîþîþîþîþîþîþîþîþîþîþîþîþîþîþîþîþîþîþîþîþîþîþîþ...

Which makes no sense. The function returns the right value, so when I return text in the function it contains the source code of the shader, but when I do

const char* testStr = ReadShaderSource("test.glsl", size).c_str();

testStr is full of rubbish.

Any ideas?

Thank you!

thats because the scope of the returned string is limited to the lifetime of expression. after line is complete, the return string is destroyed. capture your return string as `const string &testStr = ..` — Anycorn, May 08 '14 at 21:38
Is it possible `string text` is deleted from memory when the function exits? — Stefan, May 08 '14 at 21:39
Ok, makes I get it. How would I do it so that it is not destroyed? — Captain Picard, May 08 '14 at 21:39
Referring to std::string.cstr() is a dangling pointer if you do not store the resulting string of the function. — , May 08 '14 at 21:40
either copy the string or use const reference to extend its lifetime: http://stackoverflow.com/questions/2784262/does-a-const-reference-prolong-the-life-of-a-temporary — Anycorn, May 08 '14 at 21:40
@Anycorn: using a reference for the result does not buy anything here. — Cheers and hth. - Alf, May 08 '14 at 21:49
@Alf, `std::string const &testStr = foo();` extends the lifetime of the returned temporary, if `foo()` returns `string` (but not if it returns `string&`) — M.M, May 08 '14 at 21:51
@Cheersandhth.-Alf if you capture return value as a *local* const references the lifetime will be extended till the reference goes out of scope — Anycorn, May 08 '14 at 22:01
@Anycom (and Matt), yes, and as I said, that does not buy anything here (rather it introduces needless complexity). well on the old usenet i would now follow up in these well-meaning explanations by noting that the `const` is not necessary with an rvalue reference, and launching into a tutorial on references and lifetime extension etc., including Bjarne's rationale, ARM vagueness, Petru Marginean's original ScopeGuard, etc. etc., and there would be a back-and-forth exchange of useful opinions & insights, but Stack Overflow is not suited for that. old days gone. harumph. — Cheers and hth. - Alf, May 08 '14 at 22:18
@Alf I'm not sure what you're trying to say; OP's code is wrong, and the `std::string const &` suggestion makes the code correct. So this suggestion buys correctness at least. **e:** I see now that you're making a roundabout reference (no pun intended) to the answer you have posted.. never mind — M.M, May 08 '14 at 22:31
This bug is so common in C++ code that compilers should build in warnings for it. I've also seen it in Unicode libraries. — Zan Lynx, Mar 03 '17 at 17:36

R Sahu · Accepted Answer · 2017-03-03T17:30:47.153

You need to use

string str = ReadShaderSource("test.glsl", size);
const char* testStr = str.c_str();

instead of

const char* testStr = ReadShaderSource("test.glsl", size).c_str();

When you use the second form, you are storing a pointer in testStr that is not valid any more since the returned value of the function is a temporary string.

As was pointed out by @IInspectable, you could also use a const& to extend the lifetime of the temporary object.

string const& str = ReadShaderSource("test.glsl", size);
const char* testStr = str.c_str();

The following program is well behaved:

#include <iostream>
#include <string>

std::string foo()
{
   return "This is a test.";
}

void bar(std::string const& str)
{
   std::cout << str.c_str() << std::endl;
}

int main()
{
   std::string const& str = foo();
   bar(str);
   std::cout << str.c_str() << std::endl;
}

This is not strictly the only solution. You could also extend the [lifetime](http://en.cppreference.com/w/cpp/language/lifetime) of the temporary by binding it to a const lvalue reference. — IInspectable, Mar 03 '17 at 13:58

score 2 · Answer 2 · answered May 08 '14 at 21:39

Re

“when I do const char* testStr = ReadShaderSource("test.glsl", size).c_str(); testStr is full of rubbish.”

you're initializing the pointer to point to a buffer in a temporary string, that has ceased to exist already when the initialization finishes.

Instead use a string for the result variable.

Note that the conclusion that the function returns garbage is unwarranted, it does not follow from that observation of garbage, but might still be true.

You should test anew, with proper result variable type, to check that.

C++ function returning string gives garbage in the text of the string

2 Answers2

Linked

Related