3

I received a CRC function written in C from a hardware partner. Messages send by his devices are signed using this code. Can anyone help me to translate it to Java?

int8 crc8(int8*buf, int8 data_byte)
{
    int8 crc=0x00;
    int8 data_bit=0x80;
    while(data_byte>0)
    {
        if(((crc&0x01)!=0)!=((buf[data_byte]&data_bit)!=0))
        {
            crc>>=1;
            crc^=0xCD;
        }
        else 
            crc>>=1;
        data_bit>>=1;
        if(!data_bit)
        {
            data_bit=0x80;
            data_byte--;
        }
    }
    return(crc);
}

I tried to convert this to Java, but the result is not I expect.

public static byte crc8(byte [] buf, byte data_byte)
{
  byte crc = 0x00;
  byte data_bit = (byte)0x80;
  while(data_byte>0)
  {
    if(((crc&0x01)!=0)!=((buf[data_byte]&data_bit)!=0))
    {
      crc>>=1;
      crc^=0xCD;
    }
    else
    {
      crc>>=1;
    }
    data_bit>>=1;
    if(data_bit == 0)
    {
      data_bit= (byte)0x80;
      data_byte--;
    }
  }
  return crc;
}

I suppose that this is the error: if(data_bit != 0)

EDIT:

I changed the code to byte in my conversion method. I receive my data from a socket and convert this then to a String where I get a byteArray out from.

An input example is 16, 0, 1, -15, 43, 6, 1, 6, 8, 0, 111, 0, 0 ,49 where the last field (49) should be the checksum

I also tried Durandals version, but my result is still not valid.

This is how I read the data

BufferedReader bufferedReader = new BufferedReader(new InputStreamReader(socket.getInputStream()));
char[] buffer = new char[14];
int count= bufferedReader.read(buffer, 0, 14); 
String msg = new String(buffer, 0, count);
byte[] content = msg.getBytes();
  • Do they have bytes in java? Why shorts? – Charlie Burns Nov 04 '13 at 15:08
  • You want if(data_bit == 0) – Charlie Burns Nov 04 '13 at 15:09
  • It might be best to use bytes. Short in Java is 16 bits. – jonhopkins Nov 04 '13 at 15:10
  • 1
    I forget whether C `int8` uses unsigned shifts but you may need `>>>` in Java to duplicate the C `>>`. – OldCurmudgeon Nov 04 '13 at 15:12
  • Could you edit your question to include some sample input and the expected output as well as the output you're getting with the Java version? – jonhopkins Nov 04 '13 at 15:33
  • @OldCurmudgeon whether shifts are sign-extending (in c) depends on the compiler (and possibly hardware) implementation. He would be best off asking the original author if he intended zero-fill, but I suspect you're right and he did. – Kevin Nov 04 '13 at 18:29
  • If that's all you got for the C code, then it is incomplete. There must be a `typedef` or `#define` for `int8`, since that is not part of any standard. There is an `int8_t` in `stddef.h`, but it is a `signed char`. I am fairly certain that the `int8` here must be an unsigned char, with a bad name. You need the rest of the code from your hardware partner. – Mark Adler Nov 04 '13 at 18:37
  • As noted in an answer below, the C code appears to be broken, since it computes the crc on `buf[1..data_byte]` (in reverse order) instead of `buf[0..data_byte-1]` as one might expect. – Mark Adler Nov 04 '13 at 18:51
  • unfortunately this is all I have from the C code=/ – user2952987 Nov 05 '13 at 15:33
  • Using a BufferedReader (which is intended to read *characters*, not bytes), converting to String and then to byte[] is *deeply disturbing*. Its introducing so many opportunities to go wrong I don't even think I can point them all out. Use a (Buffered)InputStream and read raw bytes without implicit conversions done by the reader. Every time you see a String.getBytes() without specifying an encoding, its more likely to be a bug or an oversight than it is correct code. – Durandal Nov 05 '13 at 18:43

2 Answers2

2
if(!data_bit)

translates to

if(data_bit == 0)

You really need to use bytes and not shorts. To get around the problem you had using bytes, use this

byte data_bit = (byte)0x80;

Also, as Mark says, you need to use >>> instead of >>.

Charlie Burns
  • 6,994
  • 20
  • 29
  • if I use if(data_bit == 0), then I geht an edless loop since it never enters in this if. I used short instead of byte since byte data_bit =0x80; gives a syntax error (type mismatch). – user2952987 Nov 04 '13 at 15:19
  • I'd suggest going back to bytes and then reprasing your question. I shouldn't have answered your question, I don't have java on my machine. – Charlie Burns Nov 04 '13 at 15:28
  • I don't know jack about java but this seems to work 'byte b = (byte)0x80;' Shorts are just not going to work for you in this program. You need to use bytes. See here http://stackoverflow.com/questions/5193883/how-do-you-specify-a-byte-literal-in-java – Charlie Burns Nov 04 '13 at 15:42
0

Translate the code 1:1, paying extra attention to all operations done on bytes to account for java's implicit cast to int (e.g. (byte >>> 1) is absolutely worthless because the byte is first extendet to int, shifted and then cast back, making it effectively a signed shift no matter what).

Therefore local variables are best declared as int and when loaded from a bytearray masked to yield unsigned extension: int x = byte[i] & 0xFF; Since in the only place that is done data is already masked down to a single bit (in the if) there is nothing special to be done.

Applying to the C code yields:

int crc8(byte[] buf, int dataCount) {
    int crc = 0;
    int data_bit = 0x80;
    while(dataCount > 0) {
        if ( ((crc & 0x01)!=0) != ((buf[dataCount] & data_bit)!=0)) {
            crc >>= 1;
            crc ^= 0xCD;
        } else {
            crc >>= 1;
        }
        data_bit >>= 1;
        if (data_bit == 0) {
            data_bit = 0x80;
            dataCount--;
        }
    }
    return crc;
}

That said, the code isn't very efficient (it processes input bit by bit, there are faster implementations processing entire bytes, using a table for each possible byte added, but you probably don't care for this use case).

Also, beware when you compare the crc from this method to a byte, you must mask the byte properly with 0xFF, otherwise comparison will fail for values >=0x80:

(int) crc == (byte) crc & 0xFF

EDIT:

What worries my even about the original code, that data_byte is clearly intended to specify a length, first it calculates in reverse order and also, it will access an additional byte after the specfied number (data_byte is not decremented before the loop). I suspect the original is (already) broken code, or the calls to it are very messy.

Durandal
  • 19,919
  • 4
  • 36
  • 70