CODEWITHSUNDEEP
c++
NullVoxPopuli
NullVoxPopuli

Reputation: 65143

C++: adress of variable returned? How do I get this string to return, and then concat a small string onto it?

function:

//http://www.cplusplus.com/reference/clibrary/ctime/strftime/
char* get_current_time()
{
    time_t rawtime;
    struct tm * timeinfo;
    char buffer [80];  //main.cpp:73: warning: address of local variable ‘buffer’ returned

    time ( &rawtime );
    timeinfo = localtime ( &rawtime );

    strftime (buffer,80,"%Y %m-%d %H-%M-%S",timeinfo);
    puts (buffer);
    return buffer;
}

which is invoked by:

char* filename = get_current_time();
filename = strcat(filename, ".txt");

and the puts outputs to the console:

2011 03-18 13-51-59

so... for the most part the function works...

but when I puts (filename); after the strcat, I get this:

p??_

Upvotes: 1

Views: 765

Answers (4)

DavidMFrey
DavidMFrey

Reputation: 1668

If you want to stick with c instead of using c++ strings, this is the best way to do it

void get_current_time(char *buffer, int max_len)
{
    time_t rawtime;
    struct tm * timeinfo;

    time ( &rawtime );
    timeinfo = localtime ( &rawtime );

    strftime (buffer, max_len,"%Y %m-%d %H-%M-%S",timeinfo);
}

Then call it like this

char buffer[80];
get_current_time( buffer, 80 );
buffer = strncat(filename, ".txt", (80 - (strlen(buffer)+1)));

This will make sure you don't overrun your buffer size when writing and concatenating your strings

Upvotes: 0

Sarfaraz Nawaz
Sarfaraz Nawaz

Reputation: 361462

Returning a LOCAL address is a bad bad bad idea.

In C++, std::string solves most of such issues. Prefer using it.

Also, forget about strcat, and use + or/and += with std::string.

Re-writing your code:

std::string get_current_time()
{
    time_t rawtime;
    struct tm * timeinfo;
    char buffer [80]; 

    time ( &rawtime );
    timeinfo = localtime ( &rawtime );

    strftime (buffer,80,"%Y %m-%d %H-%M-%S",timeinfo);
    puts (buffer);
    return buffer; //it converts into std::string automatically!
}

std::string filename = get_current_time();
filename += ".txt"; //concatenate. Compare it with strcat!

It looks better now.

Upvotes: 4

Santa
Santa

Reputation: 11547

Heed the warning. buffer is a contiguous area of memory allocated on the stack. Once the call stack unwinds (your function returns), local storage is rendered garbage.

If you want to return a pointer to a memory that survives the return of the function, allocate said memory in the heap.

Upvotes: 2

Michael Krelin - hacker
Michael Krelin - hacker

Reputation: 143091

Your buffer is allocated in stack and is abandoned after you leave the function. The most you can do here is make it static, that will just make your function non-reentrant.

Better yet pass the buffer allocated in the caller function to the get time thingie.

And, as Nawaz suggests, since you're in c++ you may want to use std::string class instead if you don't want to learn how strings/memory/etc. work (you'll have to, anyway, though;-)).

Upvotes: 1

Related Questions