Check function Compare Issue

Question

Im writing a function that compares a 32bit CRC that is extracted from a Buffer (uint32_t lBuffer[10]) which first ten entries contain randomly generated data, to a calculated CRC which is computed within the CheckCRC function. Can anyone see why it doesn't want to give a true result? Any help would be much appreciated!

Here is the function code:

bool CRC32::CheckCRC(const uint32_t* plData , uint32_t lLength, uint32_t previousCrc32)
{
    bool FlagPass;
    uint32_t lCalcCRC,lMsgCRC;


    //Msg CRC needs to be extracted first
    lMsgCRC = plData[lLength-1];

    //Newly calculated CRC
    //lCalcCRC = calculate_CRC32(plData,lLength-1,lInitCRC);

    lCalcCRC = ~previousCrc32;
    unsigned char* current = (unsigned char*) plData;

    while (lLength--)
    {
        lCalcCRC = (lCalcCRC >> 8) ^ crc_table[(lCalcCRC & 0xFF) ^ *current++];
    }
    lCalcCRC = ~lCalcCRC;


    if (lCalcCRC == lMsgCRC)
    {
      FlagPass = true;
    }
    else
    {
      FlagPass = false;
    }
    return FlagPass;
}

Answer 1

The problem in how you hare handling the length of the data buffer. When you retrieve the CRC of the buffer from plData[lLength-1] the length is interpreted as the number of elements in the array. Later when you iterate over the buffer the length is assumed to be the number of bytes in the buffer.

If lLength represents the number of bytes you will need to compensate for this when retrieving the existing CRC value from the buffer. You will also need to adjust the size of the buffer so that the existing CRC itself is not included when you iterate over the buffer.

uint32_t lMsgCRC = *((const uint32_t*)((const char*)plData + lLength) - 1);
lLength -= sizeof(plData[0]);

If lLength represents the number of elements in the array rather than the number of bytes you will need to adjust the size. Again you need to take into consideration that the existing CRC is at the end of the buffer.

// Adjust length by size of elements
lLength = (lLength - sizeof(plData[0])) * sizeof(plData[0]);
while (lLength--)
{
    lCalcCRC = (lCalcCRC >> 8) ^ crc_table[(lCalcCRC & 0xFF) ^ *current++];
}

Answer 2

Some issues:

1. You are using lLength both as a number of uint32_ts in the input array, and as a number of bytes. It can't be both. If it is the number of uint32_t, you need to multiply by 4 to get the number of bytes.

2. Since the last element of the array contains the expected CRC, you want to avoid calculating the CRC of that element, so subtract 4 from the number of bytes to process.

3. Your code is dependent on the endianness of the machine it runs on due to processing a uint32_t array as an array of bytes.

4. There are different variations of CRC-32 calculation, so you need to make sure you are using the correct one.