Convert time-string to std::time_t using std::get_time: wrong result

Question

I am trying to sort photos in chronological order. Therefore I extract the time as a string from the EXIF data, which I then convert to std::time_t. However I sometimes get an incorrect result. I have reduced the problem to this minimal example. It features three time-strings, one second apart:

#include <vector>
#include <string>
#include <iostream>
#include <ctime>
#include <iomanip>
#include <sstream>

int main()
{
  std::vector<std::string> vec;

  vec.push_back("2016:07:30 09:27:06");
  vec.push_back("2016:07:30 09:27:07");
  vec.push_back("2016:07:30 09:27:08");

  for ( auto & i : vec )
  {
    struct std::tm tm;
    std::istringstream iss;
    iss.str(i);
    iss >> std::get_time(&tm,"%Y:%m:%d %H:%M:%S");

    std::time_t time = mktime(&tm);

    std::cout << i << " time = " << time << std::endl;
  }
}

Compiled with clang++ -std=c++14 test.cpp.

The output:

2016:07:30 09:27:06 time = 1469867226
2016:07:30 09:27:07 time = 1469863627
2016:07:30 09:27:08 time = 1469863628

Which is obviously wrong, they should be 1 apart, which is only true for the last two?

Edit

Since there seems to be a bug in clang's std::get_time (and std::put_time), here is my version information:

$ clang --version
Apple LLVM version 8.1.0 (clang-802.0.42)
Target: x86_64-apple-darwin16.7.0
Thread model: posix
InstalledDir: /Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin

Manyplies

Near as I can tell, All of your output times are wrong. According to epochconverter.com, 1469867226 should be Saturday, July 30, 2016 8:27:06 AM. When I run your code I get the expected result. – Rawls

Yes, all your outputs are wrong. See your code running at ideone.com with (gcc 6.3). There it works. Seems like the clang implementation of std::get_time() is buggy? – Holt

@Rawls Thanks for your comments. Indeed also I have just compiled and run using g++ to get the correct result... I will file a bug report for clang. In the meantime, any suggestions on how to work around this problem? (In my Qt application I cannot easily avoid clang I think) – Manyplies

@AndreKampling Thanks, you are right! See the above comment, any chance to go around it for the moment? – Manyplies

I have no good suggestions. What clang version are you using and can you upgrade? rextester's clang does not reproduce. – Rawls

@Rawls I'm on $ Apple LLVM version 8.1.0 (clang-802.0.42) $ Target: x86_64-apple-darwin16.7.0, not sure if I can update. – Manyplies

Answer 1

As I said in my comment, all your output is wrong. It seems that it is a bug in clang. I searched on the Bugzilla page of the LLVM project but found nothing. Maybe you want to submit the bug report with your example code.

A workaround is to parse the string manually with std::sscanf() and fill the std::tm struct. The following code does that for your input string i. See the whole example with the output of std::get_time() and the std::sscanf() method at ideone. If the string is different then adapt that solution to your needs.

This solution is using %d:%d:%d %d:%d:%d as format string. You could also use %4d:%2d:%2d %2d:%2d:%2d which would accept only numbers which have length smaller or equal than the number between % and d.

Output:

2016:07:30 09:27:06 | sscanf() time =        1469870826
2016:07:30 09:27:06 | std::get_time() time = 1469870826
2016:07:30 09:27:07 | sscanf() time =        1469870827
2016:07:30 09:27:07 | std::get_time() time = 1469870827
2016:07:30 09:27:08 | sscanf() time =        1469870828
2016:07:30 09:27:08 | std::get_time() time = 1469870828

Code:

#include <vector>
#include <string>
#include <iostream>
#include <ctime>
#include <iomanip>
#include <sstream>
#include <cstring>

int main()
{
   std::vector<std::string> vec;

   vec.push_back("2016:07:30 09:27:06");
   vec.push_back("2016:07:30 09:27:07");
   vec.push_back("2016:07:30 09:27:08");

   for (auto & i : vec)
   {
      struct std::tm tm;

      /* std::sscanf() method: */
      std::memset(&tm, 0, sizeof(tm));
      if (6 != std::sscanf(i.c_str(), "%d:%d:%d %d:%d:%d",
                           &tm.tm_year, &tm.tm_mon, &tm.tm_mday,
                           &tm.tm_hour, &tm.tm_min, &tm.tm_sec))
      {
         return -1;
      }

      /* correct the numbers according to:
       * see: http://en.cppreference.com/w/cpp/chrono/c/tm */
      --tm.tm_mon;
      tm.tm_year -= 1900;
      /* mktime determines if Daylight Saving Time was in effect
       * see: http://en.cppreference.com/w/cpp/chrono/c/mktime */
      tm.tm_isdst = -1;

      std::time_t time = std::mktime(&tm);

      std::cout << i << " | sscanf() time =        " << time << std::endl;

      /************************************************************/

      /* std::get_time() method: */
      std::istringstream iss;
      iss.str(i);
      iss >> std::get_time(&tm, "%Y:%m:%d %H:%M:%S");

      time = std::mktime(&tm);

      std::cout << i << " | std::get_time() time = " << time << std::endl;
   }
}

