equeue: Fix overflow in rtos-less timeout code #7782
Conversation
Thanks to simonnilsson
| @@ -154,7 +154,7 @@ bool equeue_sema_wait(equeue_sema_t *s, int ms) { | |||
| if (ms == 0) { | |||
| return false; | |||
| } else if (ms > 0) { | |||
| timeout.attach_us(callback(equeue_sema_timeout, s), ms*1000); | |||
| timeout.attach_us(callback(equeue_sema_timeout, s), (us_timestamp_t)ms*1000); | |||
There was a problem hiding this comment.
Shouldn't this be (us_timestamp_t)(ms * 1000) ?
There was a problem hiding this comment.
Why would it be ? result of the expression (ms * 1000) is int which means it is already truncated if it has overflowed.
We want to promote either ms or 1000 to make the calculation happen in the us_timestamp_t number system.
There was a problem hiding this comment.
True, we can cast ms to us first and then do the multiplication, though I think what i suggested is clearer to the intention but I don't care that much tbh :)
There was a problem hiding this comment.
I must be missing something but what you suggested is very different and produce different result; see here
|
/morph build |
Build : ABORTEDBuild number : 2808 |
|
/morph build |
Build : SUCCESSBuild number : 2812 Triggering tests/morph test |
Exporter Build : SUCCESSBuild number : 2440 |
Test : SUCCESSBuild number : 2547 |
…imeout-overflow equeue: Fix overflow in rtos-less timeout code
Description
See #7732
Thanks to @simonnilsson
CC @cmonr, @pan-
Pull request type