More fun with boolean logic
-
Slightly anonymized code:
CRC_Bytes = generate_crc(this.packet,this.packetsize-2)
if ((CRC_bytes[0] != this.packet[(this.packetsize - 2)])
&& (CRC_bytes[1] != this.packet[(this.packetsize - 1)])) {
result = -4; // BAD_CRC.
}It's still hard to read, but in English: we calculate a 2-byte CRC for a packet. We then compare the two calculated bytes with the bytes in the packet. If the first byte is a mismatch and the second byte is a mismatch, return a bad CRC status.
-
Is that code actualy used? How did it survived testing?
Whatever it is that this person is writting gets tested, doesn't it? I mean... At least after it is deployed, somebody always test it.
-
It probably didn't fail testing because it's likely that if one was wrong, the other would be wrong too. You'd have to construct a case where one was ok but the other was wrong artificially. Perhaps beyond the abilities of these test makers. :)
-
Yep, it appears to be in use.Of course, the real traffic I see on the wire only appears to have one byte of CRC, so...
I'm thinking this is part of the reason the creators were bought out about 10 years ago. The product and all associated documentation appears to have been taken out back, shot, burned, buried in the ground, dug up again, and blasted into the sun.
-
I take it that there is also something like this, lower down:
if (result = -4) { //BAD CRC!
//return -1; //CRC Library is broken somehow
}
-
@RichP said:
FTFYCRC_Bytes = generate_crc(this.packet,this.packetsize-2)
if ((CRC_bytes[0] != this.packet[(this.packetsize - 2)])
&& (CRC_bytes[1] != this.packet[(this.packetsize - 1)])) {
result = FILE_NOT_FOUND; // BAD_CRC.
}
-
Not seeing the issue here. The identifiers are good and the code doesn't have any strange logic. Sure, the return value could be an enum/constant/whatever, but that's a minor thing.
-
@henke37 said:
Not seeing the issue here. The identifiers are good and the code doesn't have any strange logic. Sure, the return value could be an enum/constant/whatever, but that's a minor thing.
Well, let me ask you this: how many CRC byte mismatches should there be until the CRC fails?
-
@Xyro said:
@henke37 said:
Not seeing the issue here. The identifiers are good and the code doesn't have any strange logic. Sure, the return value could be an enum/constant/whatever, but that's a minor thing.
Well, let me ask you this: how many CRC byte mismatches should there be until the CRC fails?
Umm...
a) Short-circuit evaluation means the if branch will be entered as soon as the first byte mismatches;
b) How many bytes of CRC are there? If it's a CRC16, don't you want to test both bytes? (Ignoring what was said about there only being 1 byte of CRC...)
I'm not seeing much of a problem here... it's a bit messy, but there's a chunk of data on the wire, followed by a 2-byte CRC of said chunk.
Someone doesn't know how to do ((short)bytePtr), so they test the two bytes separately? Meh.
What am I missing here?
-
@aihtdikh said:
What am I missing here?
Ohhhhh my bad.
I'm missing the &&; I'm reading the code that should be there, not the code that is there.
