fix the calculation in SleepForMilliseconds

[guanqun]

static_cast should be done after the multiplication operation. Otherwise the milliseconds is already an int.

[zzz6519003]

求问这句能解释下么XD

[guanqun]

milliseconds is already an integer, it doesn't need to be casted to integer again. Instead, kNumMicrosPerMilli is a long, so after multiplication, it needs to be casted into int.

[zzz6519003]

多谢哈 您在akuna么 我看到你的题解了XD 那里感觉如何哇

[EricWF]

If we choose to perform the cast we should assert that the value is representative is the new type. We don't want to silently truncate.

@dominichamon Should we change the interface of these functions to take size_t for extra precision and to prevent overflows?

[guanqun]

The current code doesn't check the overflow sadly...

https://github.com/google/benchmark/blob/master/src/sleep.cc#L47

[dmah42]

We could change to size_t, or int64_t, but we'd end up truncating in some cases as timespec ultimately takes int.

I'm not sure that choosing int64_t for the constants was a good move in the first place as even `kNumNanosPerSecond comfortably fits in 32 bits. The number of nanoseconds required to overflow the 32-bits would be about an hour, if i did my math correctly, which i'm comfortable with as a max sleep.

We should check for overflow, or at least potential overflow, before the multiplications.

[pleroy]

I'm afraid you didn't do your math correctly, @dominichamon. 32 bits in nanoseconds is about 4 seconds.

[dmah42]

see, this is why i shouldn't be allowed to write anything on the internet before coffee. thanks, @pleroy.

in SleepForMicroseconds we take an int (32-bit) and mod/multiply by 1000 for the nanoseconds. We divide by 1000000 for the seconds. The longest sleep time we can have is, then, 2^32/1000000, which is indeed 4.3 seconds.

There's a subtle bug in SleepForSeconds which takes a double (ie, 64-bits) and silently truncates to a 32-bit int.

I think the correct approach is to take int64_t, do the math in 64-bit, and then check for overflow before truncating to 32-bits right before calling the library functions.

[dmah42]

@guanqun can you make the changes from the above discussion, or would you rather abandon this PR and i'll create an issue for the above?

[guanqun]

@dominichamon give me two days and I'll try to fix this. Otherwise it's all yours. :D

[EricWF]

This patch isn't needed. You're right that the static cast is incorrect, but it's also entirely unneeded. kNumMicrosPerMilli is also an int so the result type of the multiplication also has the type int.

The correct thing is to remove the cast entirely, which has already been done.