0

By accident I came across this behavior and would like to know if this is expected (does not look right to me).

I force an error in one particular tm-structure and all others become corrupted.

This is the code (stripped down to a minimun to reproduce the problem)

int main()
{
    cout << "-----   Bug test - tm struc   -----" << endl;
    //--------------------------------------------

    //--- Setup struct tm ---
    time_t timet_Now = time(NULL);
    struct tm* tm1 = localtime(&timet_Now);
    struct tm* tm2 = localtime(&timet_Now);

    //--- Verify OK  -  cout shows "28/10/2016"---
    cout << tm1->tm_mday << " " << tm1->tm_mon << " " << tm1->tm_year << endl;
    cout << tm2->tm_mday << " " << tm2->tm_mon << " " << tm2->tm_year << endl;

    // ... so far, so good 

    // --- Force an error in a different tm struct (xxtm)
    time_t xtimet = 1464778020000;
    struct tm* xxtm = localtime(&xtimet);  //<<< xxtm = null - no runtime error

    //--- tm1 and tm2 are now corrupted - cout shows "-1/-1/-1"
    cout << tm1->tm_mday << " " << tm1->tm_mon << " " << tm1->tm_year << endl;
    cout << tm2->tm_mday << " " << tm2->tm_mon << " " << tm2->tm_year << endl;

    //--- This next line crashes the application, as tm1 is corrupted
    char* c = asctime(tm1);

    return 0;
}

The crash error is: Unhandled exception at 0x0FA520B5 (ucrtbased.dll) in MyTest.exe: An invalid parameter was passed to a function that considers invalid parameters fatal.

Barros
  • 1
  • 2

1 Answers1

3

Citing http://en.cppreference.com/w/cpp/chrono/c/localtime

Return value

pointer to a static internal std::tm object on success, or NULL otherwise. The structure may be shared between std::gmtime, std::localtime, and std::ctime, and may be overwritten on each invocation.

In other words all of your struct tm *s wind up pointing to the exact same place. You probably want to

struct tm tm1 = *localtime(&timet_Now);

and make a copy to work with if you're going to hold on to it for any length of time.

Kenny Ostrom raises an excellent point in the comments. I didn't handle the NULL return case and copying NULL... Not such a good idea.

struct tm * temp = localtime(&timet_Now);
if (temp == nullptr)
{
    // handle error. Throw exception, return, whatever, just don't run the next line 
}
struct tm tm1 = *temp;
Community
  • 1
  • 1
user4581301
  • 33,082
  • 7
  • 33
  • 54
  • this is why `std::put_time` should take a `time_point` and not `tm*` – Mgetz Oct 28 '16 at 18:47
  • And check for nullptr return, and consider the existing localtime_r or localtime_s alternatives. – Kenny Ostrom Oct 28 '16 at 18:47
  • 1
    Poked around for a good way to do this with `chrono` and the [best I've found so far is Howard Hinnant's answer here](http://stackoverflow.com/questions/7313919/c11-alternative-to-localtime-r) – user4581301 Oct 28 '16 at 19:03
  • Oh ... so the cause was in localtime, then. Thanks for the explanation, heads-up and links. (...one more learned) – Barros Oct 28 '16 at 19:41