[C] - warning C4172: returning address of local variable or temporary

Question

I've read a lot of other topics on this site about the same problem knowing it would be common. But I guess I'm dumb and can't figure out the proper way to do it. So, I apologize for yet another one of these questions and I'm hoping someone can give me a simple solution and/or explanation.

Here's the entire code:

Main.c

#define WIN32_LEAN_AND_MEAN

#include <Windows.h>
#include <stdlib.h>
#include <tchar.h>


LPTSTR GetApplicationPath ( HINSTANCE Instance );


int APIENTRY _tWinMain ( HINSTANCE Instance, HINSTANCE PreviousInstance, LPTSTR CommandLine, int Show )
{
    LPTSTR sMessage = GetApplicationPath ( Instance );

    MessageBox (
        NULL,
        sMessage,
        _T ( "Caption!" ),
        MB_OK
    );

    return 0;
}


LPTSTR GetApplicationPath ( HINSTANCE Instance )
{
    _TCHAR sReturn[MAX_PATH];

    GetModuleFileName ( (HMODULE) Instance, sReturn, MAX_PATH );

    return sReturn;
}

Answer 1

Right now, you're returning the address of an automatic (stack) array. This is always wrong, because as soon as the function ends, so does that memory's lifetime.

You need to use malloc (and free), or other dynamic allocation. E.g.:

_TCHAR *sReturn = malloc(sizeof(_TCHAR) * MAX_PATH);

I've omitted error checking. Then later, the calling code should free it. After the MessageBox in _tWinMain:

free(sMessage);

Answer 2

The warning says it all. In the function GetApplicationPath you are returning sReturn which is local to the function. Local variables cease to exist once the function returns and reference to them after function returns is incorrect.

To overcome this you can dynamically allocate space sof sReturn as:

sReturn = malloc(sizeof(_TCHAR) * MAX_PATH);

Answer 3

Consider modifying:

  LPTSTR GetApplicationPath ( HINSTANCE Instance )  

to something like

  void GetApplicationPath ( HINSTANCE Instance, LPTSTR str )  

This will eliminate the need to return a local variable.

Answer 4

Change

    _TCHAR sReturn[MAX_PATH];

in your "GetApplicationPath" to

    static _TCHAR sReturn[MAX_PATH];

If only one thread is going to be calling GetApplicationPath(), dynamically allocating this string is overkill. Marking a local variable as static allocates space for it with the globals, so that space doesn't get stepped on when you exit from the function.