Security fixes, code modernization, and community PR integration by zknpr · Pull Request #100 · epsilonrt/mbpoll

zknpr · 2026-02-06T19:28:42Z

Summary

This PR consolidates security fixes, code modernization, and integrates several valuable community PRs that have been pending.

Security Fixes

Memory leak fix: piStartRef was not freed in vSigIntHandler() and vFailureExit() cleanup paths
NULL checks: Added proper NULL checks after all malloc/calloc calls instead of relying on assert() (which is disabled by NDEBUG in release builds)
Compiler security flags: Added -fstack-protector-strong, -D_FORTIFY_SOURCE=2, -Wformat-security for release builds

Code Modernization

CMakeLists.txt overhaul: Modernized to use target-based CMake properties, proper Release/Debug build separation, and generator expressions
Build type handling: NDEBUG is now only defined in Release builds; Debug builds have -g -O0 for proper debugging
Added -Wpedantic with exception for 3rdparty getopt code (uses deprecated register keyword)
Updated libmodbus minimum version to 3.1.7 (required for quirks support)
Translated all French comments to English for maintainability
Updated copyright years to 2015-2025

Community PR Integration

This PR implements the functionality from the following open PRs:

check result of modbus_set_slave() #87 (@r00t-): Check modbus_set_slave() return value to prevent crashes with invalid slave addresses
Add option -x to print reference in hexadecimal format #90 (@ciakval): Add -x option to print register addresses in hexadecimal format
Add support for MAX_SLAVE and REPLY_TO_BROADCAST quirks #99 (@JesseRiemens): Add -Q and -X options for libmodbus quirks (MAX_SLAVE and REPLY_TO_BROADCAST)

New Command-Line Options

-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

Added GitHub Actions workflow for automated builds on:
- Ubuntu (Release + Debug configurations)
- macOS (Release)
- Windows (Release)

Test Plan

Build on Linux with libmodbus >= 3.1.7
Build on macOS
Build on Windows (with bundled libmodbus)
Test -x option displays hex addresses correctly
Test -Q option allows slave addresses 0-255
Test -X option for broadcast reply
Verify memory leak is fixed with valgrind
Verify invalid slave address now returns proper error instead of crash

Breaking Changes

libmodbus minimum version increased from 3.1.4 to 3.1.7 (required for modbus_enable_quirks() function)

🤖 Generated with Claude Code

@r00t-

## 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>

Copilot

Pull request overview

This PR consolidates security hardening and modernization work for mbpoll, including safer memory allocation handling, new CLI features, updated build tooling (CMake), and a new multi-platform GitHub Actions CI workflow.

Changes:

Replace assert()-based allocation checks with explicit NULL checks and improve cleanup paths.
Add new CLI options -x, -Q, and -X (hex address printing + libmodbus quirks support), plus corresponding help/README updates.
Modernize CMakeLists.txt (target-based flags, Release/Debug separation, updated libmodbus minimum) and add CI builds for Linux/macOS/Windows.

Reviewed changes

Copilot reviewed 9 out of 9 changed files in this pull request and generated 4 comments.

Show a summary per file

File	Description
`src/mbpoll.c`	Adds new CLI flags, enables libmodbus quirks, improves allocation failure handling, and fixes a cleanup leak.
`CMakeLists.txt`	Modernizes build configuration, hardening flags, dependency discovery, and libmodbus minimum version.
`.github/workflows/ci.yml`	Adds CI jobs for Linux/macOS/Windows builds and basic smoke checks.
`README.md`	Updates libmodbus minimum version and documents new CLI options.
`mbpoll-config.h`	Updates copyright year.
`src/serial.h`	Translates comments to English and updates copyright year.
`src/serial.c`	Updates copyright year.
`src/custom-rts.h`	Updates copyright year.
`src/custom-rts.c`	Updates copyright year.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

CMakeLists.txt

src/mbpoll.c

.github/workflows/ci.yml

