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

Use Resource ID type for SB Pipe ID #985

Closed
jphickey opened this issue Oct 30, 2020 · 1 comment · Fixed by #1092 or #1109
Closed

Use Resource ID type for SB Pipe ID #985

jphickey opened this issue Oct 30, 2020 · 1 comment · Fixed by #1092 or #1109
Assignees
Milestone

Comments

@jphickey
Copy link
Contributor

Is your feature request related to a problem? Please describe.
The CFE SB pipe ID table is not safe from aliasing or other issues.

Describe the solution you'd like
Redefine the CFE_SB_PipeID_t type to be a form of CFE_ES_ResourceID_t like many other resources have already been converted (mem pool handles, CDS blocks, etc).

Re-Use all the same management patterns of this structure.

Additional context
The only potential downside is that Resource IDs are defined as 32 bit values but Pipe IDs were only 8 bits. So this will make sizeof(CFE_SB_PipeId_t) into 4 instead of 1. But as long as apps are properly using the typedef and not assuming uint8 or otherwise depending on this being a single byte, this shouldn't be noticeable.

This is somewhat related to previous issue #100 - implementing this would be a good step in the right direction for that issue too.

Requester Info
Joseph Hickey, Vantage Systems, Inc.

@jphickey jphickey self-assigned this Jan 12, 2021
@jphickey
Copy link
Contributor Author

As part of debugging #1073 it is clear that many SB functions - particularly in the Pipe table and Pipe IDs - are not properly locking data structures when accessing the global table(s).

Using Resource IDs as SB Pipe IDs helps find these instances of code improperly/randomly accessing the CFE_SB.PipeTbl and therefore is important to finding and solving the race conditions.

jphickey added a commit to jphickey/cFE that referenced this issue Jan 13, 2021
Move certain definitions related to the CFE_ES_ResourceID_t type
into the global CFE include files.  This introduces two new headers:

cfe_resourceid.h (public)
cfe_resourceid_internal.h (private to CFE core apps)

This allows other CFE core apps, such as SB, to use the
CFE_ES_ResourceID_t using the same manipulators.
jphickey added a commit to jphickey/cFE that referenced this issue Jan 15, 2021
Move certain definitions related to the CFE_ES_ResourceID_t type
into the global CFE include files.  This introduces two new headers:

cfe_resourceid.h (public)
cfe_resourceid_internal.h (private to CFE core apps)

This allows other CFE core apps, such as SB, to use the
CFE_ES_ResourceID_t using the same manipulators.
@astrogeco astrogeco added this to the 7.0.0 milestone Jan 25, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants