Unexpected ifstream behavior?

Question

I am trying to load GLSL vertex/fragment shader sources into const char* array to be used with OpenGL. The function I am using to do that is

const char* loadSource(const char* path)
{
   string line;
   stringstream output;
   ifstream source(path);

   if(source.is_open())
   {
       while(source.good())
       { 
           getline(source, line);
           output << line << endl;
       }
       source.close();
   }

   return output.str().c_str();
}

The source input works fine on first call. However if I call for the second source, the first source gets "corrupted" (both calls are inside scope of a single function):

const char* vs_source = loadSource("vertex.vert");  // vs_source loads fine
const char* fs_source = loadSource("fragment.frag"); // fs_source loads fine. vs_source is terminated sooner than after previous call.

Note: I tried to code vs_source directly into *.cpp file and both shaders compiled. This points out to the fact that I must be doing something silly in the loadSource function.

Question: What causes such weird behaviour of the text input?

score 3 · Accepted Answer · answered Aug 25 '13 at 01:37

When you return the result from your function, you create a dangling reference:

return output.str().c_str();

At the end of the expression the temporary std::string obtained from the stream is destroyed and the memory returned become invalid: any access to the array result in undefined bahvior.

BTW, this approach to input is wrong:

while(source.good())
{ 
    getline(source, line);
    output << line << endl;
}

You always need to check that the read was successful after reading:

while (std::getline(source, line)) {
    output << line << '\n';
}

Also, don't use std::endl. If you really mean to flush the stream use std::flush. Finally, you can get the same effect a lot easier and probably faster:

out << source.rdbuf();

... and, of course, out should be declared as std::ostringstream (not the extra o).

Thanks a lot, this was very helpful - I really appreciate the tips as well. Btw.: What exactly is wrong with using std::endl instead of '\n'? — vojta havlíček, Aug 25 '13 at 01:47
@vojtahavlíček: Primarily it is a common source of performance problems. See this [blog entry](http://kuhllib.com/2012/01/14/stop-excessive-use-of-stdendl/) for more details. — Dietmar Kühl, Aug 25 '13 at 01:51

billz · Answer 2 · 2013-08-25T03:01:00.687

What causes such weird behaviour of the text input?

return output.str().c_str();

returns a pointer which points to a local variable, when function loadSource returns output goes out of scope vs_source/fs_source are dangling pointers. Access to vs_source/fs_source have undefined behavior.

To fix your issue, you could return std::string instead:

std::string loadSource(const char* path)
{
    //...
    return output.str();
}

std::string vs_source = loadSource("vertex.vert");  // vs_source loads fine
std::string fs_source = loadSource("fragment.frag");

score 1 · Answer 3 · edited May 23 '17 at 12:13

1

Why does everyone insist on doing this line-by-line reading of files? Just load the whole file at once. It's really simple:

std::string LoadFile(const std::string &filename)
{
    std::ifstream file (filename.c_str());
    if (file) {
        std::ostringstream os;
        os << file.rdbuf();
        return os.str();
    }
    else {
        // error
    }
}

edited May 23 '17 at 12:13

Community

1
1

answered Aug 25 '13 at 02:00

Nicol Bolas

378,677
53
635
829

Thanks! Although this was suggested by @Dietmar as well, your post clarifies it nicely (wish I could upvote it :)). Btw.: Is there any advantage between checking for file.is_open() and simply file? – vojta havlíček Aug 25 '13 at 02:07

Unexpected ifstream behavior?

3 Answers3