-
Notifications
You must be signed in to change notification settings - Fork 202
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
Incorrect ReturnCode check in CFE_ES_CreateObjects #1698
Comments
Calling this an "enhancement" since they are the same value and type in practice (so not really "broken" in behavior), but should be fixed in the interest of correctness. |
jphickey
added a commit
to jphickey/cFE
that referenced
this issue
Jul 21, 2021
The return code of CFE_ES_StartAppTask is a CFE status code, so it should be compared to CFE_SUCCESS, not OS_SUCCESS.
astrogeco
added a commit
that referenced
this issue
Jul 21, 2021
Fix #1698, correct return code check
I actually noticed a couple more cases of similar status code mismatch in other functions. I can submit a second PR to address them. Should probably NOT close this issue with the current IC, though. |
paulober
pushed a commit
to paulober/cFE
that referenced
this issue
Aug 1, 2021
The return code of CFE_ES_StartAppTask is a CFE status code, so it should be compared to CFE_SUCCESS, not OS_SUCCESS.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Is your feature request related to a problem? Please describe.
The
CFE_ES_StartAppTask
returns a CFE status. However, when checking the the return code, it is compared toOS_SUCCESS
, rather thanCFE_SUCCESS
.cFE/modules/es/fsw/src/cfe_es_start.c
Lines 786 to 799 in 2afdbc1
Describe the solution you'd like
Check against
CFE_SUCCESS
Describe alternatives you've considered
N/A
Additional context
This is just a minor/pedantic correctness issue, since in practice OS_SUCCESS and CFE_SUCCESS are the same value (0). However when scrubbing for type correctness this shows up.
Requester Info
Joseph Hickey, Vantage Systems, Inc.
The text was updated successfully, but these errors were encountered: