Repository navigation
Misc fixes from Claude Code review - #634
Merged
Merged
Conversation
The data parsing loops in the CANFD/CANXL evaluation functions jumped three characters forward after each hex byte without checking the separator. When a line ended directly after a hex byte (e.g. last line without newline or a line split by fgets()) the pointer skipped the string termination and continued parsing stale content from previous lines - or in the worst case read beyond the input buffer. Factor out the four identical loops into get_hex_data() which requires a whitespace separator behind each hex byte. Assisted-by: Claude:claude-opus-5-5 Signed-off-by: Oliver Hartkopp <socketcan@hartkopp.net>
The DLC and data length values were read into signed integers with %x and %d. Negative values (e.g. '-1' or 'ffffffff') passed the range checks and led to len values of 255 or 65535. Via the union cfu these classic CAN frames were then printed as CAN XL frames. Use unsigned int for dlc/dlen and %u for the data length so that negative values are caught by the existing range checks. Assisted-by: Claude:claude-opus-5-5 Signed-off-by: Oliver Hartkopp <socketcan@hartkopp.net>
The helper functions for the error frame content added the return values of snprintf() to the buffer offset without checking for truncation. With a too small buffer the following 'len - n' calculation wrapped around (size_t) and subsequent snprintf() calls could write beyond the end of the buffer. As the maximum output length is known, introduce CANERR_BUFFSZ in lib.h with a description how the required buffer size is calculated and return an empty string if the provided buffer is smaller. The only user candump provides a buffer of more than 6000 bytes for this function. Assisted-by: Claude:claude-opus-5-5 Signed-off-by: Oliver Hartkopp <socketcan@hartkopp.net>
parse_mod() and parse_fdmod() did not check the return value of strchr() when looking for the ':' delimiters. A modification definition without ':' (e.g. '-m AND') led to a segmentation fault. Add the missing NULL pointer checks. Assisted-by: Claude:claude-opus-5-5 Signed-off-by: Oliver Hartkopp <socketcan@hartkopp.net>
pty2can() detected complete SLCAN messages by looking for the '\r' termination but did not use this message length for parsing. The content was read at fixed positions and the start of the following message was derived from the parsed content. Too short messages (e.g. 'T1\r') read the DLC and data from stale buffer content and sent a phantom CAN frame. Following commands got lost after short messages, unknown commands or frames with timestamp. Terminate the current message at its '\r' position, check that the CAN ID fits into the message and always continue with the next message behind the '\r'. Assisted-by: Claude:claude-opus-5-5 Signed-off-by: Oliver Hartkopp <socketcan@hartkopp.net>
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
1 participant
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.
I bought a subscription for one month - so let's have Opus-5 LLM do something for its money :o)