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

Implement single endianness handling pattern #1209

Open
skliper opened this issue Mar 4, 2021 · 2 comments
Open

Implement single endianness handling pattern #1209

skliper opened this issue Mar 4, 2021 · 2 comments

Comments

@skliper
Copy link
Contributor

skliper commented Mar 4, 2021

Is your feature request related to a problem? Please describe.
Multiple ways to handle target endian in various contexts:
fsw/cfe-core/src/es/cfe_es_perf.c:CFE_ES_SetupPerfVariables - determines it at runtime
fsw/cfe-core/src/tbl/cfe_tbl_internal.c:CFE_TBL_ReadHeaders - determines it at runtime
fsw/cfe-core/src/tbl/cfe_tbl_task_cmds.c:CFE_TBL_DumpToFile - determines it at runtime
fsw/cfe-core/src/fs/cfe_fs_api.c:CFE_FS_ReadHeader (and many other functions in here) - determines it at runtime

fsw/cfe-core/src/inc/ccsds.h has macros for conversion (CFE_MAKE_BIG*) who’s implementation depends on SOFTWARE_BIG_BIT_ORDER
There’s an endian flag in the CCSDS header
osal/src/os/inc/common_types.h defines either SOFTWARE_LITTLE_BIT_ORDER or SOFTWARE_BIG_BIT_ORDER based on 8 possible defines

cfe/cmake/sample_defs/cpu1_platform_cfg.h defines CFE_PLATFORM_ENDIAN as either CCSDS_LITTLE_ENDIAN or CCSDS_BIG_ENDIAN, and also has a separately configurable CFE_PLATFORM_TIME_CFG_BIGENDIAN

CCSDS extended header has an endian bit

Then there’s all the different ways the various defines are used and custom swapping routines, examples:

/* * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * */
/* */
/* CFE_FS_ByteSwapUint32() -- byte swap an uint32 */
/* */
/* * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * */
void CFE_FS_ByteSwapUint32(uint32 *Uint32ToSwapPtr)
{
int32 Temp = *Uint32ToSwapPtr;
char *InPtr = (char *)&Temp;
char *OutPtr = (char *)Uint32ToSwapPtr;
OutPtr[0] = InPtr[3];
OutPtr[1] = InPtr[2];
OutPtr[2] = InPtr[1];
OutPtr[3] = InPtr[0];
} /* End of CFE_FS_ByteSwapUint32() */

/*******************************************************************
**
** CFE_TBL_ByteSwapUint32
**
** NOTE: For complete prolog information, see above
********************************************************************/
void CFE_TBL_ByteSwapUint32(uint32 *Uint32ToSwapPtr)
{
int32 Temp = *Uint32ToSwapPtr;
char *InPtr = (char *)&Temp;
char *OutPtr = (char *)Uint32ToSwapPtr;
OutPtr[0] = InPtr[3];
OutPtr[1] = InPtr[2];
OutPtr[2] = InPtr[1];
OutPtr[3] = InPtr[0];
} /* End of CFE_TBL_ByteSwapUint32() */

/* Macro to convert 16/32 bit types from platform "endianness" to Big Endian */
#ifdef SOFTWARE_BIG_BIT_ORDER
#define CFE_MAKE_BIG16(n) (n)
#define CFE_MAKE_BIG32(n) (n)
#else
#define CFE_MAKE_BIG16(n) ((((n) << 8) & 0xFF00) | (((n) >> 8) & 0x00FF))
#define CFE_MAKE_BIG32(n) \
((((n) << 24) & 0xFF000000) | (((n) << 8) & 0x00FF0000) | (((n) >> 8) & 0x0000FF00) | (((n) >> 24) & 0x000000FF))
#endif

Note some cfe_endian.h macros evaluate n multiple times.

Describe the solution you'd like
See #202 (comment) for a compile time suggestion:
#define IS_LITTLE_ENDIAN (((union {unsigned x; char c;}){1}).c)

Describe alternatives you've considered
None

Additional context
Triggered from email discussion on setting another endian flag in a toolchain file for an app to use

Requester Info
Jacob Hageman - NASA/GSFC

Ping - @klystron78 @ejtimmon @wmoleski

@wmoleski
Copy link

wmoleski commented Mar 4, 2021 via email

@jphickey
Copy link
Contributor

jphickey commented Mar 4, 2021

The challenge here is that the C standard (and library) don't want to expose endianness. They actively discourage one from writing endian-specific code, and there is no standardized "marker" to say what endianness the current CPU has. In particular a machine is allowed to arrange the bits however it pleases and still be compliant (see PDP-11). AFAIK There is even a variant that takes the weird PDP-11 order and reverses it. While none of these are common, they do exist.

The problem with any "endian" check is that it is usually binary on the common ones (big or little), and does not account for other variations.

Rather than introducing another endianness-check to rule them all, I'd rather see migration away from any sort of endian-specific code/dependencies.

FWIW, the SOFTWARE_LITTLE_BIT_ORDER (a misnomer because its actually byte order) was intended for compile-time checks where the union trick does not work.

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

3 participants