0

I am seeing pretty weird issue. Somehow with my below code, I am seeing negative number getting printed out as shown below in my holder variable. I am not sure why it is happening.

-2147483648 days -2147483648 hours -2147483648 minutes ago

Here is my timestamp (current_unix_timestamp) value 1437943320 which is getting passed to my below method and then afterwards holder value is coming as shown above everything as negative.

char holder[100];
get_timestamp_value(current_unix_timestamp, holder);

inline void get_timestamp_value(long sec_since_epoch_time, char* holder) {
    uint64_t timestamp = current_timestamp();
    double delta = timestamp/1000000 - sec_since_epoch_time;
    int days = floor(delta/60/60/24);
    int hours = floor((delta - days * 60 * 60 * 24)/60/60);
    int minutes = floor((delta - days * 60 * 60 * 24 - hours * 60 * 60)/60);
    holder[0] = 0;
    if (days) sprintf(holder, "%d days ", days);
    if (hours) sprintf(holder, "%s%d hours ", holder, hours);
    sprintf(holder, "%s%d minutes ago", holder, minutes);
    std::cout<< "Timestamp: " << timestamp << ", sec_since_epoch_time: " << sec_since_epoch_time << ", Delta:" << delta << ", Days: " << days << ", hours: " << hours << ", mins: " << mins << std::endl;
}

// get current system time in microseconds since epoch
inline uint64_t current_timestamp()
{
    std::chrono::time_point<std::chrono::steady_clock> ts = std::chrono::steady_clock::now();
    return std::chrono::duration_cast<std::chrono::microseconds>(ts.time_since_epoch()).count();
}

Now this is what got printed out from the above cout logs:

Timestamp: 433430278724, sec_since_epoch_time: 1437943320, Delta:1.84467e+19, Days: -2147483648, hours: -2147483648, mins: -2147483648
Timestamp: 433679536303, sec_since_epoch_time: 1437943380, Delta:1.84467e+19, Days: -2147483648, hours: -2147483648, mins: -2147483648
Timestamp: 433929683258, sec_since_epoch_time: 1437943440, Delta:1.84467e+19, Days: -2147483648, hours: -2147483648, mins: -2147483648
Timestamp: 434179628271, sec_since_epoch_time: 1437943500, Delta:1.84467e+19, Days: -2147483648, hours: -2147483648, mins: -2147483648

Is there anything wrong happening in the above code which is causing this issue? Any suggestions will be of great help.

user1950349
  • 4,738
  • 19
  • 67
  • 119

1 Answers1

1

You should try to avoid doing arithmetic with a mixture of signed and unsigned integer types. The results are often surprising.

Clearly, timestamp is not the value you expect it to be, since timestamp/1000000 is 433430, which is considerably less than sec_since_epoch_time. Consequently, you might expect timestamp/1000000 - sec_since_epoch_time to be a negative number, but (surprisingly, as above), it will be a large positive number because the signed long sec_since_epoch_time is converted to an unsigned long prior to the subtraction, following the rules for the usual arithmetic conversions. The subtraction is then done using unsigned arithmetic, so the result is a positive number slightly less than 264, as seen by the value of delta.

Dividing that large number by 86400 is not sufficient to bring it into the range of an int, so the assignment

int days = floor(delta/60/60/24);

will overflow with undefined consequences (in this case, setting days to -231.)

To me, it seems a bit odd to ask for a duration in microseconds and then divide it by a million. Why not just ask for a duration in seconds?

But the underlying problem is that you are comparing the value returned by current_timestamp, which is the number of microseconds since the epoch of std::chrono::steady_clock, with the argument sec_since_epoch_time. It looks to me like sec_since_epoch_time is the number of seconds since the Unix epoch (Jan. 1, 1970). However, there is no guarantee that the epoch for a std::chrono clock has that value. (Apparently, on Linux, the epoch for std::chrono:system_clock is the system epoch, but I repeat that there is no guarantee.)

You cannot compare two "seconds since epoch" values unless they are seconds since the same epoch. Which means that the only way your code can work is if the clock which was originally used to get the value of sec_since_epoch_time is the same clock as you are using in current_timestamp.

In addition to ensuring that timestamp actually has the expected value, you should change timestamp to an int64_t (or cast it to int64_t for the purposes of the computation of delta).

rici
  • 234,347
  • 28
  • 237
  • 341
  • Ok got it. So to fix this issue instead of using `uint64_t` for timestamp I should use `int64_t` right? So I should use like this right? `int64_t timestamp = reinterpret_cast(current_timestamp());` – user1950349 Jul 26 '15 at 22:32
  • @user1950349: You could just change the `current_timestamp` function to return an `int64_t`. But the most important thing is to make sure that `current_timestamp` returns the correct value, which it obviously is not. – rici Jul 26 '15 at 22:37
  • I see why do you think this doesn't return the correct value. It is using chrono package to get the value? Am I using anything wrong? – user1950349 Jul 26 '15 at 22:39
  • @user1950349: second last paragraph of the answer. – rici Jul 26 '15 at 22:44
  • ok, now problem is `current_timestamp()` is being used by lot of other methods and classes. If I directly convert `uint64_t` to `int64_t`, will that not be sufficient? – user1950349 Jul 26 '15 at 22:58
  • It looks like there is some weird problem with chrono package I guess. I am having some different issues as well in my other question [here](http://stackoverflow.com/questions/31642087/how-to-validate-whether-my-data-is-x-seconds-old-using-chrono-package). – user1950349 Jul 26 '15 at 22:58
  • @user1950349: That looks to me like the same problem. Did you read the paragraph in my answer? – rici Jul 26 '15 at 23:07