- Bump CMake minimum to 3.13 (required for target_link_directories) - Move find_package(PkgConfig) to Unix-only block (Windows uses bundled libmodbus) - Fix printf format UB: cast iAddr to unsigned int for %X specifier - Allow RTU slave address 0 when -Q quirk is enabled - Apply consistent quirk logic to TCP mode for symmetry Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

r00t- · 2026-02-10T15:53:13Z

This PR implements the functionality from the following open PRs:

so people are now using "AI" bots for stealing other people's contributions from open merge requests and claiming ownership of them?

zknpr · 2026-02-10T19:11:41Z

This PR implements the functionality from the following open PRs:

so people are now using "AI" bots for stealing other people's contributions from open merge requests and claiming ownership of them?

I found your Pr useful and worth adding. Where do you see me claiming your contribution as my own? As far as I can tell, you and your Pr are both tagged.

JesseRiemens · 2026-02-16T07:55:02Z

This PR implements the functionality from the following open PRs:

so people are now using "AI" bots for stealing other people's contributions from open merge requests and claiming ownership of them?

I found your Pr useful and worth adding. Where do you see me claiming your contribution as my own? As far as I can tell, you and your Pr are both tagged.

Could have cherry-picked and included the contributions itself - then the code authored by us is merged. I don't think you had bad intentions necessarily, but I feel @r00t- s annoyance as well.

zknpr · 2026-02-16T10:13:14Z

@r00t- @JesseRiemens @ciakval

You're right, and I owe you all an apology. I should have cherry-picked your commits instead of reimplementing the functionality. Mentioning your PRs in the description doesn't replace proper git authorship — your names belong in git log, not just in a PR body. That was a mistake on my part and I'm sorry.

If @epsilonrt is open to it, I'd like to fix this properly: revert the portions of this PR that overlap with #87, #90, and #99, then cherry-pick your original commits so you get the authorship credit you deserve. Happy to put together that follow-up PR if the maintainer agrees.

Again, apologies for the misstep.

epsilonrt requested review from Copilot and epsilonrt February 7, 2026 08:48

Copilot started reviewing on behalf of epsilonrt February 7, 2026 08:48 View session

Copilot AI reviewed Feb 7, 2026

View reviewed changes

CMakeLists.txt Outdated Show resolved Hide resolved

src/mbpoll.c Outdated Show resolved Hide resolved

src/mbpoll.c Show resolved Hide resolved

.github/workflows/ci.yml Show resolved Hide resolved

This was referenced Feb 7, 2026

Release v1.5.3: Security hardening, refactoring, and community fixes #101

Closed

Release v1.5.3: Security hardening, refactoring, and community fixes #102

Merged

epsilonrt merged commit b2c03e1 into epsilonrt:master Feb 8, 2026

This was referenced Feb 10, 2026

check result of modbus_set_slave() #87

Closed

Add option -x to print reference in hexadecimal format #90

Closed

Add support for MAX_SLAVE and REPLY_TO_BROADCAST quirks #99

Closed

zknpr mentioned this pull request Feb 16, 2026

fix: restore git authorship for community contributors (PR #87, #90, #99) #106

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Security fixes, code modernization, and community PR integration#100

Security fixes, code modernization, and community PR integration#100
epsilonrt merged 2 commits intoepsilonrt:masterfrom
zknpr:security-fixes-and-community-prs

zknpr commented Feb 6, 2026 •

edited

Loading

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

r00t- commented Feb 10, 2026

Uh oh!

zknpr commented Feb 10, 2026

Uh oh!

JesseRiemens commented Feb 16, 2026

Uh oh!

zknpr commented Feb 16, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

Conversation

zknpr commented Feb 6, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Security Fixes

Code Modernization

Community PR Integration

New Command-Line Options

CI/CD

Test Plan

Breaking Changes

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

r00t- commented Feb 10, 2026

Uh oh!

zknpr commented Feb 10, 2026

Uh oh!

JesseRiemens commented Feb 16, 2026

Uh oh!

zknpr commented Feb 16, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

zknpr commented Feb 6, 2026 •

edited

Loading