CRC 16, C to C#

Question

I have this code sample of CRC-16 calculation and I want to convert it to C#. It have to stay with the same logic because it works with other machines.

I saw this crc16 from c to c# but it doesn't work (different result), maybe because of the variable types.

The original code is: Input char = 'A', remainder = 0, Output = 390

#define CRC_POLYNOMIAL 0x8005 /* CRC polynomial */
short Calculate_CRC ( char ch, short remainder ){
    char *ch_ptr;
    int i;
    // the different part
    ch_ptr = (char*)&remainder;
    *(ch_ptr+1) = *(ch_ptr+1) ^ ch; /* exclusive or ch with msb of remainder */
    for (i=0; i<=7; i++)
        if (remainder < 0)
        { /* msb of remainder = 1 */
            remainder = remainder << 1;
            remainder = remainder ^ CRC_POLYNOMIAL;
        }
        else
            remainder = remainder << 1;
    return (remainder);
}

and my code is: Input char = 'A', remainder = 0, Output = 514

    static public ushort func(byte ch, ushort remainder, int f) {
        // the different part
        byte[] rawCrc = new byte[2];
        rawCrc[0] = (byte)(remainder & 0x00FF);
        rawCrc[1] = (byte)((remainder >> 8) & 0x00FF);
        rawCrc[0] ^= ch;
        remainder &= (byte)(rawCrc[0]);
        for (int i = 0; i <= 7; i++)
            if ((remainder & (0x8000)) == 0) { /* msb of remainder = 1 */
                remainder <<= 1;
                remainder ^= POL;
            }
            else
                remainder <<= 1;
        return remainder;
    }

The result is different but as I can see, my code still doing the xor with the msb as needed.

How can I fix it?

TIA

Answer 1

Your C# code is not equivalent to your C code. Specifically, this:

        rawCrc[0] ^= ch;

... XORs ch with the least-significant byte of remainder, whereas the corresponding operation in the C code is an XOR with the most-significant byte.

Furthermore, this C# line:

        remainder &= (byte)(rawCrc[0]);

... does not appear to correspond to anything in the C code, and it looks wrong on its face. I suppose that you mean to recompose the two bytes into remainder, but that doesn't do it. I would suggest doing without rawCrc altogether, and instead doing something like

            remainder = remainder ^ ((ch & 0xff) << 8);

Also, the test in this C# if statement:

            if ((remainder & (0x8000)) == 0) { /* msb of remainder = 1 */

... is reversed relative to the corresponding test in the C code, and inconsistent with the meaning asserted by the trailing comment.

Answer 2

Try this... It should (SHOULD) be equivalent to the C version. Note that I removed the byte[] array.

const short CRC_POLYNOMIAL = unchecked((short)0x8005);

static short Calculate_CRC(byte ch, short remainder)
{
    short ch_ptr = (short)(remainder & 0xFF);
    ch_ptr |= (short)((remainder & 0xFF00) ^ (ch << 8));

    // You could directly use ch_ptr from now on!
    remainder = ch_ptr;

    for (int i = 0; i <= 7; i++)
    {
        if (remainder < 0)
        { /* msb of remainder = 1 */
            remainder = (short)(remainder << 1);
            remainder = (short)(remainder ^ CRC_POLYNOMIAL);
        }
        else
        {
            remainder = (short)(remainder << 1);
        }
    }

    return remainder;
}

Note that I would normally remove the short and change it to ushort. Just thinking that there is a bit sign in something that should be treated as bits makes me want to puke.

const ushort CRC_POLYNOMIAL = (ushort)0x8005;

static ushort Calculate_CRC(byte ch, ushort remainder)
{
    ushort remainder2 = (ushort)(remainder & 0xFF);
    remainder2 |= (ushort)((remainder & 0xFF00) ^ (ch << 8));

    for (int i = 0; i <= 7; i++)
    {
        if (remainder2 > short.MaxValue)
        { /* msb of remainder = 1 */
            remainder2 = (ushort)(remainder2 << 1);
            remainder2 = (ushort)(remainder2 ^ CRC_POLYNOMIAL);
        }
        else
        {
            remainder2 = (ushort)(remainder2 << 1);
        }
    }

    return remainder2;
}

Answer 3

This line in the C code: *(ch_ptr+1) = *(ch_ptr+1) ^ ch; is exclusive-oring ch onto the high byte of remainder. It is a horrible way to do it, since that would fail on a big-endian machine. What they should have done, in C, is remainder ^= (short)ch << 8;. That would be portable, as well as more obvious to the reader what's going on. That is what I did in the C# code below, with the appropriate casting for C# (which seems to have really odd implicit promotion rules):

static ushort Calculate_CRC(byte ch, ushort crc) {
    crc ^= (ushort)(ch << 8);
    for (int i = 0; i < 8; i++)
        crc = (ushort)((crc & 0x8000) == 0 ? crc << 1 : (crc << 1) ^ 0x8005);
    return crc;
}