Assert now > 0 in GetTime GetTimeMillis GetTimeMicros #7094

pull pstratem wants to merge 1 commits into bitcoin:master from pstratem:2015-11-24-assert-time changing 1 files +11 −5

pstratem commented at 2:41 AM on November 25, 2015: contributor

Previously all of these functions could return negative values (for different readons). Large portions of the codebase currently assume that these functions return positive values.

Assert now > 0 in GetTime GetTimeMillis GetTimeMicros

Previously all of these functions could return negative values (for different
readons).  Large portions of the codebase currently assume that these
functions return positive values.

1bb289fe1b

pstratem commented at 2:42 AM on November 25, 2015: contributor

(Besides if your time is before 1970-1-1 you will reject every single block as being too far in the future)
dcousens commented at 3:16 AM on November 25, 2015: contributor

Sure? Concept ACK. Any motivation for this @pstratem ?
pstratem commented at 3:23 AM on November 25, 2015: contributor

@dcousens I was reviewing another PR and noticed that it (sort of) relied on this behavior.

Went and looked a bit and noticed basically every caller does.
dcousens commented at 3:28 AM on November 25, 2015: contributor

utACK
laanwj commented at 7:10 AM on November 25, 2015: member

Anything concerning time deltas is a different issue. I certainly think a monotonic clock would be useful for some measurements (such as pings).

However this code change would just make sure that the absolute time is positive. More of a sanity check of the OS and boost (eg time() returns -1 on error).
laanwj added the label Utils and libraries on Nov 25, 2015
jtimon commented at 4:13 PM on November 27, 2015: contributor

utACK
sipa commented at 4:26 PM on November 27, 2015: member

In what cases do we expect these assertions to fail?
paveljanik commented at 11:22 AM on November 28, 2015: contributor

IIUIC, it is only for "just to be sure".

E.g. on Linux kernel, you can't make time to fail, especially in the case when NULL is used as an argument. If non-NULL is passed, it can fail when kernel's put_user fails (and in such case, more bad things can happen ;-). The other question is boost...

ACK
laanwj commented at 1:48 PM on November 30, 2015: member

@paveljanik I guess time() could fail if the relevant syscall is somehow blocked off, say, through seccomp_bpf. And yes boost... who knows All cases in which it's better to fail fast.
laanwj merged this on Nov 30, 2015
laanwj closed this on Nov 30, 2015
laanwj referenced this in commit 34e02e0147 on Nov 30, 2015
luke-jr referenced this in commit be2633856d on Jan 10, 2016
luke-jr referenced this in commit 2c7ba8ddf9 on Jan 10, 2016
MarcoFalke locked this on Sep 8, 2021

Contributors

Labels