search

littlekernel / lk

LK embedded kernel
MIT License
3.15k stars 621 forks source link

time comparison flawed #150

Open dcurrie opened 8 years ago

dcurrie commented 8 years ago

The conversion of unsigned to signed type for TIME_LT() and friends in sys/types.h depends on C compiler implementation-defined behavior.

travisg commented 8 years ago

Yeah, I've known about that one for a while, though in practice it's been fine. If you can put together a patch with similar semantics and efficiency I'd absolutely take it.

Can probably do it with an inline routine.

dcurrie commented 8 years ago

I believe these are equivalent and do not use signed arithmetic:

define TIME_GTE(a, b) (((a) - (b)) <= (INFINITE_TIME / 2u))

define TIME_LTE(a, b) ((((a) - (b)) - 1u) > ((INFINITE_TIME / 2u) - 1u))

define TIME_GT(a, b) ((((a) - (b)) - 1u) <= ((INFINITE_TIME / 2u) - 1u))

define TIME_LT(a, b) (((a) - (b)) > (INFINITE_TIME / 2u))

These macros also make it more explicit that times cannot differ by more than UINT32_MAX/2 to be comparable; otherwise the difference overflows an int32_t. This is about 24 days of msec.

The easiest solution is to use 64-bit unsigned integers... these don't overflow for over a half-billion years of msec, and can be compared directly.

dcurrie commented 8 years ago

Here's a unit test for my macros. test_time_macros.c.txt