checksum division by zero

Projects Programming
1

There is a library i use,
Conceptinetics
for sending and receiving DMX and for RDM, anyway i never use the RDM part, but in one of the functions (on line 597) there is this

if ((m_csRecv.checksum % (uint16_t)0x10000) == m_csRecv.checksum)

and the compiler, in my opinion correctly puts out a warning "division by zero' and yes, this (uint16_t)0x10000 results in 0
I am actually quite curious what the whole 'if' statement is about. I don't know much about checksums, so if anyone can explain.

2

It's just bad code. If that statement is executed, the system is crashed. But, if there is no else clause followed, it is mostly likely to be optimized out.

3

That does seem a funny way to write it.

Can you tell us the type of m_csRecv.checksum?

It looks like this is kind of a mask operation. Like the variable is 32-bit and it's just trying to strip it down to 16 bits. Then it checks if it is equal to itself, so it's really checking if there's any junk in the top 16 bits.

The modulo operation on Arduino is really expensive, particularly on the 8-bit ones. It does a long division, which can be really time consuming on long integers.

I think this will have the same effect (but will always be true if the variable is 16-bit)

  if (m_csRecv.checksum & 0xffff == m_csRecv.checksum)

Edit: fixed code, thanks for finding thatn odometer

4

arduino_new:
It's just bad code. If that statement is executed, the system is crashed. But, if there is no else clause followed, it is mostly likely to be optimized out.

well, in my experience an accidental division by zero does not crash an Arduino (but does crash an ESP)

Can you tell us the type of m_csRecv.checksum?

it is uint16_t,
RDM_Checksum    m_csRecv; , and

union RDM_Checksum
{
    uint16_t checksum;
    struct
    {
        uint8_t csl;
        uint8_t csh;
    };
};

That is what is so confusing, on a 16-bit variable.

5

That kind of union is used to allow access to the individual bytes when necessary. Perfectly normal but it is somewhat of a relic of the 1970s.

6

MorganS:

  if (m_csRecv.checksum & 0x1111 == m_csRecv.checksum)

Nope.

Try this:

  if (m_csRecv.checksum & 0xFFFFU == m_csRecv.checksum)

EDIT: OOPS, I forgot the U (for "unsigned")! Actually, it might work without the U, but I want to make sure the thing knows I mean 65535 and not -1.

7

but I want to make sure the thing knows I mean 65535 and not -1.

On a bit-wise operation, does that really matter ?
All in all though this condition (as in the OP) should always be true and so doesn't do anything. Anybody disagree ?

8

Deva_Rishi:
On a bit-wise operation, does that really matter ?

No.

The UL/U modifier is important to append when the numerical value of the result is to be evaluated after an arithmetic operation on data.

checksum = CHKSUM: Add together all the data bytes, discard the accumulated carries and take the 2's complement of the rest and send it as the last byte of the transmission frame. This CHKSUM is used for simple error checking in some communication systems.