crc-16 IBM, 0x00 not taken in consideration

Question

I did test a crc-16/ibm implementation i found on the net. when I test it with hex byte array it works fine but if I include some 0x00 values, then it doesn't give the proper result. here is its code

unsigned short ComputeCRC16(const unsigned char* buf, unsigned int len) {
    unsigned short crc = 0;
    for (unsigned int j = 0; j < len; j++)
    {
        unsigned char b = buf[j];
        for (unsigned char i = 0; i < 8; i++)
        {
            crc = ((b ^ (unsigned char)crc) & 1) ? ((crc >> 1) ^ 0xA001) : (crc >> 1);
            b >>= 1;
        }
    }
    return crc;
}

I tested it with this code:

int main() {

    //fe   b5     5f       f7
    unsigned char buf1[4096] = { 0xfe, 0xb5, 0x5f, 0xf7 };

    //fe   b5     00    5f     f7   00
    unsigned char buf2[4096] = { 0xfe, 0xb5, 0x00, 0x5f, 0xf7, 0x00 };

    int a = strlen(buf1);
    unsigned short res = ComputeCRC16(buf1, a);
    printf("res = %04x\n", res); //res : 7858, the result is correct

    int b = strlen(buf2);
    unsigned short res = ComputeCRC16(buf2, b);
    printf("res = %04x\n", res); //res : d781, the result is not correct
    return 0;                   //the correct result : 26EE
}

to verify the result I use this website: https://www.lammertbies.nl/comm/info/crc-calculation

Do not use `strlen`, it is counting up to a first `0` byte and is in general intended to be used with strings, not arbitrary binary data.. — Eugene Sh., Dec 13 '21 at 16:02
In your case you can use `sizeof`, as the arrays are static (in terms of known size to the compiler, not in terms of storage class). If they are not, you will have to track the size by other means. — Eugene Sh., Dec 13 '21 at 16:06
Right.. because the size of your arrays is fixed `4096`, didn't notice it. Change to `unsigned char buf1[] = { 0xfe, 0xb5, 0x5f, 0xf7 };` and `unsigned char buf2[] = { 0xfe, 0xb5, 0x00, 0x5f, 0xf7, 0x00 };` Or just pass 4 and 5 respectively as `a` and `b` — Eugene Sh., Dec 13 '21 at 16:11
but how does the fact the array is static affects the result, i didn't get it tbh. — othman chanaa, Dec 13 '21 at 16:15
This is a separate question. Take a look here: https://stackoverflow.com/questions/37538/how-do-i-determine-the-size-of-my-array-in-c and don't skip the answers talking about array "decay" — Eugene Sh., Dec 13 '21 at 16:21
He meant you can't obtain the size of the array created using `TYPE *p = malloc(sizeof(*p) * n);` It's not actually an array. You would simply use `n`. — ikegami, Dec 13 '21 at 16:21

score 1 · Accepted Answer · answered Dec 13 '21 at 16:21

Your CRC routine gives correct results. It is your test that's wrong. strlen(p) returns how many bytes there are before the first zero byte at p. For buf2, that's four, not five as you intended. For buf1 it's not even defined, since there can be anything in memory after that array. You might be getting four, if the compiler happened to put zeros after the array.

For testing, you should simply provide len manually. (buf1, 4), (buf2, 5).

By the way, that code could be more efficient. It doesn't have to test with b every time. Just exclusive-oring with b to start has the same effect:

    crc ^= buf[j];
    for (unsigned char i = 0; i < 8; i++)
        crc = crc & 1 ? (crc >> 1) ^ 0xa001 : crc >> 1;

thanks for the clarification. – othman chanaa Dec 13 '21 at 16:25 — othman chanaa, Dec 13 '21 at 16:25

crc-16 IBM, 0x00 not taken in consideration

1 Answers1