canbus: fix frame comparison semantics - #3
Open
Opriego wants to merge 1 commit into
Open
Conversation
Compare CAN frame IDs and DLC explicitly and use memcmp() for binary payload data. This fixes the inverted payload comparison semantics and avoids treating embedded NUL bytes as string terminators. Add regression tests covering identical frames, different CAN IDs, different DLC values, different payloads, and binary payload differences after an embedded NUL byte.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fix
canbus_framecmp()so CAN frames are compared consistently and as binary data.The previous implementation had three issues:
1when CAN IDs differed, but also returned1when payloads were equal, resulting in inconsistent comparison semantics.strcmp()for CAN payloads, even though CAN data is binary and may contain embedded NUL (0x00) bytes.can_dlc, so frames with different payload lengths could be treated incorrectly.Changes
Compare CAN IDs explicitly.
Compare DLC values explicitly.
Replace
strcmp()withmemcmp()over exactlycan_dlcbytes.Keep comparison semantics consistent:
0= frames are equalAdd regression tests covering:
Testing
The regression suite was first run against the original implementation:
The failures demonstrated the inconsistent comparison behavior for identical frames and differing payloads.
After applying the fix:
The new
check_canbustarget was also validated through Automake generation, confirming that bothtests/check_canbus.candsrc/canbus.care included in the test executable.