2

I have a simple log function that needs to print the current date and time. I'm doing it inside a function that returns a char *. When I try to set this char * into fprintf(), it doesn't print me the string into the file: why?

Here is the function that constructs the date-time:

char * UT::CurrentDateTime()
{
     char buffer [50];
     time_t t = time(0);   // get time now
     struct tm * now = localtime( & t ); 
     int n=sprintf(buffer, "%d:%d:%d %d:%d:%d:", (now->tm_year + 1900),
                   (now->tm_mon + 1), now->tm_mday, now->tm_hour, now->tm_min,
                   now->tm_sec);
     return buffer;
}

Here is the log:

const char *time =__TIME__; // compilation time 
char *currentTime = UT::CurrentDateTime(); // it's a static method; also tried to set it to const
fprintf(fp, "%s %s %s %s %s %d %s\n", __TIME__, pType, __DATE__,
        currentTime, pFileName, lineNo, pMsg.c_str());
fflush(fp);

Every thing is printed except the date/time char *. Why?

Jonathan Leffler
  • 730,956
  • 141
  • 904
  • 1,278
user63898
  • 29,839
  • 85
  • 272
  • 514
  • You should _embrace_ C++ rather than just write C code with a C++ compiler. C++ has better (better in terms of C++ness anyway) ways to do formatted output to strings (and file output) - see `stringstream` for example. – paxdiablo Dec 29 '11 at 06:16
  • Consider using `strftime()` to format date/time values from a `struct tm`. Also, the 'const char *time` variable is unused and unlikely to be beneficial. You seldom need to log the compilation date and time in every log message; you might do it once when the log is opened to record which version of the product is writing to the log. – Jonathan Leffler Dec 29 '11 at 08:05

1 Answers1

5
char * UT::CurrentDateTime()
{
     char buffer [50];
     /* ... */
     return buffer;
}

You've returned a pointer to a memory buffer that dies immediately. Any function that uses the pointer returned from CurrentDateTime() is relying upon garbage.

Your compiler should have warned you about this. Disregard your compiler warnings at your own peril.

Instead, either allocate this via char *buffer = malloc(50 * sizeof char); or use C++'s memory allocation mechanisms to allocate memory that can live longer than the time the function is 'live' and running.

sarnold
  • 102,305
  • 22
  • 181
  • 238
  • 1
    Hehehe, that's the most enthusiastic response I've seen yet. :) Thanks! – sarnold Dec 29 '11 at 06:42
  • do i need to delete it each time im using it ? or in the end of the function ? – user63898 Dec 29 '11 at 06:58
  • 1
    If your function allocates and returns the memory, then the calling code is responsible for freeing it. An alternative design has the calling code pass a buffer (and its size) to the function, and the function writes in that buffer. Again, it is the calling function that deletes the buffer if it needs deleting (freeing). OTOH, the calling function might well pass a local variable which doesn't need explicit memory management. – Jonathan Leffler Dec 29 '11 at 08:00
  • so if i understand right in my method the calling fucntion need to delete the char* that is returned from the CurrentDateTime method like this : char* foo = CurrentDateTime(); ...do stuff...; free(foo) or delete foo ? – user63898 Dec 29 '11 at 11:37