Skip to content
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

Improve Validation of Config #1084

Merged
merged 17 commits into from
Mar 14, 2024
Merged

Conversation

HTRamsey
Copy link
Contributor

@HTRamsey HTRamsey commented Jan 30, 2024

Improve Validation of Config

Description

Verifies FreeRTOSConfig.h values that are required
Uses static asserts on a few ipconfig values that can't be compile time checked normally

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.

Related Issue

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

@HTRamsey HTRamsey requested a review from a team as a code owner January 30, 2024 15:02
@paulbartell
Copy link
Contributor

@HTRamsey Thanks for the PR. Would you mind running uncrustify on your changes?

@HTRamsey
Copy link
Contributor Author

/bot run formatting

paulbartell
paulbartell previously approved these changes Feb 1, 2024
Comment on lines +172 to +174
#if ( configSUPPORT_DYNAMIC_ALLOCATION == 0 )
#error configSUPPORT_DYNAMIC_ALLOCATION must be set to 1
#endif
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, this is good. We can remove this once #449 has been resolved.

@HTRamsey
Copy link
Contributor Author

HTRamsey commented Feb 6, 2024

@tony-josi-aws we can change the static assert definition, that semi colon is just to allow multiple asserts on the same line.

Have to change unsigned comparisons against 0 too

@tony-josi-aws
Copy link
Member

@HTRamsey, makes sense to remove the semicolon if we are not using multiple asserts on the same line.

And MISRA doesn't like literals without a sign (U) explicitly specified.

@HTRamsey
Copy link
Contributor Author

HTRamsey commented Feb 6, 2024

@tony-josi-aws Should I add literal signs to all of the default configs?

@tony-josi-aws
Copy link
Member

@HTRamsey, not all, but those that are compared/used/assigned to unsigned variables. The latest main branch is validated with coverity for MISRA 2012, so unless new macros are being added, the existing default macro definitions should be good.

@HTRamsey
Copy link
Contributor Author

Is there an actual minimum kernel version that is supported?

@tony-josi-aws
Copy link
Member

@HTRamsey

FreeRTOS+TCP requires kernel version that supports event groups, which is added with kernel version v8.1.

@tony-josi-aws tony-josi-aws merged commit 4364e78 into FreeRTOS:main Mar 14, 2024
10 checks passed
@HTRamsey HTRamsey deleted the dev-config-check branch March 14, 2024 15:44
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.

6 participants