Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Redundancy Check issues #97

Open
dmhouston95 opened this issue Jul 22, 2020 · 4 comments
Open

Redundancy Check issues #97

dmhouston95 opened this issue Jul 22, 2020 · 4 comments

Comments

@dmhouston95
Copy link

Function validate_crc does not evaluate the CRC bytes correctly. Incorrect use of not and ==

@OrangeTux
Copy link
Collaborator

What do you mean? Instead of

    if not struct.unpack('<H', get_crc(msg[:-2])) ==\
            struct.unpack('<H', msg[-2:]):
        raise CRCError('CRC validation failed.')

you suggest:

    if  struct.unpack('<H', get_crc(msg[:-2])) !=\
            struct.unpack('<H', msg[-2:]):
        raise CRCError('CRC validation failed.')

@dmhouston95
Copy link
Author

Yes. The expression evaluates false because not of an integer is always false unless the integer is zero. And then if you are trying to compare a boolean false (evaluated from the not expression) with an integer that will also always be false.

@dmhouston95
Copy link
Author

dmhouston95 commented Jul 23, 2020

This would also be vaild

if not (struct.unpack('<H', get_crc(msg[:-2])) ==struct.unpack('<H', msg[-2:])):
        raise CRCError('CRC validation failed.')

@rgov
Copy link
Contributor

rgov commented Jul 24, 2020

See operator precedence: https://docs.python.org/3/reference/expressions.html#operator-precedence

not is less binding than == so I believe that not a == b is (in most cases?) the same as a != b. Since == and is have the same precedence, this is easier to see this way:

>>> not 1
False
>>> not 1 is True
True

If not bound more tightly, then it would be the same as (not 1) is True which doesn't match the behavior. But instead we get not (1 is True).

However != reads better to me.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants