Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Clean up endian temporary compatibility macro #843

Closed
skliper opened this issue Mar 10, 2021 · 2 comments · Fixed by #878 or #927
Closed

Clean up endian temporary compatibility macro #843

skliper opened this issue Mar 10, 2021 · 2 comments · Fixed by #878 or #927
Assignees
Milestone

Comments

@skliper
Copy link
Contributor

skliper commented Mar 10, 2021

Is your feature request related to a problem? Please describe.
Missing an else (likely error) case (or !defined(__BYTE_ORDER))

/*
* TEMPORARY COMPATIBILITY MACRO
*
* Any code that depends on this macro should be fixed so as to not need it.
* The value for this had been set by the BSP makefiles but this is not reliable,
* especially on processors that support both big- and little- endian modes e.g.
* ARM and MIPS.
*
* This is deprecated and only here to bridge the gap until code that depends
* on this can be fixed. Do not write any new code that uses this macro.
*
* If using an older makefile that defines one of the BIT_ORDER macros already,
* then this entire section is skipped and the macro is used as-is.
*/
#if !defined(SOFTWARE_BIG_BIT_ORDER) && !defined(SOFTWARE_LITTLE_BIT_ORDER)
#if defined(__BYTE_ORDER) && __BYTE_ORDER == __BIG_ENDIAN || defined(__BIG_ENDIAN__) || defined(__ARMEB__) || \
defined(__THUMBEB__) || defined(__AARCH64EB__) || defined(_MIBSEB) || defined(__MIBSEB) || defined(__MIBSEB__)
/* It is a big-endian target architecture */
#define SOFTWARE_BIG_BIT_ORDER
#elif defined(__BYTE_ORDER) && __BYTE_ORDER == __LITTLE_ENDIAN || defined(__LITTLE_ENDIAN__) || defined(__ARMEL__) || \
defined(__THUMBEL__) || defined(__AARCH64EL__) || defined(_MIPSEL) || defined(__MIPSEL) || defined(__MIPSEL__)
/* It is a little-endian target architecture */
#define SOFTWARE_LITTLE_BIT_ORDER
#endif
#endif /* !defined(SOFTWARE_BIG_BIT_ORDER) && !defined(SOFTWARE_LITTLE_BIT_ORDER) */

It's also a misnomer (BYTE not BIT), and one of many various patterns (see nasa/cFE#1209)

Describe the solution you'd like
At minimum add the error case, ideally clean/remove/consolidate.

Describe alternatives you've considered
None

Additional context
nasa/cFE#1209

Requester Info
Jacob Hageman - NASA/GSFC, OSAL code review

@jphickey
Copy link
Contributor

Here's the conundrum with this one:

  • CFS Developers should be discouraged from writing any endian-specific code.
  • So while these macros are defined via some commonly-used C library macros, none of the symbols checked are standardized in any way, nothing is guaranteed to work.
  • These BIG/LITTLE macros are only there for backward compatibility. We don't actually want anyone to use them. Nothing in OSAL itself uses these at all, either.

SO - at the end of the day - we shouldn't be puting an #error in a file that is used everywhere in the event that a (nonstandardized) test was inconclusive as to how to define a macro that we actively discourage you from using.

My recommendation would be to take this out of OSAL completely, and only put it into the CFE file that defines CFE_MAKE_BIG16/32 and then we can just punt the problem to CFE to discourage developers from writing endian -specific code.

@jphickey
Copy link
Contributor

Submitted nasa/cFE#1217 to stop CFE from depending on this, after which it can be removed from OSAL.

jphickey added a commit to jphickey/osal that referenced this issue Mar 10, 2021
The "common_types.h" file will no longer provide these.
astrogeco added a commit that referenced this issue Mar 24, 2021
Fix #843, remove BIG/LITTLE bit order macros
# for free to join this conversation on GitHub. Already have an account? # to comment
Projects
None yet
2 participants