[BUG] Missing tick overflow protection on u.xTCP.xLastActTime and u.xTCP.xLastAliveTime

[lzungri]

No tick overflow protection mechanism in place for u.xTCP.xLastActTime and u.xTCP.xLastAliveTime:

• FreeRTOS-Plus-TCP/source/FreeRTOS_TCP_Transmission.c

  Line 969 in d701079

  TickType_t xAge = xTaskGetTickCount() - pxSocket->u.xTCP.xLastAliveTime;

• FreeRTOS-Plus-TCP/source/FreeRTOS_TCP_State_Handling.c

  Line 183 in d701079

  TickType_t xAge = xTaskGetTickCount() - pxSocket->u.xTCP.xLastActTime;

Consider replacing raw arithmetic on the tick count with the safer xTaskCheckForTimeOut.

[htibosch]

Hello @lzungri , thank you for reporting this, but I don't think that there is a problem with mentioned code.

   TickType_t uxAge = xTaskGetTickCount() - pxSocket->u.xTCP.xLastActTime;

Both xTaskGetTickCount() and xLastActTime have the type TickType_t, which is an uint32_t or bigger.

Suppose that xLastActTime has the value 0xFFFFFFF0, and now the clocktick has overflown to 0x00000002. Is that the overflow you mean? uxAge will become 0x00000012, thanks to the magic of two's complement.

--- BEGIN EDIT ---

Using signed variables would give the same result:

int xLastActTime    = -16
xTaskGetTickCount() =   2
uxAge               =   2 - ( -16 ) = 18

--- END EDIT ---

If you feel that I miss the point, please give an example in which things might go wrong.

[lzungri]

@htibosch Thanks for looking into that.

Suppose that xLastActTime has the value 0xFFFFFFF0, and now the clocktick has overflown to 0x00000002. Is that the overflow you mean? uxAge will become 0x00000012, thanks to the magic of two's complement.

When only one tick overflow has occurred, the result will be correct. I believe the problem will arise when more than 1 tick overflow occurred during that evaluation period. In that case, uxAge will not hold the right time delta as it doesn't account for the number of overflows.
This issue may not be that problematic on 32/64 bits CPUs where TickType_t is big enough to make sure an overflow only happens after a year or so. However, on ports where TickType_t is defined as uint16_t or uint8_t, overflows will happen much more frequently and could lead to timing/accuracy problems. Having a high configTICK_RATE_HZ will make things worse.

xTaskCheckForTimeOut/vTaskSetTimeOutState internally keep track of the number of overflows and thus will not have this problem.

[htibosch]

I do not think that the +TCP is being used on a "16-bit" CPU. At least I don't know of any such project.

[lzungri]

I do not think that the +TCP is being used on a "16-bit" CPU. At least I don't know of any such project.

Got it, please let me know if I should close this issue.

[htibosch]

If nobody else has comments on this subject, you may close it.
Thanks.

[aggarg]

I am closing this issue as per the above discussion. Feel free to reopen is you need anything else.