-
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 #635, Reduce use of uint32, add more OSAL typedefs #654
Fix #635, Reduce use of uint32, add more OSAL typedefs #654
Conversation
CCB 2020-11-12 APPROVED
|
@astrogeco - note this PR does depend on PR nasa/cFE#1003 which is currently a draft. I can either update this draft to a final rev or we can hold off on merging this until next cycle. |
Adds typedefs for: - osal_priority_t - osal_stackptr_t - osal_index_t - osal_objtype_t - osal_blockcount_t Note that by using `uint8` as the priority type, all values are valid and it is no longer possible to pass a value which is out of range. Checks/tests for this are no longer valid and would cause compiler warnings.
263ed34
to
2899dfe
Compare
Note - commit 2899dfe is a rebase to current "main" branch. One thing to consider before final merge: the Other alternatives:
(In general I prefer the simplicity of using the same type everywhere but if large filesystems on 32 bit platforms are a thing, this might not be feasible) |
Additional type corrections to avoid implicit sign and width conversions.
Describe the contribution
Scrub all uses of
uint32
type across OSAL, and replace most usage of this generic type with more appropriate, purpose-specific typedefs.All use of
uint32
to store an object size is now replaced withsize_t
.Adds proper typedefs for:
osal_priority_t
- used for indicating a task priorityosal_stackptr_t
- for indicating a task stack pointerosal_index_t
- for indicating a table position/array index of all internal tablesosal_objtype_t
- for indicating the type of an objectFixes #635
Testing performed
Build and sanity test OSAL on supported platforms
Run all unit tests
Expected behavior changes
None. This is mainly a scrub of all APIs to use the proper type, but keeps it as
uint32
most of the time, so it really shouldn't change anything. Except for sizes which are nowsize_t
- so they will be 64 bits on a 64 bit platform.System(s) tested on
Ubuntu 20.04
RTEMS 4.11
Additional context
This does expose another API problem with functions that return a size as an
int32
(e.g. OS_read, OS_write, etc) .... as anint32
cannot represent the full range of possible values. We may need a type to correlate with POSIXssize_t
to address this.Contributor Info - All information REQUIRED for consideration of pull request
Joseph Hickey, Vantage Systems, Inc.