Skip to content

check result of modbus_set_slave()#87

Closed
r00t- wants to merge 1 commit intoepsilonrt:masterfrom
r00t-:check_set_slave_result
Closed

check result of modbus_set_slave()#87
r00t- wants to merge 1 commit intoepsilonrt:masterfrom
r00t-:check_set_slave_result

Conversation

@r00t-
Copy link

@r00t- r00t- commented Oct 22, 2024

libmodbus considers some slave addresses invalid.
it will report an error, but mbpoll previously ignored this, leading to a crash with an assertion failure.

@r00t-
Copy link
Author

r00t- commented Oct 22, 2024

before this change:

[~/mbpoll]# ./mbpoll -m rtu -b 9600 -P none /dev/ttyUSB0 -a 248 -r 1 -c 1 -t 3 -u -v
[...]
Slave configuration...: address = 248, report slave id
Communication.........: /dev/ttyUSB0,       9600-8N1 
                        t/o 1.00 s, poll rate 1000 ms
mbpoll: modbus-rtu.c:94: _modbus_rtu_build_request_basis: Assertion `ctx->slave != -1' failed.
Aborted

after:

[~/mbpoll]# ./mbpoll -a 248 -b 9600 -t 3 -r 1 -c 2 /dev/ttyUSB0 
[...]
Slave configuration...: address = [248]
                        start reference = 1, count = 2
Communication.........: /dev/ttyUSB0,       9600-8E1 
                        t/o 1.00 s, poll rate 1000 ms
Data type.............: 16-bit register, input register table

./mbpoll: setting slave address failed: Invalid argument.

@r00t-
Copy link
Author

r00t- commented Oct 22, 2024

the same crash can be seen in #75

@r00t- r00t- force-pushed the check_set_slave_result branch from d755866 to 3ff31b0 Compare October 22, 2024 04:37
libmodbus considers some slave addresses invalid.
it will report an error, but mbpoll previously ignored this,
leading to a crash with an assertion failure.
@r00t- r00t- mentioned this pull request Apr 22, 2025
zknpr added a commit to zknpr/mbpoll that referenced this pull request Feb 6, 2026
## Security Fixes
- Fix memory leak: piStartRef was not freed in cleanup paths
- Add NULL checks after all malloc/calloc calls to prevent crashes
- Add compiler security flags: -fstack-protector-strong, -D_FORTIFY_SOURCE=2, -Wformat-security

## Code Modernization
- Modernize CMakeLists.txt with proper Release/Debug build separation
- Use target-based CMake properties instead of global definitions
- Add -Wpedantic flag (with exception for 3rdparty getopt code)
- Update copyright years to 2015-2025
- Update libmodbus minimum version to 3.1.7
- Translate all French comments to English

## Community PR Integration
- Implement PR epsilonrt#87: Check modbus_set_slave() return value to prevent crashes with invalid slave addresses (credit: @r00t-)
- Implement PR epsilonrt#90: Add -x option to print register addresses in hexadecimal format (credit: @ciakval)
- Implement PR epsilonrt#99: Add -Q and -X options for libmodbus quirks (MAX_SLAVE and REPLY_TO_BROADCAST) (credit: @JesseRiemens)

## New Features
- `-x` Print address (reference) in hexadecimal format
- `-Q` Enable MAX_SLAVE quirk (accept slave id 0-255)
- `-X` Enable REPLY_TO_BROADCAST quirk (send reply to broadcast)

## CI/CD
- Add GitHub Actions workflow for automated builds on Linux, macOS, and Windows

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@epsilonrt
Copy link
Owner

fix by #100

@epsilonrt epsilonrt closed this Feb 10, 2026
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

Successfully merging this pull request may close these issues.

2 participants