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

CFE_SB_GetLastSenderId() returns a pointer to a shared data location #745

Closed
dmccomas opened this issue Jun 17, 2020 · 6 comments
Closed
Assignees

Comments

@dmccomas
Copy link

Is your feature request related to a problem? Please describe.
CFE_SB_GetLastSenderId() in cfe_sb_api.c returns a pointer to shared data location which is risky. The function description also warns of some unpredictable or at least inconsistent behavior.

Describe the solution you'd like
I think the use cases for this function should be identified and a safer & behaviorally consistent implementation should be implemented.
Are the use cases?

  1. Determine if app X send the last message
  2. Determine whether the last message sent is from a lsit of apps

A couple of options are

  1. Caller passes in the name of an app or a list of apps to SB determines whether the last message sent is a match.
  2. The current type implementation copies the last message information rather than returning a pointer.

Additional context
n/a

Requester Info
David McComas, NASA Emeritus

@jwilmot
Copy link

jwilmot commented Jun 17, 2020

Use case is to disregard messages from applications that are not "allowed" to send you commands or telemetry. Example, science app can not send GN&C app a command to turn on/off the thrusters.

Yes, returning a ptr to the global is insecure. That area should be protected. Option 2 would be my preferred approach as the comparison logic is in the applications domain. SB does not have to lock the area while doing the search across a potentially long list.

@CDKnightNASA
Copy link
Contributor

I prefer the API return (via an "out" ptr parameter) the AppId of the app. If the caller needs to know the name, there should be a separate call to crosswalk the ID to a name.

@CDKnightNASA CDKnightNASA self-assigned this Jun 18, 2020
@CDKnightNASA
Copy link
Contributor

Note that my proposed solution requires first changing the CFE_SB_SenderId_t type to include uint32 AppId, or replacing SenderId with AppId. Note also ticket #744 which is closely related and should be sequenced with this one lest there be conflicts.

@CDKnightNASA
Copy link
Contributor

Also note that the documentation of the API in cfe_sb.h describe the return value as the AppId when, if you look at the code, it returns a CFE status code (like CFE_SUCCESS). What a mess. :D

@CDKnightNASA
Copy link
Contributor

@dmccomas please look at #759, I propose closing this and #744.

@skliper
Copy link
Contributor

skliper commented Sep 9, 2020

This is OBE by #784 which deprecates it. Marking as duplicate of #759. Merge to main was in #816.

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.

4 participants