In my auto-generated HAL code for implementing CRC I have the following function:
uint32_t HAL_CRC_Accumulate(CRC_HandleTypeDef *hcrc, uint32_t pBuffer[], uint32_t BufferLength)
{
uint32_t index; /* CRC input data buffer index */
uint32_t temp = 0U; /* CRC output (read from hcrc->Instance->DR register) */
/* Change CRC peripheral state */
hcrc->State = HAL_CRC_STATE_BUSY;
switch (hcrc->InputDataFormat)
{
case CRC_INPUTDATA_FORMAT_WORDS:
/* Enter Data to the CRC calculator */
for (index = 0U; index < BufferLength; index )
{
hcrc->Instance->DR = pBuffer[index];
}
temp = hcrc->Instance->DR;
break;
case CRC_INPUTDATA_FORMAT_BYTES:
temp = CRC_Handle_8(hcrc, (uint8_t *)pBuffer, BufferLength);
break;
case CRC_INPUTDATA_FORMAT_HALFWORDS:
temp = CRC_Handle_16(hcrc, (uint16_t *)(void *)pBuffer, BufferLength); /* Derogation MisraC2012 R.11.5 */
break;
default:
break;
}
/* Change CRC peripheral state */
hcrc->State = HAL_CRC_STATE_READY;
/* Return the CRC computed value */
return temp;
}
The problem is that my own inBuff is of type uint8_t * inBuff
and I can see in the auto-generated code that it needs a uint32_t as input, and also later on just typecasts it to a uint8_t as i use the CRC_INPUTDATA_FORMAT_BYTES option for my CRC. The reason for needing my uint8_t buff is to make it work with the rest of my code (pretty big project). Is there any reason to work around this in an efficient way without damaging my own inBuff content? It needs to be aligned correctly.
CodePudding user response:
Problems:
- A function which does not intend to access a pointer parameter through the pointed-at type should not be declared with that type. If
HAL_CRC_Accumulate
never intends to accesspBuffer
as an (array of)uint32_t
then it shouldn't be using that type. It should then have been declared asuint8_t*
. - Passing an
uint8_t*
pointing at the first item of an array to a function acceptinguint32_t*
is highly fishy code and could indeed cause misalignment problems. Most of your problems boils down to: why does this function use uint32_t[]? It doesn't make any sense. - Casting a
uint32*
to auint16_t*
is similarly bad practice and undefined behavior in case the function that thisuint16_t*
is passed to will access that parameter asuint16_t
. Not only because of possible misalignment but also because of strict aliasing. Again, if that function has no intention of using theuint16_t*
to access auint16_t
object, then it shouldn't be using that pointer type. Again, it seems like this whole code should be working onuint8_t*
types. - "Chaining" multiple casts like this
(uint16_t *)(void *)
is nonsense. Thevoid*
adds nothing and solves nothing. - Normally a function that calculates CRC would work on a
const
qualified buffer, because it isn't supposed to change anything. If the so-called "FCS" (calculated checksum) should be appended at the end of the data buffer, then it might be better design to do that separately. - Since MISRA-C is mentioned in comments: all of the above is particularly unacceptable in a mission-critical code base. Auto-generated code is no excuse for sloppy, potentially broken use of types - on the contrary. More importantly than advisory rule 11.5, you have multiple violations of (Required) MISRA-C:2012 rule 11.3 and none of this is MISRA compliant.
CodePudding user response:
- Allocate your buffer to the desired size plus
sizeof(uint32_t) - 1
.
For example, if you buffer is statically allocated to SIZE
, then you can do:
uint8_t buff[SIZE sizeof(uint32_t) - 1];
- Set your pointer to the lowest address within your buffer, which aligned to
uint32_t
:
size_t addr = (size_t)buff sizeof(uint32_t) - 1;
uint8_t* inBuff = (uint8_t*)(addr / sizeof(uint32_t) * sizeof(uint32_t));
CodePudding user response:
How to convert/align my uint8_t pointer buffer to uint32 for it to work with auto-generated HAL functions
Do nothing.
The problem is that my own inBuff is of type uint8_t * inBuff and I can see in the auto-generated code that it needs a uint32_t as input
So cast the pointer.
HAL_CRC_Accumulate(..., (uint32_t*)your_buffer, ...)
Is there any reason to work around this in an efficient way without damaging my own inBuff content?
No. (?)
Yes, it is unpleasant that it doesn't take a void*
pointer or that there aren't just separate API for each INPUTDATA_FORMAT. Still, there is nothing for you to do, just pass the pointer value - the internal routines will access them via uint8_t*
proper handle anyway.
The alignment needed for INPUTDATA_FORMAT_BYTES is 1, at bytes boundaries.