Skip to content

Redundancy Check issues #97

Open
Open

Description

@dmhouston95

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

Activity

  1. OrangeTux commented on Jul 23, 2020

    @OrangeTux
    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.')
  2. dmhouston95 commented on Jul 23, 2020

    @dmhouston95
    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.

  3. dmhouston95 commented on Jul 23, 2020

    @dmhouston95
    Author

    This would also be vaild

    if not (struct.unpack('<H', get_crc(msg[:-2])) ==struct.unpack('<H', msg[-2:])):
            raise CRCError('CRC validation failed.')
    
  4. rgov commented on Jul 24, 2020

    @rgov
    Contributor

    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

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions