-
Notifications
You must be signed in to change notification settings - Fork 217
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
Fix #1208, typesafe definition of osal_id_t #1209
Merged
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Modifies the osal_id_t typedef to be a non-integer value. The intent is to catch cases where it inappropriately being used as an integer value. This is transparent so long as the osal_id_t typedef and provided check and conversion routines are used.
jphickey
added
the
CCB:Ready
Pull request is ready for discussion at the Configuration Control Board (CCB)
label
Jan 18, 2022
There is one exception to CFE compliance here, see nasa/cFE#2024 |
skliper
approved these changes
Jan 18, 2022
astrogeco
added
CCB:Approved
Indicates code review and approval by community CCB
and removed
CCB:Ready
Pull request is ready for discussion at the Configuration Control Board (CCB)
labels
Jan 19, 2022
CCB:2022-01-19 APPROVED |
astrogeco
added a commit
that referenced
this pull request
Jan 24, 2022
Fix #1208, typesafe definition of osal_id_t
astrogeco
added a commit
to nasa/cFS
that referenced
this pull request
Jan 24, 2022
Missed merging it to IC:Caelum+dev2, added this PR to nasa/cFS#414 and it passess all checks so merging straight to main. |
astrogeco
added a commit
to nasa/cFS
that referenced
this pull request
Jan 24, 2022
astrogeco
added a commit
to nasa/cFS
that referenced
this pull request
Feb 3, 2022
This was referenced Feb 3, 2022
astrogeco
added a commit
to nasa/cFS
that referenced
this pull request
Feb 3, 2022
*cFE v7.0.0-rc4+dev70* nasa/cFE#2041, Improve CFE_SB_IsValidMsgId handler nasa/cFE#2034, Update CodeQL workflow nasa/cFE#2042, Replace CFE_SB_ValueToMsgId(0) with CFE_SB_INVALID_MSG_ID *osal v6.0.0-rc4+dev32* nasa/osal#1209, typesafe definition of osal_id_t *sample_app v1.3.0-rc4+dev9* nasa/sample_app#165, Use preferred UT patterns Co-authored-by: Jacob Hageman <[email protected]> Co-authored-by: Paul <[email protected]> Co-authored-by: Ariel Adams <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Labels
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Describe the contribution
Modifies the osal_id_t typedef to be a non-integer value. The intent is to catch cases where it inappropriately being used as an integer value.
This is transparent so long as the osal_id_t typedef and provided check and conversion routines are used.
Fixes #1208
Testing performed
Build and sanity check CFE and OSAL with and without OSAL_OMIT_DEPRECATED flag
Expected behavior changes
When OSAL_OMIT_DEPRECATED is enabled (opt-in) this will catch misuse of OSAL IDs as integers, or failure to use correct
osal_id_t
typedef to hold the value.System(s) tested on
Ubuntu 21.10
Additional context
OSAL and CFE should all be compliant, but CFS apps probably are not yet compliant with this. Hence why it is "opt-in" via OSAL_OMIT_DEPRECATED flag, like other similar changes done in the past.
Contributor Info - All information REQUIRED for consideration of pull request
Joseph Hickey, Vantage Systems, Inc.