Skip to content

Reject out-of-range --buffer values - #605

Open
lee3Q wants to merge 2 commits into
orf:mainfrom
lee3Q:lee3Q-patch-1
Open

lee3Q wants to merge 2 commits into
orf:mainfrom
lee3Q:lee3Q-patch-1

Conversation

@lee3Q

@lee3Q lee3Q commented Nov 4, 2026

Copy link
Copy Markdown

--buffer is a u64 of seconds, but it was converted with i64/chrono::Duration conversions that can fail, and the graph window arithmetic (started +/- display_interval, now - buffer) can leave chrono's representable DateTime range. With a very large value, gping panicked while running (in PlotData::update) instead of reporting a bad argument.

This change:

  • Validates --buffer once at startup (validate_buffer). A value that does not fit chrono::Duration, or whose window started - buffer ..= started + buffer would leave the DateTime range, exits with Invalid buffer size: <n> seconds is out of range. The same started instant is passed to App so validation and use agree.
  • PlotData and App now take a chrono::Duration instead of converting a u64 themselves.
  • PlotData::update computes its retention cutoff with checked_sub_signed. If the cutoff is not representable (for example the system clock moved backwards after startup), no point is treated as stale and all points are kept until a later reading makes it representable.

Behavior changes to be aware of:

  • Values that used to start and then panic are now rejected at startup. Values that worked before (0, 30, 8e12 seconds) are still accepted.
  • In the unrepresentable-cutoff case, update used to panic; it now keeps all points. If that state persisted, samples would keep accumulating, because there is no upper filter.

Testing: 22 unit tests pass with --locked and cargo fmt --all --check is clean; clippy with all targets and features shows no warnings. The new None branch test fails if the checked guard is reverted. I did not run the full workspace tests, release builds, the TUI on this exact revision, or a real system-clock rollback. The branch has two commits, so only the final tree builds.

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.

1 participant