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.

Sign up for GitHub

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

Jump to bottom

Fix #555, provide typedef for OSAL ID #568

Merged
merged 3 commits into from
Sep 2, 2020
Merged

Fix #555, provide typedef for OSAL ID #568

merged 3 commits into from
Sep 2, 2020

Conversation

jphickey
Copy link
Contributor

Describe the contribution
Introduces a new typedef, osal_id_t, in common_types.h, which should be used to represent an OSAL ID. All API structures/functions are updated to use this typedef in place of the uint32 type wherever it actually refers to an OSAL ID.

Adds inline functions for basic equality/validity check, and conversion back to bare integer for logging purposes.

Fixes #555

Testing performed
Build and run all unit tests
Execute and sanity test CFE.

Expected behavior changes
Should be none. As the osal_id_t typedef is a uint32, this is just an alias for the same thing.

System(s) tested on
Ubuntu 20.04
RTEMS 4.11.3

Contributor Info - All information REQUIRED for consideration of pull request
Joseph Hickey, Vantage Systems, Inc.

@jphickey jphickey marked this pull request as draft August 19, 2020 15:11
@jphickey
Copy link
Contributor Author

jphickey commented Aug 19, 2020 •
edited
Loading

Marked as draft because it depends on some other changes being merged first - removal of deprecated code in particular as otherwise this would require updating all of them to use the typedef (which doesn't seem worthwhile).

Actual change for review is in de3c985 Will rebase after next cycle.

@jphickey jphickey added the CCB:Ready Pull request is ready for discussion at the Configuration Control Board (CCB) label Aug 19, 2020
@jphickey
Copy link
Contributor Author

Marking as CCB ready, the concept can be reviewed/discussed prior to rebase

@astrogeco
Copy link
Contributor

CCB 2020-08-19 APPROVED

@skliper skliper added this to the 6.0.0 milestone Aug 21, 2020
@astrogeco astrogeco removed the CCB:Ready Pull request is ready for discussion at the Configuration Control Board (CCB) label Aug 26, 2020
@skliper skliper marked this pull request as ready for review August 26, 2020 16:35
@astrogeco astrogeco changed the base branch from main to integration-candidate August 26, 2020 16:36
@jphickey
Copy link
Contributor Author

Rebased to integration-candidate - should be ready for merge

@jphickey
Copy link
Contributor Author

NOTE - testing after rebase, I noticed there are some newly-written unit tests that will need to be updated to use osal_id_t rather than uint32 i.e. file-sys-add-fixed-map-api-test.c and network-api-test.c. I'll get these updated ASAP - please hold off on merge until then.

@jphickey
Copy link
Contributor Author

Commit 0f54017 above updates the new test routines to use the typedef correctly/consistently.

Now this should be good to merge.

yammajamma and others added 3 commits September 1, 2020 17:06
@yammajamma
Merge pull request nasa#582 from nasa/integration-candidate
6720437 
Integration Candidate: 2020-08-26
@jphickey
Fix nasa#555, Introduce typedef for OSAL IDs
b2a49b1 
Update all internal OSAL code to use the typedef when creating, storing,
or otherwise dealing with abstract ID values.

Removes/fixes any code that directly compares or casts the IDs between
normal integer values.  Replace equality comparisons with inline functions
to check for equality and validity.
@jphickey
Update nasa#555, fix newly-added test code
a277cd1 
Update file-sys-add-fixed-map-api-test and network-api-test to
use the osal_id_t typedef and proper conversion/test functions
for manipulating object IDs.
@jphickey
Copy link
Contributor Author

jphickey commented Sep 1, 2020

Rebased to main again ... still hoping to get this into a build....

@jphickey jphickey mentioned this pull request Sep 2, 2020
Closed
@yammajamma yammajamma added the CCB:Approved Indicates code review and approval by community CCB label Sep 2, 2020
@yammajamma yammajamma merged commit d12808c into nasa:integration-candidate Sep 2, 2020
@yammajamma yammajamma mentioned this pull request Sep 3, 2020
Merged
@jphickey jphickey mentioned this pull request Sep 8, 2020
Merged
@jphickey jphickey deleted the fix-555-id-typedef branch December 3, 2020 17:26
@skliper skliper linked an issue Dec 9, 2020 that may be closed by this pull request
Use of hardcoded numbers for bad IDs in OSAL unit tests #515
Closed
@skliper skliper mentioned this pull request Dec 9, 2020
Closed
jphickey pushed a commit to jphickey/osal that referenced this pull request Aug 10, 2022
@astrogeco
Merge pull request nasa#568 from skliper/fix297-cmd-sec-hdr
ff3aa94 
Fix nasa#297, CCSDS Command Secondary Header Endian Agnostic
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
CCB:Approved Indicates code review and approval by community CCB
Projects
None yet
Milestone
Caelum
Development

Successfully merging this pull request may close these issues.

OSAL should provide a typedef for "ID" values Use of hardcoded numbers for bad IDs in OSAL unit tests
4 participants
@jphickey @astrogeco @skliper @yammajamma