warning C4172: returning address of local variable or temporary

c compiler-warnings

12,698

Solution 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);

Solution 2

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.

Solution 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.

Solution 4

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);

View more solutions

12,698

Author by

guitar-

Updated on June 04, 2022

Comments

guitar- almost 2 years

Possible Duplicate:
Pointer to local variable

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;
}

guitar- over 13 years

Fixed, thanks. Apparently, I have to wait 9 minutes to accept, but I will. Thanks again. :)
guitar- over 13 years

Just had to add a cast, I'm sure you and everyone already knows this though: _TCHAR * sReturn = (_TCHAR *) malloc ( sizeof ( _TCHAR ) * MAX_PATH );
Matthew Flaschen over 13 years

You could do it this way, but GetApplicationPath should just take a LPTSTR, since you're just writing chars to that location, not setting the caller's variable. Also, it must take a length parameter or document that it must be MAX_PATH.
Matthew Flaschen over 13 years

@guitar, you do not need a cast in C. That probably means you're really using C++.
skimobear over 13 years

@Matthew - I fixed the pointer issue you mentioned. Thx. I was thinking I would leave the MAX_PATH issue you mentioned to the imagination of the user. The argument really could be what ever they prefer (ex. char pathname[MAX_PATH]). Cheers
guitar- over 13 years

It compiled fine without it, it was just underlined in Visual Studio and showed an error, so I thought it'd be best to have it there.
Matt Joiner over 13 years

Not entirely accurate to say it's destroyed, but that it's undefined what has happened to it...
Matt Joiner over 13 years

It doesn't really make sense to do this unless you were optimizing for both stack and heap use. Generally nobody bothers to optimize stack usage, and it's safer to not presume the threading situtation.
Jens Gustedt over 13 years

In C, the return of malloc should not be casted. When forgetting to include "stdlib.h" this may loose bits of your address on 64 archs... Casting malloc is a C++ habit. There you need to do it, but you should usually use new, anyhow.
R.. GitHub STOP HELPING ICE over 13 years

@guitar: the cast is considered very bad style for several reasons.