Repository navigation
Handle failed slave selection and stale client buffers safely - #183
Open
rupayon123 wants to merge 2 commits into
Open
rupayon123 wants to merge 2 commits into
rupayon123 wants to merge 2 commits into
Conversation
|
|
|
Memory usage change @ 4f8dcb4
Click for full report table
Click for full report CSV |
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.
The client ignores
modbus_set_slave()failures and can continue a read/write using an invalid or stale destination. This is the error-propagation problem reported in #151.Check selection before single reads/writes and before preparing buffered requests. Return each API's documented failure value while retaining the backend error. Reselect the buffered destination at send time and retire the attempted transmission if that selection fails. Address validity stays with libmodbus so RTU/TCP-specific limits are not hard-coded in the client.
A host regression compiles the production
ModbusClient.cppagainst a recording backend. The original client fails the invalid-ID assertion. The fix passes checks for negative/too-large IDs, all read kinds, single writes, buffered writes, no backend requests after selection failure, valid reads, and buffered destination reselection. Clang C++11 builds with-Wall -Wextra -Werrorand address/undefined-behavior sanitizers pass.This validates wrapper error handling, not the reported physical Opta crash. The RTU client example also compiles for the locally installed Mega AVR core (16,158 bytes flash, 1,647 bytes globals). The libraries warn that AVR is outside their declared architecture lists; this is an additional compile check, not validation of the supported-board CI matrix or physical device behavior. Prepared with AI assistance; source diff and executed regression results reviewed.
A separate buffer-lifetime reproduction found that a failed smaller read leaves the previous larger read count active, so
read()can index beyond the resized buffer.end()likewise leaves readable state after freeing the buffer. Invalidate old read/write views before resizing and on shutdown, and retain the old allocation when realloc fails. The extended sanitizer regression covers a failed shrinking read, forced allocation failures, cleanup, and safe post-end reads. This is an additional fix to the same client; it is not a claim of physical-device validation.