2

So I've been learning OpenGL and decided to write a small static function for loading shaders from files. Here it is :

std::string HM::GetShaderSource(std::string filename)
{
std::string shaderSource = "";
std::string line = "";

std::ifstream shaderFile(filename);
if (shaderFile.is_open())
{
    while (getline(shaderFile, line))
    {
        shaderSource.append(line + "\n");
    }

    shaderFile.close();
}
else std::cout << "Unable to open shader file : " << filename << std::endl;



std::cout << " first output : " << std::endl << (const GLchar *)shaderSource.c_str() << std::endl;

return shaderSource;
}

Then I call it out in my code like this : 

const char * vertexShaderSource = HM::GetShaderSource("VertexShader.txt").c_str();
std::cout << "second output: " << vertexShaderSource << std::endl;
std::cout << "third output: " << HM::GetShaderSource("VertexShader.txt").c_str() << std::endl;

And this prints out : 

first output: ...Normal Shader Code...
second output: second output: ▌▌▌▌▌▌▌▌▌▌▌▌▌▌▌▌▌▌▌▌▌▌▌▌▌▌▌▌▌▌▌▌▌▌▌▌▌▌▌▌▌▌▌▌▌▌▌▌▌▌▌▌▌▌▌▌▌▌▌▌▌▌▌▌▌
▌▌▌▌▌▌▌▌▌▌▌▌▌▌▌▌▌▌▌▌▌▌▌▌▌▌▌▌▌▌▌▌▌▌▌▌▌▌▌7
first output: ...Normal Shader Code...
third output: ...Normal Shader Code...

So why is it behaving so oddly? Why when I place this in a variable it gives this strange output?

Ulrich Eckhardt
  • 16,572
  • 3
  • 28
  • 55
Rokner
  • 169
  • 2
  • 13
  • 1
    Think about the lifetime of the associated objects. Then, why do you think you need a C-style string in the first place? Lastly, do not cast. Seriously, you hide more errors than you "fix" with casts. – Ulrich Eckhardt Nov 12 '15 at 18:03
  • Well to answer about the c strings I need them because I have to give one(or GLchar *) to the shader load function which is `glShaderSource(vertexShader, 1, **&vertexShaderSource**, NULL);` – Rokner Nov 12 '15 at 18:08
  • *This* code has nothing to do with shader load functions. It only reads strings from files and prints them. There is not a single reason in the world to use a pointer here. – n. m. could be an AI Nov 12 '15 at 18:32

1 Answers1

3
const char * vertexShaderSource = HM::GetShaderSource("VertexShader.txt").c_str();

Is going to create a pointer to the underlying c-style string of the string returned from GetShaderSource. Since you never capture that string it is destroyed at the end of the expression. So now you have a pointer pointing to memory you no longer own. Using that pointer is now undefined behavior.

There are two ways you could fix this. You could capture the string and then use it in your function

std::string vertexShaderSource = HM::GetShaderSource("VertexShader.txt");
the_function_you_want_to_call(vertexShaderSource.c_str());

Or you can just make the function call a paramter if you are never going to use it again.

the_function_you_want_to_call(HM::GetShaderSource("VertexShader.txt").c_str());
NathanOliver
  • 171,901
  • 28
  • 288
  • 402