It is necessary to subtract one of the tm_mon because it begins at 0. Further the tm_year member are years since 1900. See the description of std::tm here.

Further it is needed to set tm_isdst to -1 to let std::mktime() determine if the Daylight Saving Time was in effect.

---

To convert the std::time_t Linux timestamp back to a std::string of your given format: %Y:%m:%d %H:%M:%S you can use std::strftime() with std::localtime(). See live example on ideone.

Output:

time = 1469870826 | 2016:07:30 09:27:06

Code:

#include <vector>
#include <string>
#include <iostream>
#include <ctime>

int main()
{
   char buff[20];
   time_t timestamp = 1469870826;
   std::strftime(buff, sizeof(buff), "%Y:%m:%d %H:%M:%S", std::localtime(&timestamp));
   std::string timeStr(buff);

   std::cout << "time = " << timestamp << " | " << timeStr;
}

Holt

Great, thanks!! Since I also use std::put_time which evidences the same bug, I will also write the reverse of this function. I have also email a bug-report (I cannot directly report because I'm not registered and the bugzilla is blocked). – Manyplies

@TomdeGeus: I've added a method to revert from std::time_t back to std::string with your given format %Y:%m:%d %H:%M:%S. – Holt

Answer 2

Here is another library you could use to work around the bug. And this library is portable. If you have any problems with it, I'll get it fixed within hours. It is just a single header, no sources, and so very easy to "install". Just put the header somewhere in your include path:

#include "date.h"
#include <vector>
#include <string>
#include <iostream>
#include <sstream>

int main()
{
  std::vector<std::string> vec;

  vec.push_back("2016:07:30 09:27:06");
  vec.push_back("2016:07:30 09:27:07");
  vec.push_back("2016:07:30 09:27:08");

  for ( auto & i : vec )
  {
    date::sys_seconds tm;
    std::istringstream iss{i};
    iss >> date::parse("%Y:%m:%d %H:%M:%S", tm);

    std::cout << i << " time = " << tm.time_since_epoch().count() << std::endl;
    std::cout << date::format("%Y:%m:%d %H:%M:%S\n", tm);
  }
}

Output:

2016:07:30 09:27:06 time = 1469870826
2016:07:30 09:27:06
2016:07:30 09:27:07 time = 1469870827
2016:07:30 09:27:07
2016:07:30 09:27:08 time = 1469870828
2016:07:30 09:27:08

Frobisher

This indeed seems a very good solution to have a clean workaround that I can keep potentially forever. Since you seem to be the author, can you comment how to accomplish the opposite: std::time_t -> "%Y:%m:%d %H:%M:%S" (I can of course find out myself, but it might be only one second for you) – Manyplies

Updated the answer with the use of format which returns a std::string. – Frobisher

+1 Wow really nice library which evaluate as much as it can on compile-time. Should be faster than my answer which uses std::sscanf(). – Holt

Answer 3

there is a bug in the posted code - not in the library. it's a subtle one. replace:

struct std::tm tm;

with:

struct std::tm tm{};

this makes sure tm gets zero-initialized. after applying that change the output is as expected:

2016:07:30 09:27:06 time = 1469867226
2016:07:30 09:27:07 time = 1469867227
2016:07:30 09:27:08 time = 1469867228

the reason that 2nd and 3rd runs were "correct" was because mktime updates its parameter via pointer. in particular it sets DST and GMT offset, so after the loop runs for the 1st time, the memory region that gets assigned to tm already has these set. next loop iteration grabs the same memory region, puts (uninitialized) value and updates only some fields. however with DST and GMT offset already "set", behavior of mktime changes.

because of this, 1st run is not consistent with 2nd and 3rd.

Epicedium

That is very interesting, thanks! So you are saying, there are some things that std::mtkime need that are not initialised by std::get_time? – Manyplies

yes in particular tm_isdst was not initialized in struct tm. on my Linux there's also TZ information missing. this does make sense, as the input does not contain TZ information, so it cannot be inferred. it's good practice to always initialize variables before usage, as performance impact is typically minimal (or even none), yet usage of uninitialized variable/field can create nasty issues, that are hard to track down. – Epicedium