GLSL shaders work when hardcoded, not when loaded from file

Question

I am trying to learn OpenGL. I have a main.cpp where I create the vertex and fragment shaders as strings, compile and link and it works fine. When I try to load the identical shaders from text files, it no longer links. Having trouble figuring out why. (I am using Linux if that matters)

Here is where I define the shader source:

    /*
    const char* vertex_shader = "#version 410\n"
                    "in vec3 vp;"
                    "void main () {"
                    "  gl_Position = vec4 (vp, 1.0);"
                    "}";

    const char* fragment_shader =  "#version 410\n"
                       "uniform vec4 inputColour;"
                       "out vec4 frag_colour;"
                       "void main () {"
                       "  frag_colour = inputColour;"
                       "}";

    */
    const char* vertex_shader = load_shader("test.vert").c_str();
    const char* fragment_shader = load_shader("test.frag").c_str();

Again, if I change what I comment out, so that the shader source is hardcoded, it works fine. If I try to load from files like I show here, it fails to link. Each individual shader seems to compile fine.

Here is my load_shader() function, it just gets the text from the file and logs some data.

std::string load_shader(std::string file){
    std::ifstream fin(file);
    if(fin.is_open()){
        std::string shader = "";
        std::string line = "";
        while(!fin.eof()){
            std::getline(fin, line);
            shader.append(line +"\n");
        }
        fin.close();
        l.add_string("LOADED SHADER");
        l.add_line(file);
        l.add_line(shader);
        l.add_line("---------------------------------");
        return shader;
    }else{
        l.add_string("FAILED TO LOAD SHADER");
        l.add_line(file);
        return "";
    }
}

Could my load_shader() function be making a bad string somehow?

EDIT: I got my log functions working slightly better. glGetProgramInfo() now returns this:

Vertex shader(s) failed to link, fragment shader(s) failed to link.
Vertex link error: INVALID_OPERATION.
ERROR: error(#97) No program main found
fragment link error: INVALID_OPERATION.
ERROR: error(#97) No program main found

EDIT 2: According to this, it is most likely a bad source string. But it doesn't say why. I'll keep working at it.

EDIT 3: I tried to compare the strings I was getting from each method and noticed something weird. Here is my check code, where I make strings for each input method, then loop through and print the chars to a file.

const char* vertex_shader = "#version 410\n"
                "in vec3 vp;"
                "void main () {"
                "  gl_Position = vec4 (vp, 1.0);"
                "}";

const char* fragment_shader =  "#version 410\n"
                   "uniform vec4 inputColour;"
                   "out vec4 frag_colour;"
                   "void main () {"
                   "  frag_colour = inputColour;"
                   "}";


const char* vertex_shader2 = load_shader("test.vert").c_str();
const char* fragment_shader2 = load_shader("test.frag").c_str();
std::ofstream shader("shaders.txt");
int i = 0;
while(vertex_shader[i] != '\0'){
    shader << vertex_shader[i];
    i++;
}
shader << std::endl;
i = 0;
while(fragment_shader[i] != '\0'){
    shader << fragment_shader[i];
    i++;
}
shader << std::endl;
i = 0;
while(vertex_shader2[i] != '\0'){
    shader << vertex_shader2[i];
    i++;
}
shader << std::endl;
i = 0;
while(fragment_shader2[i] != '\0'){
    shader << fragment_shader2[i];
    i++;
}
shader << std::endl;
shader.close();

And here is the shaders.txt output:

#version 410
in vec3 vp;void main () {  gl_Position = vec4 (vp, 1.0);}
#version 410
uniform vec4 inputColour;out vec4 frag_colour;void main () {  frag_colour = inputColour;}
#version 410
uniform vec4 inputColour;out vec4 frag_colour;void main () {  frag_colour = inputColour;}
#version 410
uniform vec4 inputColour;out vec4 frag_colour;void main () {  frag_colour = inputColour;}

As you can see, I'm getting a copy of the fragment shader text where I should be seeing the vertex shader. To be sure, here are the shader files I'm trying to upload.

test.vert:

#version 410
in vec3 vp;void main () {  gl_Position = vec4 (vp, 1.0);}

test.frag:

#version 410
uniform vec4 inputColour;out vec4 frag_colour;void main () {  frag_colour = inputColour;}

Also, when I changed test.frag a little, that change appeared in the wrong line in the shaders.txt. This shows that the extra copy is coming from the file input instead of the hardcoded version.

EDIT 4: I am apparently getting some sort of string overwrite problem. I tested with std::cout like so:

const char* vertex_shader2 = load_shader("test.vert").c_str();
const char* fragment_shader2 = load_shader("test.frag").c_str();
std::cout << vertex_shader2 << std::endl;
std::cout << fragment_shader2 << std::endl;

In this case, both cout statements give the test.frag string. But this one works fine:

const char* vertex_shader2 = load_shader("test.vert").c_str();
std::cout << vertex_shader2 << std::endl;
const char* fragment_shader2 = load_shader("test.frag").c_str();
std::cout << fragment_shader2 << std::endl;

So the second load_shader() call is overwriting the first call.

Answer 1

You are using pointers to data inside a temporary object:

const char* vertex_shader = load_shader("test.vert").c_str();
const char* fragment_shader = load_shader("test.frag").c_str();

Which means both vertex_shader and fragment_shader are pointing to invalid memory, since the strings returned by load_shader get destroyed at the end of each statement. Accessing that memory results in undefined behaviour.

Try keeping the strings instead:

std::string vertex_shader = load_shader("test.vert");
std::string fragment_shader = load_shader("test.frag");

And then giving their .c_str()s to the shader compiler.

Answer 2

The other answers already mention that you're getting a dangling pointer and should return the strings. But reading shaders from a file, really comes down to putting the whole file into a string. Here's one way to do it:

// Read shader file into one string
std::string shaderSource;
if ( !readFileIntoString( shaderFileName, shaderSource ) ) {
    return false;
}

const GLchar *source[] = { shaderSource.c_str() };
// (...) Use the source

And readFileIntoString can be written in different ways. Here you send the name of your shader file and pass a string to read the file into.

bool readFileIntoString( const std::string &fileName, std::string &destination ) {
    std::ifstream in( fileName, std::ios::in | std::ios::binary );
    if ( in ) {
        in.seekg( 0, std::ios::end );
        destination.resize( in.tellg() );
        in.seekg( 0, std::ios::beg );
        in.read( &destination[ 0 ], destination.size() );
        in.close();
        return true;
    }
    return false;
}

Answer 3

const char* vertex_shader = load_shader("test.vert").c_str();
const char* fragment_shader = load_shader("test.frag").c_str();

will result in dangling pointers (with undefined behavior if you dereference it), after you assign the char* to the variable the temp std::string (and backing array you just got a pointer to) gets deleted.

const std::string vertex_shader = load_shader("test.vert");
const std::string fragment_shader = load_shader("test.frag");

Keep those variables alive as long as you need the strings.