Skip to content

Avoid portTICK_PERIOD_MS usage in library code #1242

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 6 commits into
base: main
Choose a base branch
from

Conversation

StefanBalt
Copy link
Contributor

@StefanBalt StefanBalt commented Apr 15, 2025

Avoid portTICK_PERIOD_MS usage in library code

Description

portTICK_PERIOD_MS evaluates to zero when configTICK_RATE_HZ > 1000, which (while uncommon) is a valid configuration (e.g., for stress testing).

To ensure compatibility and robustness, this change replaces its usage with safer FreeRTOS-provided time conversion macros pdMS_TO_TICKS() and pdTICKS_TO_MS().

Notably, the FreeRTOS kernel itself avoids using portTICK_PERIOD_MS.

Some of the conversion code was fixed as a side effect:

  • In source/FreeRTOS_DNS_Cache.c the ulCurrentTimeSeconds was not correctly converted to seconds previously, if portTICK_PERIOD_MS != 1
  • In source/include/FreeRTOSIPConfigDefaults.h the portMAX_DELAY multiplication would typically overflow previously (depending on the architecure)

Test Steps

Checklist:

  • I have tested my changes. No regression in existing tests.
  • I have modified and/or added unit-tests to cover the code changes in this Pull Request.

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

portTICK_PERIOD_MS evaluates to zero when configTICK_RATE_HZ > 1000,
which (while uncommon) is a valid configuration (e.g., for stress
testing).

To ensure compatibility and robustness, this change replaces its usage
with safer FreeRTOS-provided time conversion macros.

Notably, the FreeRTOS kernel itself avoids using portTICK_PERIOD_MS.
@StefanBalt StefanBalt requested a review from a team as a code owner April 15, 2025 15:51
@jasonpcarroll
Copy link
Member

Hi @StefanBalt,

Thank you for this PR. I will ping someone from the team to take a look.

Best,

Jason Carroll

@tony-josi-aws
Copy link
Member

Thanks for this PR @StefanBalt

Do you wish to update the usage of portTICK_PERIOD_MS some of the config files present in test and utilities files?

Also could you please apply the formattingChanges patch that can be found in the end of this page: https://github.com/FreeRTOS/FreeRTOS-Plus-TCP/actions/runs/14574889771?pr=1242 to fix the formatting checks.

@StefanBalt
Copy link
Contributor Author

Do you wish to update the usage of portTICK_PERIOD_MS some of the config files present in test and utilities files?

I will replace some of the usages.

Also could you please apply the formattingChanges

Yes, I will also format the code.

Use safer pdMS_TO_TICKS() macro instead.
Keep some portTICK_PERIOD_MS usages to test both implementations.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants