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

Improve separation/distinction between OSAL and CFE error codes #1676

Open
jphickey opened this issue Jul 20, 2021 · 4 comments
Open

Improve separation/distinction between OSAL and CFE error codes #1676

jphickey opened this issue Jul 20, 2021 · 4 comments

Comments

@jphickey
Copy link
Contributor

Is your feature request related to a problem? Please describe.
Application code should not conflate/intermix these two sets of error/status code values.

OSAL and CFE differ in how their error codes are defined. For OSAL, in osapi-errors.h, the codes are in signed decimal format (e.g. -10). For CFE, in cfe_error.h, the codes are in hexadecimal format (e.g. 0xc2000003) with bits having certain meanings.

Describe the solution you'd like

  • When storing an error code in a local stack variable, applications should create a separate variable for storing an OSAL status code from a CFE status code.
  • The correct typedef should be used, e.g. CFE_Status_t for CFE codes, and (ideally) OS_Status_t for OSAL codes (which does not exist yet, but see issue Add typedef for OSAL status codes osal#1108).
  • When logging/printing or sending events, use the correct conversion specifier such that the format is consistent with how it is defined. This means %d for OSAL codes, and %08x for CFE codes. (In many cases, OSAL error codes are printed using the %08x conversion currently).

Additional context
For Caelum, the approach will be to document the functions/cases where OSAL and CFE status codes are conflated (see issue #1599) but ideally for better code modularity, the two sets of status codes should not be intermixed at all. While issue #1599 will just document where this is a problem, this issue should be to fix those problem areas and keep things isolated.

Also note that PSP also has a few of its own status codes, which also should be kept separate.

Requester Info
Joseph Hickey, Vantage Systems, Inc.

@jphickey
Copy link
Contributor Author

After reviewing existing code, here is a list of functions which intermix an OSAL status code with a CFE status code (e.g. storing it in the same local variable, etc). These do not appear to return the value to the caller, at least in the current form:

CFE_ES_StartApplications
CFE_ES_CleanUpApp
CFE_ES_CleanupTaskResources
CFE_ES_CDS_EarlyInit
CFE_ES_LockCDS
CFE_ES_UnlockCDS
CFE_ES_PoolCreateEx
CFE_ES_RunPerfLogDump
CFE_ES_InitializeFileSystems
CFE_ES_CreateObjects
CFE_ES_SysLogDump
CFE_ES_QueryAllCmd
CFE_ES_QueryAllTasksCmd
CFE_ES_DumpCDSRegistryCmd
CFE_FS_RunBackgroundFileDump
CFE_SB_CreatePipe
CFE_SB_GetPipeName
CFE_SB_GetPipeIdByName
CFE_TBL_LockRegistry
CFE_TBL_UnlockRegistry
CFE_TBL_GetWorkingBuffer
CFE_TBL_LoadFromFile
CFE_TBL_ReadHeaders
CFE_TBL_HousekeepingCmd
CFE_TBL_LoadCmd
CFE_TBL_DumpToFile

@jphickey
Copy link
Contributor Author

The following functions are OK as far as implementation goes (they use a dedicated/separate variable for OSAL status codes, and do not mix with a CFE status code). However they use a non-ideal or inconsistent conversion when sending events or log messages (typically they print an OSAL error has hex, should print as decimal to match what OSAL defines).

CFE_ES_DeleteChildTask
CFE_ES_LockSharedData
CFE_ES_UnlockSharedData
CFE_ES_LoadModule
CFE_ES_StartAppTask
CFE_ES_CleanupObjectCallback
CFE_ES_BackgroundTask
CFE_ES_Main
CFE_FS_LockSharedData
CFE_FS_UnlockSharedData
CFE_SB_BroadcastBufferToRoute
CFE_SB_ReceiveBuffer
CFE_SB_LockSharedData
CFE_SB_UnlockSharedData

@jphickey
Copy link
Contributor Author

Finally, this set of functions (from issue #1599) should be fixed so that they do NOT return an OSAL status to the caller - they should rewrite to a CFE status code before returning:

CFE_ES_BackgroundInit
CFE_EVS_WriteLogDataFileCmd
CFE_EVS_EarlyInit
CFE_EVS_WriteAppDataFileCmd
CFE_FS_ReadHeader
CFE_FS_WriteHeader
CFE_FS_SetTimestamp
CFE_FS_EarlyInit
CFE_SB_EarlyInit
CFE_TBL_EarlyInit
CFE_TIME_TaskInit

@jphickey
Copy link
Contributor Author

jphickey commented Aug 3, 2021

Related issue #1755 and PR #1756 resolves part of this issue, specifically it ensures that a separate stack variable is created for OSAL status vs. CFE status, and a translation to CFE error occurs in places where an OSAL status was previously returned by a CFE function.

Remaining work to do is to change type from int32 to osal_status_t (depending on nasa/osal#1108) and also to use the appropriate macros/inline functions to check status codes where applicable.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

1 participant