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

Fix #790, Return status from OS_ConsoleAPI_Init #792

Merged

Conversation

skliper
Copy link
Contributor

@skliper skliper commented Feb 10, 2021

Describe the contribution
Fix #790 - now returning status from OS_ConsoleAPI_Init so debug warnings will get reported correctly on errors

Testing performed
Build and executed unit tests, passed (note the OS_ConsoleAPI_Init test does not check status)

Expected behavior changes
Will send debug message on error

System(s) tested on

  • Hardware: cFS Dev Server
  • OS: Ubuntu 18.04
  • Versions: cFS Bundle main + this commit

Additional context
See #791 to add full branch coverage to unit tests w/ return checking

Third party code
None

Contributor Info - All information REQUIRED for consideration of pull request
Jacob Hageman - NASA/GSFC

Copy link
Contributor

@jphickey jphickey left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is fine in general however I just wanted to note there is a chance of unintended consequence where FSW might now entirely fail to start if initializing the OS_printf() subsystem (debug) doesn't work.

Returning the failure status here will ultimately trigger a PSP panic/reset whereas before it would simply run in a degraded state where OS_printf() calls didn't work but other things might.

That being said, if this init fails it probably indicates a real underlying problem that will manifest in other ways, so I don't know if allowing startup to continue in a degraded state is a good thing or not.

@skliper
Copy link
Contributor Author

skliper commented Feb 10, 2021

Returning the failure status here will ultimately trigger a PSP panic/reset...

Understood but I see that as a separate issue (update error handling to not cause panic/reset for this case if desired), either way this routine should report it's real status and it should get reported via some mechanism.

@astrogeco astrogeco changed the base branch from main to integration-candidate February 12, 2021 20:41
@astrogeco astrogeco merged commit 7448a5e into nasa:integration-candidate Feb 12, 2021
@skliper skliper deleted the fix790_os_consoleapi_init_rtn branch April 1, 2021 20:07
jphickey pushed a commit to jphickey/osal that referenced this pull request Aug 10, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Return code not passed back in OS_ConsoleAPI_Init - static analysis warning
4 participants