Hi,
I've had another look at the issue and found a solution, which I hope will satisfy everyone. I'll start with a brief argument, followed by the code and a more detailed explanation.
Looking at the function chTimeAddX(), the idea is to add some sysinterval_t value to a given systime_t and get the result again as systime_t. As with any summation, there is the possibility of overflows to occur. Especially if the sysinterval_t is wider than systime_t, this risk is exceptionally high. But since systime_t is expected to overflow over time anyway, the calculation of chTimeAddX() may overflow just as well. In this case, however, the information
how often the original systime_t value revolved can not be obtained trivially. This again can be considered as legitimate limitation, though, as long as CH_CFG_ST_TIMEDELTA <= TIME_MAX_SYSTIME holds, because the system is expected to stay within the specified jitter in any situation.
These considerations in mind, I enhanced the chTimeAddx() function as follows:
Code: Select all
static inline systime_t chTimeAddX(systime_t systime, sysinterval_t interval) {
#if CH_CFG_ST_RESOLUTION < CH_CFG_INTERVALS_SIZE
return systime + (systime_t)(interval % ((sysinterval_t)1 << CH_CFG_ST_RESOLUTION));
#else
return systime + interval;
#endif
}
First, I distinguish the trivial case (after #else), where systime_t can hold any interval value and thus will overflow no more than once.
For the other case, I "subtract" (modulo operator) a multiple of full systime_t revolutions, so that the remaining rest of interval will lead to one or none overflow of systime as it fits into the systime_t type. In order to prevent any side effects due to the wider sysinterval_t type, this value is casted to the correct systime_t type and added to the specified systime. Finally, the returned number is the correct systime_t value after an arbitrary number of overflows (which is unknown, though).
pro:
In contrast to the original implementation, any value of interval can be given to the function.
contra:
With the original implementation the result of chTimeAddX() would never be equal to the systime argument, which is possible now and might cause issues (although it should not, imo).
For my case, the kernel test suite does not panic anymore and everything else seems to be unaffected.
Let me know what you think about this =)
- Thomas
