0

Possible Duplicate:
Is it a good idea to return “ const char * ” from a function?
how to return char array in c++?

What is wrong with this return? I'm trying to return the current path using the following function but it doesn't seems to be correct:

Please Not: I need an char return not string.

char* getINIfile(void)
{
    char buffer[MAX_PATH];
    GetModuleFileName( NULL, buffer, MAX_PATH );
    string::size_type pos = string( buffer ).find_last_of( "\\/" );
    string path = string( buffer ).substr( 0, pos) + "\\setup.ini";

    char *ini_local= (char*)path.c_str();

    printf(ini_local); // so far output OK!

    return ini_local;
}

main
{
    printf(getINIfile()); // output Not OK! 

    char mybuffer[200];
    GetPrivateProfileStringA( "files","DLL","0",  mybuffer,200, getINIfile());
    printf(mybuffer);

}
Community
  • 1
  • 1
User6996
  • 2,953
  • 11
  • 44
  • 66
  • Why in the world... are you combining C++ and C in such a wretched fashion? Either use C and use the proper methods, or use C++ and stay away from character arrays. We have `std::string` for a reason. – Drise Aug 06 '12 at 19:10
  • 1
    I removed the C tag as this is not C (no matter how hard you try). There is also a fundamental misunderstanding here; you cannot return arrays from functions. A pointer is not an array. – Ed S. Aug 06 '12 at 19:10
  • Also, just thought I'd point out, you're using `printf` unformatted. See a nice security vulnerability: http://en.wikipedia.org/wiki/Uncontrolled_format_string – Drise Aug 06 '12 at 19:15

3 Answers3

4

path goes out of scope at the end of the function and you are returning an internal pointer in that out of scope object. try returning an std::string instead

std::string getINIfile(void)
{
    char buffer[MAX_PATH];
    GetModuleFileName( NULL, buffer, MAX_PATH );
    string::size_type pos = string( buffer ).find_last_of( "\\/" );
    string path = string( buffer ).substr( 0, pos) + "\\setup.ini";

    char *ini_local= (char*)path.c_str();

    printf(ini_local); // so far output OK!

    return path;
}
cppguy
  • 3,611
  • 2
  • 21
  • 36
3

You're returning an address that goes out of scope when the function exits, and so it's no longer valid: std::string path is local to the function getINIFile and so it's invalid after the function exits, as is the address that you get from path.c_str().

In this case you can just return the std::string from your function. If you really need a C string later, you can use c_str() then:

std::string getINIfile(void)
{
    //...

    return path;
}


int main()
{
    string path = getINIFile();

    // do something with path.c_str():
    const char *cPath = path.c_str();
}

Given your code I can't think of any reason that you must have a char* return, but if so you'll need to allocate a buffer on the heap:

char *getINIfile(void)
{
    char *buffer[MAX_PATH];
    GetModuleFileName(NULL, buffer, MAX_PATH);
    string::size_type pos = string(buffer).find_last_of( "\\/" );
    string path = string(buffer).substr( 0, pos) + "\\setup.ini";

    char *ini_local = new[path.size()];
    strncpy(ini_local, path.c_str(), path.size());

    printf(ini_local); // so far output OK!

    return ini_local;
}

But this is a really awful mix of standard C strings and std::string: just using string to manipulate the path and passing around char* everywhere else.

Using only standard C, replacing find_last_of with strrchr - note the lack of error handling:

char *getINIfile(void)
{
    char *buffer = new[MAX_PATH];
    char *pos = NULL;
    char *ini_local = NULL;

    GetModuleFileName(NULL, buffer, MAX_PATH);
    pos = strrchr(buffer, "\\/");
    // check for and handle pos == NULL

    buffer[pos] = '\0';

    strncat(buffer, "\\setup.ini", MAX_PATH - strlen(buffer));

    printf(buffer);

    return buffer;
}
pb2q
  • 58,613
  • 19
  • 146
  • 147
1

The function is returning a pointer to a local variable, which goes out of scope, leaving you with a dangling pointer. Why not just return an std::string by value?

std::string getINIfile() {
   ....
   return path;
}

Then you can just use the string's underlying char* on the caller side:

const std::string s = getINIfile();
const char* c = s.c_str();
pb2q
  • 58,613
  • 19
  • 146
  • 147
juanchopanza
  • 223,364
  • 34
  • 402
  • 480