char * - returning address of local variable or temporary

piul

Oh... I've so forgotten the usage of character arrays! for example:

char * Func (char * sT)
{
char sRet [MAX_STR_SIZE] = "";

strcpy (sRet, sT);

return sRet;

}

And when compiling I get a warning returning address of local variable or temporary I think I know why it tells me this, but not completely sure... Would it be more reasonable to do void Func (char * sT, char * sRet) ?

Alain Rist

You should really stop home cooked memory handling and write std::string sRet(sT), or (MFC) CString sRet(sT), or ATL::CString sRet(sT) ... other string classes omitted. cheers, AR

When the wise (person) points at the moon the fool looks at the finger (Chinese proverb)

piul

Err... well, I know that and agree with you, but my boss does not think the same. I have mentioned him several times that it is more secure to use std::string or any other, but apparently it is best to use char * and home cooked functions for the system the application will run in. I understand your worries, I will make sure someone checks my code before it goes on the system.

jschell

You have a buffer that is local - it exists on the stack. When you exit the routine the stack, for that routine, will no longer exist. As for the best way to do it it depends on what you are doing. But at a minimum the calling routine would need to pass its local buffer for the copy operation. Or manage it on the heap and make sure to delete it when done.

David Crow

piul wrote:

I think I know why it tells me this, but not completely sure...

Because sRet ceases to exist once Func() goes out of scope.

"One man's wage rise is another man's price increase." - Harold Wilson

"Fireproof doesn't mean the fire will never come. It means when the fire comes that you will be able to withstand it." - Michael Simmons

"Man who follows car will be exhausted." - Confucius

Sauro Viti

The problem is that you are returning a pointer to a buffer that will go out of scope after your function exit: then then returned pointer is no longer valid and what you find there is going to be pseudo-random. To fix the problem you can:

allocate the output buffer out from the function
allocate the output buffer using new or malloc (but you should remember to deallocate it when it is no more needed)

However, as Alan said, the very best way is to stop using C-style strings and use std::string instead: the STL is one of the better solutions because all its classes are inlined and highly optimized. This will give you very good performances with a minimal footprint (the unused classes and methods simply are not compiled and don't affect the executable size).

Alain Rist

piul wrote:

my boss does not think the same. I have mentioned him several times that it is more secure to use std::string or any other, but apparently it is best to use char * and home cooked functions for the system the application will run in.

I sympathize :sigh: Try find another job or another boss :) Meanwhile:

piul wrote:

char * Func (char * sT)
{
char sRet [MAX_STR_SIZE] = "";
strcpy (sRet, sT);
return sRet;
}

And when compiling I get a warning returning address of local variable or temporary

Your nice compiler warns you that sRet points to deallocated memory when returned. A solution is to make sRet static: static char sRet [MAX_STR_SIZE] = {0};. It will be overwritten on each call but valid on return. For safety use strncpy(sRet, sT, MAX_STR_SIZE - 1). cheers, AR

When the wise (person) points at the moon the fool looks at the finger (Chinese proverb)

_Superman_

You should look up to the signature of strcpy as a reference. The first parameter to strcpy is the out parameter and the second is the in parameter. So change your function to be something similar even though it really doesn't make sense in this context -

int Func(char* sRet, const char* sT)
{
return strcpy(sRet, sT);
}

Also, if you're not able to use std::string, at least use the secure versions of the C runtime functions like strcpy_s.

«_Superman_» _I love work. It gives me something to do between weekends.

_Microsoft MVP (Visual C++) Polymorphism in C

tom1443

sRet is a local variable allocated on the stack and as such the memory it uses will be reclaimed when Func returns. That means the stuff it points to will probably not contain what you think it does at some point in the future. So you have essentially 2 choices: 1. Use void Func (char * sT, char * sRet) as you suggest. Func would then copy the string into the memory supplied by the caller. That memory of course has its own lifecycle. 2. Allocate memory in Func(). For example in C++: char * Func (char * sT) { char * sRet = NULL; sRet = new char[MAX_STR_SIZE]; if (sRet) strcpy (sRet, sT); return sRet; } In this case someone has to deallocate the newed up memory later. So for example: void Foo(void) { char *mystr = Func("test"); DoSomethingInteresting(mystr); delete mystr []; }