Why is my std::string obtained via stream being overwritten?

Question

Assume I have a function like so:

std::string get_shader(std::string path) {
     std::string fullpath = "./resources/shaders/" + path;
     std::ifstream vertexShaderFile(fullpath);
     std::ostringstream vertexBuffer;
     vertexBuffer << vertexShaderFile.rdbuf();
     return vertexBuffer.str();
}

And then some code like this:

GLuint vertex_shader;
GLuint fragment_shader;
GLuint program;

const GLchar * vertex_shader_source = get_shader("triangle_vertex.vs").c_str();

// At this point vertex_shader_source is correct.

const GLchar * fragment_shader_source = get_shader("fragment.vs").c_str();

// Now vertex_shader_source is the same as fragment_shader_source

I don't understand why vertex_shader_source ends up being overwitten by a subsequent call to get_shader. How do I fix this?

Niall · Accepted Answer · 2016-02-16T06:27:12.987

3

const GLchar * vertex_shader_source = get_shader("triangle_vertex.vs").c_str();

The vertex_shader_source is bound to a value "inside" the temporary std::string returned from get_shader. This does not "extend" the lifetime of the temporary at all. Once the execution of that statement is complete and continues, that temporary, and it's memory (and the pointer you now hold) is no longer accessible in a defined manner.

Essentially you are invoking undefined behaviour.

A more appropriate declaration of the vertex_shader_source could be as a std::string. Since the value is being returned from the function, it is a rvalue and the appropriate move construction will be invoked.

std::string vertex_shader_source = get_shader("triangle_vertex.vs");

If you still the the const GLchar* at this point, vertex_shader_source.c_str() will do.

edited Feb 16 '16 at 06:27

answered Feb 16 '16 at 06:17

Niall

30,036
10
99
142

Just trying to understand: I thought get_shader() would return a copy of the string, so wouldn't the pointer to the c_str() be valid within the remainder of the scope? – Knox Feb 16 '16 at 06:23
3

Yes it does return a copy. That copy is a temporary. Once you call the `c_str()` function, you get a pointer to the internal "c string" of the temporary `std::string`. Once the temporary is destroyed, that pointer is no longer valid and attempting to access it is not going to work the way you want it to. – Niall Feb 16 '16 at 06:25
Ah, I was thinking that the call to get_shader() would be equivalent to creating an `std::string` in the current scope, but I see now it only lives till the end of the statement that called it. Thanks. – Knox Feb 16 '16 at 06:31

songyuanyao · Answer 2 · 2016-02-16T06:31:01.737

For the statement,

const GLchar * vertex_shader_source = get_shader("triangle_vertex.vs").c_str();
// The temporary `std::string` will be destroyed here.

get_shader returns a temporary std::string, which will be destroyed after the statement, that means vertex_shader_source will hold an invalid pointer, dereference of it will lead to UB.

What you have seen might be caused by the memory reuse after deallocated, but UB is UB, anything is possible.

You could define a named variable for it, such as:

std::string vertex_shader_source_string = get_shader("triangle_vertex.vs");
const GLchar * vertex_shader_source = vertex_shader_source_string.c_str();

Why is my std::string obtained via stream being overwritten?

2 Answers2

Linked