Skip to content

Fix Print::printf return value on truncation and error#74

Open
l5yth wants to merge 2 commits into
meshtastic:masterfrom
l5yth:l5y-printf-return-value
Open

Fix Print::printf return value on truncation and error#74
l5yth wants to merge 2 commits into
meshtastic:masterfrom
l5yth:l5y-printf-return-value

Conversation

@l5yth

@l5yth l5yth commented Jun 2, 2026

Copy link
Copy Markdown
  • vsnprintf returns the would-be length (not bytes written) and a negative int on encoding error.
  • Storing that in size_t and returning it gave callers an overcount on truncation and ~SIZE_MAX on error.
  • Clamp to what actually fit and return write()'s byte count.
  • Fix Print::printf returns an incorrect byte count #72

@CLAassistant

CLAassistant commented Jun 2, 2026

Copy link
Copy Markdown

CLA assistant check
All committers have signed the CLA.

@coderabbitai

coderabbitai Bot commented Jun 29, 2026

Copy link
Copy Markdown

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 0a16aaf0-0bad-4043-8a0b-495f6357cdf4

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Comment @coderabbitai help to get the list of available commands.

@thebentern
thebentern requested a review from Copilot July 16, 2026 20:32

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Note

Copilot couldn't run its full agentic review because it didn't start before the timeout. Make sure your repository has a runner available, or add a copilot-setup-steps.yml file specifying one with the runs-on attribute. See the docs for more details.

This PR fixes Print::printf’s return value semantics by correctly handling vsnprintf’s “would-have-written” length on truncation and negative values on encoding errors (Fixes #72).

Changes:

  • Store vsnprintf’s return value as int and treat <= 0 as no bytes written.
  • Clamp the output length to the buffer’s actual written capacity on truncation.
  • Return the byte count from write() instead of the vsnprintf would-be length.

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

if (n <= 0) return 0;
// vsnprintf returns the would-be length; clamp to what actually fit.
size_t len = (n < (int)sizeof(buf)) ? (size_t)n : sizeof(buf) - 1;
return write((const uint8_t *)buf, len);
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.

Print::printf returns an incorrect byte count

3 participants