-
Notifications
You must be signed in to change notification settings - Fork 206
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
Remove undefined error defines in cfe_error.h #552
Comments
Actually, I would propose removing all of the There should be no need for these duplicate names for the same error codes. It would be a different story if they were providing some abstraction, but they are not- they are only providing a different name for the same thing. |
Yeah, I thought this topic had been touched on recently but I didn't find an issue (maybe in comments somewhere?) I'd be interested to hear from @acudmore. |
It came up in the review of a recent change (issue #308/pull #452) where it was initially mixing the Logically speaking - If calling an OSAL function directly, it should be comparing to the OSAL error codes directly too. The gray area comes in when CFE API calls in turn call the OSAL API, and return the OSAL error directly. For true abstraction this is a bad paradigm and shouldn't even be done (it should explicitly translate the code). But for efficiency the CFE error codes are simply chosen to not overlap/alias OSAL codes to make this practice OK, and it is understood that a return code from a CFE API might be either an OSAL error code or a CFE error code. Since both sets agree on the general pattern that negative is error and non-negative is success, its fine. At any rate, So long as we aren't explicitly translating OSAL error codes to CFE error codes, we do not need to redefine the OSAL error constants in CFE either (and should not) . |
Bigger issue probably worth 2 steps - deprecate then remove, the undefined referenced here could just be removed (they'd likely already be broken). |
See #580 to cover deprecation of the entire set. |
Is your feature request related to a problem? Please describe.
cFE/fsw/cfe-core/src/inc/cfe_error.h
Lines 757 to 763 in f1be048
neither of these exist in OSAL
Describe the solution you'd like
Delete
Describe alternatives you've considered
None
Additional context
None
Requester Info
Jacob Hageman - NASA/GSFC
The text was updated successfully, but these errors were encountered: