Skip to content

Commit

Permalink
Fix #437: Avoid alignment warnings on some CPUs
Browse files Browse the repository at this point in the history
On CPUs with strict alignment requirements, some CFE code
that uses a char-type pointer (e.g. uint8*) to compute
memory addresses triggers an alignment warning when it gets
cast back to the actual data type.

This code should be alignment-safe already, because the address
computation already takes CPU alignment requirements into account
when calculating the addresses/offsets.

However, the compiler still flags the final conversion from a
pointer with no special alignment to something with alignment
requirements.

- For the CFE_SB pool buffers, using the `cpuaddr` type, which
  is integer in nature, avoids the warning.
- For the CFE_TBL internal table pointer, use a `void*` internally
  to store the buffer pointer, rather than a `uint8_t*`.  This
  changes the casting needs elsewhere.
  • Loading branch information
jphickey authored and skliper committed Jan 10, 2020
1 parent e84e6a2 commit f71141f
Show file tree
Hide file tree
Showing 6 changed files with 11 additions and 10 deletions.
7 changes: 4 additions & 3 deletions fsw/cfe-core/src/sb/cfe_sb_api.c
Original file line number Diff line number Diff line change
Expand Up @@ -1802,7 +1802,7 @@ CFE_SB_Msg_t *CFE_SB_ZeroCopyGetPtr(uint16 MsgSize,
{
int32 stat1;
uint32 AppId = 0xFFFFFFFF;
uint8 *address = NULL;
cpuaddr address = 0;
CFE_SB_ZeroCopyD_t *zcd = NULL;
CFE_SB_BufferD_t *bd = NULL;

Expand Down Expand Up @@ -1849,7 +1849,7 @@ CFE_SB_Msg_t *CFE_SB_ZeroCopyGetPtr(uint16 MsgSize,
}/* end if */

/* first set ptr to actual msg buffer the same as ptr to descriptor */
address = (uint8 *)bd;
address = (cpuaddr)bd;

/* increment actual msg buffer ptr beyond the descriptor */
address += sizeof(CFE_SB_BufferD_t);
Expand Down Expand Up @@ -1914,6 +1914,7 @@ int32 CFE_SB_ZeroCopyReleasePtr(CFE_SB_Msg_t *Ptr2Release,
{
int32 Status;
int32 Stat2;
cpuaddr Addr = (cpuaddr)Ptr2Release;

Status = CFE_SB_ZeroCopyReleaseDesc(Ptr2Release, BufferHandle);

Expand All @@ -1922,7 +1923,7 @@ int32 CFE_SB_ZeroCopyReleasePtr(CFE_SB_Msg_t *Ptr2Release,
if(Status == CFE_SUCCESS){
/* give the buffer back to the buffer pool */
Stat2 = CFE_ES_PutPoolBuf(CFE_SB.Mem.PoolHdl,
(uint32 *) (((uint8 *)Ptr2Release) - sizeof(CFE_SB_BufferD_t)));
(uint32 *) (Addr - sizeof(CFE_SB_BufferD_t)));
if(Stat2 > 0){
/* Substract the size of the actual buffer from the Memory in use ctr */
CFE_SB.StatTlmMsg.Payload.MemInUse-=Stat2;
Expand Down
2 changes: 1 addition & 1 deletion fsw/cfe-core/src/sb/cfe_sb_buf.c
Original file line number Diff line number Diff line change
Expand Up @@ -116,7 +116,7 @@ CFE_SB_BufferD_t * CFE_SB_GetBufferFromPool(CFE_SB_MsgId_t MsgId, uint16 Size) {

CFE_SB_BufferD_t * CFE_SB_GetBufferFromCaller(CFE_SB_MsgId_t MsgId,
void *Address) {
CFE_SB_BufferD_t *bd = (CFE_SB_BufferD_t *)(((uint8 *)Address) - sizeof(CFE_SB_BufferD_t));
CFE_SB_BufferD_t *bd = (CFE_SB_BufferD_t *)((cpuaddr)Address - sizeof(CFE_SB_BufferD_t));

/* Initialize the MsgId in the buffer descriptor (the rest has already been initialized in this case). */
bd->MsgId = MsgId;
Expand Down
6 changes: 3 additions & 3 deletions fsw/cfe-core/src/tbl/cfe_tbl_internal.c
Original file line number Diff line number Diff line change
Expand Up @@ -486,7 +486,7 @@ int32 CFE_TBL_RemoveAccessLink(CFE_TBL_Handle_t TblHandle)
if (RegRecPtr->UserDefAddr == false)
{
/* Free memory allocated to buffers */
Status = CFE_ES_PutPoolBuf(CFE_TBL_TaskData.Buf.PoolHdl, (uint32 *)RegRecPtr->Buffers[0].BufferPtr);
Status = CFE_ES_PutPoolBuf(CFE_TBL_TaskData.Buf.PoolHdl, RegRecPtr->Buffers[0].BufferPtr);
RegRecPtr->Buffers[0].BufferPtr = NULL;

if (Status < 0)
Expand All @@ -498,7 +498,7 @@ int32 CFE_TBL_RemoveAccessLink(CFE_TBL_Handle_t TblHandle)
/* If a double buffered table, then free the second buffer as well */
if (RegRecPtr->DoubleBuffered)
{
Status = CFE_ES_PutPoolBuf(CFE_TBL_TaskData.Buf.PoolHdl, (uint32 *)RegRecPtr->Buffers[1].BufferPtr);
Status = CFE_ES_PutPoolBuf(CFE_TBL_TaskData.Buf.PoolHdl, RegRecPtr->Buffers[1].BufferPtr);
RegRecPtr->Buffers[1].BufferPtr = NULL;

if (Status < 0)
Expand Down Expand Up @@ -986,7 +986,7 @@ int32 CFE_TBL_LoadFromFile(CFE_TBL_LoadBuff_t *WorkingBufferPtr,
}

NumBytes = OS_read(FileDescriptor,
&WorkingBufferPtr->BufferPtr[TblFileHeader.Offset],
((uint8*)WorkingBufferPtr->BufferPtr) + TblFileHeader.Offset,
TblFileHeader.NumBytes);

if (NumBytes != TblFileHeader.NumBytes)
Expand Down
2 changes: 1 addition & 1 deletion fsw/cfe-core/src/tbl/cfe_tbl_task.h
Original file line number Diff line number Diff line change
Expand Up @@ -145,7 +145,7 @@ typedef struct
*/
typedef struct
{
uint8 *BufferPtr; /**< \brief Pointer to Load Buffer */
void *BufferPtr; /**< \brief Pointer to Load Buffer */
uint32 FileCreateTimeSecs; /**< \brief File creation time from last file loaded into table */
uint32 FileCreateTimeSubSecs; /**< \brief File creation time from last file loaded into table */
uint32 Crc; /**< \brief Last calculated CRC for this buffer's contents */
Expand Down
2 changes: 1 addition & 1 deletion fsw/cfe-core/src/tbl/cfe_tbl_task_cmds.c
Original file line number Diff line number Diff line change
Expand Up @@ -441,7 +441,7 @@ int32 CFE_TBL_LoadCmd(const CFE_TBL_Load_t *data)
{
/* Copy data from file into working buffer */
Status = OS_read(FileDescriptor,
&WorkingBufferPtr->BufferPtr[TblFileHeader.Offset],
((uint8*)WorkingBufferPtr->BufferPtr) + TblFileHeader.Offset,
TblFileHeader.NumBytes);

/* Make sure the appropriate number of bytes were read */
Expand Down
2 changes: 1 addition & 1 deletion fsw/cfe-core/unit-test/tbl_UT.c
Original file line number Diff line number Diff line change
Expand Up @@ -1688,7 +1688,7 @@ void Test_CFE_TBL_HousekeepingCmd(void)
CFE_TBL_LoadBuff_t *DumpBuffPtr = &DumpBuff;
CFE_TBL_RegistryRec_t RegRecPtr;
uint8 Buff;
uint8 *BuffPtr = &Buff;
void *BuffPtr = &Buff;
uint32 Secs = 0;
uint32 SubSecs = 0;
int32 LoadInProg = 0;
Expand Down

0 comments on commit f71141f

Please sign in to comment.