4

I'm a C/C++ beginner trying to build what seems like a pretty simple program: it loads a file into a c-string (const char*). However, although the program is incredibly simple, it's not working in a way I understand. Take a look:

#include <iostream>
#include <fstream>

std::string loadStringFromFile(const char* file)
{
    std::ifstream shader_file(file, std::ifstream::in);
    std::string str((std::istreambuf_iterator<char>(shader_file)), std::istreambuf_iterator<char>());
    return str;
}

const char* loadCStringFromFile(const char* file)
{
    std::ifstream shader_file(file, std::ifstream::in);
    std::string str((std::istreambuf_iterator<char>(shader_file)), std::istreambuf_iterator<char>());
    return str.c_str();
}

int main()
{
    std::string hello = loadStringFromFile("hello.txt");
    std::cout << "hello: " << hello.c_str() << std::endl;

    const char* hello2 = loadCStringFromFile("hello.txt");
    std::cout << "hello2: " << hello2 << std::endl;

    hello2 = hello.c_str();
    std::cout << "hello2 = hello.c_str(), hello2: " << hello2 << std::endl;

    return 0;
}

The output looks like this:

hello: Heeeeyyyyyy

hello2:       青!
hello2 = hello, hello2: Heeeeyyyyyy

The initial hello2 value changes every time, always some random kanji (I'm using a Japanese computer, so I'm guessing that's why it's kanji).

In my naive view, it seems like the two values should print identically. One function returns a c++ string, which I then convert to a c-string, and the other loads the string, converts the c-string from that and returns it. I made sure that the string was loading properly in loadCStringFromFile by couting the value before I returned it, and indeed it was what I had thought, e.g.:

/*(inside loadCStringFromFile)*/
const char* result = str.c_str();
std::cout << result << std::endl;//prints out "Heeeyyyyyy" as expected
return result;

So why should the value change? Thanks for the help...

limp_chimp
  • 13,475
  • 17
  • 66
  • 105
  • In loadCStringFromFile you load the text to a local variable then you return the internal buffer of that variable. When the function ends then you have a pointer to deallocated memory (and it's garbage). With std::string you return a full valid copy. – Adriano Repetti Aug 05 '12 at 10:18
  • 1
    I haven't read your question yet, but +1 *just for* your first paragraph. It's mind-blowingly well-written (at least compared to most other similar kinds of questions here on StackOverflow). I hope you get an awesome answer and continue to come back to ask more questions on SO! :) – user541686 Aug 05 '12 at 10:45
  • Since this question is 'c++' tagged, I suggest for future reference not to use C-strings without a good reason. std::string is as efficient for most programs (and sometimes even more). – MasterMastic Aug 05 '12 at 11:40

3 Answers3

4

Your problem is that str in loadCStringFromFile is a local variable, and is destructed when the function returns. At that point the return value from c_str() is invalid.

More detail here

Your first function, loadStringFromFile, is a more C++-like way of doing it, and illustrates the benefit of having a class manage memory for you. If you use char* then you have to take much more care where memory is allocated and freed.

Community
  • 1
  • 1
Bryan
  • 11,398
  • 3
  • 53
  • 78
  • Handy rule for whoever reads this: 90% of the time you call `c_str`, it should be only used as an argument to a function, just as input. If you follow this rule and don't try storing the result in local variables or returning the result from a functions, it's very difficult to go wrong. (The only exception is if you pass *the string itself* as well as the `char*` as another argument, in which case, this alias had better be `const`.) – user541686 Aug 05 '12 at 10:50
3

the function

std::string loadStringFromFile(const char* file)

returns a string copy of the string created inside the function which is copied before the string goes out of scope i.e. the function ends, that is why it works.

const char* loadCStringFromFile(const char* file)

on the other hand returns a pointer to the local string which goes out of scope when the function returns and is destroyed so the returned address, the const char*, points to somewhere undefined.

in order for the second way to work you either need to create the string before calling the function :

const char* loadCStringFromFile(const char* file, string& str); // return str.c_str()

..
string str;
const char* result = loadCStringFromFile(file,str);

or you create a string on the heap in the function and pass the address back, but that gets a bit messy since the caller would need to delete the string to avoid memleak.

AndersK
  • 35,813
  • 6
  • 60
  • 86
-1

You should duplicate output of str.c_str():

return strdup(str.c_str);

function strdup can be found in cstring header.

KCH
  • 2,794
  • 2
  • 23
  • 22