Opened 9 years ago

Closed 5 years ago

#9584 closed Bugs (fixed)

date_time::difference produces incorrect values

Reported by: Alexey Spiridonov <bstbg.20.lesha@…> Owned by: Artyom Beilis
Milestone: To Be Determined Component: locale
Version: Boost 1.54.0 Severity: Problem
Keywords: Cc:

Description

The program below attempts to compute the local timezone's GMT offset for a given timestamp. The idea is simple. First, get the year-month-day-hour-minute-second representation of the timestamp in the local timezone. Then, create a UTC date_time with exactly the same components. The difference between those times is exactly the UTC offset. The program gives this:

g++ -std=c++11 c.cpp -o c -l boost_locale ; ./c 1383469199 ; ./c 0; 
-25199
-28799

The expected output is 28800 (8 hours) both times, since my computer's local timezone is America/Los_Angeles (or Pacific Time).

#include <boost/locale.hpp>
#include <iostream>

int compute_utc_offset_seconds(time_t time_since_utc_epoch) {
  using namespace boost::locale;
  std::locale loc = generator()("");

  // Get the local time coordinates for the given timestamp
  date_time local_dt{(double)time_since_utc_epoch, calendar(loc)};

  // Copy the date_time coordinates into a UTC value
  date_time utc_dt{calendar(loc, "UTC")};
  for (auto p : {
    period::year(), period::month(), period::day(),
    period::hour(), period::minute(), period::second()
  }) {
    utc_dt.set(p, local_dt.get(p));
  }
   
  return local_dt.difference(utc_dt, period::second());
}

int main(int argc, char **argv) {
  if (argc < 2) {
    return 1;
  }
  std::cout << compute_utc_offset_seconds(atoi(argv[1])) << std::endl;
  return 0;
}

Change History (8)

comment:1 by Alexey Spiridonov <bstbg.20.lesha@…>, 9 years ago

FWIW, the obvious workaround is to return utc_dt.time() - local_dt.time(), but that doesn't make the difference() method any less broken.

comment:2 by anonymous, 9 years ago

Sorry, my "expected output" was wrong. I'm going to assume that the sign is correct because the docs for ::difference() are unclear (is it subtracting this from other?).

Then, I expect to see -25200 and -28800 (because the first timestamp is in daylight savings time, while the latter is not).

The off-by-one error with difference() is pretty systematic, so maybe it's by design?

Also, I tried to use that function to compute the difference in seconds across 40 years, and it returned garbage, so something probably overflowed.

My current guess is that one should always prefer to subtract the results of calling the ::time() method instead of relying on ::difference().

comment:3 by Artyom Beilis, 9 years ago

You incorrectly assume that for an object date_time, setting 6 parameters Y-M-D-H-M-S one afte another is the same as setting them all together.

Additionally, if you open a bug ticked you need to provide an information about the backend you are using. Also it is hard to check anything if you refer to a current time.

 date_time utc_dt{calendar(loc, "UTC")}

Is the time in current time.

So unless you provide some more valid information I can't help.

comment:4 by Alexey Spiridonov <bstbg.20.lesha@…>, 9 years ago

What does my *original* code print for you?

a) I'm clearly not intending to use the current time, since I'm overwriting all of YMDHMS with the values from a fixed timestamp. The current time should have no effect on this code.

b) How do you want me to be setting YMDHMS? The documentation does not suggest a better way to set all of YMDHMS together, and in any case, why is this incorrect? How can the results be different? If they can be, such an API is extremely unsafe to use (and the documentation again provides no warning).

c) I found no method that reveals the current back-end name. Can you suggest one? This is Ubuntu 13.10 with a full install of Boost, and ICU is installed also, so I assume that it's using ICU.

d) The off-by-one-second error I originally reported is not so good (it might be due to rounding errors in the way I construct local_dt above), and I'd encourage you to actually try my original code to see if you can reproduce it. However, this seems even worse:

#include <boost/locale.hpp>
#include <iostream>

int main(int argc, char **argv) {
  if (argc < 4) {
    return 1;
  }

  using namespace std;
  using namespace boost::locale;
  
  locale loc = generator{}("");
  auto cal = (strlen(argv[3]) == 0) ? calendar(loc) : calendar(loc, argv[3]);

  date_time a((double)atoi(argv[1]), cal);
  date_time b((double)atoi(argv[2]), cal);

  cout << a << " - " << b << " = " << a.difference(b, period::second()) << endl
    << b << " - " << a << " = " << b.difference(a, period::second()) << endl;
  return 0;
}

Neither of its outputs is even remotely correct. If the library cannot handle a 40-year difference, it should probably throw rather than make up numbers...

g++ -std=c++11 bug.cpp -o bug -lboost_locale; ./bug 0 1390247464 '' 
0 - 1.39025e+09 = 0
1.39025e+09 - 0 = 536870912

comment:5 by maksqwe1@…, 8 years ago

Fix:

github com/boostorg/locale/pull/6

comment:6 by bstbg.20.lesha@…, 8 years ago

Thank you!

comment:6 by bstbg.20.lesha@…, 8 years ago

Thank you!

comment:7 by Artyom Beilis, 5 years ago

Resolution: fixed
Status: newclosed
Note: See TracTickets for help on using tickets.