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

Deprecate OStask_id field from OS_task_prop_t #437

Closed
jphickey opened this issue Apr 23, 2020 · 3 comments · Fixed by #446 or #458
Closed

Deprecate OStask_id field from OS_task_prop_t #437

jphickey opened this issue Apr 23, 2020 · 3 comments · Fixed by #446 or #458
Assignees
@jphickey
Labels
enhancement
Milestone
Bootes

Comments

@jphickey
Copy link
Contributor

Is your feature request related to a problem? Please describe.
This field is not necessarily applicable to all OS types and it breaks the abstraction. OSAL should not be reporting the raw/unabstracted values back to the application.

This field is fundamentally broken in any environment where the underlying OS task ID isn't convertible to a uint32. This includes real systems, such as:

  • Cygwin, where pthread_t is a compound data type, not an integer (POSIX specifically allows this)
  • Systems where the task ID is actually pointer to the TCB (in which case it will be 64 bits on 64 bit systems)

Currently this field serves one purpose, which is to allow the CFE ES exception processing to find the OSAL task ID associated with the exception. However, this design is being reworked in nasa/cFE#411 where more of the exception processing is done in the CFE PSP. With this, there should be no need for applications to ever use the OStask_id member.

Describe the solution you'd like
The OStask_id member of OS_task_prop_t should be marked as deprecated. As a replacement, an new API can be added which can be invoked by the PSP/BSP to aid in exception processing. This new API should use an abstract pointer and size, not assume that the task identifier information fits within a 32-bit integer.

Requester Info
Joseph Hickey, Vantage Systems, Inc.
@jphickey
Copy link
Contributor Author

@skliper, @acudmore, @jwilmot - please comment if you have any thoughts on this, I'd like to include this as part of the overall strategy for fixing nasa/cFE#411 and nasa/cFE#76.

@skliper
Copy link
Contributor

skliper commented Apr 23, 2020

Concur.

@skliper skliper added this to the 5.1.0 milestone Apr 23, 2020
@ghost
Copy link

ghost commented Apr 23, 2020

The proposed solution sounds good to me.

jphickey added a commit to jphickey/osal that referenced this issue Apr 29, 2020
@jphickey
Fix nasa#437, add new API for obtaining exception task ID
dbee24a 
Deprecate the OStask_ID field within the property structure.

Replace with a new API to perform a task ID reverse-lookup
based on an abstract data object (the system task ID).
jphickey added a commit to jphickey/osal that referenced this issue Apr 30, 2020
@jphickey
Fix nasa#437, add new API for obtaining exception task ID
afb70b0 
Deprecate the OStask_ID field within the property structure.

Replace with a new API to perform a task ID reverse-lookup
based on an abstract data object (the system task ID).
jphickey added a commit to jphickey/osal that referenced this issue Apr 30, 2020
@jphickey
Update nasa#437, add UT Test cases
93c0832
jphickey added a commit to jphickey/osal that referenced this issue May 5, 2020
@jphickey
Fix nasa#437, add new API for obtaining exception task ID
f3f3b65 
Deprecate the OStask_ID field within the property structure.

Replace with a new API to perform a task ID reverse-lookup
based on an abstract data object (the system task ID).
@jphickey jphickey mentioned this issue May 5, 2020
Merged
astrogeco added a commit that referenced this issue May 8, 2020
@astrogeco
Merge pull request #446 from jphickey/fix-437-exception-api
998377b 
Fix #437, Add new API for obtaining exception task ID
@astrogeco astrogeco mentioned this issue May 13, 2020
Merged
jphickey added a commit to jphickey/osal that referenced this issue Aug 10, 2022
@jphickey
Fix nasa#437: Avoid alignment warnings on some CPUs
4607e4e 
On CPUs with strict alignment requirements, some CFE code
that uses a char-type pointer (e.g. uint8*) to compute
memory addresses triggers an alignment warning when it gets
cast back to the actual data type.

This code should be alignment-safe already, because the address
computation already takes CPU alignment requirements into account
when calculating the addresses/offsets.

However, the compiler still flags the final conversion from a
pointer with no special alignment to something with alignment
requirements.

- For the CFE_SB pool buffers, using the `cpuaddr` type, which
  is integer in nature, avoids the warning.
- For the CFE_TBL internal table pointer, use a `void*` internally
  to store the buffer pointer, rather than a `uint8_t*`.  This
  changes the casting needs elsewhere.
jphickey pushed a commit to jphickey/osal that referenced this issue Aug 10, 2022
@skliper
Merge pull request nasa#450 from jphickey/fix-437-alignment-warnings
c69605c 
Fix nasa#437: Avoid alignment warnings on some CPUs
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@jphickey jphickey

Labels
enhancement
Projects
None yet
Milestone
Bootes
2 participants
@jphickey @skliper