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

Fix #1208, typesafe definition of osal_id_t #1209

Merged
merged 1 commit into from
Jan 24, 2022

Conversation

jphickey
Copy link
Contributor

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.

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 jphickey added the CCB:Ready Pull request is ready for discussion at the Configuration Control Board (CCB) label Jan 18, 2022
@jphickey
Copy link
Contributor Author

There is one exception to CFE compliance here, see nasa/cFE#2024
The fix for the CFE issue should be merged first.

@skliper skliper added this to the Draco milestone Jan 18, 2022
@astrogeco 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
@astrogeco
Copy link
Contributor

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
@astrogeco
Copy link
Contributor

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 astrogeco merged commit e3d2f4c into nasa:main Jan 24, 2022
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
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]>
@jphickey jphickey deleted the fix-1208-typesafe-id branch February 23, 2022 18:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CCB:Approved Indicates code review and approval by community CCB dependency draco-rc1
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Type-safe implementation for osal_id_t
3 participants